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

Merged

andres merged 3 commits from build/62-publish-deb-token into main 2026-09-04 06:16:38 +00:00

Conversation 5 Commits 3 Files changed 3 +139 -26

codex-bot-andresmgsl commented 2026-09-04 02:55:10 +00:00

Member

Copy link

Closes #62

Worklog

• Claim #62, post the design record, and create the isolated worktree
• Add runtime coverage and prove the current argv leak and curl-failure cleanup defect
• Store the authorization header and response body in one trap-cleaned temp directory
• Add the changelog fragment
• Verify focused tests, the full suite, governance, and the exact three-file diff
• Push the complete head and post the build reply

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.
• The same run shows curl invoked with -H @<path>, where <path> is inside the script 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 stderr dump of the response body are byte-identical to today, proven by a test per status.
• A non-empty STOKE_TOKEN still gets past the guard, and an empty one still exits 1 with #57 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.

TDD evidence

Before implementation, node --test test/publish-deb.test.js failed 2/6 for the intended reasons: the sentinel appeared in curl argv, and curl exit 7 left /tmp/stoke-publish-response.<pid> behind. The existing empty-token, 201, 409, and HTTP-error behaviors remained green. After the implementation, the focused suite passes 6/6. Fresh final verification at 4d17b8b passes focused 6/6, full suite 147/147, governance, shell syntax, diff check, and remote-head equality. The current-head CI checkbox remains unchecked until the engine-observed check is green.

Round log

Round at 4d17b8b1

Round passed with no written reply.

Closes #62 ## Worklog - [x] Claim #62, post the design record, and create the isolated worktree - [x] Add runtime coverage and prove the current argv leak and curl-failure cleanup defect - [x] Store the authorization header and response body in one trap-cleaned temp directory - [x] Add the changelog fragment - [x] Verify focused tests, the full suite, governance, and the exact three-file diff - [x] Push the complete head and post the build reply ## Acceptance criteria - [x] 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. - [x] The same run shows curl invoked with `-H @<path>`, where `<path>` is inside the script temp directory, contains exactly `Authorization: token <sentinel>`, and has mode `0600`. - [x] 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. - [x] The `409` and `201` messages and the failure path stderr dump of the response body are byte-identical to today, proven by a test per status. - [x] A non-empty `STOKE_TOKEN` still gets past the guard, and an empty one still exits 1 with #57 message — the guard was not disturbed. - [x] `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. ## TDD evidence Before implementation, `node --test test/publish-deb.test.js` failed 2/6 for the intended reasons: the sentinel appeared in curl argv, and curl exit 7 left `/tmp/stoke-publish-response.<pid>` behind. The existing empty-token, 201, 409, and HTTP-error behaviors remained green. After the implementation, the focused suite passes 6/6. Fresh final verification at `4d17b8b` passes focused 6/6, full suite 147/147, governance, shell syntax, diff check, and remote-head equality. The current-head CI checkbox remains unchecked until the engine-observed check is green. ## Round log <!-- round:4d17b8b1131e921dea43d6f04c3d45bb028d39e1 --> **Round at 4d17b8b1** _Round passed with no written reply._

codex-bot-andresmgsl added 1 commit 2026-09-04 02:55:11 +00:00

test: cover private deb publish credentials 1dfa2c173d

codex-bot-andresmgsl changed title from fix: keep Debian publish token out of curl argv to WIP: fix: keep Debian publish token out of curl argv 2026-09-04 02:55:21 +00:00

forgejo-actions added the

state:building

label 2026-09-04 02:55:27 +00:00

codex-bot-andresmgsl added 1 commit 2026-09-04 02:55:59 +00:00

fix: protect Debian publish credentials dcb169e8ab

forgejo-actions added the

scope:packaging

label 2026-09-04 02:56:10 +00:00

codex-bot-andresmgsl added 1 commit 2026-09-04 02:56:48 +00:00

docs: record private Debian publish headers 4d17b8b113

codex-bot-andresmgsl commented 2026-09-04 02:58:58 +00:00

Author

Member

Copy link

Build complete at head 4d17b8b113.

• publish-deb.sh now creates one trap-owned temporary directory after token validation, writes the authorization header there at mode 0600, passes curl -H @<path>, and stores the response body beside it.
• The trap removes both files and their directory on 201, 409, HTTP failure, curl failure, and shell exits; the old process-wide /tmp/stoke-publish-response.* path is gone.
• Runtime tests preserve the exact 201, 409, and HTTP-error transcripts while proving the token never enters curl argv, the header contents/mode are exact, both files share the RUNNER_TEMP directory, and every exit cleans up.
• Added changelog.d/62.md.

TDD evidence: before implementation, the focused suite failed 2/6 because the sentinel was present in curl argv and curl exit 7 leaked /tmp/stoke-publish-response.<pid>. After implementation, focused tests pass 6/6.

Fresh final verification: npm test passes 147/147; npm run check:governance passes with 4 identities and 5 scope rows; bash -n scripts/publish-deb.sh and git diff --check origin/main...HEAD pass; the clean pushed diff contains exactly scripts/publish-deb.sh, test/publish-deb.test.js, and changelog.d/62.md. The draft head currently has no checks reported, which is the ruled no-check green case; marking ready may start them, and the engine owns waiting before requesting the panel.

Build complete at head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1. - `publish-deb.sh` now creates one trap-owned temporary directory after token validation, writes the authorization header there at mode `0600`, passes curl `-H @<path>`, and stores the response body beside it. - The trap removes both files and their directory on 201, 409, HTTP failure, curl failure, and shell exits; the old process-wide `/tmp/stoke-publish-response.*` path is gone. - Runtime tests preserve the exact 201, 409, and HTTP-error transcripts while proving the token never enters curl argv, the header contents/mode are exact, both files share the `RUNNER_TEMP` directory, and every exit cleans up. - Added `changelog.d/62.md`. TDD evidence: before implementation, the focused suite failed 2/6 because the sentinel was present in curl argv and curl exit 7 leaked `/tmp/stoke-publish-response.<pid>`. After implementation, focused tests pass 6/6. Fresh final verification: `npm test` passes 147/147; `npm run check:governance` passes with 4 identities and 5 scope rows; `bash -n scripts/publish-deb.sh` and `git diff --check origin/main...HEAD` pass; the clean pushed diff contains exactly `scripts/publish-deb.sh`, `test/publish-deb.test.js`, and `changelog.d/62.md`. The draft head currently has no checks reported, which is the ruled no-check green case; marking ready may start them, and the engine owns waiting before requesting the panel.

codex-bot-andresmgsl commented 2026-09-04 02:59:14 +00:00

Author

Member

Copy link

📣 round answered at head 4d17b8b113

📣 round answered at head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1

codex-bot-andresmgsl changed title from WIP: fix: keep Debian publish token out of curl argv to fix: keep Debian publish token out of curl argv 2026-09-04 02:59:19 +00:00

forgejo-actions added

state:addressing

and removed

state:building

labels 2026-09-04 03:00:14 +00:00

codex-bot-andresmgsl requested review from glm-bot-andresmgsl 2026-09-04 03:06:03 +00:00

codex-bot-andresmgsl requested review from claude-bot-andresmgsl 2026-09-04 03:06:04 +00:00

codex-bot-andresmgsl requested review from kimi-bot-andresmgsl 2026-09-04 03:06:04 +00:00

codex-bot-andresmgsl added the

state:bots-reviewing

label 2026-09-04 03:06:05 +00:00

forgejo-actions removed the

state:addressing

label 2026-09-04 03:06:24 +00:00

claude-bot-andresmgsl commented 2026-09-04 03:10:52 +00:00

Member

Copy link

🔎 reviewing head 4d17b8b113

Acceptance criteria against #62, all settled:

• sentinel zero times in curl argv — stub records argv; assert at test/publish-deb.test.js:113
• -H @<path> inside the script temp dir, exact contents, mode 0600 — :114-:117
• temp dir gone and no /tmp/stoke-publish-response.* on 201, HTTP-failure, and stub-exit-7 — assertCleaned, :87-:92
• 201 / 409 / failure-stderr transcripts byte-identical — :128, :137, :147
• guard undisturbed: empty token still exits 1 with #57's message; non-empty passes — :94-:106
• diff is exactly scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md
• npm test 147/147 here after npm ci; ci / test and labels / labels both success on this head

Verification run: new tests against base 2230ca25 fail 2/6 (argv, curl-failure leak); dropping the
EXIT trap fails 5/6; umask 022+chmod 0644 fails the mode assertion; restoring the inline -H
fails the argv assertion. shellcheck and bash -n clean, check:governance passes.
curl -H @file confirmed live over a loopback server (curl 8.14.1): the header is sent as written.

Verdict submitted.

🔎 reviewing head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1 Acceptance criteria against #62, all settled: - [x] sentinel zero times in curl argv — stub records argv; `assert` at test/publish-deb.test.js:113 - [x] `-H @<path>` inside the script temp dir, exact contents, mode `0600` — :114-:117 - [x] temp dir gone and no `/tmp/stoke-publish-response.*` on 201, HTTP-failure, and stub-exit-7 — `assertCleaned`, :87-:92 - [x] 201 / 409 / failure-stderr transcripts byte-identical — :128, :137, :147 - [x] guard undisturbed: empty token still exits 1 with #57's message; non-empty passes — :94-:106 - [x] diff is exactly `scripts/publish-deb.sh`, `test/publish-deb.test.js`, `changelog.d/62.md` - [x] `npm test` 147/147 here after `npm ci`; `ci / test` and `labels / labels` both success on this head Verification run: new tests against base `2230ca25` fail 2/6 (argv, curl-failure leak); dropping the EXIT trap fails 5/6; `umask 022`+`chmod 0644` fails the mode assertion; restoring the inline `-H` fails the argv assertion. `shellcheck` and `bash -n` clean, `check:governance` passes. `curl -H @file` confirmed live over a loopback server (curl 8.14.1): the header is sent as written. Verdict submitted.

glm-bot-andresmgsl commented 2026-09-04 03:12:14 +00:00

Member

Copy link

🔎 reviewing head 4d17b8b113

• Announced; marker posted (comment 34785)
• Issue #62 acceptance criteria read and checked (7/7 met)
• Whole-PR code review at this head in a detached worktree (review-67)
• Tests run: focused 6/6, full suite 147/147, governance, bash -n; mutation check — new tests vs the old script fail exactly the 2 intended cases
• ci / test (pull_request) = success on this exact head (statuses endpoint)
• Diff touches exactly scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md
• Verdict submitted: APPROVE at head 4d17b8b113 (verified via submit-verdict.sh exit 0)

🔎 reviewing head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1 - [x] Announced; marker posted (comment 34785) - [x] Issue #62 acceptance criteria read and checked (7/7 met) - [x] Whole-PR code review at this head in a detached worktree (review-67) - [x] Tests run: focused 6/6, full suite 147/147, governance, bash -n; mutation check — new tests vs the old script fail exactly the 2 intended cases - [x] ci / test (pull_request) = success on this exact head (statuses endpoint) - [x] Diff touches exactly scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md - [x] Verdict submitted: APPROVE at head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1 (verified via submit-verdict.sh exit 0)

glm-bot-andresmgsl approved these changes 2026-09-04 03:16:25 +00:00

glm-bot-andresmgsl left a comment

Member

Copy link

Approve — head 4d17b8b113, all 7 acceptance criteria of #62 verified.

Acceptance criteria, each checked:

1. Stub curl on PATH records its full argv; run with STOKE_TOKEN=deb-token-that-must-not-enter-argv; sentinel asserted absent from every recorded argument (test 2). Sentinel is named for meaning, mirroring publish-release.test.js.
2. curl invoked with -H @<path>; asserted path is directly inside the RUNNER_TEMP-rooted temp dir, header content exactly Authorization: token <sentinel>\n, mode 0600 (umask 077 + explicit chmod).
3. Cleanup proven on the 201 path, the HTTP-failure path, and the stub-exits-7 path: RUNNER_TEMP entry gone, header/response files gone, and no new /tmp/stoke-publish-response.* legacy file (assertCleaned on all six tests).
4. 201/409 transcripts and the failure-path stderr dump pinned byte-exactly, one test per status; the script's case block is untouched by the diff.
5. Guard undisturbed: empty token still exits 1 with #57's message (test 1 unchanged); non-empty token reaches curl.
6. git diff origin/main...HEAD touches exactly scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md.
7. npm test 147/147 pass locally at this head; ci / test (pull_request) = success on this exact SHA (read from the commit statuses endpoint).

What I ran: focused suite 6/6; full suite 147/147 (after linking node_modules into the throwaway worktree — the first run's 66 failures were all Cannot find module 'commander', i.e. missing untracked deps, not code); npm run check:governance (4 identities, 5 scope rows); bash -n. Mutation check: the new test file against the pre-PR script fails exactly 2/6 — the sentinel in argv and the curl-failure temp leak — so the failure cases actually fail.

Constraints: mirrors publish-release.sh's sanctioned pattern exactly (RUNNER_TEMP-conditional mktemp -d, EXIT trap installed before first write, umask 077 + chmod 0600); CommonJS, node:test, bash shebang conventions all held; changelog fragment present.

Could not verify: a real upload against a live registry (no network publish from this box) — relied on the issue's live -H @file verification (401/200 on this forge; form exists since curl 7.88 on the runner) and on the stub-based runtime assertions; signal-path cleanup beyond bash's EXIT trap (SIGKILL is inherently uncleansable) — relied on the trap semantics and the identical exemplar. The response file also becomes 0600 via umask — strictly safer, noted as intended.

Non-blocking nit (builder's discretion): none material; the curl failure test's empty-stderr assertion reflects the stub (real curl -S would add its own error line), which is fine as the script's own transcript is what is pinned.

Approve — head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1, all 7 acceptance criteria of #62 verified. Acceptance criteria, each checked: 1. Stub `curl` on PATH records its full argv; run with STOKE_TOKEN=deb-token-that-must-not-enter-argv; sentinel asserted absent from every recorded argument (test 2). Sentinel is named for meaning, mirroring publish-release.test.js. 2. curl invoked with `-H @<path>`; asserted path is directly inside the RUNNER_TEMP-rooted temp dir, header content exactly `Authorization: token <sentinel>\n`, mode 0600 (umask 077 + explicit chmod). 3. Cleanup proven on the 201 path, the HTTP-failure path, and the stub-exits-7 path: RUNNER_TEMP entry gone, header/response files gone, and no new `/tmp/stoke-publish-response.*` legacy file (assertCleaned on all six tests). 4. 201/409 transcripts and the failure-path stderr dump pinned byte-exactly, one test per status; the script's case block is untouched by the diff. 5. Guard undisturbed: empty token still exits 1 with #57's message (test 1 unchanged); non-empty token reaches curl. 6. `git diff origin/main...HEAD` touches exactly `scripts/publish-deb.sh`, `test/publish-deb.test.js`, `changelog.d/62.md`. 7. `npm test` 147/147 pass locally at this head; `ci / test (pull_request)` = success on this exact SHA (read from the commit statuses endpoint). What I ran: focused suite 6/6; full suite 147/147 (after linking node_modules into the throwaway worktree — the first run's 66 failures were all `Cannot find module 'commander'`, i.e. missing untracked deps, not code); `npm run check:governance` (4 identities, 5 scope rows); `bash -n`. Mutation check: the new test file against the pre-PR script fails exactly 2/6 — the sentinel in argv and the curl-failure temp leak — so the failure cases actually fail. Constraints: mirrors publish-release.sh's sanctioned pattern exactly (RUNNER_TEMP-conditional mktemp -d, EXIT trap installed before first write, umask 077 + chmod 0600); CommonJS, node:test, bash shebang conventions all held; changelog fragment present. Could not verify: a real upload against a live registry (no network publish from this box) — relied on the issue's live `-H @file` verification (401/200 on this forge; form exists since curl 7.88 on the runner) and on the stub-based runtime assertions; signal-path cleanup beyond bash's EXIT trap (SIGKILL is inherently uncleansable) — relied on the trap semantics and the identical exemplar. The response file also becomes 0600 via umask — strictly safer, noted as intended. Non-blocking nit (builder's discretion): none material; the `curl failure` test's empty-stderr assertion reflects the stub (real curl -S would add its own error line), which is fine as the script's own transcript is what is pinned.

kimi-bot-andresmgsl commented 2026-09-04 03:17:48 +00:00

Member

Copy link

🔎 reviewing head 4d17b8b113

• Read #62 spec, PR body, full diff (exactly 3 files: scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md).
• Full suite 147/147 green at head (node v22.23.2 after npm ci); mutation check: new tests vs main script fail exactly 2/6 (argv leak, curl-failure cleanup) for the intended reasons.
• Independent bash-stub probe of the real script: sentinel 0x in recorded argv, -H @ inside trap-cleaned temp dir, cleanup on 201/409/500/curl-7 paths, transcripts byte-identical.
• CI on head: ci/test + labels both success; governance + bash -n green; changelog fragment format matches siblings.

🔎 reviewing head 4d17b8b1131e921dea43d6f04c3d45bb028d39e1 - Read #62 spec, PR body, full diff (exactly 3 files: scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md). - Full suite 147/147 green at head (node v22.23.2 after npm ci); mutation check: new tests vs main script fail exactly 2/6 (argv leak, curl-failure cleanup) for the intended reasons. - Independent bash-stub probe of the real script: sentinel 0x in recorded argv, -H @<file> inside trap-cleaned temp dir, cleanup on 201/409/500/curl-7 paths, transcripts byte-identical. - CI on head: ci/test + labels both success; governance + bash -n green; changelog fragment format matches siblings.

claude-bot-andresmgsl approved these changes 2026-09-04 03:21:49 +00:00

claude-bot-andresmgsl left a comment

Member

Copy link

Approve at 4d17b8b1131e921dea43d6f04c3d45bb028d39e1.

The change is exactly #62's spec: one trap-owned temp directory created after the L33 guard, the
authorization header written there with printf (a bash builtin — no argv exposure of its own) at
mode 0600, curl -H @"$HEADER_FILE", the response body moved into the same directory, and both
hand-rolled rm -f /tmp/stoke-publish-response.$$ calls gone. It mirrors publish-release.sh
L19-31 line for line, which is what #54 decided and what the issue asked for. Nothing in the frozen
surface moved: the L29-31 token resolution, the #57 message, --upload-file, the 201/409/other
status handling, the stderr dump and exit 1 are byte-identical.

Acceptance criteria — all seven met

#	criterion	evidence
1	sentinel zero times in curl argv	stub records full argv; test/publish-deb.test.js:113, plus :119-:120 for stdout/stderr
2	-H @<path> in the script temp dir, exact contents, mode 0600	:114 proves the dir is a child of RUNNER_TEMP; :116 the exact Authorization: token <sentinel>\n; :117 the mode
3	temp dir gone, no /tmp/stoke-publish-response.*, on 201 / failure / stub-exit-non-zero	assertCleaned :87-:92, called from all six tests; remainingTempEntries reads RUNNER_TEMP directly
4	201, 409 and failure-stderr transcripts byte-identical	:128, :137, :147 — full-string equality, not match
5	guard undisturbed	:94-:106 empty token → exit 1 with #57's message and ordering; every other test passes a non-empty one
6	diff is exactly three files	git diff --name-only 2230ca25...HEAD → changelog.d/62.md, scripts/publish-deb.sh, test/publish-deb.test.js
7	npm test passes, ci / test green on the head	147/147 locally after npm ci; ci / test and labels / labels both success on 4d17b8b via the commit status API

What I ran

• The failure cases actually fail. New tests against base 2230ca25: 2/6 fail — test 2 (the
  sentinel is in argv) and test 6 (curl exit 7 leaves /tmp/stoke-publish-response.<pid>). This
  matches the TDD evidence in the PR body, independently reproduced.
• Mutation checks on the head, to prove the new assertions discriminate rather than pass by
  construction:
  • delete the trap ... EXIT line → 5/6 fail, remainingTempEntries reporting the orphaned
    stoke-publish.XXXXXX directory by name;
  • umask 022 + chmod 0644 → test 2 fails on headerMode;
  • restore the inline -H "Authorization: token $TOKEN" while keeping the temp dir → test 2 fails
    on the argv assertion.
    Worktree restored clean after each.
• curl -H @file semantics, live. Against a loopback HTTP server, curl -sS -o … -w '%{http_code}' -X PUT -H @<file> --upload-file … sent authorization: token SENTINEL-abc123 and returned 201.
  The form works alongside -X PUT/--upload-file, which the stub cannot show.
• bash -n and shellcheck clean on scripts/publish-deb.sh; npm run check:governance passes
  (4 identities, 5 scope rows). changelog.d/62.md matches the wording and (#N). shape of its
  neighbours 57.md and 60.md.

What I could not verify

• No real upload. curl is stubbed in the tests by design (#62's test plan says no network), and
  I did not publish a .deb to the registry. Criterion 2's "the registry accepted it" half rests on
  the live loopback check above plus the issue's own 401/200 measurement, not on a real publish.
• The release container's curl. My check used curl 8.14.1 in this box. The deb job runs in
  node:22-bookworm (curl 7.88); -H @file has existed since curl 7.55, so it is present there, but
  I did not run that image. CI's green ci / test does not cover it either — nothing in the test
  suite invokes the real curl.
• Signal paths. Only normal exits are exercised (0, 1, and curl's 7). The claim that the EXIT trap
  also covers SIGINT/SIGTERM is untested here; it is the same exposure publish-release.sh already
  carries, so this PR does not widen it.

Non-blocking notes

1. The local config token still enters node's argv, at the lines #62 deliberately froze. L29
   always runs node -e '…loadConfig()…' and L30-31 pass the resulting CONFIG_JSON — which contains
   token — as an argument to node -pe. I confirmed it with a PATH-stubbed node that logs argv:
   the sentinel appears twice. This is not a regression and not this PR's job — the issue says
   L29-31 "stay exactly as they are", and I am not re-litigating that. It is CI-harmless (no stoke
   config on the runner, so CONFIG_JSON is empty there) and local-only. Worth a proposal, since it
   is the same defect class in the same file.
2. A newline inside $TOKEN would now inject extra headers. -H @file reads one header per
   line — verified: a two-line file produced both authorization and x-injected. Real Forgejo
   tokens cannot contain one, and the old inline form was merely malformed rather than safe, so this
   is a curiosity, not a defect.
3. The PR body's last acceptance checkbox ("npm test passes and ci / test is green") is still
   unchecked; both are now green on this head. Tick it at your discretion.

Approving. Nits 1-3 are yours to take or leave.

**Approve** at `4d17b8b1131e921dea43d6f04c3d45bb028d39e1`. The change is exactly #62's spec: one trap-owned temp directory created after the L33 guard, the authorization header written there with `printf` (a bash builtin — no argv exposure of its own) at mode `0600`, `curl -H @"$HEADER_FILE"`, the response body moved into the same directory, and both hand-rolled `rm -f /tmp/stoke-publish-response.$$` calls gone. It mirrors `publish-release.sh` L19-31 line for line, which is what #54 decided and what the issue asked for. Nothing in the frozen surface moved: the L29-31 token resolution, the #57 message, `--upload-file`, the 201/409/other status handling, the stderr dump and `exit 1` are byte-identical. ## Acceptance criteria — all seven met | # | criterion | evidence | |---|---|---| | 1 | sentinel zero times in curl argv | stub records full argv; `test/publish-deb.test.js:113`, plus `:119-:120` for stdout/stderr | | 2 | `-H @<path>` in the script temp dir, exact contents, mode `0600` | `:114` proves the dir is a child of `RUNNER_TEMP`; `:116` the exact `Authorization: token <sentinel>\n`; `:117` the mode | | 3 | temp dir gone, no `/tmp/stoke-publish-response.*`, on 201 / failure / stub-exit-non-zero | `assertCleaned` `:87-:92`, called from all six tests; `remainingTempEntries` reads `RUNNER_TEMP` directly | | 4 | 201, 409 and failure-stderr transcripts byte-identical | `:128`, `:137`, `:147` — full-string equality, not `match` | | 5 | guard undisturbed | `:94-:106` empty token → exit 1 with #57's message and ordering; every other test passes a non-empty one | | 6 | diff is exactly three files | `git diff --name-only 2230ca25...HEAD` → `changelog.d/62.md`, `scripts/publish-deb.sh`, `test/publish-deb.test.js` | | 7 | `npm test` passes, `ci / test` green on the head | 147/147 locally after `npm ci`; `ci / test` and `labels / labels` both `success` on `4d17b8b` via the commit status API | ## What I ran - **The failure cases actually fail.** New tests against base `2230ca25`: **2/6 fail** — test 2 (the sentinel is in argv) and test 6 (curl exit 7 leaves `/tmp/stoke-publish-response.<pid>`). This matches the TDD evidence in the PR body, independently reproduced. - **Mutation checks on the head, to prove the new assertions discriminate rather than pass by construction:** - delete the `trap ... EXIT` line → **5/6 fail**, `remainingTempEntries` reporting the orphaned `stoke-publish.XXXXXX` directory by name; - `umask 022` + `chmod 0644` → test 2 fails on `headerMode`; - restore the inline `-H "Authorization: token $TOKEN"` while keeping the temp dir → test 2 fails on the argv assertion. Worktree restored clean after each. - **`curl -H @file` semantics, live.** Against a loopback HTTP server, `curl -sS -o … -w '%{http_code}' -X PUT -H @<file> --upload-file …` sent `authorization: token SENTINEL-abc123` and returned `201`. The form works alongside `-X PUT`/`--upload-file`, which the stub cannot show. - `bash -n` and **`shellcheck`** clean on `scripts/publish-deb.sh`; `npm run check:governance` passes (4 identities, 5 scope rows). `changelog.d/62.md` matches the wording and `(#N).` shape of its neighbours `57.md` and `60.md`. ## What I could not verify - **No real upload.** `curl` is stubbed in the tests by design (#62's test plan says no network), and I did not publish a `.deb` to the registry. Criterion 2's "the registry accepted it" half rests on the live loopback check above plus the issue's own 401/200 measurement, not on a real publish. - **The release container's curl.** My check used curl **8.14.1** in this box. The `deb` job runs in `node:22-bookworm` (curl 7.88); `-H @file` has existed since curl 7.55, so it is present there, but I did not run that image. CI's green `ci / test` does not cover it either — nothing in the test suite invokes the real curl. - **Signal paths.** Only normal exits are exercised (0, 1, and curl's 7). The claim that the EXIT trap also covers SIGINT/SIGTERM is untested here; it is the same exposure `publish-release.sh` already carries, so this PR does not widen it. ## Non-blocking notes 1. **The local config token still enters `node`'s argv, at the lines #62 deliberately froze.** L29 always runs `node -e '…loadConfig()…'` and L30-31 pass the resulting `CONFIG_JSON` — which contains `token` — as an argument to `node -pe`. I confirmed it with a PATH-stubbed `node` that logs argv: the sentinel appears **twice**. This is not a regression and not this PR's job — the issue says L29-31 "stay exactly as they are", and I am not re-litigating that. It is CI-harmless (no stoke config on the runner, so `CONFIG_JSON` is empty there) and local-only. Worth a proposal, since it is the same defect class in the same file. 2. **A newline inside `$TOKEN` would now inject extra headers.** `-H @file` reads one header per line — verified: a two-line file produced both `authorization` and `x-injected`. Real Forgejo tokens cannot contain one, and the old inline form was merely malformed rather than safe, so this is a curiosity, not a defect. 3. The PR body's last acceptance checkbox ("`npm test` passes and `ci / test` is green") is still unchecked; both are now green on this head. Tick it at your discretion. Approving. Nits 1-3 are yours to take or leave.

kimi-bot-andresmgsl approved these changes 2026-09-04 03:22:27 +00:00

kimi-bot-andresmgsl left a comment

Member

Copy link

Approved at head 4d17b8b1. Reviewed the whole PR against #62's acceptance criteria, the repo's constraints, and the code, with runtime verification on a detached worktree (node v22.23.2).

Acceptance criteria — all seven verified:

1. Token out of argv. The stub-curl test records curl's real arguments and asserts the sentinel (deb-token-that-must-not-enter-argv) appears zero times. I reproduced this independently with my own PATH-stubbed bash curl against the real script: the sentinel occurs 0 times in the recorded argv; curl is invoked as -H @/…/stoke-publish.XXXXXX/authorization-header.
2. Header file. The test asserts the -H @<path> target sits inside the script's temp dir (itself under RUNNER_TEMP), contains exactly Authorization: token <sentinel>\n, and stats at mode 0600 at runtime. The script earns this with umask 077 plus an explicit chmod 0600, mirroring publish-release.sh's --token-file discipline that #54 specified.
3. Cleanup on every exit path. Asserted per-path (201, 409, HTTP failure, and stub exit 7) — temp dir removed, no new /tmp/stoke-publish-response.* left behind, and the two hand-rolled rm -f lines are gone. My probe confirmed RUNNER_TEMP is empty after all four runs; curl's exit status (7) propagates unchanged.
4. Byte-identical transcripts. Exact-string tests per status for the 201 and 409 messages and the failure path's stderr body dump; my probe's outputs match main's messages line for line.
5. Guard untouched. The empty-token test passes with #57's message verbatim and asserts curl is never invoked; the non-empty path still passes the guard.
6. Exact three-file diff. git diff main...HEAD --stat shows only scripts/publish-deb.sh, test/publish-deb.test.js, changelog.d/62.md.
7. npm test and CI green. Local full suite 147/147. Failure cases actually fail: running the new test file against main's script fails exactly 2/6 (the argv-leak and curl-failure-cleanup tests) for the intended reasons, matching the TDD evidence. ci / test (pull_request) and labels / labels both report success on this head.

Constraints: changelog fragment present and formatted like its 57.md/60.md siblings; scripts/check-governance.js green; bash -n clean; no workflow or third-party-action surface touched; CommonJS/node:test conventions kept.

Could not verify: the real upload against the live registry (needs the release secret; out of scope per the issue's own test plan — the stub never reaches the network). The 0600 mode is proven by the test's in-run stat, not by my probe, since the trap removes the file before exit — that is the required behavior, not a gap.

Converges with the panel; ready for the human.

Approved at head 4d17b8b1. Reviewed the whole PR against #62's acceptance criteria, the repo's constraints, and the code, with runtime verification on a detached worktree (node v22.23.2). **Acceptance criteria — all seven verified:** 1. **Token out of argv.** The stub-curl test records curl's real arguments and asserts the sentinel (`deb-token-that-must-not-enter-argv`) appears zero times. I reproduced this independently with my own PATH-stubbed bash `curl` against the real script: the sentinel occurs 0 times in the recorded argv; curl is invoked as `-H @/…/stoke-publish.XXXXXX/authorization-header`. 2. **Header file.** The test asserts the `-H @<path>` target sits inside the script's temp dir (itself under `RUNNER_TEMP`), contains exactly `Authorization: token <sentinel>\n`, and `stat`s at mode `0600` at runtime. The script earns this with `umask 077` plus an explicit `chmod 0600`, mirroring `publish-release.sh`'s `--token-file` discipline that #54 specified. 3. **Cleanup on every exit path.** Asserted per-path (201, 409, HTTP failure, and stub exit 7) — temp dir removed, no new `/tmp/stoke-publish-response.*` left behind, and the two hand-rolled `rm -f` lines are gone. My probe confirmed `RUNNER_TEMP` is empty after all four runs; curl's exit status (7) propagates unchanged. 4. **Byte-identical transcripts.** Exact-string tests per status for the 201 and 409 messages and the failure path's stderr body dump; my probe's outputs match `main`'s messages line for line. 5. **Guard untouched.** The empty-token test passes with #57's message verbatim and asserts curl is never invoked; the non-empty path still passes the guard. 6. **Exact three-file diff.** `git diff main...HEAD --stat` shows only `scripts/publish-deb.sh`, `test/publish-deb.test.js`, `changelog.d/62.md`. 7. **`npm test` and CI green.** Local full suite 147/147. Failure cases actually fail: running the new test file against `main`'s script fails exactly 2/6 (the argv-leak and curl-failure-cleanup tests) for the intended reasons, matching the TDD evidence. `ci / test (pull_request)` and `labels / labels` both report success on this head. **Constraints:** changelog fragment present and formatted like its `57.md`/`60.md` siblings; `scripts/check-governance.js` green; `bash -n` clean; no workflow or third-party-action surface touched; CommonJS/`node:test` conventions kept. **Could not verify:** the real upload against the live registry (needs the release secret; out of scope per the issue's own test plan — the stub never reaches the network). The `0600` mode is proven by the test's in-run `stat`, not by my probe, since the trap removes the file before exit — that is the required behavior, not a gap. Converges with the panel; ready for the human.

codex-bot-andresmgsl requested review from andres 2026-09-04 03:26:11 +00:00

codex-bot-andresmgsl added the

state:needs-human

label 2026-09-04 03:26:12 +00:00

forgejo-actions removed the

state:bots-reviewing

label 2026-09-04 03:26:30 +00:00

claude-bot-andresmgsl referenced this pull request 2026-09-04 03:52:00 +00:00

.forgejo/workflows/labels*.yml + .ceremony/ — re-pin the ceremony machinery past 0.6.1, and the eleven measured defects — seven the bump answered, three it shipped, one it carried across #36

andres merged commit 74e52b4210 into main 2026-09-04 06:16:38 +00:00

andres referenced this pull request from a commit 2026-09-04 06:16:40 +00:00

Merge pull request 'fix: keep Debian publish token out of curl argv' (#67) from build/62-publish-deb-token into main

Sign in to join this conversation.

Reviewers

No reviewers

glm-bot-andresmgsl

claude-bot-andresmgsl

kimi-bot-andresmgsl

andres

Labels

Clear labels   

attention

A demand is parked here for the assignee: pick up the thread, ack by removing this label

  

blocked

Waiting on another PR or issue to land first

  

blocker:ci-red

A check is failing — the agent owes a fix (not a rebase)

  

blocker:conflict

Does not merge — the branch conflicts and the agent owes a rebase

  

blocker:drill-pending

A release PR whose version has no drills/X.Y.Z.md record — correct but unevidenced

  

blocker:unrequested

Somebody still owes a verdict and nobody was asked for one

  

bug

Something isn't working

  

claimed

A builder owns it: assignee set, draft PR expected shortly

  

documentation

Improvements or additions to documentation

  

enhancement

New feature or request

  

epic

Organizes other issues via a dependency-ordered task list — builders never pick it

  

merge-next

Head of the merge queue — merge this one next (set by hand/agent, cleared here)

  

needs-ruling

A human decision is pending — question, options and a recommendation are in the comment

  

needs-triage

Did not come through triage — owes normalization into work or a reasoned refusal

  

offsite

Issue deliverable is a PR in another repository — claim clock paused

  

post-merge

Refs-linked PR merged; post-merge criteria remain and triage owns completion

  

ready

Triaged, spec complete, unblocked — a builder can start now and succeed

  

release

Release flow and version/packaging work

  

scope:ci

.forgejo/workflows/, .github/, and .ceremony/ — the test, label, and governance gates

  

scope:cli

src/ — the command surface (cli.js, api.js, config.js)

  

scope:docs

root Markdown and docs/ — the prose contract

  

scope:manifests

manifests/ — the fleet repo registry data

  

scope:packaging

scripts/, release workflow, package manifests, changelog, and fragments — release packaging and version surfaces

  

stale

No activity for 48h — needs a poke (sweep-managed)

  

state:addressing

All bots reviewed — coding agent owes the single reply + fixes

  

state:bots-reviewing

Waiting on the bot reviewers to finish the round

  

state:building

Pre-round: the builder is still building — draft is evidence for it, not the definition

  

state:needs-human

No blockers, all bots approve — waiting on the human reviewer

No labels

attention

blocked

blocker:ci-red

blocker:conflict

blocker:drill-pending

blocker:unrequested

bug

claimed

documentation

enhancement

epic

merge-next

needs-ruling

needs-triage

offsite

post-merge

ready

release

scope:ci

scope:cli

scope:docs

scope:manifests

scope:packaging

stale

state:addressing

state:bots-reviewing

state:building

state:needs-human

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

4 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/stoke#67

No description provided.