pm scripts pass the token in argv and swallow failed requests #133

New issue

Open

opened 2026-07-30 04:46:51 +01:00 by vnprc-agent · 0 comments

vnprc-agent commented

2026-07-30 04:46:51 +01:00

Contributor

Bring the two pm scripts' Forgejo calls up to the same standard as forge's api(), which keeps the token out of the process table and reports why a request failed instead of silently returning nothing.

Primary goals:

Token out of argv — the credential moves into a --config file the way forge already passes it, so it stops being readable from /proc/<pid>/cmdline by anything running as the same user.
No silently partial data — a failed request stops the run with a reason instead of yielding an empty string that the caller treats as a legitimate absence.
One shared helper — both scripts call the same request path rather than repeating the pattern a third time.

Current state

pm/render:70 and pm/integrity-check:111 both run:

curl -sf -H "Authorization: token $token" "$FORGE/api/v1/..." 2>/dev/null || true

Two independent defects sit in that one line.

The -H places the token in the command's arguments, where any process owned by the same user can read it out of /proc. This is the precise exposure forge avoids: api() passes the credential through --config process substitution, and the test harness's mock curl exits 99 if it ever sees an Authorization: header in argv, so the property is actively guarded there and unguarded here.

The -f plus 2>/dev/null || true discards the status, the body, and the exit code together, so an empty result is indistinguishable from a successful empty response. The consequences differ per caller and both are silent:

Caller	Behavior on a failed request
`pm/render:70-72`	`continue` skips the issue; the rendered board omits it with no indication anything was missed
`pm/integrity-check:111-113`	`break` ends pagination early; the check validates whichever pages arrived and reports success

The second is the more serious: a validator that quietly examines a subset and passes is the failure mode shell.md warns about, where a guard that cannot be shown to fail on sabotaged input does not count. A single 403, rate limit, or dropped connection turns the integrity check into a no-op that still exits 0.

Scope

In: replace both call sites with a shared helper that passes the token via --config, surfaces the HTTP status and the API's message on failure, and returns a non-zero code the callers act on rather than absorb. Decide explicitly per caller whether a failed page should abort or degrade, and if it degrades, say so on stderr.

Out: forge's own api(), which is handled in issue #100 and is the reference implementation to copy rather than re-derive. Out: any change to what the pm scripts render or check when every request succeeds.

Bring the two `pm` scripts' Forgejo calls up to the same standard as `forge`'s `api()`, which keeps the token out of the process table and reports why a request failed instead of silently returning nothing. Primary goals: - **Token out of argv** — the credential moves into a `--config` file the way `forge` already passes it, so it stops being readable from `/proc/<pid>/cmdline` by anything running as the same user. - **No silently partial data** — a failed request stops the run with a reason instead of yielding an empty string that the caller treats as a legitimate absence. - **One shared helper** — both scripts call the same request path rather than repeating the pattern a third time. ### Current state `pm/render:70` and `pm/integrity-check:111` both run: ```sh curl -sf -H "Authorization: token $token" "$FORGE/api/v1/..." 2>/dev/null || true ``` Two independent defects sit in that one line. The `-H` places the token in the command's arguments, where any process owned by the same user can read it out of `/proc`. This is the precise exposure `forge` avoids: `api()` passes the credential through `--config` process substitution, and the test harness's mock `curl` exits 99 if it ever sees an `Authorization:` header in argv, so the property is actively guarded there and unguarded here. The `-f` plus `2>/dev/null || true` discards the status, the body, and the exit code together, so an empty result is indistinguishable from a successful empty response. The consequences differ per caller and both are silent: | Caller | Behavior on a failed request | | --- | --- | | `pm/render:70-72` | `continue` skips the issue; the rendered board omits it with no indication anything was missed | | `pm/integrity-check:111-113` | `break` ends pagination early; the check validates whichever pages arrived and reports success | The second is the more serious: a validator that quietly examines a subset and passes is the failure mode `shell.md` warns about, where a guard that cannot be shown to fail on sabotaged input does not count. A single 403, rate limit, or dropped connection turns the integrity check into a no-op that still exits 0. ### Scope In: replace both call sites with a shared helper that passes the token via `--config`, surfaces the HTTP status and the API's message on failure, and returns a non-zero code the callers act on rather than absorb. Decide explicitly per caller whether a failed page should abort or degrade, and if it degrades, say so on stderr. Out: `forge`'s own `api()`, which is handled in issue #100 and is the reference implementation to copy rather than re-derive. Out: any change to what the `pm` scripts render or check when every request succeeds.

vnprc-agent referenced this issue

2026-07-30 04:47:16 +01:00

forge label create exits 22 without diagnostics #100