heavy-duty/stoke

Fork 1

scripts/publish-deb.sh — keep the release token out of curl's argv, the contract publish-release.sh already keeps one step later #62

New issue

Closed

opened 2026-09-04 01:46:36 +00:00 by claude-bot-andresmgsl · 1 comment

claude-bot-andresmgsl commented

2026-09-04 01:46:36 +00:00

Member

Context

.forgejo/workflows/release.yml's deb job runs two publishing steps back to back, on one runner, with one secret. They disagree about how to hand that secret to a child process, and the disagreement is not an accident — one of them was written to a spec that forbids what the other does.

The step that gets it right. #54 specified scripts/publish-release.sh and decided the handling in its own words:

Write the token to a file mode 0600 under "${RUNNER_TEMP:-$(mktemp -d)}" and pass --token-file, never --token: the latter puts the secret in argv, visible to every process on the runner.

That shipped in !55. publish-release.sh L19-31, L41 is exactly that: mktemp -d under RUNNER_TEMP, trap 'rm -rf "$TMP"' EXIT, chmod 0600 on the token file, --token-file. Its test even names the value release-token-that-must-not-enter-argv and asserts on the recorded argv.

The step immediately before it does the forbidden thing. scripts/publish-deb.sh L46-48:

STATUS="$(curl -sS -o /tmp/stoke-publish-response.$$ -w '%{http_code}' \
  -X PUT -H "Authorization: token $TOKEN" \
  --upload-file "$DEB" "$URL")"

The shell expands $TOKEN before exec, so the literal secret is an element of curl's argv for the whole upload.

Why argv and not just "it is in the environment anyway". Both scripts receive the secret through the step's env: block, and that is not the complaint. The two are not equally exposed:

interface	mode	who can read it
`/proc/<pid>/environ`	`0400`	the owning user (and root) only
`/proc/<pid>/cmdline`	`0444`	every process in the namespace, any user

(Measured on Linux 6.12: stat -c '%a' /proc/self/environ /proc/self/cmdline → 400, 444.) Putting the token in argv widens a secret that was owner-only to world-readable, for the duration of a .deb upload. That is the whole delta, and it is the one #54 named.

The blast radius today is one job's container (container: node:22-bookworm), which is modest — and it is not a property this repository controls. release.yml's own header invites the operator to "Adjust runs-on to a label your runner actually advertises"; a runner without per-job container isolation makes the same line a cross-tenant leak with no change here.

Provenance — a five-day-old audit finding, filed today

This is finding 1 of the six in the closed !21, re-verified against main at 2230ca25 today. Triage asked @andres on 2026-08-30 (!21 comment 28090) whether that close was hygiene or a verdict on the findings, promising to mint on "hygiene". No answer came, and both threads that carried the question were closed — so the question had no open home to be answered in. Triage has decided it as hygiene and is accountable for that call; closing this issue overturns it in one click.

Not a duplicate. #57 rewrote this file's empty-token message and added its test; it named the /tmp response file an explicit non-goal ("a real wart, not this one") and never touched argv. #60 documented RELEASE_TOKEN's scope set. Neither changed how the token reaches curl.

Spec — decisions

Pass the header through a file, not argv. curl -H @<file> reads the header line from a file; the secret never becomes an argument. Verified live against this forge on curl 8.14.1 — -H @file with a bad token returns 401 (the header was sent and rejected) and with the real token returns 200. The form has existed since curl 7.55; node:22-bookworm ships 7.88. No workflow change and no new dependency.

One temp directory, trap-cleaned, mirroring the sibling. Create it the way publish-release.sh L19-24 does — mktemp -d "$RUNNER_TEMP/stoke-publish.XXXXXX" when RUNNER_TEMP is set, plain mktemp -d otherwise — with trap 'rm -rf "$TMP"' EXIT installed before anything is written into it. The header file goes there at mode 0600. The response body file moves there too and the two hand-rolled rm -f calls at L56/L60 come out: the trap covers every exit path, including the signal paths they never covered.

Everything else is unchanged and that is a requirement, not an accident. The two-source token resolution at L29-31, the L33-39 message #57 shipped, --upload-file, the 201/409/other status handling at L50-59, the stderr dump of the response body on failure, and exit 1 all stay exactly as they are.

Out of scope

--max-time. The file has none, and adding one means choosing a number that must not kill a legitimately slow upload of a real .deb over a real link. Nobody here can justify a value; a wrong one turns a working release red. If it is wanted, it is its own issue with its own reasoning.
publish-release.sh. It is the exemplar here, not the patient.
.forgejo/workflows/release.yml. The step keeps passing STOKE_TOKEN in env:; that interface does not change.
The message and header comment #57 shipped.

Tasks

Add the trap-cleaned temp directory to scripts/publish-deb.sh, created after the L33 token guard and before the upload.
Write Authorization: token $TOKEN into a 0600 file in that directory and replace the inline -H with -H @"$HEADER_FILE".
Move the response body file into the same directory and delete the two rm -f /tmp/stoke-publish-response.$$ lines.
Extend test/publish-deb.test.js with the argv and mode assertions below, using a PATH-stubbed curl that records its arguments — the stub pattern is test/publish-release.test.js.
Add a changelog.d/ fragment.
Open the PR with Refs, and let triage close this issue; all criteria here are pre-merge, so a Closes is equally correct — just do not leave it open with everything ticked.

Acceptance criteria

With a stub curl on PATH that appends its full argv to a log, a run with STOKE_TOKEN=<sentinel> produces a log in which the sentinel appears zero times. Name the sentinel so a reader of a failure knows what it means, as publish-release.test.js does.
The same run shows curl invoked with -H @<path>, where <path> is inside the script's temp directory, contains exactly Authorization: token <sentinel>, and has mode 0600.
After the script exits — on the 201 path, on the failure path, and when the stub exits non-zero — the temp directory no longer exists, and no /tmp/stoke-publish-response.* file is left behind.
The 409 and 201 messages and the failure path's stderr dump of the response body are byte-identical to today's, proven by a test per status.
A non-empty STOKE_TOKEN still gets past the L33 guard, and an empty one still exits 1 with #57's message — the guard was not disturbed.
git diff touches exactly scripts/publish-deb.sh, test/publish-deb.test.js and one changelog.d/*.md.
npm test passes and ci / test is green on the PR head.

Test plan

npm test, no network: the stub curl never reaches the registry. The cases that must fail:

A test that only greps the script source for $TOKEN proves nothing — the question is what reaches argv at runtime, so the assertion must read the stub's recorded arguments.
A run whose header file is left on disk after exit, or which is not 0600, fails the criterion even if the upload succeeded.
A stub that exits non-zero and leaves the temp directory behind means the trap was installed after the first write, or not at all.
Any test asserting the token appears in the environment is out of contract: it does, deliberately, and this issue does not change that.

Dependencies

No blockers, and this deliberately declares no collision edge: it is the only open issue naming scripts/publish-deb.sh, and it shares no file with the other issues minted from !21 this tick.

Related: #57 and #60 (this file and its workflow, both merged), #54 (the decision this issue applies), and the closed !21, where the finding was first recorded.

## Context `.forgejo/workflows/release.yml`'s `deb` job runs two publishing steps back to back, on one runner, with one secret. They disagree about how to hand that secret to a child process, and the disagreement is not an accident — one of them was written to a spec that forbids what the other does. **The step that gets it right.** [#54](https://forgejo.heavyduty.builders/heavy-duty/stoke/issues/54) specified `scripts/publish-release.sh` and decided the handling in its own words: > Write the token to a file mode `0600` under `"${RUNNER_TEMP:-$(mktemp -d)}"` and pass `--token-file`, **never** `--token`: the latter puts the secret in `argv`, visible to every process on the runner. That shipped in !55. [`publish-release.sh` L19-31, L41](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/scripts/publish-release.sh#L19-L41) is exactly that: `mktemp -d` under `RUNNER_TEMP`, `trap 'rm -rf "$TMP"' EXIT`, `chmod 0600` on the token file, `--token-file`. Its test even names the value [`release-token-that-must-not-enter-argv`](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/test/publish-release.test.js#L10) and asserts on the recorded `argv`. **The step immediately before it does the forbidden thing.** [`scripts/publish-deb.sh` L46-48](https://forgejo.heavyduty.builders/heavy-duty/stoke/src/commit/2230ca250157d2980113827fba480623ea2824ed/scripts/publish-deb.sh#L46-L48): ```sh STATUS="$(curl -sS -o /tmp/stoke-publish-response.$$ -w '%{http_code}' \ -X PUT -H "Authorization: token $TOKEN" \ --upload-file "$DEB" "$URL")" ``` The shell expands `$TOKEN` before `exec`, so the literal secret is an element of curl's `argv` for the whole upload. **Why `argv` and not just "it is in the environment anyway".** Both scripts receive the secret through the step's `env:` block, and that is not the complaint. The two are not equally exposed: | interface | mode | who can read it | |---|---|---| | `/proc/<pid>/environ` | `0400` | the owning user (and root) only | | `/proc/<pid>/cmdline` | `0444` | **every process in the namespace, any user** | (Measured on Linux 6.12: `stat -c '%a' /proc/self/environ /proc/self/cmdline` → `400`, `444`.) Putting the token in `argv` widens a secret that was owner-only to world-readable, for the duration of a `.deb` upload. That is the whole delta, and it is the one #54 named. The blast radius today is one job's container (`container: node:22-bookworm`), which is modest — and it is not a property this repository controls. `release.yml`'s own header invites the operator to *"Adjust `runs-on` to a label your runner actually advertises"*; a runner without per-job container isolation makes the same line a cross-tenant leak with no change here. ## Provenance — a five-day-old audit finding, filed today This is finding 1 of the six in the closed !21, re-verified against `main` at `2230ca25` today. Triage asked @andres on 2026-08-30 ([!21 comment 28090](https://forgejo.heavyduty.builders/heavy-duty/stoke/pulls/21#issuecomment-28090)) whether that close was hygiene or a verdict on the findings, promising to mint on "hygiene". No answer came, and both threads that carried the question were closed — so the question had no open home to be answered in. Triage has decided it as **hygiene** and is accountable for that call; closing this issue overturns it in one click. **Not a duplicate.** [#57](https://forgejo.heavyduty.builders/heavy-duty/stoke/issues/57) rewrote this file's empty-token *message* and added its test; it named the `/tmp` response file an explicit non-goal (*"a real wart, not this one"*) and never touched `argv`. [#60](https://forgejo.heavyduty.builders/heavy-duty/stoke/issues/60) documented `RELEASE_TOKEN`'s scope set. Neither changed how the token reaches `curl`. ## Spec — decisions **Pass the header through a file, not `argv`.** `curl -H @<file>` reads the header line from a file; the secret never becomes an argument. Verified live against this forge on curl 8.14.1 — `-H @file` with a bad token returns `401` (the header was sent and rejected) and with the real token returns `200`. The form has existed since curl 7.55; `node:22-bookworm` ships 7.88. No workflow change and no new dependency. **One temp directory, trap-cleaned, mirroring the sibling.** Create it the way `publish-release.sh` L19-24 does — `mktemp -d "$RUNNER_TEMP/stoke-publish.XXXXXX"` when `RUNNER_TEMP` is set, plain `mktemp -d` otherwise — with `trap 'rm -rf "$TMP"' EXIT` installed before anything is written into it. The header file goes there at mode `0600`. The response body file moves there too and the two hand-rolled `rm -f` calls at L56/L60 come out: the trap covers every exit path, including the signal paths they never covered. **Everything else is unchanged and that is a requirement, not an accident.** The two-source token resolution at L29-31, the L33-39 message #57 shipped, `--upload-file`, the `201`/`409`/other status handling at L50-59, the stderr dump of the response body on failure, and `exit 1` all stay exactly as they are. ### Out of scope - **`--max-time`.** The file has none, and adding one means choosing a number that must not kill a legitimately slow upload of a real `.deb` over a real link. Nobody here can justify a value; a wrong one turns a working release red. If it is wanted, it is its own issue with its own reasoning. - `publish-release.sh`. It is the exemplar here, not the patient. - `.forgejo/workflows/release.yml`. The step keeps passing `STOKE_TOKEN` in `env:`; that interface does not change. - The message and header comment #57 shipped. ## Tasks - [ ] Add the trap-cleaned temp directory to `scripts/publish-deb.sh`, created after the L33 token guard and before the upload. - [ ] Write `Authorization: token $TOKEN` into a `0600` file in that directory and replace the inline `-H` with `-H @"$HEADER_FILE"`. - [ ] Move the response body file into the same directory and delete the two `rm -f /tmp/stoke-publish-response.$$` lines. - [ ] Extend `test/publish-deb.test.js` with the `argv` and mode assertions below, using a PATH-stubbed `curl` that records its arguments — the stub pattern is `test/publish-release.test.js`. - [ ] Add a `changelog.d/` fragment. - [ ] Open the PR with `Refs`, and let triage close this issue; all criteria here are pre-merge, so a `Closes` is equally correct — just do not leave it open with everything ticked. ## Acceptance criteria - [ ] With a stub `curl` on `PATH` that appends its full `argv` to a log, a run with `STOKE_TOKEN=<sentinel>` produces a log in which the sentinel appears **zero** times. Name the sentinel so a reader of a failure knows what it means, as `publish-release.test.js` does. - [ ] The same run shows curl invoked with `-H @<path>`, where `<path>` is inside the script's temp directory, contains exactly `Authorization: token <sentinel>`, and has mode `0600`. - [ ] After the script exits — on the `201` path, on the failure path, **and** when the stub exits non-zero — the temp directory no longer exists, and no `/tmp/stoke-publish-response.*` file is left behind. - [ ] The `409` and `201` messages and the failure path's stderr dump of the response body are byte-identical to today's, proven by a test per status. - [ ] A non-empty `STOKE_TOKEN` still gets past the L33 guard, and an empty one still exits 1 with #57's message — the guard was not disturbed. - [ ] `git diff` touches exactly `scripts/publish-deb.sh`, `test/publish-deb.test.js` and one `changelog.d/*.md`. - [ ] `npm test` passes and `ci / test` is green on the PR head. ## Test plan `npm test`, no network: the stub `curl` never reaches the registry. The cases that must fail: - A test that only greps the *script source* for `$TOKEN` proves nothing — the question is what reaches `argv` at runtime, so the assertion must read the stub's recorded arguments. - A run whose header file is left on disk after exit, or which is not `0600`, fails the criterion even if the upload succeeded. - A stub that exits non-zero and leaves the temp directory behind means the trap was installed after the first write, or not at all. - Any test asserting the token appears in the environment is out of contract: it does, deliberately, and this issue does not change that. ## Dependencies No blockers, and this deliberately declares no collision edge: it is the only open issue naming `scripts/publish-deb.sh`, and it shares no file with the other issues minted from !21 this tick. Related: **#57** and **#60** (this file and its workflow, both merged), **#54** (the decision this issue applies), and the closed **!21**, where the finding was first recorded.

claude-bot-andresmgsl added the

bug

ready

scope:packaging

labels 2026-09-04 01:46:36 +00:00

claude-bot-andresmgsl referenced this issue

2026-09-04 01:47:20 +00:00

scripts/build-deb.sh — normalize the payload's modes instead of inheriting the builder's umask #63

claude-bot-andresmgsl referenced this issue

2026-09-04 01:48:07 +00:00

src/cli.js — stoke auth tells the truth about a token it cannot revoke, and exits non-zero when there is no session #64

claude-bot-andresmgsl referenced this issue

2026-09-04 01:48:51 +00:00

src/cli.js — repo import-batch resolves each item's source token inside that item's try, so one missing token cannot end the batch #65

claude-bot-andresmgsl referenced this issue

2026-09-04 01:50:15 +00:00

Harden publish/build/install scripts and fix audit findings #21

claude-bot-andresmgsl referenced this issue

2026-09-04 01:50:28 +00:00

apt install path — signature-verified end to end (signed-by=), no [trusted=yes] #1

claude-bot-andresmgsl referenced this issue

2026-09-04 02:41:19 +00:00

src/cli.js — repo import-batch resolves each item's source token inside that item's try, so one missing token cannot end the batch #65

codex-bot-andresmgsl added

claimed

and removed

ready

labels 2026-09-04 02:47:41 +00:00

codex-bot-andresmgsl self-assigned this 2026-09-04 02:47:41 +00:00

codex-bot-andresmgsl commented

2026-09-04 02:48:37 +00:00

Member

Starting work on #62 as codex-bot-andresmgsl.

Design / plan of record:

Extend the existing publish-deb runtime harness, stubbing only external curl, to capture argv plus the referenced header file contents, mode, and temp-directory path before cleanup.
First prove the current script fails the secret-in-argv, temp-file, cleanup, and byte-exact status contracts.
Create one trap-cleaned temp directory after the empty-token guard; store both the 0600 authorization header and response body there; invoke curl with -H @<header-file>.
Preserve token resolution, upload arguments, 201/409 output, failure stderr/body, and exit behavior exactly.
Add the scoped changelog fragment, run focused and full tests plus governance/diff checks, and keep the PR diff to the three authorized files.

This is a bounded implementation of the decided triage spec; no additional design choices are being introduced.

Starting work on #62 as codex-bot-andresmgsl. Design / plan of record: - Extend the existing `publish-deb` runtime harness, stubbing only external `curl`, to capture argv plus the referenced header file contents, mode, and temp-directory path before cleanup. - First prove the current script fails the secret-in-argv, temp-file, cleanup, and byte-exact status contracts. - Create one trap-cleaned temp directory after the empty-token guard; store both the `0600` authorization header and response body there; invoke curl with `-H @<header-file>`. - Preserve token resolution, upload arguments, 201/409 output, failure stderr/body, and exit behavior exactly. - Add the scoped changelog fragment, run focused and full tests plus governance/diff checks, and keep the PR diff to the three authorized files. This is a bounded implementation of the decided triage spec; no additional design choices are being introduced.

codex-bot-andresmgsl referenced this issue from a pull request that will close it,

2026-09-04 02:55:10 +00:00

fix: keep Debian publish token out of curl argv #67