diff --git a/docs/compatibility.md b/docs/compatibility.md index 94a7c6ab..152a7da2 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -1329,10 +1329,24 @@ Unsupported secret uses include: Action metadata cannot add secret authority to a plan. A secret used only by an optional action input becomes an empty value unless another field requires it. -Jobs with `id-token: write` expose the GitHub Actions `getIDToken()` contract to -host JavaScript actions, including those called by composite actions. The -endpoint mints a Buildkite OIDC token for the requested audience. Cloud identity -providers must trust Buildkite's issuer and claims, not GitHub's. +Jobs with `id-token: write` let JavaScript actions that run directly on the +agent call `getIDToken()`. This includes JavaScript actions called by composite +actions. The call returns a Buildkite OIDC token for the requested audience. + +Buildkite issues these tokens. It does not use or imitate GitHub's issuer. +Buildkite's issuer is `https://agent.buildkite.com`; GitHub's is +`https://token.actions.githubusercontent.com`. Update the target service's OIDC +trust policy to trust Buildkite's issuer and claims instead of GitHub's. +See [Buildkite OIDC](https://buildkite.com/docs/pipelines/security/oidc). + +The job shows this migration warning after the first successful token request. +Later token requests do not repeat it. The warning does not change HTTP status +handling for failed token requests. A failed token request does not say that +Buildkite issued a token. + +`buildkite-gha` cannot tell whether the target service later rejects the token. +An Agent API 401 or 403 means Buildkite rejected the token request, not that the +target service rejected the token. `id-token: read`, `id-token: none`, and omitted permissions do not expose the endpoint. Repository tests cover the wire contract; hosted runtime proof remains diff --git a/internal/runtime/oidc_token_service.go b/internal/runtime/oidc_token_service.go index 72480256..ec19a56c 100644 --- a/internal/runtime/oidc_token_service.go +++ b/internal/runtime/oidc_token_service.go @@ -23,7 +23,10 @@ import ( "github.com/buildkite/buildkite-gha/internal/useragent" ) -const oidcTokenResponseLimit = 64 << 10 +const ( + oidcTokenResponseLimit = 64 << 10 + oidcIssuerMigrationWarn = "Buildkite issued this job an OIDC token with issuer https://agent.buildkite.com, not GitHub's https://token.actions.githubusercontent.com. Update the target service's OIDC trust policy from GitHub's issuer and claims to Buildkite's issuer and claims. See https://buildkite.com/docs/pipelines/security/oidc." +) var oidcTokenPattern = regexp.MustCompile(`^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$`) @@ -195,6 +198,7 @@ type idTokenService struct { provider OIDCTokenProvider redactor Redactor processor *commandProcessor + warning sync.Once mu sync.RWMutex authHashes map[[sha256.Size]byte]struct{} } @@ -292,6 +296,7 @@ func (s *idTokenService) ServeHTTP(w http.ResponseWriter, request *http.Request) http.Error(w, "could not protect actions ID token", http.StatusInternalServerError) return } + s.warning.Do(func() { s.processor.trustedWarning(oidcIssuerMigrationWarn) }) w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(struct { Value string `json:"value"` diff --git a/internal/runtime/oidc_token_service_test.go b/internal/runtime/oidc_token_service_test.go index 1356f52c..382e934a 100644 --- a/internal/runtime/oidc_token_service_test.go +++ b/internal/runtime/oidc_token_service_test.go @@ -135,7 +135,8 @@ func (p *testOIDCTokenProvider) OIDCToken(ctx context.Context, audience string) func TestIDTokenServiceWireContract(t *testing.T) { provider := &testOIDCTokenProvider{token: "header.payload.signature", requireLiveContext: true} redactor := &testRedactor{} - processor := newCommandProcessor(&bytes.Buffer{}, &bytes.Buffer{}) + stderr := &bytes.Buffer{} + processor := newCommandProcessor(&bytes.Buffer{}, stderr) service, err := startIDTokenService(t.Context(), provider, redactor, processor) if err != nil { t.Fatal(err) @@ -154,15 +155,24 @@ func TestIDTokenServiceWireContract(t *testing.T) { if unauthorized.StatusCode != http.StatusUnauthorized || len(provider.audiences) != 0 { t.Fatalf("unauthorized request = %d, provider calls %#v", unauthorized.StatusCode, provider.audiences) } - request, err := http.NewRequest(http.MethodGet, env["ACTIONS_ID_TOKEN_REQUEST_URL"]+"&audience=sts.amazonaws.com", nil) - if err != nil { - t.Fatal(err) + warnings, _, _, _ := processor.workflowCommandAnnotations() + if warnings != "" || stderr.Len() != 0 { + t.Fatalf("unauthorized request emitted guidance: annotation = %q, stderr = %q", warnings, stderr) } - request.Header.Set("Authorization", "Bearer "+env["ACTIONS_ID_TOKEN_REQUEST_TOKEN"]) - response, err := http.DefaultClient.Do(request) - if err != nil { - t.Fatal(err) + authorizedRequest := func(audience string) *http.Response { + t.Helper() + request, err := http.NewRequest(http.MethodGet, env["ACTIONS_ID_TOKEN_REQUEST_URL"]+"&audience="+audience, nil) + if err != nil { + t.Fatal(err) + } + request.Header.Set("Authorization", "Bearer "+env["ACTIONS_ID_TOKEN_REQUEST_TOKEN"]) + response, err := http.DefaultClient.Do(request) + if err != nil { + t.Fatal(err) + } + return response } + response := authorizedRequest("sts.amazonaws.com") defer func() { _ = response.Body.Close() }() var body struct { Value string `json:"value"` @@ -170,19 +180,37 @@ func TestIDTokenServiceWireContract(t *testing.T) { if err := json.NewDecoder(response.Body).Decode(&body); err != nil { t.Fatal(err) } - if response.StatusCode != http.StatusOK || body.Value != provider.token || len(provider.audiences) != 1 || provider.audiences[0] != "sts.amazonaws.com" { + if response.StatusCode != http.StatusOK || body.Value != provider.token { t.Fatalf("response/provider = %d %#v / %#v", response.StatusCode, body, provider.audiences) } - if len(redactor.values) != 2 || redactor.values[0] != env["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] || redactor.values[1] != provider.token { + second := authorizedRequest("second-audience") + _ = second.Body.Close() + if second.StatusCode != http.StatusOK || len(provider.audiences) != 2 || provider.audiences[0] != "sts.amazonaws.com" || provider.audiences[1] != "second-audience" { + t.Fatalf("second response/provider = %d / %#v", second.StatusCode, provider.audiences) + } + if len(redactor.values) != 3 || redactor.values[0] != env["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] || redactor.values[1] != provider.token || redactor.values[2] != provider.token { t.Fatalf("redactions = %#v", redactor.values) } + warnings, truncated, _, _ := processor.workflowCommandAnnotations() + if truncated || strings.Count(warnings, "Buildkite issued this job an OIDC token") != 1 || !strings.Contains(warnings, "https://agent.buildkite.com") || !strings.Contains(warnings, "https://buildkite.com/docs/pipelines/security/oidc") { + t.Fatalf("OIDC guidance annotation = %q, truncated = %v", warnings, truncated) + } + if strings.Count(stderr.String(), oidcIssuerMigrationWarn) != 1 { + t.Fatalf("OIDC guidance log = %q", stderr) + } + for _, secret := range []string{env["ACTIONS_ID_TOKEN_REQUEST_TOKEN"], provider.token} { + if strings.Contains(warnings, secret) || strings.Contains(stderr.String(), secret) { + t.Fatalf("OIDC guidance leaked secret %q", secret) + } + } } func TestIDTokenServicePreservesPermanentMintFailureStatus(t *testing.T) { for _, status := range []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound, http.StatusUnprocessableEntity} { t.Run(http.StatusText(status), func(t *testing.T) { provider := &testOIDCTokenProvider{err: oidcTokenStatusError(status)} - service, err := startIDTokenService(t.Context(), provider, &testRedactor{}, newCommandProcessor(&bytes.Buffer{}, &bytes.Buffer{})) + processor := newCommandProcessor(&bytes.Buffer{}, &bytes.Buffer{}) + service, err := startIDTokenService(t.Context(), provider, &testRedactor{}, processor) if err != nil { t.Fatal(err) } @@ -205,6 +233,10 @@ func TestIDTokenServicePreservesPermanentMintFailureStatus(t *testing.T) { if response.StatusCode != status || len(provider.audiences) != 1 { t.Fatalf("request = HTTP %d with %d mint calls, want HTTP %d with one call", response.StatusCode, len(provider.audiences), status) } + warnings, _, _, _ := processor.workflowCommandAnnotations() + if warnings != "" { + t.Fatalf("failed mint emitted issued-token guidance: %q", warnings) + } }) } } @@ -265,7 +297,11 @@ const endpoint = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL); if (!endpoint.search) throw new Error("ACTIONS_ID_TOKEN_REQUEST_URL must already contain a query string"); if (process.env.NO_PROXY !== "upper.example,127.0.0.1") throw new Error("NO_PROXY does not preserve the proxy bypass list"); if (process.env.no_proxy !== "lower.example,127.0.0.1") throw new Error("no_proxy does not preserve the proxy bypass list"); -(async () => fs.writeFileSync(process.env.MARKER, await core.getIDToken("sts.amazonaws.com")))().catch(error => { console.error(error); process.exitCode = 1; }); +(async () => { + const first = await core.getIDToken("sts.amazonaws.com"); + const second = await core.getIDToken("second-audience"); + fs.writeFileSync(process.env.MARKER, first + "\n" + second); +})().catch(error => { console.error(error); process.exitCode = 1; }); `) marker := filepath.Join(workspace, "token") lockID := "a-0123456789abcdef" @@ -285,9 +321,15 @@ if (process.env.no_proxy !== "lower.example,127.0.0.1") throw new Error("no_prox if err != nil { t.Fatal(err) } - if string(contents) != provider.token || len(provider.audiences) != 1 || provider.audiences[0] != "sts.amazonaws.com" { + if string(contents) != provider.token+"\n"+provider.token || len(provider.audiences) != 2 || provider.audiences[0] != "sts.amazonaws.com" || provider.audiences[1] != "second-audience" { t.Fatalf("token/audiences = %q / %#v", contents, provider.audiences) } + if strings.Count(result.WarningAnnotations, "Buildkite issued this job an OIDC token") != 1 || !strings.Contains(result.WarningAnnotations, "https://agent.buildkite.com") || !strings.Contains(result.WarningAnnotations, "https://buildkite.com/docs/pipelines/security/oidc") { + t.Fatalf("RunJob() OIDC guidance annotation = %q", result.WarningAnnotations) + } + if strings.Contains(result.WarningAnnotations, provider.token) { + t.Fatalf("RunJob() OIDC guidance annotation leaked token: %q", result.WarningAnnotations) + } } func TestNodePostActionUsesIDTokenServiceAfterJobCancellation(t *testing.T) {