Skip to content

Surface the Agent API rejection reason on token 400s - #181

Open
JuanitoFatas wants to merge 1 commit into
mainfrom
surface-github-token-rejection-reason
Open

Surface the Agent API rejection reason on token 400s#181
JuanitoFatas wants to merge 1 commit into
mainfrom
surface-github-token-rejection-reason

Surface the Agent API rejection reason on token 400s

1277242
Select commit
Loading
Failed to load commit list.
buildsworth-bk-app / Buildsworth review succeeded Aug 12, 2026 in 3m 29s

Review submitted — changes requested

Review summary

Change reviewed

PR #181 surfaces Agent API rejection details when GitHub workflow token issuance returns HTTP 400. The client now reads a bounded error body, extracts the JSON message field when present (falling back to plain text), sanitizes and truncates the reason, and appends it to the operator-facing error. Other status branches keep their existing messages.

The tests give each response branch a distinct leak marker, verify that only a 400 response body is echoed, cover JSON and plain-text reasons, preserve the empty-body fallback and 503 behavior, and check ASCII control-character collapsing and the 200-rune cap.

Evidence examined

  • Read the full diff and both changed files: internal/runtime/github_token_service.go and internal/runtime/github_token_service_test.go.
  • Traced workflow-token errors through Runner.resolveWorkflowToken, job setup failure handling, and the run-job CLI stderr path. This confirms the rejection reason reaches an operator-visible log line.
  • Checked the separate scoped-token caller, which degrades token-mint failures to an anonymous-download warning.
  • Reviewed the repository's credential and token boundaries in docs/security.md and docs/compatibility.md.
  • Cross-checked the Agent API controller and API specs in buildkite/buildkite: 400 responses use a JSON message, including Repository provider refused to issue token, while rate-limit responses remain separate.
  • Verified the branch remained at reviewed commit 12772422a18761f3891a42b952a3fa946d2b06ea before submission and that no earlier review thread covered the finding.
  • git diff --check passed.
  • GitHub reported Buildkite build #846 and both Socket checks as passing. The dedicated buildsworth review check was still running during the review.
  • I could not run Go tests or gofmt locally because the documented Go/mise toolchain was not present in this environment. Direct Buildkite API queries were also unavailable to the configured token, so CI status came from the PR checks. No Linear issue ID appeared in the branch name, PR title, or PR description.

Finding and verdict

Submitted REQUEST_CHANGES with one blocking inline finding in internal/runtime/github_token_service.go.

The new sanitizer removes only ASCII C0 controls and DEL. Unicode C1 controls (U+0080U+009F) survive both JSON decoding and the replacement. In particular, a reason containing \u009b31m retains U+009B (CSI) and can still introduce terminal formatting when the error is written to stderr. This leaves an injection path at the untrusted-response boundary the change is intended to protect. The smallest correction is to collapse C1 controls as well and add a C1 case to the sanitization test.

The bounded response read, status-specific disclosure, structured-message extraction, and rune-based truncation otherwise keep the change narrow and avoid exposing bodies from the unchanged 401, 403, 404, 503, and unexpected-status paths.

Trigger source: automatic.