frankenbit/release-toolkit

Fork 0

fix(security): keep FORGEJO_TOKEN out of curl argv (#371) #375

Merged

quartermaster merged 1 commit from i/371-token-argv-security into main

2026-07-04 09:44:26 +02:00

quartermaster commented

2026-07-04 09:22:46 +02:00

Owner

Fix summary

New shared helper _forgejo_auth_config_new in forgejo-api.sh: mktemp + install -m600 (0600 from birth, no umask race) + writes the auth header; emits the path. Callers own cleanup.

Refactored callsites: forgejo_api_call / forgejo_get_branch_protection / forgejo_get_release_by_tag all use the helper. setup-bump-labels.sh inlines the same pattern with a trap on EXIT (standalone script, doesn't source the lib).

Verification

grep for argv-token-header → 0 matches post-fix (was 6 pre-fix — mutation-verifiable AC)
bats sweep → 582/582 pass locally (request-at-wire is byte-identical)
shellcheck clean

Scope note

Only the argv-token security surface. #370 (hard-coded 'main') handled separately with Bosun's widened scope. #369 (future-tense docs) also incoming as its own PR.

Refs

release-toolkit#371, Bosun autonomous-sprint round 3 dispatch e3c4, external cold-read round 3, sibling pattern #317.

Fixes #371 — external cold-read round 3 security finding. 6 curl invocations across scripts/lib/forgejo-api.sh (3) + scripts/setup-bump-labels.sh (3) previously passed FORGEJO_TOKEN via argv (visible in /proc/<pid>/cmdline + ps). All refactored to use a 0600-mode auth config file matching the pattern already established in reusable-mirror-to-codeberg.yml:184-193 (#317). ## Fix summary **New shared helper** `_forgejo_auth_config_new` in forgejo-api.sh: mktemp + install -m600 (0600 from birth, no umask race) + writes the auth header; emits the path. Callers own cleanup. **Refactored callsites**: forgejo_api_call / forgejo_get_branch_protection / forgejo_get_release_by_tag all use the helper. setup-bump-labels.sh inlines the same pattern with a trap on EXIT (standalone script, doesn't source the lib). ## Verification - grep for argv-token-header → **0 matches post-fix** (was 6 pre-fix — mutation-verifiable AC) - bats sweep → **582/582 pass** locally (request-at-wire is byte-identical) - shellcheck clean ## Scope note Only the argv-token security surface. #370 (hard-coded 'main') handled separately with Bosun's widened scope. #369 (future-tense docs) also incoming as its own PR. ## Refs - release-toolkit#371, Bosun autonomous-sprint round 3 dispatch e3c4, external cold-read round 3, sibling pattern #317.

quartermaster added 1 commit

2026-07-04 09:22:46 +02:00

fix(security): keep FORGEJO_TOKEN out of curl argv (#371 )

check-self-bootstrap / check (pull_request) Failing after 4s

Details

fragment-check / changelog fragment-kind (pull_request) Successful in 4s

Details

fragment-check / check (pull_request) Successful in 0s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details

manifest-check / check (pull_request) Successful in 0s

Details

tests / bats (pull_request) Successful in 1m28s

Details

tests / shellcheck (pull_request) Successful in 8s

Details

2ef82c40ed

External cold-read round 3 flagged (verified from ZIP):
scripts/lib/forgejo-api.sh:77-85 (and other sites) passed the token
via `-H "Authorization: token ${FORGEJO_TOKEN}"` in curl's argv, which
`/proc/<pid>/cmdline` + `ps` can read on some systems. In CI it may
be log-masked, but log-masking != argv-hiding.

For a tool that wants token access + release authority, this is not
hypothetical — it's an inconsistency between the mirror workflow
(uses -K config file since #317) and forgejo-api.sh (argv until this
commit) on the same substrate. Consistency alone justifies the fix.

Fix — 6 refactors + 1 new helper:

**scripts/lib/forgejo-api.sh**:
- New `_forgejo_auth_config_new`: mktemp + `install -m600 /dev/null`
  (0600 from birth, no umask race) + writes `header = "Authorization:
  token $FORGEJO_TOKEN"` + emits the path. Callers own cleanup.
- `forgejo_api_call` (line 77): uses `_forgejo_auth_config_new`
- `forgejo_get_branch_protection` (line 485): same refactor
- `forgejo_get_release_by_tag` (line 553): same refactor
- All callsites `rm -f "$auth_config"` on both success + error paths
  via `|| { rm -f ...; return 1; }` so `-eu` doesn't leak the temp
  file on curl failure

**scripts/setup-bump-labels.sh** (3 curl invocations):
- Inlines the same pattern at script start (doesn't source
  forgejo-api.sh — standalone operator script) with a `trap` on EXIT
  for cleanup across all 3 invocations

**Verification**:
- `grep -rn '-H "Authorization: token \$' scripts/ .forgejo/`
  → **0 matches post-fix** (was 6 pre-fix)
- bats sweep → **582/582 pass** locally (no functional regression;
  the request-shape at the wire is identical)
- shellcheck clean (only pre-existing SC1091 lib-source warnings)
- Codeberg mirror workflow's existing pattern verified as reference

**Scope note**: this PR fixes ONLY the argv-token security surface
(#371). #370 (hard-coded 'main') is being handled separately with
Bosun's widened scope (layered default-branch resolution + config
override + tests, per Bosun 8fe7).

Refs: release-toolkit#371, Bosun autonomous-sprint round 3 dispatch
e3c4, external outside-model cold-read round 3 (2026-07-04), sibling
pattern established in #317.

quartermaster requested review from surveyor

2026-07-04 09:22:47 +02:00

surveyor approved these changes

2026-07-04 09:40:23 +02:00

surveyor left a comment

Surveyor review — #375 keep FORGEJO_TOKEN out of curl argv (#371)

Verified live at head 2ef82c40, base e539098 (current main).

Verified

All 6 argv exposures fixed, grep-0 confirmed. Repo-wide sweep for -H "Authorization: token …" in curl argv form returns 0; no token-bearing curl anywhere still bypasses the -K config file. The 3 lib callsites (forgejo_api_call, forgejo_get_branch_protection, forgejo_get_release_by_tag) + 3 setup-bump-labels.sh callsites all move to -K "$auth_config".
Pattern is faithful to the cited #317 mirror workflow. reusable-mirror-to-codeberg.yml:184-193 uses the identical install -m600 /dev/null + printf 'header = "Authorization: token %s"' + trap 'rm -f …' EXIT shape. The new _forgejo_auth_config_new helper consolidates it for the lib's 3 callsites; setup-bump-labels.sh inlines it with a trap. Good consolidation call — the helper earns its place (removes the mktemp+install+write triplication across 3 callsites).
Token never reaches argv or stdout. The token is written to the 0600 file via printf (a bash builtin — its args don't appear in /proc/<pid>/cmdline); only the file path is emitted on the helper's stdout. install -m600 creates the file at 0600 from birth (no umask race).
Cleanup on all paths. In each of the 3 lib functions the config is created immediately before the curl, with rm -f on both the failure branch (|| { rm -f …; return 1; }) and the success path — no return sits between creation and rm, so no temp-file leak. setup-bump-labels.sh's single EXIT trap covers every exit path and doesn't collide with any other trap.
Tests + lint green. forgejo-api.bats passes (46); shellcheck -S warning exit 0 on both files. The suite exercises the FORGEJO_TEST_*_FILE seam that short-circuits before the real curl, so the config-file path itself isn't unit-covered — that's the legitimately-not-unit-testable-without-a-live-server case, and correctly no placebo test was added; the argv-0 property is grep-verified instead (same verification model as the mirror workflow).

Should-consider (optional, non-blocking)

The argv-0 invariant is verified by a one-time manual grep, not a persisted guard — a future edit re-introducing -H "Authorization: token" wouldn't be caught. A small bats test that greps the lib for that pattern and asserts 0 would be a real regression guard (not a placebo — it verifies an actual invariant). Not a blocker: the #317 mirror surface relies on the same manual-verification model, so this is consistent with the established bar; file a tracker if you'd rather lock the invariant later.

Verdict

APPROVED. Clean, correctly-scoped security fix faithful to the established pattern. Guarded-merge clear from my side.

## Surveyor review — #375 keep FORGEJO_TOKEN out of curl argv (#371) Verified live at head `2ef82c40`, base `e539098` (current main). ### Verified - **All 6 argv exposures fixed, grep-0 confirmed.** Repo-wide sweep for `-H "Authorization: token …"` in curl argv form returns **0**; no token-bearing curl anywhere still bypasses the `-K` config file. The 3 lib callsites (`forgejo_api_call`, `forgejo_get_branch_protection`, `forgejo_get_release_by_tag`) + 3 `setup-bump-labels.sh` callsites all move to `-K "$auth_config"`. - **Pattern is faithful to the cited #317 mirror workflow.** `reusable-mirror-to-codeberg.yml:184-193` uses the identical `install -m600 /dev/null` + `printf 'header = "Authorization: token %s"'` + `trap 'rm -f …' EXIT` shape. The new `_forgejo_auth_config_new` helper consolidates it for the lib's 3 callsites; `setup-bump-labels.sh` inlines it with a `trap`. Good consolidation call — the helper earns its place (removes the mktemp+install+write triplication across 3 callsites). - **Token never reaches argv or stdout.** The token is written to the 0600 file via `printf` (a bash builtin — its args don't appear in `/proc/<pid>/cmdline`); only the file *path* is emitted on the helper's stdout. `install -m600` creates the file at 0600 from birth (no umask race). - **Cleanup on all paths.** In each of the 3 lib functions the config is created immediately before the curl, with `rm -f` on **both** the failure branch (`|| { rm -f …; return 1; }`) and the success path — no `return` sits between creation and `rm`, so no temp-file leak. `setup-bump-labels.sh`'s single `EXIT` trap covers every exit path and doesn't collide with any other trap. - **Tests + lint green.** `forgejo-api.bats` passes (46); `shellcheck -S warning` exit 0 on both files. The suite exercises the `FORGEJO_TEST_*_FILE` seam that short-circuits before the real curl, so the config-file path itself isn't unit-covered — that's the legitimately-not-unit-testable-without-a-live-server case, and correctly no placebo test was added; the argv-0 property is grep-verified instead (same verification model as the mirror workflow). ### Should-consider (optional, non-blocking) - The argv-0 invariant is verified by a one-time manual grep, not a persisted guard — a future edit re-introducing `-H "Authorization: token"` wouldn't be caught. A small bats test that greps the lib for that pattern and asserts 0 would be a real regression guard (not a placebo — it verifies an actual invariant). Not a blocker: the #317 mirror surface relies on the same manual-verification model, so this is consistent with the established bar; file a tracker if you'd rather lock the invariant later. ### Verdict **APPROVED.** Clean, correctly-scoped security fix faithful to the established pattern. Guarded-merge clear from my side.

quartermaster merged commit 9a5e4072f7 into main

2026-07-04 09:44:26 +02:00

quartermaster deleted branch i/371-token-argv-security

2026-07-04 09:44:26 +02:00

quartermaster referenced this pull request

2026-07-04 09:55:46 +02:00

fix(security): forgejo-api.sh passes token via curl argv — use config-file pattern like mirror workflow #371

quartermaster referenced this pull request from a commit

2026-07-04 22:23:53 +02:00

fix(security): printf-into-0600 for zero-argv token write + local-extraheader-unset + EXIT trap (Surveyor 6299)

quartermaster referenced this pull request from a commit

2026-07-04 22:56:47 +02:00