From d71db6de98ce71fe0579aa5b19196015b894d529 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 16 Jun 2026 14:01:52 -0400 Subject: [PATCH] fix: wrap bare secret-name tokens in setup-cli step A manifest that sets release_token (or state_token, or a notify token) to a bare secret name such as CASCADE_STATE_TOKEN caused the generator to emit that name verbatim into the Setup CLI step (token: CASCADE_STATE_TOKEN). The setup-cli action then ran gh release download with GH_TOKEN set to the literal string, which GitHub rejected with 401 Bad credentials. Normalize token values in GetReleaseToken, GetStateToken, and NotifyConfig.GetToken: a full ${{ ... }} expression passes through, an unwrapped context form (secrets.X, vars.X) is wrapped, and a bare name is treated as a secret. The Setup CLI download targets a public release, so the wrapped expression resolves correctly. Adds unit coverage for the normalizer and the token getters, a generator regression test asserting no emitted token is a bare identifier, and an e2e scenario exercising a bare release_token. Signed-off-by: Joshua Temple --- e2e/harness/scenario.go | 4 ++ .../22-release-token-bare-secret-name.yaml | 44 ++++++++++++ internal/config/types.go | 39 +++++++++-- internal/config/types_test.go | 49 +++++++++++++ internal/generate/generator_test.go | 70 +++++++++++++++++++ 5 files changed, 201 insertions(+), 5 deletions(-) create mode 100644 e2e/scenarios/orchestrate/22-release-token-bare-secret-name.yaml diff --git a/e2e/harness/scenario.go b/e2e/harness/scenario.go index 0d66e0b4..c20a7526 100644 --- a/e2e/harness/scenario.go +++ b/e2e/harness/scenario.go @@ -31,6 +31,10 @@ type Config struct { TrunkBranch string `yaml:"trunk_branch"` Environments []string `yaml:"environments"` JobTimeoutMinutes int `yaml:"job_timeout_minutes,omitempty"` + // ReleaseToken carries the release_token field through to the generated + // manifest. It accepts a full ${{ secrets.* }} expression or a bare secret + // name; the generator normalizes a bare name to a resolvable expression. + ReleaseToken string `yaml:"release_token,omitempty"` Builds []BuildConfig `yaml:"builds"` Deploys []DeployConfig `yaml:"deploys"` Publish *PublishConfig `yaml:"publish,omitempty"` diff --git a/e2e/scenarios/orchestrate/22-release-token-bare-secret-name.yaml b/e2e/scenarios/orchestrate/22-release-token-bare-secret-name.yaml new file mode 100644 index 00000000..5293fc1b --- /dev/null +++ b/e2e/scenarios/orchestrate/22-release-token-bare-secret-name.yaml @@ -0,0 +1,44 @@ +name: "Release Token Bare Secret Name Is Wrapped" +description: | + Reproduces a manifest that sets release_token to a bare secret name (the shape + that previously broke a multi-env example repo). The Setup CLI step downloads + the cascade binary from a public release; it must use a resolvable token. When + the operator writes a bare name like CASCADE_STATE_TOKEN, the generator must + normalize it to ${{ secrets.CASCADE_STATE_TOKEN }} rather than emit the bare + identifier, which would reach gh release download as a literal string and fail + with 401 Bad credentials. + + Generator-output verification scenario; assertion runs on the staged repo after + StageRepoFromConfig generates workflows but before any orchestrate runs. + +config: + trunk_branch: main + environments: ["dev", "test", "staging", "prod"] + release_token: CASCADE_STATE_TOKEN + builds: + - name: app + workflow: build-app.yaml + triggers: ["src/**"] + deploys: + - name: app + workflow: deploy-app.yaml + triggers: [] + +steps: + - name: "Initial commit; assert the bare release_token is wrapped" + action: commit + commit: + message: "feat: add app build and deploy with a bare release_token" + files: + src/main.go: | + package main + func main() {} + expect: + workflow_files: + - path: ".github/workflows/orchestrate.yaml" + contains: + # Setup CLI token is the wrapped, resolvable secrets expression. + - "token: ${{ secrets.CASCADE_STATE_TOKEN }}" + not_contains: + # The bare secret name must never be emitted as the token value. + - "token: CASCADE_STATE_TOKEN" diff --git a/internal/config/types.go b/internal/config/types.go index 0230fd61..b1ff582e 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -272,13 +272,42 @@ func (c *TrunkConfig) GetTagPrefix() string { return c.TagPrefix } -// GetReleaseToken returns the configured release token expression or "${{ secrets.GITHUB_TOKEN }}" if not specified. -// Users should provide the full GitHub Actions expression, e.g. "${{ secrets.MY_TOKEN }}". +// normalizeTokenExpression returns a GitHub Actions expression that resolves to +// a token at run time. It accepts either a full expression +// ("${{ secrets.MY_TOKEN }}"), an unwrapped context form ("secrets.MY_TOKEN", +// "vars.MY_TOKEN"), or a bare secret name ("MY_TOKEN") and always returns a +// wrapped "${{ ... }}" expression. A bare name is treated as a secret, matching +// the documented "GitHub secret name" intent of the token fields. This prevents +// a bare name from being emitted verbatim into a workflow, which would resolve +// to a literal string token and fail authentication. +func normalizeTokenExpression(value string) string { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return "" + } + // Already a full expression: leave it untouched. + if strings.HasPrefix(trimmed, "${{") && strings.HasSuffix(trimmed, "}}") { + return trimmed + } + // Unwrapped context form (secrets.X, vars.X, env.X, ...): wrap it. + if i := strings.IndexByte(trimmed, '.'); i > 0 { + switch trimmed[:i] { + case "secrets", "vars", "env", "inputs", "github": + return "${{ " + trimmed + " }}" + } + } + // Bare name: treat it as a secret. + return "${{ secrets." + trimmed + " }}" +} + +// GetReleaseToken returns the configured release token as a resolvable GitHub +// Actions expression, or "${{ secrets.GITHUB_TOKEN }}" if not specified. A bare +// secret name (e.g. "MY_TOKEN") is normalized to "${{ secrets.MY_TOKEN }}". func (c *TrunkConfig) GetReleaseToken() string { if c.ReleaseToken == "" { return "${{ secrets.GITHUB_TOKEN }}" } - return c.ReleaseToken + return normalizeTokenExpression(c.ReleaseToken) } // GetStateToken returns the configured state-write token expression or @@ -292,7 +321,7 @@ func (c *TrunkConfig) GetStateToken() string { if c.StateToken == "" { return "${{ secrets.GITHUB_TOKEN }}" } - return c.StateToken + return normalizeTokenExpression(c.StateToken) } // GetManifestFile returns the configured manifest file path or ".github/manifest.yaml" if not specified @@ -552,7 +581,7 @@ func (n *NotifyConfig) GetToken() string { if n.Token == "" { return "${{ secrets.PRIMARY_REPO_TOKEN }}" } - return n.Token + return normalizeTokenExpression(n.Token) } // ChangelogConfig defines changelog generation settings diff --git a/internal/config/types_test.go b/internal/config/types_test.go index 159e2eed..0ac0cd05 100644 --- a/internal/config/types_test.go +++ b/internal/config/types_test.go @@ -426,6 +426,16 @@ func TestGetReleaseToken(t *testing.T) { // Configured value (full expression) cfg.ReleaseToken = "${{ secrets.CUSTOM_RELEASE_TOKEN }}" assert.Equal(t, "${{ secrets.CUSTOM_RELEASE_TOKEN }}", cfg.GetReleaseToken()) + + // Bare secret name is normalized to a resolvable secrets expression. + // The field doc advertises a "GitHub secret name", so a bare name must + // not be emitted verbatim (that produces a literal token and a 401). + cfg.ReleaseToken = "CASCADE_STATE_TOKEN" + assert.Equal(t, "${{ secrets.CASCADE_STATE_TOKEN }}", cfg.GetReleaseToken()) + + // Unwrapped context form is also wrapped. + cfg.ReleaseToken = "secrets.CASCADE_STATE_TOKEN" + assert.Equal(t, "${{ secrets.CASCADE_STATE_TOKEN }}", cfg.GetReleaseToken()) } func TestGetStateToken(t *testing.T) { @@ -436,6 +446,45 @@ func TestGetStateToken(t *testing.T) { // Configured value (full expression) cfg.StateToken = "${{ secrets.CASCADE_BOT_TOKEN }}" assert.Equal(t, "${{ secrets.CASCADE_BOT_TOKEN }}", cfg.GetStateToken()) + + // Bare secret name is normalized. + cfg.StateToken = "CASCADE_BOT_TOKEN" + assert.Equal(t, "${{ secrets.CASCADE_BOT_TOKEN }}", cfg.GetStateToken()) +} + +func TestNotifyConfigGetToken(t *testing.T) { + // Default when not set + n := &NotifyConfig{} + assert.Equal(t, "${{ secrets.PRIMARY_REPO_TOKEN }}", n.GetToken()) + + // Full expression passes through. + n.Token = "${{ secrets.CROSS_REPO_TOKEN }}" + assert.Equal(t, "${{ secrets.CROSS_REPO_TOKEN }}", n.GetToken()) + + // Bare secret name is normalized. + n.Token = "CROSS_REPO_TOKEN" + assert.Equal(t, "${{ secrets.CROSS_REPO_TOKEN }}", n.GetToken()) +} + +func TestNormalizeTokenExpression(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {name: "full secrets expression", input: "${{ secrets.X }}", want: "${{ secrets.X }}"}, + {name: "full vars expression", input: "${{ vars.Y }}", want: "${{ vars.Y }}"}, + {name: "bare secret name", input: "CASCADE_STATE_TOKEN", want: "${{ secrets.CASCADE_STATE_TOKEN }}"}, + {name: "unwrapped secrets context", input: "secrets.MY_TOKEN", want: "${{ secrets.MY_TOKEN }}"}, + {name: "unwrapped vars context", input: "vars.MY_VAR", want: "${{ vars.MY_VAR }}"}, + {name: "surrounding whitespace trimmed", input: " CASCADE_STATE_TOKEN ", want: "${{ secrets.CASCADE_STATE_TOKEN }}"}, + {name: "empty stays empty", input: "", want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, normalizeTokenExpression(tt.input)) + }) + } } func TestGetGitMode(t *testing.T) { diff --git a/internal/generate/generator_test.go b/internal/generate/generator_test.go index 346c4361..cf5a292a 100644 --- a/internal/generate/generator_test.go +++ b/internal/generate/generator_test.go @@ -45,6 +45,76 @@ func TestGenerator_OrchestrateHasConcurrencyBlock(t *testing.T) { assert.Contains(t, result, "cancel-in-progress: true", "default concurrency cancels in-progress") } +// bareTokenPattern matches a `token: NAME` (or `GH_TOKEN: NAME`) line whose +// value is a bare, unresolved identifier rather than a `${{ ... }}` expression. +// Such a value reaches a step as a literal string and fails authentication. +var bareTokenPattern = regexp.MustCompile(`(?m)^\s*(?:GH_)?[Tt][Oo][Kk][Ee][Nn]:\s+([A-Za-z_][A-Za-z0-9_]*)\s*$`) + +// TestGenerator_ReleaseTokenBareSecretNameIsWrapped reproduces the +// cascade-example-primary manifest shape: a `release_token` set to a bare +// secret name. The generator previously emitted that name verbatim into the +// Setup CLI step (`token: CASCADE_STATE_TOKEN`), which `gh release download` +// then treated as a literal token and rejected with 401. The token input must +// be a resolvable `${{ secrets.* }}` expression. +func TestGenerator_ReleaseTokenBareSecretNameIsWrapped(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev", "test", "staging", "prod"}, + ReleaseToken: "CASCADE_STATE_TOKEN", // bare secret name, as in the primary example + 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) + + // The Setup CLI token must be the wrapped, resolvable expression. + assert.Contains(t, result, "token: ${{ secrets.CASCADE_STATE_TOKEN }}", + "Setup CLI token must be a resolvable secrets expression") + // And must never appear as the bare name. + assert.NotContains(t, result, "token: CASCADE_STATE_TOKEN\n", + "Setup CLI token must not be a bare secret name") + + // Regression guard: no emitted token: (or GH_TOKEN:) line may be a bare + // identifier. Every token must be a ${{ ... }} expression. + if m := bareTokenPattern.FindStringSubmatch(result); m != nil { + t.Errorf("generated workflow emits a bare, unresolved token value %q; tokens must be ${{ ... }} expressions", m[1]) + } +} + +// TestGenerator_DefaultReleaseTokenIsGitHubToken confirms the single-env shape +// (no release_token configured) still emits the GITHUB_TOKEN expression, which +// can read public releases. +func TestGenerator_DefaultReleaseTokenIsGitHubToken(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + + 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, "token: ${{ secrets.GITHUB_TOKEN }}", + "default Setup CLI token must be GITHUB_TOKEN") + if m := bareTokenPattern.FindStringSubmatch(result); m != nil { + t.Errorf("generated workflow emits a bare token value %q", m[1]) + } +} + // TestGenerator_OrchestrateConcurrencyOverride asserts manifest config // can override both group and cancel-in-progress. func TestGenerator_OrchestrateConcurrencyOverride(t *testing.T) {