Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/safe-outputs-step-token-same-job-validation.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 39 additions & 0 deletions docs/adr/54632-compile-time-step-token-reference-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# ADR-54632: Compile-Time Validation of Same-Job Step-Output Token References

**Date**: 2026-08-21
**Status**: Accepted
**Deciders**: gh-aw maintainers

---

### Context

`safe-outputs.github-token` accepts expressions of the form `${{ steps.<id>.outputs.<name> }}`, which reference the output of a step that runs in the same job. GitHub Actions step outputs are strictly job-scoped: a step declared in job A cannot produce an output consumed by job B. The gh-aw workflow compiler previously did not enforce this constraint — it emitted a lock file containing the unresolvable reference, which then failed `actionlint` and produced an empty token at runtime. A workflow that minted the token via `safe-outputs.steps` (which runs *after* the `safe_outputs` job's checkout) or that omitted the minting step in the `conclusion` job compiled without error, then silently failed at runtime. There was no mechanism to surface the misconfiguration until the generated workflow actually executed.

### Decision

We add a compile-time validation pass (`validateSafeOutputStepTokenReferences` in `pkg/workflow/safe_outputs_step_token_validation.go`) as the final step of `buildJobs`. After all jobs are assembled, the compiler collects every step id referenced by a `steps.<id>.outputs.*` expression in any `github-token` field (global and per-output), then checks each consuming job to ensure that (a) the step with that id exists in the job and (b) it is declared before the first step that consumes the token. If either condition fails, compilation returns an error naming the job and the `pre-steps` frontmatter snippet needed to fix it. The check is scoped to YAML mapping-value positions so that references inside run scripts or prompt text do not trigger false positives.

### Alternatives Considered

#### Alternative 1: Auto-propagate the minting step into every consuming job

The compiler could detect that a job needs the minting step and inject it automatically, mirroring the existing `applyBuiltinJobPreSteps` pattern. This keeps author-facing configuration minimal. It was rejected because it would silently side-effect every consuming job with whatever the minting action does (OIDC token issuance, network calls, permissions) without the author explicitly opting in. It also hides a real configuration gap: if the author deploys the workflow to a context where the minting action is unavailable, the error would surface at runtime rather than at compile time.

#### Alternative 2: Emit a warning or annotation instead of a hard error

The compiler could allow the lock file to be emitted and annotate the affected lines or print a warning to stderr. This was rejected because the runtime consequence is a critical failure — `actionlint` rejects the generated lock file and the token is empty, causing downstream safe-output tool calls to fail. A warning does not prevent the broken lock file from being committed or deployed. A hard compile error ensures the configuration is fixed before any generated artifact is written.

### Consequences

#### Positive
- Authors receive an actionable compile-time error that names the exact job and provides the `pre-steps` snippet required to resolve the misconfiguration, eliminating a class of silent runtime failures.
- All 286 existing repository workflows continue to compile unchanged, confirming the validation is purely additive for correct configurations.

#### Negative
- Configurations that previously compiled (and silently failed at runtime) now fail at compile time, requiring authors to migrate `safe-outputs.steps` token-minting to `pre-steps` under the appropriate job.
- The validation is restricted to YAML mapping-value positions in the rendered job YAML; step references embedded in `run:` scripts or free-form text are not detected as consumers, which is a conservative scope that may miss some edge cases.

#### Neutral
- The new file introduces two index-based helper functions (`jobStepIDDeclarationIndex`, `jobStepOutputConsumptionIndex`) that operate on rendered job YAML strings; these are deliberately scoped to this validation pass rather than integrated into the broader step-ordering infrastructure.
- Documentation in `reference/safe-outputs.md` was updated to show a keyless OIDC minting example covering all three consuming jobs (`agent`, `safe_outputs`, `conclusion`), replacing a simpler single-job example that implied that pattern was sufficient.
48 changes: 41 additions & 7 deletions docs/src/content/docs/reference/safe-outputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1673,17 +1673,51 @@ safe-outputs:
- `needs.<job>.outputs.<name>`
- `steps.<id>.outputs.<name>`

The `steps.*.outputs.*` form is useful when the safe-outputs job mints a short-lived token in `pre-steps:` or `setup-steps:` and then reuses that token for `Process Safe Outputs` in the same job.
The `steps.*.outputs.*` form is useful when a short-lived token is minted inside the job that uses it, for example with a keyless OIDC token-minting action. Step outputs are only readable inside the job that produced them, so the minting step must be injected into **every** job that consumes the token: the `agent` job (top-level `pre-steps:`), the `safe_outputs` job and the `conclusion` job (`jobs.<job>.pre-steps:` or `jobs.<job>.setup-steps:`).

`pre-steps:` run before the job's checkout, git-credential and token-consuming steps, so the minted token is available everywhere it is needed. `safe-outputs.steps:` is not a valid place to mint such a token because it runs *after* the `safe_outputs` job checkout.

```yaml wrap
pre-steps:
- id: fetch_token
run: echo "token=${TOKEN}" >> "$GITHUB_OUTPUT"
permissions:
contents: read
id-token: write

pre-steps: # agent job
- name: Mint token
id: mint_token
uses: octo-sts/action@v1.1.1
with:
scope: ${{ github.repository }}
identity: my-policy

safe-outputs:
github-token: ${{ steps.fetch_token.outputs.token }}
create-pull-request:
```
github-token: ${{ steps.mint_token.outputs.token }}
push-to-pull-request-branch:

jobs:
safe_outputs:
permissions:
id-token: write
pre-steps:
- name: Mint token
id: mint_token
uses: octo-sts/action@v1.1.1
with:
scope: ${{ github.repository }}
identity: my-policy
conclusion:
permissions:
id-token: write
pre-steps:
- name: Mint token
id: mint_token
uses: octo-sts/action@v1.1.1
with:
scope: ${{ github.repository }}
identity: my-policy
```

The compiler fails compilation when a job consumes `${{ steps.<id>.outputs.* }}` but never declares a step with that id, or declares it after the first consumer, rather than emitting a lock file with an unresolvable reference.

### Using a GitHub App for Authentication (`github-app:`)

Expand Down
46 changes: 26 additions & 20 deletions pkg/workflow/compiler_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,30 +288,30 @@ func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error {

// Build safe outputs jobs if configured
if err := c.buildSafeOutputsJobs(data, string(constants.AgentJobName), markdownPath); err != nil {
return fmt.Errorf("failed to build safe outputs jobs: %w", err)
return fmt.Errorf("safe outputs jobs could not be built: %w. Check the safe-outputs configuration for valid job types", err)
}

// Build BinEval evals job if evals are declared in frontmatter.
if evalsJob, err := c.buildEvalsJob(data); err != nil {
return fmt.Errorf("failed to build evals job: %w", err)
return fmt.Errorf("evals job could not be built: %w. Check that the evals frontmatter section is a valid object", err)
} else if evalsJob != nil {
if err := c.jobManager.AddJob(evalsJob); err != nil {
return fmt.Errorf("failed to add evals job: %w", err)
return fmt.Errorf("evals job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}
}

// Apply jobs.<builtin-job>.pre-steps customizations to already-created built-in jobs
// before processing non-built-in custom jobs.
if err := c.applyBuiltinJobPreSteps(data); err != nil {
return fmt.Errorf("failed to apply built-in job pre-steps: %w", err)
return fmt.Errorf("built-in job pre-steps could not be applied: %w. Check that pre-steps is an array of valid step objects", err)
}

// Build additional custom jobs from frontmatter jobs section
if len(data.Jobs) > 0 {
compilerJobsLog.Printf("Building %d custom jobs from frontmatter", len(data.Jobs))
}
if err := c.buildCustomJobs(data, activationJobCreated); err != nil {
return fmt.Errorf("failed to build custom jobs: %w", err)
return fmt.Errorf("custom jobs could not be built: %w. Check the jobs section in frontmatter for valid job definitions", err)
}

// Build memory management jobs (repo-memory and cache-memory)
Expand All @@ -322,7 +322,7 @@ func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error {
// Apply additive jobs.<built-in>.needs augmentations once all jobs are created,
// so referenced custom/imported jobs can be validated against the final job set.
if err := c.applyBuiltinJobAugmentations(data); err != nil {
return fmt.Errorf("failed to apply built-in job needs augmentations: %w", err)
return fmt.Errorf("built-in job needs augmentations could not be applied: %w. Check that jobs referenced in needs actually exist in the workflow", err)
}

// Final pass: ensure conclusion job depends on ALL remaining workflow jobs.
Expand All @@ -337,6 +337,12 @@ func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error {
// to each job individually after all jobs have been created.
c.ensureOTLPOIDCJobPermissions(data)

// Final pass: same-job `steps.<id>.outputs.*` token expressions must be produced by a
// step of the job that consumes them, otherwise the token is empty at runtime.
if err := c.validateSafeOutputStepTokenReferences(data); err != nil {
return err
Comment on lines +342 to +343
}

compilerJobsLog.Print("Successfully built all jobs for workflow")
return nil
}
Expand Down Expand Up @@ -401,10 +407,10 @@ func (c *Compiler) buildPreActivationAndActivationJobs(data *WorkflowData, front
compilerJobsLog.Print("Building pre-activation job")
preActivationJob, err := c.buildPreActivationJob(data, needsPermissionCheck)
if err != nil {
return false, false, fmt.Errorf("failed to build %s job: %w", constants.PreActivationJobName, err)
return false, false, fmt.Errorf("%s job could not be built: %w. Check the activation-related frontmatter fields (roles, if, stop-time, etc.)", constants.PreActivationJobName, err)
}
if err := c.jobManager.AddJob(preActivationJob); err != nil {
return false, false, fmt.Errorf("failed to add %s job: %w", constants.PreActivationJobName, err)
return false, false, fmt.Errorf("%s job could not be added: %w. Check that no other job in the workflow reuses its name", constants.PreActivationJobName, err)
}
compilerJobsLog.Printf("Successfully added pre-activation job: %s", constants.PreActivationJobName)
preActivationJobCreated = true
Expand All @@ -422,10 +428,10 @@ func (c *Compiler) buildPreActivationAndActivationJobs(data *WorkflowData, front
compilerJobsLog.Print("Building activation job")
activationJob, err := c.buildActivationJob(data, preActivationJobCreated, workflowRunRepoSafety, lockFilename)
if err != nil {
return preActivationJobCreated, false, fmt.Errorf("failed to build activation job: %w", err)
return preActivationJobCreated, false, fmt.Errorf("activation job could not be built: %w. Check the workflow triggers and activation-related frontmatter fields", err)
}
if err := c.jobManager.AddJob(activationJob); err != nil {
return preActivationJobCreated, false, fmt.Errorf("failed to add activation job: %w", err)
return preActivationJobCreated, false, fmt.Errorf("activation job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}
compilerJobsLog.Print("Successfully added activation job")
activationJobCreated = true
Expand All @@ -439,10 +445,10 @@ func (c *Compiler) buildMainJobWrapper(data *WorkflowData, activationJobCreated
compilerJobsLog.Print("Building main job")
mainJob, err := c.buildMainJob(data, activationJobCreated)
if err != nil {
return fmt.Errorf("failed to build main job: %w", err)
return fmt.Errorf("main job could not be built: %w. Check the engine and steps configuration in frontmatter", err)
}
if err := c.jobManager.AddJob(mainJob); err != nil {
return fmt.Errorf("failed to add main job: %w", err)
return fmt.Errorf("main job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}
compilerJobsLog.Printf("Successfully added main job: %s", string(constants.AgentJobName))
return nil
Expand Down Expand Up @@ -495,7 +501,7 @@ func (c *Compiler) buildPushRepoMemoryJobWrapper(data *WorkflowData, threatDetec
compilerJobsLog.Print("Building push_repo_memory job")
pushRepoMemoryJob, err := c.buildPushRepoMemoryJob(data, threatDetectionEnabled)
if err != nil {
return "", fmt.Errorf("failed to build push_repo_memory job: %w", err)
return "", fmt.Errorf("push_repo_memory job could not be built: %w. Check the repo-memory configuration in frontmatter", err)
}

if pushRepoMemoryJob == nil {
Expand All @@ -507,7 +513,7 @@ func (c *Compiler) buildPushRepoMemoryJobWrapper(data *WorkflowData, threatDetec
// and its condition checks needs.detection.result == 'success'

if err := c.jobManager.AddJob(pushRepoMemoryJob); err != nil {
return "", fmt.Errorf("failed to add push_repo_memory job: %w", err)
return "", fmt.Errorf("push_repo_memory job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}

compilerJobsLog.Printf("Successfully added push_repo_memory job: %s", pushRepoMemoryJob.Name)
Expand All @@ -528,15 +534,15 @@ func (c *Compiler) buildUpdateCacheMemoryJobWrapper(data *WorkflowData, threatDe
compilerJobsLog.Print("Building update_cache_memory job")
updateCacheMemoryJob, err := c.buildUpdateCacheMemoryJob(data, threatDetectionEnabled)
if err != nil {
return "", fmt.Errorf("failed to build update_cache_memory job: %w", err)
return "", fmt.Errorf("update_cache_memory job could not be built: %w. Check the cache-memory configuration in frontmatter", err)
}

if updateCacheMemoryJob == nil {
return "", nil
}

if err := c.jobManager.AddJob(updateCacheMemoryJob); err != nil {
return "", fmt.Errorf("failed to add update_cache_memory job: %w", err)
return "", fmt.Errorf("update_cache_memory job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}

compilerJobsLog.Printf("Successfully added update_cache_memory job: %s", updateCacheMemoryJob.Name)
Expand All @@ -553,14 +559,14 @@ func (c *Compiler) buildPushExperimentsStateJobWrapper(data *WorkflowData) (stri
compilerJobsLog.Print("Building push_experiments_state job")
job, err := c.buildPushExperimentsStateJob(data)
if err != nil {
return "", fmt.Errorf("failed to build push_experiments_state job: %w", err)
return "", fmt.Errorf("push_experiments_state job could not be built: %w. Check the experiments configuration in frontmatter", err)
}
if job == nil {
return "", nil
}

if err := c.jobManager.AddJob(job); err != nil {
return "", fmt.Errorf("failed to add push_experiments_state job: %w", err)
return "", fmt.Errorf("push_experiments_state job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}

compilerJobsLog.Printf("Successfully added push_experiments_state job: %s", job.Name)
Expand All @@ -577,14 +583,14 @@ func (c *Compiler) buildPushEvalsStateJobWrapper(data *WorkflowData) (string, er
compilerJobsLog.Print("Building push_evals_state job")
job, err := c.buildPushEvalsStateJob(data)
if err != nil {
return "", fmt.Errorf("failed to build push_evals_state job: %w", err)
return "", fmt.Errorf("push_evals_state job could not be built: %w. Check the evals configuration in frontmatter", err)
}
if job == nil {
return "", nil
}

if err := c.jobManager.AddJob(job); err != nil {
return "", fmt.Errorf("failed to add push_evals_state job: %w", err)
return "", fmt.Errorf("push_evals_state job could not be added: %w. Check that no other job in the workflow reuses its name", err)
}

compilerJobsLog.Printf("Successfully added push_evals_state job: %s", job.Name)
Expand Down
36 changes: 34 additions & 2 deletions pkg/workflow/github_token_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,26 @@ tools:
}

func TestGitHubTokenValidationInSafeOutputs(t *testing.T) {
// preSteps mints the "fetch-token" step referenced by the same-job token
// expression below in every job that consumes it (agent and conclusion),
// so the token resolves in each job that produces/consumes it.
const preSteps = `pre-steps:
- id: fetch-token
run: echo "my-token=x" >> "$GITHUB_OUTPUT"
jobs:
safe_outputs:
pre-steps:
- id: fetch-token
run: echo "my-token=x" >> "$GITHUB_OUTPUT"
conclusion:
pre-steps:
- id: fetch-token
run: echo "my-token=x" >> "$GITHUB_OUTPUT"
`
tests := []struct {
name string
token string
preSteps string
expectError bool
}{
{
Expand All @@ -177,6 +194,7 @@ func TestGitHubTokenValidationInSafeOutputs(t *testing.T) {
{
name: "valid same-job step output token in safe-outputs",
token: "${{ steps.fetch-token.outputs.my-token }}",
preSteps: preSteps,
expectError: false,
},
{
Expand All @@ -196,7 +214,7 @@ on:
issues:
types: [opened]
engine: copilot
safe-outputs:
` + tt.preSteps + `safe-outputs:
github-token: ` + tt.token + `
create-issue:
---
Expand Down Expand Up @@ -226,9 +244,22 @@ safe-outputs:
}

func TestGitHubTokenValidationInIndividualSafeOutput(t *testing.T) {
// preSteps mints the "fetch-token" step referenced by the same-job token
// expression below in every job that consumes it (agent and safe_outputs),
// so the token resolves in each job that produces/consumes it.
const preSteps = `pre-steps:
- id: fetch-token
run: echo "my-token=x" >> "$GITHUB_OUTPUT"
jobs:
safe_outputs:
pre-steps:
- id: fetch-token
run: echo "my-token=x" >> "$GITHUB_OUTPUT"
`
tests := []struct {
name string
token string
preSteps string
expectError bool
}{
{
Expand All @@ -244,6 +275,7 @@ func TestGitHubTokenValidationInIndividualSafeOutput(t *testing.T) {
{
name: "valid same-job step output token in individual safe-output",
token: "${{ steps.fetch-token.outputs.my-token }}",
preSteps: preSteps,
expectError: false,
},
{
Expand All @@ -263,7 +295,7 @@ on:
issues:
types: [opened]
engine: copilot
safe-outputs:
` + tt.preSteps + `safe-outputs:
create-agent-session:
github-token: ` + tt.token + `
---
Expand Down
Loading
Loading