From 79b04bebc02a59d14e29a1e65a28f156ff520eaf Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 16 Jun 2026 18:23:15 -0400 Subject: [PATCH] fix: coerce dry_run and boolean dispatch_inputs to real booleans in callback inputs On push/schedule/workflow_run events github.event.inputs.dry_run evaluates to an empty string. Forwarding that bare value into a deploy reusable-workflow callback whose dry_run input is type: boolean makes GitHub Actions reject the call before any job materializes, so the deploy reports failure with no visible job and Finalize aborts. Emit github.event.inputs.dry_run == 'true' so the forwarded value is always a real boolean: false on a push-triggered run and true only when an operator dispatches dry_run=true. Apply the same coercion to boolean dispatch_inputs forwarded into callbacks, which share the empty-string hazard on non-dispatch events. Signed-off-by: Joshua Temple --- .../orchestrate/dry-run-input-expression.yaml | 19 ++- internal/generate/dry_run_test.go | 148 ++++++++++++++++-- internal/generate/generator.go | 16 +- internal/generate/promote.go | 6 +- 4 files changed, 163 insertions(+), 26 deletions(-) diff --git a/e2e/scenarios/orchestrate/dry-run-input-expression.yaml b/e2e/scenarios/orchestrate/dry-run-input-expression.yaml index e5ead247..b6961286 100644 --- a/e2e/scenarios/orchestrate/dry-run-input-expression.yaml +++ b/e2e/scenarios/orchestrate/dry-run-input-expression.yaml @@ -1,14 +1,16 @@ -name: "Dry-Run Passthrough Uses Null-Safe Expression" +name: "Dry-Run Passthrough Uses Coerced Null-Safe Expression" description: | Verifies that the generator emits the null-safe github.event.inputs.dry_run - accessor when a deploy callback has supports_dry_run: true. + accessor, coerced to a real boolean, when a deploy callback has + supports_dry_run: true. The orchestrate workflow is triggered by push, schedule, and workflow_run events -- none of which populate the inputs context. A bare inputs.dry_run expression renders as an empty string on those triggers, which breaks the boolean dispatch to the reusable callback workflow. The generator must use - github.event.inputs.dry_run instead so the value is null (and the boolean - coercion to false) rather than an empty string that fails type validation. + github.event.inputs.dry_run and compare it against 'true' so the forwarded + value is a real boolean rather than an empty string that fails the callback's + boolean type validation. Generator-output verification scenario; the assertion runs on the staged repo after StageRepoFromConfig generates workflows but before any orchestrate run, @@ -40,8 +42,13 @@ steps: workflow_files: - path: ".github/workflows/orchestrate.yaml" contains: - # The null-safe accessor works across push, schedule, and workflow_run triggers. - - "dry_run: ${{ github.event.inputs.dry_run }}" + # The null-safe accessor, coerced to a boolean, works across push, + # schedule, and workflow_run triggers and satisfies the callback's + # boolean dry_run input. + - "dry_run: ${{ github.event.inputs.dry_run == 'true' }}" not_contains: # The bare inputs accessor renders empty on non-dispatch triggers and must not appear. - "dry_run: ${{ inputs.dry_run }}" + # The uncoerced accessor yields an empty string for a boolean callback + # input and must not appear in the with: block. + - "dry_run: ${{ github.event.inputs.dry_run }}\n" diff --git a/internal/generate/dry_run_test.go b/internal/generate/dry_run_test.go index 4c36c4b3..ce18a421 100644 --- a/internal/generate/dry_run_test.go +++ b/internal/generate/dry_run_test.go @@ -13,7 +13,7 @@ import ( // TestPromote_SupportsDryRun_SingleDeploy verifies that a deploy callback with // supports_dry_run: true is invoked (not skipped) during a dry-run promote and -// receives dry_run: ${{ github.event.inputs.dry_run }} in its with: block. +// receives dry_run: ${{ github.event.inputs.dry_run == 'true' }} in its with: block. func TestPromote_SupportsDryRun_SingleDeploy(t *testing.T) { cfg := &config.TrunkConfig{ TrunkBranch: "main", @@ -41,10 +41,18 @@ func TestPromote_SupportsDryRun_SingleDeploy(t *testing.T) { "contains(fromJSON(needs.preflight.outputs.deploys_to_run), 'app')", "supports_dry_run deploy must still be gated on deploys_to_run") - // dry_run must be forwarded in the with: block. + // dry_run must be forwarded in the with: block, coerced to a real boolean so + // the reusable callback's boolean dry_run input accepts it on every trigger. assert.Contains(t, content, - "dry_run: ${{ github.event.inputs.dry_run }}", - "supports_dry_run deploy must receive dry_run input") + "dry_run: ${{ github.event.inputs.dry_run == 'true' }}", + "supports_dry_run deploy must receive coerced dry_run input") + + // The bare (uncoerced) form must not appear in the with: block; it renders as + // an empty string on push/schedule/workflow_run and fails boolean validation. + jobSection := extractJobSection(t, content, "deploy-app:") + assert.NotContains(t, jobSection, + "dry_run: ${{ github.event.inputs.dry_run }}\n", + "supports_dry_run deploy must not forward the uncoerced dry_run value") } // TestPromote_NoSupportsDryRun_SingleDeploy verifies that a deploy callback @@ -75,7 +83,7 @@ func TestPromote_NoSupportsDryRun_SingleDeploy(t *testing.T) { // (Check within the deploy-app job block only to avoid false matches.) jobSection := extractJobSection(t, content, "deploy-app:") assert.NotContains(t, jobSection, - "dry_run: ${{ github.event.inputs.dry_run }}", + "dry_run: ${{", "non-supports_dry_run deploy must not receive dry_run input") } @@ -137,9 +145,9 @@ func TestPromote_SupportsDryRun_NormalRunUnaffected(t *testing.T) { } // TestOrchestrate_SupportsDryRun_WithInputPassthrough verifies that the -// orchestrate generator passes dry_run: ${{ inputs.dry_run }} to a deploy -// callback that declares supports_dry_run: true, given the workflow file -// declares a dry_run input. +// orchestrate generator passes a coerced dry_run value to a deploy callback that +// declares supports_dry_run: true, given the workflow file declares a dry_run +// boolean input. func TestOrchestrate_SupportsDryRun_WithInputPassthrough(t *testing.T) { tmpDir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) @@ -176,12 +184,17 @@ on: require.NoError(t, err) // dry_run must be forwarded in the with: block to the deploy job using the - // null-safe github.event.inputs accessor. The bare inputs.dry_run form renders - // empty on push/schedule/workflow_run, which is invalid for a boolean callback - // input and fails the reusable-workflow dispatch. + // null-safe github.event.inputs accessor, coerced to a real boolean. The bare + // (uncoerced) form renders empty on push/schedule/workflow_run, which is + // invalid for a boolean callback input and fails the reusable-workflow dispatch. assert.Contains(t, content, - "dry_run: ${{ github.event.inputs.dry_run }}", - "orchestrate generator must pass dry_run to a supports_dry_run callback") + "dry_run: ${{ github.event.inputs.dry_run == 'true' }}", + "orchestrate generator must pass coerced dry_run to a supports_dry_run callback") + + // The uncoerced form must not appear in the with: block. + assert.NotContains(t, content, + "dry_run: ${{ github.event.inputs.dry_run }}\n", + "orchestrate generator must not forward the uncoerced dry_run value") } // TestOrchestrate_NoSupportsDryRun_NoDryRunInput verifies that a deploy @@ -222,10 +235,117 @@ on: deploySection := extractJobSection(t, content, "deploy-app:") assert.NotContains(t, deploySection, - "dry_run: ${{ github.event.inputs.dry_run }}", + "dry_run: ${{", "non-supports_dry_run callback must not receive dry_run in orchestrate") } +// TestOrchestrate_SupportsDryRun_CoercedNotBare verifies the orchestrate +// generator emits the boolean-coerced dry_run passthrough and never the bare +// uncoerced expression inside a with: block. +func TestOrchestrate_SupportsDryRun_CoercedNotBare(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + + deployWorkflow := ` +name: Deploy +on: + workflow_call: + inputs: + environment: + type: string + dry_run: + type: boolean +` + require.NoError(t, os.WriteFile( + filepath.Join(tmpDir, ".github/workflows/deploy.yaml"), + []byte(deployWorkflow), 0644, + )) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Deploys: []config.DeployConfig{ + { + Name: "app", + Workflow: ".github/workflows/deploy.yaml", + SupportsDryRun: true, + }, + }, + } + + gen := NewGenerator(cfg, tmpDir) + content, err := gen.Generate() + require.NoError(t, err) + + assert.Contains(t, content, + "dry_run: ${{ github.event.inputs.dry_run == 'true' }}", + "orchestrate must forward the coerced dry_run value") + assert.NotContains(t, content, + "dry_run: ${{ github.event.inputs.dry_run }}\n", + "orchestrate must not forward the bare dry_run value in a with: block") +} + +// TestOrchestrate_DispatchInputs_BooleanCoercion verifies that a boolean +// dispatch_input forwarded into a callback with: block is coerced +// (NAME: ${{ inputs.NAME == 'true' }}) while a string dispatch_input stays bare +// (NAME: ${{ inputs.NAME }}). +func TestOrchestrate_DispatchInputs_BooleanCoercion(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + + deployWorkflow := ` +name: Deploy +on: + workflow_call: + inputs: + environment: + type: string + verbose: + type: boolean + region: + type: string +` + require.NoError(t, os.WriteFile( + filepath.Join(tmpDir, ".github/workflows/deploy.yaml"), + []byte(deployWorkflow), 0644, + )) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Deploys: []config.DeployConfig{ + { + Name: "app", + Workflow: ".github/workflows/deploy.yaml", + }, + }, + DispatchInputs: map[string]config.DispatchInput{ + "verbose": {Type: config.DispatchInputTypeBoolean}, + "region": {Type: config.DispatchInputTypeString}, + }, + } + + gen := NewGenerator(cfg, tmpDir) + content, err := gen.Generate() + require.NoError(t, err) + + // Boolean dispatch input must be coerced to a real boolean. + assert.Contains(t, content, + "verbose: ${{ inputs.verbose == 'true' }}", + "boolean dispatch_input must be coerced in the callback with: block") + assert.NotContains(t, content, + "verbose: ${{ inputs.verbose }}\n", + "boolean dispatch_input must not be forwarded uncoerced") + + // String dispatch input stays bare. + assert.Contains(t, content, + "region: ${{ inputs.region }}", + "string dispatch_input must be forwarded verbatim") + assert.NotContains(t, content, + "region: ${{ inputs.region == 'true' }}", + "string dispatch_input must not be coerced") +} + // extractJobSection returns the YAML lines for a named job block, stopping at // the next top-level job or end of file. Used to scope assertions to one job. func extractJobSection(t *testing.T, content, jobKey string) string { diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 60f6131c..8fc62dea 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -1097,9 +1097,11 @@ func (g *Generator) writeWithInputs(sb *strings.Builder, info CallbackInfo) { // renders empty. Passing "" into a callback's boolean dry_run input fails the // reusable-workflow dispatch. The github.event.inputs accessor is null-safe on // those events (the callback falls back to its dry_run default), and on dispatch - // it still forwards the operator's value. + // it still forwards the operator's value. Compare against 'true' so the result is + // a real boolean: the callback input is type: boolean, and GitHub Actions rejects + // the empty string that github.event.inputs.dry_run yields on non-dispatch events. if info.SupportsDryRun { - inputs = append(inputs, " dry_run: ${{ github.event.inputs.dry_run }}") + inputs = append(inputs, " dry_run: ${{ github.event.inputs.dry_run == 'true' }}") } // For build callbacks with a matrix, pass each dimension's current value to @@ -1128,7 +1130,15 @@ func (g *Generator) writeWithInputs(sb *strings.Builder, info CallbackInfo) { sort.Strings(names) for _, name := range names { if g.jobHasInput(info.JobID, name) { - inputs = append(inputs, fmt.Sprintf(" %s: ${{ inputs.%s }}", name, name)) + // Boolean dispatch_inputs forward through the inputs context, which is + // a string. A callback declaring the matching input as type: boolean + // rejects the bare string, so compare against 'true' to coerce it to a + // real boolean. Non-boolean inputs are forwarded verbatim. + if g.config.DispatchInputs[name].Type == config.DispatchInputTypeBoolean { + inputs = append(inputs, fmt.Sprintf(" %s: ${{ inputs.%s == 'true' }}", name, name)) + } else { + inputs = append(inputs, fmt.Sprintf(" %s: ${{ inputs.%s }}", name, name)) + } } } } diff --git a/internal/generate/promote.go b/internal/generate/promote.go index 86849568..5f10f97a 100644 --- a/internal/generate/promote.go +++ b/internal/generate/promote.go @@ -785,7 +785,7 @@ func (g *PromoteGenerator) writeDeployJobs(sb *strings.Builder) { // When the callback opts in to dry-run passthrough, forward the // dispatch input so it can emulate internally. if d.SupportsDryRun { - sb.WriteString(" dry_run: ${{ github.event.inputs.dry_run }}\n") + sb.WriteString(" dry_run: ${{ github.event.inputs.dry_run == 'true' }}\n") } // Passthrough-expression inputs (e.g. ${{ vars.X }}) are excluded @@ -844,7 +844,7 @@ func (g *PromoteGenerator) writeDeployJobs(sb *strings.Builder) { // When the callback opts in to dry-run passthrough, forward the // dispatch input so it can emulate internally. if d.SupportsDryRun { - sb.WriteString(" dry_run: ${{ github.event.inputs.dry_run }}\n") + sb.WriteString(" dry_run: ${{ github.event.inputs.dry_run == 'true' }}\n") } } writeSecretsBlock(sb, d.Secrets) @@ -882,7 +882,7 @@ func (g *PromoteGenerator) writeDeployJobs(sb *strings.Builder) { // When the callback opts in to dry-run passthrough, forward the // dispatch input so it can emulate internally. if d.SupportsDryRun { - sb.WriteString(" dry_run: ${{ github.event.inputs.dry_run }}\n") + sb.WriteString(" dry_run: ${{ github.event.inputs.dry_run == 'true' }}\n") } writeSecretsBlock(sb, d.Secrets) }