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
79 changes: 79 additions & 0 deletions docs/adr/27476-extend-safe-outputs-dependencies-via-needs-field.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# ADR-27476: Extend `safe_outputs` Job Dependencies via `safe-outputs.needs` Field

**Date**: 2026-04-21
**Status**: Draft
**Deciders**: pelikhan

---

## Part 1 — Narrative (Human-Friendly)

### Context

The consolidated `safe_outputs` job in gh-aw workflows had a hardcoded dependency set (`agent`, `activation`, optional `detection`, optional `unlock`). Some workflows need credentials supplied by a user-defined job (e.g., a `secrets_fetcher` that retrieves app IDs and private keys from a vault) before the `safe_outputs` job can run. Because the `safe_outputs` dependency list was not extensible, `needs.<custom-job>.outputs.*` expressions inside `safe-outputs` handler configs (notably `github-app`) referenced jobs that were not declared as upstream dependencies, causing `actionlint` lint failures and runtime breakage.

### Decision

We will add a `safe-outputs.needs` array field to the frontmatter schema that lets workflow authors declare explicit custom upstream jobs for the consolidated `safe_outputs` job. At compile time, the compiler merges these user-declared dependencies with the built-in set and deduplicates them. A validation pass rejects references to unknown jobs and built-in/control job names, providing actionable error messages.

### Alternatives Considered

#### Alternative 1: Rely Solely on Environment-Scoped Secrets

Workflows could store credentials in GitHub environment secrets, accessible to the `safe_outputs` job without any custom upstream job. This avoids the dependency wiring problem entirely. It was not chosen because it requires credentials to be pre-registered in the GitHub environment UI and cannot dynamically fetch or rotate secrets at workflow runtime — a real constraint for vault-backed or short-lived credentials.

#### Alternative 2: Auto-Detect Dependencies from Expression References

The compiler could parse expression strings inside `safe-outputs` config values (e.g., `${{ needs.secrets_fetcher.outputs.app_id }}`) and automatically infer the required `needs` entries. This approach was not chosen because parsing arbitrary GitHub Actions expression syntax inside YAML values is fragile, error-prone, and creates a tight coupling between the expression evaluator and the compiler. An explicit declaration (`safe-outputs.needs`) is simpler to validate, easier to understand, and immune to expression parsing edge cases.

### Consequences

#### Positive
- Unlocks the credential-fetcher pattern: workflows can now supply dynamic, vault-retrieved credentials to `safe_outputs` handlers such as `github-app`.
- Eliminates `actionlint` failures caused by undeclared `needs.*` references in the compiled GitHub Actions workflow.
- The deduplication logic prevents duplicate dependency entries regardless of how `needs` is populated (built-in vs. user-declared vs. imported).

#### Negative
- Authors must know which job names are reserved (built-in control jobs) and cannot accidentally use them as custom dependencies; this adds cognitive overhead and requires consulting documentation or error messages.
- Import merge behavior for `safe-outputs.needs` must be maintained in sync with other merge fields as the import system evolves.

#### Neutral
- A new validation function (`validateSafeOutputsNeeds`) is added to the compiler pipeline, following the same structure as the existing `validateSafeJobNeeds` validator.
- The schema change (`main_workflow_schema.json`) affects all schema-validation tooling and IDE integrations that consume the schema.

---

## Part 2 — Normative Specification (RFC 2119)

> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).

### Dependency Declaration

1. Workflow authors **MAY** declare additional upstream job dependencies for the consolidated `safe_outputs` job using the `safe-outputs.needs` array field in workflow frontmatter.
2. Each entry in `safe-outputs.needs` **MUST** be a string matching the pattern `^[a-zA-Z_][a-zA-Z0-9_-]*$`.
3. The `safe-outputs.needs` array **MUST** have `uniqueItems: true` enforced at the schema level; duplicate entries **SHOULD** be deduplicated silently at compile time.

### Compiler Behavior

1. The compiler **MUST** merge `safe-outputs.needs` entries with the built-in dependency set (`agent`, `activation`, and optionally `detection` and `unlock`) before writing the compiled `safe_outputs` job's `needs` list.
2. The compiler **MUST** deduplicate the final `needs` list so that no job name appears more than once.
3. The compiler **MUST** validate `safe-outputs.needs` entries before generating the compiled workflow and **MUST** reject invalid entries with an actionable error message.

### Validation Rules

1. A `safe-outputs.needs` entry **MUST NOT** reference a built-in or control job name (`agent`, `activation`, `pre_activation`, `pre-activation`, `conclusion`, `safe_outputs`, `safe-outputs`, `detection`, `unlock`, `push_repo_memory`, `update_cache_memory`).
2. A `safe-outputs.needs` entry **MUST NOT** reference a job name that is not declared in the workflow's top-level `jobs:` map.
3. When a violation of rules 1 or 2 is detected, the compiler **MUST** emit a compile-time error and **MUST NOT** produce a compiled workflow artifact.

### Import Merge Behavior

1. When a workflow imports another workflow, the importer's `safe-outputs.needs` **MUST** be merged with the imported workflow's `safe-outputs.needs` as a deduplicated union.
2. Import merge **MUST NOT** introduce duplicate entries into the merged `needs` list.

### Conformance

An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24701927396) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
32 changes: 32 additions & 0 deletions docs/src/content/docs/reference/safe-outputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,38 @@ safe-outputs:

Accepts a plain string or an object with `name` and optional `url`, consistent with the top-level `environment:` syntax.

### Safe Outputs Dependencies (`needs:`)

Extend the consolidated `safe_outputs` job dependencies with custom workflow jobs (for example, credential fetchers). `safe-outputs.needs` is merged with built-in dependencies (`agent`, `activation`, optional `detection`, optional `unlock`) and deduplicated.

```yaml wrap
jobs:
secrets_fetcher:
runs-on: ubuntu-latest
outputs:
app_id: ${{ steps.fetch.outputs.app_id }}
app_private_key: ${{ steps.fetch.outputs.app_private_key }}
steps:
- id: fetch
run: |
echo "app_id=123" >> "$GITHUB_OUTPUT"
echo "app_private_key=***" >> "$GITHUB_OUTPUT"

safe-outputs:
needs: [secrets_fetcher]
github-app:
app-id: ${{ needs.secrets_fetcher.outputs.app_id }}
private-key: ${{ needs.secrets_fetcher.outputs.app_private_key }}
```

Use the single `safe-outputs.needs` field for all explicit custom dependencies.

Validation rules:

- Values must reference workflow custom jobs from top-level `jobs:`
- Built-in jobs are rejected (`agent`, `activation`, `pre_activation`/`pre-activation`, `conclusion`, `safe_outputs`, `detection`, `unlock`, `push_repo_memory`, `update_cache_memory`)
- Unknown jobs fail compilation with an actionable error

### Text Sanitization (`allowed-domains:`, `allowed-github-references:`)

The text output by AI agents is automatically sanitized to prevent injection of malicious content and ensure safe rendering on GitHub. The auto-sanitization applied is: XML escaped, HTTPS only, domain allowlist (GitHub by default), 0.5MB/65k line limits, control char stripping.
Expand Down
72 changes: 72 additions & 0 deletions pkg/cli/compile_safe_outputs_needs_imports_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//go:build integration

package cli

import (
"os"
"os/exec"
"path/filepath"
"testing"

goyaml "github.com/goccy/go-yaml"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestCompileSafeOutputsNeedsMergedWithImports(t *testing.T) {
setup := setupIntegrationTest(t)
defer setup.cleanup()

srcMainPath := filepath.Join(projectRoot, "pkg/cli/workflows/test-safe-outputs-needs-imports.md")
srcImportPath := filepath.Join(projectRoot, "pkg/cli/workflows/shared/safe-outputs-needs-import.md")

dstMainPath := filepath.Join(setup.workflowsDir, "test-safe-outputs-needs-imports.md")
dstImportDir := filepath.Join(setup.workflowsDir, "shared")
dstImportPath := filepath.Join(dstImportDir, "safe-outputs-needs-import.md")

require.NoError(t, os.MkdirAll(dstImportDir, 0755), "Failed to create shared import directory")

srcMainContent, err := os.ReadFile(srcMainPath)
require.NoError(t, err, "Failed to read source workflow fixture")
require.NoError(t, os.WriteFile(dstMainPath, srcMainContent, 0644), "Failed to write main workflow fixture")

srcImportContent, err := os.ReadFile(srcImportPath)
require.NoError(t, err, "Failed to read source imported workflow fixture")
require.NoError(t, os.WriteFile(dstImportPath, srcImportContent, 0644), "Failed to write imported workflow fixture")

cmd := exec.Command(setup.binaryPath, "compile", dstMainPath)
output, err := cmd.CombinedOutput()
require.NoError(t, err, "Compile failed:\n%s", string(output))

lockFilePath := filepath.Join(setup.workflowsDir, "test-safe-outputs-needs-imports.lock.yml")
lockContent, err := os.ReadFile(lockFilePath)
require.NoError(t, err, "Failed to read lock file")

var workflow map[string]any
require.NoError(t, goyaml.Unmarshal(lockContent, &workflow), "Lock file should be valid YAML")

jobs, ok := workflow["jobs"].(map[string]any)
require.True(t, ok, "Compiled workflow should include jobs map")
safeOutputsJob, ok := jobs["safe_outputs"].(map[string]any)
require.True(t, ok, "Compiled workflow should include safe_outputs job")
needsRaw, ok := safeOutputsJob["needs"].([]any)
require.True(t, ok, "safe_outputs job should include needs array")

needs := make([]string, 0, len(needsRaw))
for _, need := range needsRaw {
require.IsType(t, "", need, "safe_outputs needs entries should be strings")
needs = append(needs, need.(string))
}

assert.Contains(t, needs, "main_job", "safe_outputs.needs should include top-level custom dependency")
assert.Contains(t, needs, "imported_job", "safe_outputs.needs should include imported custom dependency")
assert.Contains(t, needs, "shared_job", "safe_outputs.needs should include imported custom dependency")

importedJobCount := 0
for _, need := range needs {
if need == "imported_job" {
importedJobCount++
}
}
assert.Equal(t, 1, importedJobCount, "safe_outputs.needs should dedupe duplicate custom dependencies")
}
6 changes: 6 additions & 0 deletions pkg/cli/workflows/shared/safe-outputs-needs-import.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
safe-outputs:
needs:
- shared_job
- imported_job
---
27 changes: 27 additions & 0 deletions pkg/cli/workflows/test-safe-outputs-needs-imports.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
on:
workflow_dispatch:
imports:
- ./shared/safe-outputs-needs-import.md
safe-outputs:
needs:
- main_job
- imported_job
jobs:
main_job:
runs-on: ubuntu-latest
steps:
- run: echo "main"
imported_job:
runs-on: ubuntu-latest
steps:
- run: echo "imported"
shared_job:
runs-on: ubuntu-latest
steps:
- run: echo "shared"
---

# Safe outputs needs imports fixture

Verify that safe-outputs.needs from imports is merged with top-level needs.
1 change: 1 addition & 0 deletions pkg/parser/schema_compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ var safeOutputMetaFields = map[string]bool{
"jobs": true,
"runs-on": true,
"messages": true,
"needs": true,
}

// GetSafeOutputTypeKeys returns the list of safe output type keys from the embedded main workflow schema.
Expand Down
1 change: 1 addition & 0 deletions pkg/parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ func TestGetSafeOutputTypeKeys(t *testing.T) {
"jobs",
"runs-on",
"messages",
"needs",
}

for _, meta := range metaFields {
Expand Down
12 changes: 12 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -8473,6 +8473,18 @@
"description": "Concurrency group for the safe-outputs job. When set, the safe-outputs job will use this concurrency group with cancel-in-progress: false. Supports GitHub Actions expressions.",
"examples": ["my-workflow-safe-outputs", "safe-outputs-${{ github.repository }}"]
},
"needs": {
"type": "array",
"description": "Explicit additional custom workflow jobs that the consolidated safe_outputs job should depend on.",
"items": {
"type": "string",
"pattern": "^[a-zA-Z_][a-zA-Z0-9_-]*$"
},
"additionalItems": false,
"uniqueItems": true,
"default": [],
"examples": [["secrets_fetcher"]]
},
"environment": {
"description": "Override the GitHub deployment environment for the safe-outputs job. When set, this environment is used instead of the top-level environment: field. When not set, the top-level environment: field is propagated automatically so that environment-scoped secrets are accessible in the safe-outputs job.",
"oneOf": [
Expand Down
6 changes: 6 additions & 0 deletions pkg/workflow/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,12 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath
return formatCompilerError(markdownPath, "error", err.Error(), err)
}

// Validate safe-outputs needs declarations
log.Printf("Validating safe-outputs needs declarations")
if err := validateSafeOutputsNeeds(workflowData); err != nil {
return formatCompilerError(markdownPath, "error", err.Error(), err)
}

// Validate safe-job needs: declarations against known generated job IDs
log.Printf("Validating safe-job needs declarations")
if err := validateSafeJobNeeds(workflowData); err != nil {
Expand Down
14 changes: 14 additions & 0 deletions pkg/workflow/compiler_safe_outputs_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,20 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa
needs = append(needs, "unlock")
consolidatedSafeOutputsJobLog.Print("Added unlock job dependency to safe_outputs job")
}
seenNeeds := make(map[string]bool, len(needs))
for _, need := range needs {
seenNeeds[need] = true
}
if data.SafeOutputs != nil {
for _, need := range data.SafeOutputs.Needs {
if seenNeeds[need] {
continue
}
needs = append(needs, need)
seenNeeds[need] = true
consolidatedSafeOutputsJobLog.Printf("Added explicit safe-outputs needs dependency to safe_outputs job: %s", need)
}
}

// Extract workflow ID from markdown path for GH_AW_WORKFLOW_ID
workflowID := GetWorkflowIDFromPath(markdownPath)
Expand Down
Loading
Loading