Surface the Agent API rejection reason on token 400s - #181
Conversation
A denied workflow or scoped token printed only "request was rejected". The reason existed solely in server-side telemetry, so diagnosing a denial needed Datadog access that no operator running a build has. The body is untrusted, so the reason is bounded, control characters collapse to spaces, and it is capped at 200 runes. Only 400 carries dynamic detail from the endpoint, so only that branch echoes it; the leak test now uses a distinct secret per status to keep proving that every other branch stays silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
The sanitizer still leaves one terminal-control path open; the concrete case is inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 14946, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
About buildsworth
Model: gpt-5.6-sol with xhigh thinking.
How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.
Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. To let it approve, grant L2 approval by mentioning @buildsworth-bk (see L2 approval grant):
- L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
- L2 — Standard risk (new UI, additive API fields, refactors). Approved only with an L2 grant; otherwise comment-only.
- L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.
| } | ||
| } | ||
|
|
||
| var githubTokenControlCharPattern = regexp.MustCompile(`[\x00-\x1F\x7F]+`) |
There was a problem hiding this comment.
Blocking: This range leaves the C1 controls (U+0080–U+009F) untouched. For example, JSON decoding a message with a U+009B character followed by 31m leaves CSI in the string; the error later written to stderr can therefore still inject terminal formatting from the untrusted response. Please collapse these controls too and cover a C1 value in the sanitization test.
1 participant
A denied GitHub token printed only
GitHub workflow token request was rejected. The reason lived only inDeveloper.transaction_contextserver-side, so an operator watching a build could not tell a stale app installation from a missing permission from a cross-org request.Real case that motivated this: a dead GitHub App installation id made GitHub return
404 POST /app/installations/<id>/access_tokens, which the Rails side reports asprovider_refusedwith a 400. Finding that took Datadog APM access. With this change the job log says it:The response body is untrusted, so the reason is read under the existing
githubTokenResponseLimit, control characters collapse to single spaces, and it is capped at 200 runes. Only the 400 branch echoes it — that is the only status where the endpoint returns dynamic detail; 401/403/404/503 are byte-for-byte unchanged.TestAgentGitHubTokensRejectsRedirectsAndUntrustedResponsespreviously shared oneghs_must_not_leakbody across every status and asserted it never appeared. Each status now gets its own distinct secret, so the no-leak assertion still holds for every other branch and a failure names the branch that leaked.Verified
go build ./...,go test ./internal/runtime/...,go vet.make lintfails on two pre-existingunusedfindings ininternal/runtime/test_platform_unix_test.go, unrelated to this change and fixed separately.🤖 Generated with Claude Code