diff --git a/CHANGELOG.md b/CHANGELOG.md index 465acfd..7aeb509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,14 @@ A `Migration` section is added to any release that bumps `schema_version`. published instead of failing. Release-artifact names and paths are now also validated against the character set the emitted upload shell can carry safely, since the path is spliced unquoted so its glob can expand +- **generate:** Route the promote matrix input JSON (`DEFAULT_INPUTS` / + `ENV_INPUTS`) through the Build Deploy Matrices step `env:` map instead of + embedding it in a shell single-quote literal, and emit dispatch-input + defaults as properly escaped YAML single-quoted scalars. An apostrophe in an + ordinary input value (for example `message: "it's live"`) previously + generated a promote script that failed shell parsing on every promotion, and + a crafted value could have executed in the promote job; an apostrophe in a + `dispatch_inputs` default produced an invalid workflow document - **promote:** Record only the publishing component's own release marker (`version`, `sha`, `released_on`) under `latest_release.components.`. diff --git a/e2e/scenarios/66-input-value-quoting.yaml b/e2e/scenarios/66-input-value-quoting.yaml new file mode 100644 index 0000000..367efcf --- /dev/null +++ b/e2e/scenarios/66-input-value-quoting.yaml @@ -0,0 +1,97 @@ +name: "Input Value Quoting" +description: | + Operator-authored input values are ordinary human text and may carry + apostrophes. The generated workflows must stay well-formed anyway: + + 1. Promote matrix inputs: the serialized DEFAULT_INPUTS / ENV_INPUTS JSON is + routed through the Build Deploy Matrices step env: map (YAML single-quoted, + embedded quotes doubled) and read back via quoted shell variables. The JSON + must never sit inside a shell single-quote literal in the run script, where + an apostrophe breaks parsing and a crafted value would execute. + + 2. Dispatch input defaults: a default containing an apostrophe must be + emitted as a valid YAML single-quoted scalar (embedded quote doubled) in + the workflow_dispatch inputs block. + + Generator-output verification only. The harness localizes and parses every + generated workflow, so a malformed document fails the run outright. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: + - name: app + workflow: deploy.yaml + triggers: ["src/**"] + inputs: + message: "it's live" + env_inputs: + prod: + message: "don't stop" + dispatch_inputs: + notice: + type: string + default: "it's enabled" + description: "Operator note" + +# The deploy callback declares the message input the generated promote +# workflow threads through the matrix, keeping the reusable-workflow call +# valid. +setup_workflows: + ".github/workflows/deploy.yaml": | + name: deploy + on: + workflow_call: + inputs: + environment: + required: false + type: string + sha: + required: false + type: string + message: + required: false + type: string + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - run: echo "deploy" + +steps: + - name: "Seed source; assert apostrophe-bearing values emit well-formed workflows" + action: commit + commit: + message: "seed source" + files: + src/main.go: | + package main + + func main() {} + expect: + workflow_files: + - path: ".github/workflows/promote.yaml" + contains: + # Input JSON is env-routed on the step, YAML-quoted with the + # embedded apostrophe doubled. + - "DEFAULT_INPUTS_APP: '{\"message\":\"it''s live\"}'" + - "ENV_INPUTS_APP: '{\"prod\":{\"message\":\"don''t stop\"}}'" + # The script reads the JSON back through quoted variables. + - "DEFAULT_INPUTS=\"$DEFAULT_INPUTS_APP\"" + - "ENV_INPUTS=\"$ENV_INPUTS_APP\"" + not_contains: + # The broken form: JSON inside a shell single-quote literal. + - "DEFAULT_INPUTS='{" + - "ENV_INPUTS='{" + - path: ".github/workflows/orchestrate.yaml" + contains: + - " notice:" + # Valid YAML single-quoted scalar: embedded apostrophe doubled. + - " default: 'it''s enabled'" + not_contains: + # The broken form: raw apostrophe terminating the scalar early. + - "default: 'it's enabled'" diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 0c6e58a..5c7f486 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -776,7 +776,7 @@ func (g *Generator) writeWorkflowTriggers(sb *strings.Builder) { } } if di.Default != nil { - fmt.Fprintf(sb, " default: '%v'\n", di.Default) + fmt.Fprintf(sb, " default: %s\n", yamlSingleQuote(fmt.Sprintf("%v", di.Default))) } if di.IsRequired() { sb.WriteString(" required: true\n") diff --git a/internal/generate/input_passthrough_test.go b/internal/generate/input_passthrough_test.go index e714325..f3f076d 100644 --- a/internal/generate/input_passthrough_test.go +++ b/internal/generate/input_passthrough_test.go @@ -196,7 +196,7 @@ func TestPromoteLiteralInputsUnchanged(t *testing.T) { result, err := gen.Generate() require.NoError(t, err) - assert.Contains(t, result, `DEFAULT_INPUTS='{"cluster":"dev-eks"}'`) + assert.Contains(t, result, `DEFAULT_INPUTS_APP: '{"cluster":"dev-eks"}'`) assert.Contains(t, result, `"prod":{"cluster":"prod-eks"}`) assert.Contains(t, result, "cluster: ${{ matrix.cluster }}") } diff --git a/internal/generate/input_quoting_test.go b/internal/generate/input_quoting_test.go new file mode 100644 index 0000000..d61b68c --- /dev/null +++ b/internal/generate/input_quoting_test.go @@ -0,0 +1,180 @@ +package generate + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + + "github.com/stablekernel/cascade/internal/config" +) + +// requireShellParseable writes script to a temp file and asserts bash -n +// accepts it. The generated matrix-building step must stay parseable no +// matter what characters an operator puts in an input value. +func requireShellParseable(t *testing.T, script string) { + t.Helper() + path := filepath.Join(t.TempDir(), "step.sh") + require.NoError(t, os.WriteFile(path, []byte(script), 0o600)) + out, err := exec.Command("bash", "-n", path).CombinedOutput() + require.NoError(t, err, "bash -n rejected the emitted run script:\n%s\nscript:\n%s", out, script) +} + +// findStep parses a generated workflow and returns the step with the given +// name from any job, proving along the way that the document is valid YAML. +func findStep(t *testing.T, content, stepName string) map[string]interface{} { + t.Helper() + var doc struct { + Jobs map[string]struct { + Steps []map[string]interface{} `yaml:"steps"` + } `yaml:"jobs"` + } + require.NoError(t, yaml.Unmarshal([]byte(content), &doc), "generated workflow must be valid YAML") + for _, job := range doc.Jobs { + for _, step := range job.Steps { + if step["name"] == stepName { + return step + } + } + } + require.Failf(t, "step not found", "step %q not present in generated workflow", stepName) + return nil +} + +// TestBuildDeployMatrices_ApostropheInputValue_EnvRoutedAndShellParseable +// covers the promote matrix input sink: an ordinary human value containing an +// apostrophe (and any crafted value) must never sit inside a shell quote +// literal in the emitted run script. The JSON blobs are routed through the +// step env: map and referenced as quoted shell variables, so the script +// parses regardless of input content. +func TestBuildDeployMatrices_ApostropheInputValue_EnvRoutedAndShellParseable(t *testing.T) { + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: config.EnvNames("dev", "prod"), + Deploys: []config.DeployConfig{ + { + Name: "app", + Workflow: ".github/workflows/deploy-app.yaml", + Inputs: map[string]interface{}{ + "message": "it's live", + }, + EnvInputs: map[string]map[string]interface{}{ + "prod": {"message": "don't y'all stop"}, + }, + }, + }, + } + + gen := NewPromoteGenerator(cfg, "") + content, err := gen.Generate() + require.NoError(t, err) + + step := findStep(t, content, "Build Deploy Matrices") + + // The run script must be shell-parseable even with apostrophes in values. + run, ok := step["run"].(string) + require.True(t, ok, "Build Deploy Matrices step has no run script") + requireShellParseable(t, run) + + // The JSON reaches the shell through env: indirection, never through a + // quoted literal inside the script. + env, ok := step["env"].(map[string]interface{}) + require.True(t, ok, "Build Deploy Matrices step has no env map") + assert.Equal(t, `{"message":"it's live"}`, env["DEFAULT_INPUTS_APP"]) + assert.Equal(t, `{"prod":{"message":"don't y'all stop"}}`, env["ENV_INPUTS_APP"]) + assert.NotContains(t, run, "DEFAULT_INPUTS='") + assert.NotContains(t, run, "ENV_INPUTS='") + assert.Contains(t, run, `DEFAULT_INPUTS="$DEFAULT_INPUTS_APP"`) + assert.Contains(t, run, `ENV_INPUTS="$ENV_INPUTS_APP"`) +} + +// TestBuildDeployMatrices_PlainInputValues_ShellParseable pins the same +// guarantee for apostrophe-free values across multiple deploys: the script +// still parses and each deploy gets its own env-routed JSON pair. +func TestBuildDeployMatrices_PlainInputValues_ShellParseable(t *testing.T) { + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: config.EnvNames("dev", "prod"), + Deploys: []config.DeployConfig{ + { + Name: "app", + Workflow: ".github/workflows/deploy-app.yaml", + Inputs: map[string]interface{}{ + "cluster": "dev-eks", + }, + EnvInputs: map[string]map[string]interface{}{ + "prod": {"cluster": "prod-eks"}, + }, + }, + { + Name: "infra-core", + Workflow: ".github/workflows/deploy-infra.yaml", + Inputs: map[string]interface{}{ + "stack": "main", + }, + }, + }, + } + + gen := NewPromoteGenerator(cfg, "") + content, err := gen.Generate() + require.NoError(t, err) + + step := findStep(t, content, "Build Deploy Matrices") + run, ok := step["run"].(string) + require.True(t, ok, "Build Deploy Matrices step has no run script") + requireShellParseable(t, run) + + env, ok := step["env"].(map[string]interface{}) + require.True(t, ok, "Build Deploy Matrices step has no env map") + assert.Equal(t, `{"cluster":"dev-eks"}`, env["DEFAULT_INPUTS_APP"]) + assert.Equal(t, `{"prod":{"cluster":"prod-eks"}}`, env["ENV_INPUTS_APP"]) + assert.Equal(t, `{"stack":"main"}`, env["DEFAULT_INPUTS_INFRA_CORE"]) + assert.Equal(t, `{}`, env["ENV_INPUTS_INFRA_CORE"]) +} + +// TestGenerator_DispatchInputDefaultWithApostrophe_ValidYAML covers the +// dispatch-input default sink: an apostrophe in a default value must produce +// a valid YAML scalar (single-quote style doubles embedded quotes), so the +// emitted orchestrate workflow parses and the value round-trips intact. +func TestGenerator_DispatchInputDefaultWithApostrophe_ValidYAML(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n workflow_call:\n"), 0o644)) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: config.EnvNames("dev"), + Builds: []config.BuildConfig{ + {Name: "app", Workflow: ".github/workflows/build.yaml", Triggers: []string{"src/**"}}, + }, + DispatchInputs: map[string]config.DispatchInput{ + "greeting": { + Type: config.DispatchInputTypeString, + Default: "it's a default", + Description: "operator note", + }, + }, + } + + gen := NewGenerator(cfg, tmpDir) + result, err := gen.Generate() + require.NoError(t, err) + + var doc struct { + On struct { + WorkflowDispatch struct { + Inputs map[string]struct { + Default interface{} `yaml:"default"` + } `yaml:"inputs"` + } `yaml:"workflow_dispatch"` + } `yaml:"on"` + } + require.NoError(t, yaml.Unmarshal([]byte(result), &doc), "generated orchestrate workflow must be valid YAML") + require.Contains(t, doc.On.WorkflowDispatch.Inputs, "greeting") + assert.Equal(t, "it's a default", doc.On.WorkflowDispatch.Inputs["greeting"].Default) +} diff --git a/internal/generate/promote.go b/internal/generate/promote.go index fac5d01..aa6d982 100644 --- a/internal/generate/promote.go +++ b/internal/generate/promote.go @@ -237,6 +237,19 @@ func (g *PromoteGenerator) writeMatrixBuildingStep(sb *strings.Builder) { sb.WriteString(" id: build-matrices\n") sb.WriteString(" env:\n") sb.WriteString(" PROMOTION_RESULT: ${{ steps.preflight.outputs.promotion_result }}\n") + // Route each deploy's serialized input JSON through env: indirection so + // operator-authored values never appear inside a shell quote literal in + // the run script (same pattern as the dispatch-input handling in the + // external update workflow). + for _, d := range g.config.Deploys { + if len(d.Inputs) == 0 { + continue + } + upper := strings.ToUpper(strings.ReplaceAll(d.Name, "-", "_")) + defaultInputsJSON, envInputsJSON := g.matrixInputsJSON(&d) + fmt.Fprintf(sb, " DEFAULT_INPUTS_%s: %s\n", upper, yamlSingleQuote(defaultInputsJSON)) + fmt.Fprintf(sb, " ENV_INPUTS_%s: %s\n", upper, yamlSingleQuote(envInputsJSON)) + } sb.WriteString(" run: |\n") sb.WriteString(" # Extract promotions array from promotion result\n") sb.WriteString(" PROMOTIONS=$(echo \"$PROMOTION_RESULT\" | jq -c '.promotions // []')\n") @@ -252,8 +265,8 @@ func (g *PromoteGenerator) writeMatrixBuildingStep(sb *strings.Builder) { fmt.Fprintf(sb, " # Build matrix for deploy: %s\n", d.Name) fmt.Fprintf(sb, " MATRIX_%s='['\n", strings.ToUpper(outputName)) - // Serialize default inputs and env-specific inputs as JSON for bash - g.writeMatrixBuildingLogic(sb, &d, outputName) + // The input JSON is env-routed on the step; the logic reads it back. + g.writeMatrixBuildingLogic(sb, outputName) fmt.Fprintf(sb, " MATRIX_%s=\"${MATRIX_%s}]\"\n", strings.ToUpper(outputName), strings.ToUpper(outputName)) fmt.Fprintf(sb, " echo \"deploy_%s_matrix=$MATRIX_%s\" >> \"$GITHUB_OUTPUT\"\n", outputName, strings.ToUpper(outputName)) @@ -385,24 +398,32 @@ func (g *PromoteGenerator) matrixEnvInputs(deploy *config.DeployConfig) map[stri return out } -// writeMatrixBuildingLogic generates the bash logic to build a matrix for a single deploy. -// It iterates through promotions and builds matrix entries with resolved inputs. -func (g *PromoteGenerator) writeMatrixBuildingLogic(sb *strings.Builder, deploy *config.DeployConfig, outputName string) { - // Serialize default inputs (passthrough expressions excluded; state.* - // refs resolved per-env into env_inputs below). +// matrixInputsJSON serializes a deploy's default inputs and env_inputs as +// compact JSON (passthrough expressions excluded; state.* refs resolved +// per-env into env_inputs). +func (g *PromoteGenerator) matrixInputsJSON(deploy *config.DeployConfig) (string, string) { defaultInputsJSON, err := json.Marshal(g.matrixDefaultInputs(deploy)) if err != nil { defaultInputsJSON = []byte("{}") } - - // Serialize env_inputs (passthrough excluded, state.* resolved). envInputsJSON, err := json.Marshal(g.matrixEnvInputs(deploy)) if err != nil { envInputsJSON = []byte("{}") } + return string(defaultInputsJSON), string(envInputsJSON) +} - fmt.Fprintf(sb, " DEFAULT_INPUTS='%s'\n", string(defaultInputsJSON)) - fmt.Fprintf(sb, " ENV_INPUTS='%s'\n", string(envInputsJSON)) +// writeMatrixBuildingLogic generates the bash logic to build a matrix for a single deploy. +// It iterates through promotions and builds matrix entries with resolved inputs. +func (g *PromoteGenerator) writeMatrixBuildingLogic(sb *strings.Builder, outputName string) { + // The serialized input JSON carries operator-authored values verbatim, so + // it must never sit inside a shell quote literal in the script (a single + // quote in an ordinary value would break parsing; a crafted value would + // execute). The step env: map carries the JSON and the script reads it + // back through quoted variable references. + upper := strings.ToUpper(outputName) + fmt.Fprintf(sb, " DEFAULT_INPUTS=\"$DEFAULT_INPUTS_%s\"\n", upper) + fmt.Fprintf(sb, " ENV_INPUTS=\"$ENV_INPUTS_%s\"\n", upper) sb.WriteString(" \n") sb.WriteString(" # Iterate through promotions\n") sb.WriteString(" FIRST=true\n") diff --git a/internal/generate/promote_matrix_script_test.go b/internal/generate/promote_matrix_script_test.go index 7b517f7..dc7879d 100644 --- a/internal/generate/promote_matrix_script_test.go +++ b/internal/generate/promote_matrix_script_test.go @@ -15,9 +15,11 @@ import ( "github.com/stablekernel/cascade/internal/config" ) -// extractStepRun parses a generated workflow and returns the run script of the -// step with the given id, failing the test if no such step exists. -func extractStepRun(t *testing.T, workflowYAML, stepID string) string { +// extractStepRun parses a generated workflow and returns the run script of +// the step with the given id plus its env: entries verbatim (the caller +// overrides expression-valued ones like PROMOTION_RESULT), failing the test +// if no such step exists. +func extractStepRun(t *testing.T, workflowYAML, stepID string) (string, map[string]string) { t.Helper() var wf map[string]interface{} @@ -43,12 +45,20 @@ func extractStepRun(t *testing.T, workflowYAML, stepID string) string { if step["id"] == stepID { run, ok := step["run"].(string) require.True(t, ok && run != "", "step %q has no run script", stepID) - return run + env := make(map[string]string) + if rawEnv, ok := step["env"].(map[string]interface{}); ok { + for k, v := range rawEnv { + if val, ok := v.(string); ok { + env[k] = val + } + } + } + return run, env } } } t.Fatalf("step %q not found in generated workflow", stepID) - return "" + return "", nil } // TestGeneratedMatrixScript_ResolvesDeployInputsAtRuntime executes the emitted @@ -93,7 +103,7 @@ func TestGeneratedMatrixScript_ResolvesDeployInputsAtRuntime(t *testing.T) { content, err := gen.Generate() require.NoError(t, err) - script := extractStepRun(t, content, "build-matrices") + script, stepEnv := extractStepRun(t, content, "build-matrices") // runScript executes the emitted step exactly as GitHub Actions would // (bash -e), feeding the preflight promotion result through the same env @@ -105,7 +115,14 @@ func TestGeneratedMatrixScript_ResolvesDeployInputsAtRuntime(t *testing.T) { require.NoError(t, os.WriteFile(outFile, nil, 0o600)) cmd := exec.Command("bash", "-e", "-c", script) - cmd.Env = append(os.Environ(), + cmd.Env = os.Environ() + // Wire the step's env: entries (the env-routed input JSON) exactly as + // GitHub Actions would; test-controlled values are appended last so + // they override the expression-valued entries. + for k, v := range stepEnv { + cmd.Env = append(cmd.Env, k+"="+v) + } + cmd.Env = append(cmd.Env, "PROMOTION_RESULT="+promotionResult, "GITHUB_OUTPUT="+outFile, ) diff --git a/internal/generate/promote_test.go b/internal/generate/promote_test.go index 313e1d0..45e544c 100644 --- a/internal/generate/promote_test.go +++ b/internal/generate/promote_test.go @@ -995,8 +995,10 @@ func TestPreflightDeployMatrixOutputs(t *testing.T) { // Should have matrix building logic for app deploy assert.Contains(t, content, "# Build matrix for deploy: app") assert.Contains(t, content, "MATRIX_APP='['") - assert.Contains(t, content, `DEFAULT_INPUTS='{"cluster":"dev-eks","environment":"${{ matrix.environment }}","sha":"${{ matrix.sha }}"}'`) - assert.Contains(t, content, `ENV_INPUTS='{"prod":{"cluster":"prod-eks"}}'`) + assert.Contains(t, content, `DEFAULT_INPUTS_APP: '{"cluster":"dev-eks","environment":"${{ matrix.environment }}","sha":"${{ matrix.sha }}"}'`) + assert.Contains(t, content, `ENV_INPUTS_APP: '{"prod":{"cluster":"prod-eks"}}'`) + assert.Contains(t, content, `DEFAULT_INPUTS="$DEFAULT_INPUTS_APP"`) + assert.Contains(t, content, `ENV_INPUTS="$ENV_INPUTS_APP"`) // Should have matrix building logic for infra deploy assert.Contains(t, content, "# Build matrix for deploy: infra") diff --git a/internal/generate/yaml_quote.go b/internal/generate/yaml_quote.go new file mode 100644 index 0000000..1df1df1 --- /dev/null +++ b/internal/generate/yaml_quote.go @@ -0,0 +1,12 @@ +package generate + +import "strings" + +// yamlSingleQuote renders s as a YAML single-quoted scalar. The only escape +// in that style is doubling embedded single quotes, so any operator-authored +// value (apostrophes included) round-trips intact instead of breaking the +// emitted document. Callers pass single-line values; JSON-serialized input +// blobs and dispatch-input defaults never carry raw newlines. +func yamlSingleQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "''") + "'" +}