From e066fb5bea7f42f98f844d5cf8765d592cfc017f Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 16 Jun 2026 14:28:59 -0400 Subject: [PATCH 1/2] fix: propagate callback permissions into top-level workflow permissions Per-callback permissions declared in the manifest (such as id-token: write for OIDC) were parsed into CallbackInfo but never emitted. A reusable-workflow caller job cannot set job-level permissions, so the calling workflow's top-level permissions must grant the union of base scopes and every scope any invoked callback requires. Without id-token: write at the caller top level, GitHub rejects the callee with 'requesting id-token: write, but is only allowed id-token: none'. Collect the union of callback permissions across the dependency graph and emit it on top of each generator's base scopes, with callback-only scopes appended in sorted order for deterministic output. Applied to orchestrate, promote, hotfix, and rollback generators. The no-callback-permissions path is byte identical to prior output. Signed-off-by: Joshua Temple --- .../callback-permissions-oidc.yaml | 55 +++++ .../generate/callback_permissions_test.go | 206 ++++++++++++++++++ internal/generate/determinism_test.go | 11 + internal/generate/generator.go | 14 +- internal/generate/graph.go | 66 ++++++ internal/generate/hotfix.go | 14 +- internal/generate/promote.go | 17 +- internal/generate/rollback.go | 16 +- 8 files changed, 376 insertions(+), 23 deletions(-) create mode 100644 e2e/scenarios/orchestrate/callback-permissions-oidc.yaml create mode 100644 internal/generate/callback_permissions_test.go diff --git a/e2e/scenarios/orchestrate/callback-permissions-oidc.yaml b/e2e/scenarios/orchestrate/callback-permissions-oidc.yaml new file mode 100644 index 00000000..21833e63 --- /dev/null +++ b/e2e/scenarios/orchestrate/callback-permissions-oidc.yaml @@ -0,0 +1,55 @@ +name: "Callback Permissions Union (OIDC)" +description: | + Verifies that per-callback permissions declared on a build callback are + unioned into the calling orchestrate.yaml's top-level permissions block. A + reusable-workflow caller job cannot carry job-level permissions, so any scope + a callback needs (notably id-token: write for OIDC) must be granted at the top + level of the calling workflow instead. + + The api build declares permissions: {id-token: write, packages: read}. The + generated orchestrate.yaml must keep its base scopes (contents: write, + actions: read) and append the callback scopes in deterministic alphabetical + order (id-token before packages). The caller job itself must NOT carry a + job-level permissions block, since GitHub Actions forbids permissions on a + reusable-workflow call. + + Generator-output verification scenario; the assertion runs on the staged repo + after StageRepoFromConfig generates workflows but before any orchestrate runs. + +config: + trunk_branch: main + environments: [] + builds: + - name: api + workflow: build-api.yaml + triggers: ["src/**"] + permissions: + id-token: write + packages: read + deploys: [] + +steps: + - name: "Initial commit; assert callback permissions union in orchestrate.yaml" + action: commit + commit: + message: "feat: add api callback requiring id-token for oidc" + files: + src/main.go: | + package main + func main() {} + expect: + workflow_files: + - path: ".github/workflows/orchestrate.yaml" + contains: + # Base scopes are preserved. + - "contents: write" + - "actions: read" + # Callback scopes are unioned in at the top level. + - "id-token: write" + - "packages: read" + not_contains: + # A reusable-workflow caller job cannot carry job-level permissions; + # the scopes live only in the top-level block, so the per-callback + # OIDC grant never appears as a job-scoped permissions line directly + # under the build-api caller's uses:. + - "uses: ./.github/workflows/build-api.yaml\n permissions:" diff --git a/internal/generate/callback_permissions_test.go b/internal/generate/callback_permissions_test.go new file mode 100644 index 00000000..282a9942 --- /dev/null +++ b/internal/generate/callback_permissions_test.go @@ -0,0 +1,206 @@ +package generate + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// topLevelPermissions extracts the top-level permissions: block from a generated +// workflow. It returns the lines indented under the first "permissions:" line +// that begins at column zero (a top-level key), stopping at the next top-level +// key or blank line. Job-level permissions blocks are indented and therefore +// never match. +func topLevelPermissions(t *testing.T, content string) string { + t.Helper() + lines := strings.Split(content, "\n") + var out []string + inBlock := false + for _, line := range lines { + if line == "permissions:" { + inBlock = true + out = append(out, line) + continue + } + if inBlock { + if strings.HasPrefix(line, " ") { + out = append(out, line) + continue + } + break + } + } + require.True(t, inBlock, "no top-level permissions block found") + return strings.Join(out, "\n") +} + +// writeCallWorkflow writes a minimal reusable workflow file so the generator can +// resolve a callback's workflow path. +func writeCallWorkflow(t *testing.T, dir, rel string) { + t.Helper() + full := filepath.Join(dir, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0755)) + require.NoError(t, os.WriteFile(full, []byte("on:\n workflow_call:\n"), 0644)) +} + +// TestOrchestrate_TopLevelPermissions_UnionIncludesCallbackScopes asserts that a +// build callback declaring id-token: write propagates into the calling +// workflow's top-level permissions union, alongside the base contents/actions +// scopes. A reusable-workflow caller job cannot set job-level permissions, so the +// top-level block must grant the union. +func TestOrchestrate_TopLevelPermissions_UnionIncludesCallbackScopes(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + writeCallWorkflow(t, tmpDir, ".github/workflows/build.yaml") + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Builds: []config.BuildConfig{ + { + Name: "app", + Workflow: ".github/workflows/build.yaml", + Triggers: []string{"src/**"}, + Permissions: map[string]string{"id-token": "write"}, + }, + }, + } + + gen := NewGenerator(cfg, tmpDir) + result, err := gen.Generate() + require.NoError(t, err) + + perms := topLevelPermissions(t, result) + assert.Contains(t, perms, "contents: write") + assert.Contains(t, perms, "actions: read") + assert.Contains(t, perms, "id-token: write") +} + +// TestOrchestrate_TopLevelPermissions_Deterministic asserts the appended +// callback-only scopes are emitted in stable sorted order and that repeated +// generation is byte-identical. +func TestOrchestrate_TopLevelPermissions_Deterministic(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + writeCallWorkflow(t, tmpDir, ".github/workflows/build.yaml") + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Builds: []config.BuildConfig{ + { + Name: "app", + Workflow: ".github/workflows/build.yaml", + Triggers: []string{"src/**"}, + Permissions: map[string]string{ + "id-token": "write", + "packages": "read", + }, + }, + }, + } + + var first string + for i := 0; i < 25; i++ { + gen := NewGenerator(cfg, tmpDir) + result, err := gen.Generate() + require.NoError(t, err) + if i == 0 { + first = result + continue + } + assert.Equal(t, first, result, "generation must be byte-identical across runs") + } + + perms := topLevelPermissions(t, first) + // Base scopes keep their existing order; callback-only scopes are appended + // sorted alphabetically: id-token before packages. + idIdx := strings.Index(perms, "id-token: write") + pkgIdx := strings.Index(perms, "packages: read") + require.NotEqual(t, -1, idIdx) + require.NotEqual(t, -1, pkgIdx) + assert.Less(t, idIdx, pkgIdx, "callback-only scopes must be sorted alphabetically") +} + +// TestOrchestrate_TopLevelPermissions_NoCallbackPermsByteIdentical asserts that +// when no callback declares permissions, the top-level block is exactly the +// historical base block (no churn, existing golden assertions preserved). +func TestOrchestrate_TopLevelPermissions_NoCallbackPermsByteIdentical(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + writeCallWorkflow(t, tmpDir, ".github/workflows/build.yaml") + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Builds: []config.BuildConfig{ + { + Name: "app", + Workflow: ".github/workflows/build.yaml", + Triggers: []string{"src/**"}, + }, + }, + } + + gen := NewGenerator(cfg, tmpDir) + result, err := gen.Generate() + require.NoError(t, err) + assert.Contains(t, result, "permissions:\n contents: write\n actions: read\n") +} + +// TestPromote_TopLevelPermissions_UnionIncludesCallbackScopes asserts deploy +// callback OIDC permissions propagate into the promote workflow's top-level +// union, alongside its base contents/actions scopes. +func TestPromote_TopLevelPermissions_UnionIncludesCallbackScopes(t *testing.T) { + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev", "prod"}, + Deploys: []config.DeployConfig{ + { + Name: "services", + Workflow: ".github/workflows/deploy.yaml", + Permissions: map[string]string{"id-token": "write"}, + }, + }, + } + + gen := NewPromoteGenerator(cfg, "") + result, err := gen.Generate() + require.NoError(t, err) + + perms := topLevelPermissions(t, result) + assert.Contains(t, perms, "contents: write") + assert.Contains(t, perms, "actions: write") + assert.Contains(t, perms, "id-token: write") +} + +// TestRollback_TopLevelPermissions_UnionIncludesCallbackScopes asserts deploy +// callback OIDC permissions propagate into the rollback workflow's top-level +// union, alongside its base contents/actions scopes. +func TestRollback_TopLevelPermissions_UnionIncludesCallbackScopes(t *testing.T) { + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev", "prod"}, + Deploys: []config.DeployConfig{ + { + Name: "services", + Workflow: ".github/workflows/deploy.yaml", + Permissions: map[string]string{"id-token": "write"}, + }, + }, + } + + gen := NewRollbackGenerator(cfg, "") + result, err := gen.Generate() + require.NoError(t, err) + + perms := topLevelPermissions(t, result) + assert.Contains(t, perms, "contents: write") + assert.Contains(t, perms, "actions: write") + assert.Contains(t, perms, "id-token: write") +} diff --git a/internal/generate/determinism_test.go b/internal/generate/determinism_test.go index 2b6460e4..36edff50 100644 --- a/internal/generate/determinism_test.go +++ b/internal/generate/determinism_test.go @@ -80,6 +80,10 @@ func determinismConfig() *config.TrunkConfig { "arch": {"amd64", "arm64"}, }, }, + // A second callback contributing to the top-level permissions + // union, so the union spans multiple nodes (map iteration order + // across nodes must not leak into output). + Permissions: map[string]string{"attestations": "write"}, }, { Name: "bundle", @@ -102,6 +106,13 @@ func determinismConfig() *config.TrunkConfig { Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, DependsOn: []string{"bundle"}, + // Per-callback permissions exercise the top-level permissions + // union, whose appended scopes are map-sourced and must emit in a + // stable sorted order run to run. + Permissions: map[string]string{ + "id-token": "write", + "packages": "read", + }, }, // sidecar is an independent deploy root, again adding parallel // nodes to the graph so seed order can diverge run to run. diff --git a/internal/generate/generator.go b/internal/generate/generator.go index e96b8aa0..91fbd02b 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -710,11 +710,15 @@ func (g *Generator) writeConcurrency(sb *strings.Builder) { } func (g *Generator) writePermissions(sb *strings.Builder) { - // Permissions needed for release management (tags, releases) and state commits - sb.WriteString("permissions:\n") - sb.WriteString(" contents: write\n") - sb.WriteString(" actions: read\n") - sb.WriteString("\n") + // Base: permissions needed for release management (tags, releases) and state + // commits. A reusable callback cannot set its own job permissions, so any + // scope a callback declares (e.g. id-token: write for OIDC) is unioned in at + // the top level here. + base := [][2]string{ + {"contents", "write"}, + {"actions", "read"}, + } + writeTopLevelPermissions(sb, base, collectCallbackPermissions(g.config)) } func (g *Generator) writeJobs(sb *strings.Builder) { diff --git a/internal/generate/graph.go b/internal/generate/graph.go index de97722b..43524904 100644 --- a/internal/generate/graph.go +++ b/internal/generate/graph.go @@ -2,6 +2,8 @@ package generate import ( "fmt" + "sort" + "strings" "github.com/stablekernel/cascade/internal/config" ) @@ -266,6 +268,70 @@ func defaultString(s, def string) string { return s } +// collectCallbackPermissions unions the per-callback permissions of every +// callback in the manifest (validate, builds, deploys). A reusable-workflow +// caller job cannot carry job-level permissions, so any scope a callback needs +// (notably id-token: write for OIDC) must be granted by the calling workflow's +// top-level permissions block instead. The result is that union keyed by scope. +// +// Precedence when two callbacks set the same scope to different values: "write" +// always wins over any other value, since GHA permission scopes are monotonic +// (write subsumes read). Otherwise the last-declared value for the scope is +// kept; in practice callbacks agree on a scope's value, so this only matters +// for the read/write distinction handled above. +func collectCallbackPermissions(cfg *config.TrunkConfig) map[string]string { + graph := BuildDependencyGraph(cfg) + union := make(map[string]string) + for _, node := range graph.Nodes { + for scope, value := range node.Permissions { + if existing, ok := union[scope]; ok && existing == "write" { + continue + } + union[scope] = value + } + } + return union +} + +// writeTopLevelPermissions emits a top-level permissions: block. The base scopes +// are written first in their given order (preserving each generator's historical +// output), then any callback-union scope not already present in base is appended +// in alphabetical order for deterministic output. When a scope appears in both +// base and the callback union, the more-permissive value wins ("write" over a +// non-write value), so base scopes can be promoted to write by a callback that +// requires it. A trailing blank line is emitted to match the prior blocks. +// +// When the callback union is empty and contributes nothing, the output is +// byte-identical to the historical hardcoded base block. +func writeTopLevelPermissions(sb *strings.Builder, base [][2]string, callbackUnion map[string]string) { + sb.WriteString("permissions:\n") + + inBase := make(map[string]bool, len(base)) + for _, kv := range base { + scope, value := kv[0], kv[1] + inBase[scope] = true + // Promote to write if a callback requires write on this base scope. + if value != "write" && callbackUnion[scope] == "write" { + value = "write" + } + fmt.Fprintf(sb, " %s: %s\n", scope, value) + } + + // Append callback-only scopes (not in base) in deterministic sorted order. + extra := make([]string, 0, len(callbackUnion)) + for scope := range callbackUnion { + if !inBase[scope] { + extra = append(extra, scope) + } + } + sort.Strings(extra) + for _, scope := range extra { + fmt.Fprintf(sb, " %s: %s\n", scope, callbackUnion[scope]) + } + + sb.WriteString("\n") +} + // ensureValidateDependency adds "validate" to deps if not already present func ensureValidateDependency(deps []string) []string { for _, d := range deps { diff --git a/internal/generate/hotfix.go b/internal/generate/hotfix.go index 3b5d280e..dae8c180 100644 --- a/internal/generate/hotfix.go +++ b/internal/generate/hotfix.go @@ -141,11 +141,15 @@ func (g *HotfixGenerator) writeTriggers(sb *strings.Builder) { // to push the cherry-pick branch, pull-requests:write to open the resolution PR, // and actions:read for workflow introspection. func (g *HotfixGenerator) writePermissions(sb *strings.Builder) { - sb.WriteString("permissions:\n") - sb.WriteString(" contents: write\n") - sb.WriteString(" pull-requests: write\n") - sb.WriteString(" actions: read\n") - sb.WriteString("\n") + // Base scopes the hotfix workflow needs. A reusable callback cannot set its + // own job permissions, so any scope a callback declares (e.g. id-token: write + // for OIDC) is unioned in at the top level here. + base := [][2]string{ + {"contents", "write"}, + {"pull-requests", "write"}, + {"actions", "read"}, + } + writeTopLevelPermissions(sb, base, collectCallbackPermissions(g.config)) } // writeConcurrency keys the group per target environment. On dispatch the env is diff --git a/internal/generate/promote.go b/internal/generate/promote.go index f3e3cefe..86849568 100644 --- a/internal/generate/promote.go +++ b/internal/generate/promote.go @@ -596,13 +596,16 @@ func (g *PromoteGenerator) writeWorkflowTriggers(sb *strings.Builder) { } sb.WriteString("\n") - // Permissions needed for release management, state commits, and job queries. - // actions:write is required to dispatch the Release workflow from the - // finalize job when a final release is published. - sb.WriteString("permissions:\n") - sb.WriteString(" contents: write\n") - sb.WriteString(" actions: write\n") - sb.WriteString("\n") + // Base: permissions needed for release management, state commits, and job + // queries. actions:write is required to dispatch the Release workflow from + // the finalize job when a final release is published. A reusable callback + // cannot set its own job permissions, so any scope a deploy callback declares + // (e.g. id-token: write for OIDC) is unioned in at the top level here. + base := [][2]string{ + {"contents", "write"}, + {"actions", "write"}, + } + writeTopLevelPermissions(sb, base, collectCallbackPermissions(g.config)) } func (g *PromoteGenerator) writeJobs(sb *strings.Builder) { diff --git a/internal/generate/rollback.go b/internal/generate/rollback.go index 6ea4a636..70439d38 100644 --- a/internal/generate/rollback.go +++ b/internal/generate/rollback.go @@ -138,12 +138,16 @@ func (g *RollbackGenerator) writeTriggers(sb *strings.Builder) { sb.WriteString(" default: false\n") sb.WriteString("\n") - // contents:write to commit the rolled-back state; actions:write for parity - // with the promote workflow's release/dispatch surface. - sb.WriteString("permissions:\n") - sb.WriteString(" contents: write\n") - sb.WriteString(" actions: write\n") - sb.WriteString("\n") + // Base: contents:write to commit the rolled-back state; actions:write for + // parity with the promote workflow's release/dispatch surface. A reusable + // callback cannot set its own job permissions, so any scope a deploy callback + // declares (e.g. id-token: write for OIDC) is unioned in at the top level + // here. + base := [][2]string{ + {"contents", "write"}, + {"actions", "write"}, + } + writeTopLevelPermissions(sb, base, collectCallbackPermissions(g.config)) } // writeConcurrency serializes rollback runs so concurrent state writes cannot From e0be16153fd5744a61b00e19d91d11df66846dd2 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 16 Jun 2026 14:31:49 -0400 Subject: [PATCH 2/2] refactor: make callback permission collision precedence order-independent Use a lexicographic comparison so the union and base promotion pick the same deterministic value when callbacks set a scope to different values, removing the latent dependency on map iteration order. write still wins over read wins over none, matching the monotonic permission order. Signed-off-by: Joshua Temple --- internal/generate/graph.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/generate/graph.go b/internal/generate/graph.go index 43524904..f75369de 100644 --- a/internal/generate/graph.go +++ b/internal/generate/graph.go @@ -274,17 +274,18 @@ func defaultString(s, def string) string { // (notably id-token: write for OIDC) must be granted by the calling workflow's // top-level permissions block instead. The result is that union keyed by scope. // -// Precedence when two callbacks set the same scope to different values: "write" -// always wins over any other value, since GHA permission scopes are monotonic -// (write subsumes read). Otherwise the last-declared value for the scope is -// kept; in practice callbacks agree on a scope's value, so this only matters -// for the read/write distinction handled above. +// Precedence when two callbacks set the same scope to different values: the +// lexicographically greatest value wins. This keeps "write" ahead of "read" +// ahead of "none", matching the monotonic GHA permission order (write subsumes +// read), and is independent of map iteration order so the union is fully +// deterministic. In practice callbacks agree on a scope's value, so this only +// matters for the read/write distinction. func collectCallbackPermissions(cfg *config.TrunkConfig) map[string]string { graph := BuildDependencyGraph(cfg) union := make(map[string]string) for _, node := range graph.Nodes { for scope, value := range node.Permissions { - if existing, ok := union[scope]; ok && existing == "write" { + if existing, ok := union[scope]; ok && existing >= value { continue } union[scope] = value @@ -310,9 +311,10 @@ func writeTopLevelPermissions(sb *strings.Builder, base [][2]string, callbackUni for _, kv := range base { scope, value := kv[0], kv[1] inBase[scope] = true - // Promote to write if a callback requires write on this base scope. - if value != "write" && callbackUnion[scope] == "write" { - value = "write" + // Promote the base scope if a callback requires a more-permissive value + // (e.g. write over read), using the same lexicographic order as the union. + if cb, ok := callbackUnion[scope]; ok && cb > value { + value = cb } fmt.Fprintf(sb, " %s: %s\n", scope, value) }