publish-crates-github: version and branch reach a shell with the bot token, plus two operational defects

[milyin]

Three problems in publish-crates-github. The first is a security issue; the other two are operational.

1. version and branch reach a shell, with the bot token in the environment

publish-crates-github.ts builds the command as one string:

const command = ["gh", "release", "create", input.version];
command.push("--repo", input.repo);
command.push("--target", input.branch);
…
sh(command.join(" "), { env });          // env = { GH_TOKEN: input.githubToken }

and sh in src/command.ts runs that string through a shell:

const returns = spawnSync(cmd, { …, shell: true });

So input.version and input.branch are concatenated into a line that sh -c then parses. Both arrive from workflow_dispatch inputs in the calling workflows, and Git accepts $, (, ), ; and | in tag names. Command substitution happens while the line is built, so a later --verify-tag failure does not prevent it.

The consequence is privilege escalation: anyone able to dispatch a release workflow can run commands in a job holding BOT_TOKEN_WORKFLOW, without having access to that secret.

Fix. The same module defines a second helper that passes an argument array and sets shell: false:

const returns = spawnSync(program, args, { …, shell: false });

Using it for both gh calls removes the shell from the path, and no escaping scheme is then needed.

2. Every invocation installs toml-cli2, which this action never uses

publish-crates-github.ts imports build-crates-debian.ts for one value, the archive filename pattern:

import { artifactRegExp } from "./build-crates-debian.js";   // /^.*-debian\.zip$/

build-crates-debian.ts initialises TOML at module scope, so importing it runs that:

const toml = await TOML.init();     // → installBinaryCached("toml-cli2")

Observed in a zenoh-java release job:

08:32:38  cargo +stable install toml-cli2@0.3.2 --force
08:33:17  Installed package `toml-cli2 v0.3.2`
08:33:20  gh release list --repo eclipse-zenoh/zenoh-java …

Thirty-nine seconds before the first gh call, for a parser the action does not use.

installBinaryCached does cache, but the key includes os.release(), the kernel version:

const key = `${os.platform()}-${os.release()}-${os.arch()}-${name}-${version}`;

That entry is invalidated whenever the runner image kernel moves, and unused caches expire after 7 days. A release workflow runs a few times a year, so a release is realistically always a cache miss.

Fix. Move the archive patterns into a module with no TOML dependency, or make TOML.init() lazy rather than a module-scope await. Either one also helps every other action that imports these modules for unrelated values.

3. Generated notes start at the newest release, not at the predecessor of the version being released

const releasesRaw = sh(`gh release list --repo ${input.repo} --exclude-drafts --order desc --json tagName`, { env });
const releaseLatest = JSON.parse(releasesRaw).at(0);
…
if (releaseLatest != undefined) command.push("--notes-start-tag", releaseLatest.tagName);

During a normal release the newest release is the predecessor, so this is correct and has never misbehaved.

That choice breaks when the version being released is not the newest — creating a GitHub release for a version that reached Maven Central without one, after a later version has since shipped. Releasing 1.10.0 while 1.11.0 exists gives --notes-start-tag 1.11.0, so the notes run from a later tag back to an earlier one and come out empty.

Fix. Sort the version into the release list and take its predecessor. The result is identical to the current behaviour in the ordinary case:

{ gh release list --repo "$REPO" --exclude-drafts --limit 100 --json tagName --jq '.[].tagName'
  printf '%s\n' "$VERSION"
} | sort -V -u | awk -v v="$VERSION" '$0 == v { print prev; exit } { prev = $0 }'

Status in zenoh-kotlin

eclipse-zenoh/zenoh-kotlin#708 validates version and branch against ^[A-Za-z0-9][A-Za-z0-9._/-]*$ before calling the action, which closes problem 1 at that one call site. Every other repository calling this action has the same exposure. Problems 2 and 3 are not worked around there, on the view that a fix here reaches every Zenoh repository.

Happy to open a pull request for any of the three.

[milyin]

Folded into the issue description above, which now covers all three problems.