Skip to content

Prompt OIDC users to migrate trust policies - #382

Open
dannymidnight wants to merge 3 commits into
mainfrom
pb-3066-oidc-issuer-guidance
Open

Prompt OIDC users to migrate trust policies#382
dannymidnight wants to merge 3 commits into
mainfrom
pb-3066-oidc-issuer-guidance

Clarify OIDC compatibility guidance

4f40df6
Select commit
Loading
Failed to load commit list.
buildsworth-bk-app / Buildsworth review succeeded Aug 24, 2026 in 2m 16s

Review submitted — commented

Review summary

Change

PR #382 addresses PB-3066 by warning jobs when an action first receives a Buildkite OIDC token. The warning explains that downstream services must trust Buildkite's issuer and claims rather than GitHub's.

internal/runtime/oidc_token_service.go adds a fixed migration message and a per-service sync.Once. ServeHTTP emits the trusted warning only after the provider returns a token and the runtime registers that token with both masking mechanisms. Unauthorized requests, mint failures, and redaction failures return before the warning. Because each job creates its own ID-token service, the warning appears at most once per job while later token requests continue normally.

internal/runtime/oidc_token_service_test.go extends the wire-contract and end-to-end action tests to make two token requests, verify one annotation and one log warning, preserve failure statuses, and check that neither the endpoint credential nor minted token appears in the guidance. docs/compatibility.md documents the Buildkite and GitHub issuers, the warning boundary, and the distinction between Agent mint denial and downstream trust-policy rejection.

Review coverage

I read the full diff and all three changed files. I also traced:

  • ID-token service creation and per-job lifetime in internal/runtime/action_execution.go.
  • Per-action credential injection and revocation in internal/runtime/runtime.go.
  • commandProcessor.trustedWarning, its mutex and masking behavior, and final annotation collection.
  • The existing Node 16 trusted-warning pattern for comparison.
  • The PR and Linear issue PB-3066 descriptions, acceptance intent, commits, and conversation. There were no earlier reviews or comment threads to reconcile.

The implementation preserves the endpoint response and status handling. The warning is static, does not contain audience or token data, is serialized safely with concurrent requests, and is recorded before the successful response can reach the requesting action.

GitHub reported Buildkite build 1828 and its public-action and shell-smoke checks as passing. The Socket checks also passed. The documentation target at https://buildkite.com/docs/pipelines/security/oidc returned HTTP 200. Direct Buildkite CLI access to the pipeline was unavailable, so I verified CI through GitHub's check rollup.

I attempted targeted normal and race-enabled Go tests for the OIDC runtime paths, but this environment had neither mise nor go installed, so I could not run local tests. I did not claim those checks as executed.

Findings and verdict

I found no actionable code-review issues and posted no inline comments. The submitted review is COMMENT, not approval: the change modifies behavior in the OIDC token issuance path and therefore still needs human review.

Trigger source: automatic.