[object Object]

← back to Cli Printing Press

fix(cli): apply Bearer prefix in composed and cookie AuthHeader paths (#1500)

ca4898a396731026e1f9e33497d966ebf9d2165d · 2026-05-16 01:15:25 -0700 · Trevin Chow

* fix(cli): apply Bearer prefix in composed and cookie AuthHeader paths

The generator's composed and cookie auth branches in `AuthHeader()` returned
the raw env-var token or chrome-imported AccessToken without applying the
spec's HeaderPrefix, so any CLI configured via env-var fallback hit 401 on
every call (`Authorization: <raw>` instead of `Authorization: Bearer <raw>`).
The `auth login --chrome` path masked this because the token exchanged
through Clerk already had the prefix; the env-var path documented in the
SKILL did not.

Composed and cookie branches now wrap returned tokens in a new
`ensureAuthScheme(scheme, token)` helper that applies `Auth.HeaderPrefix`
(defaulting to `Bearer`) and skips the prefix when the value already
carries it case-insensitively. This handles users who pre-prefixed their
env var to work around the bug without double-prefixing.

bearer_token/api_key/oauth2 branches are untouched.

Closes #1419

* test(cli): extend ensureAuthScheme runtime test to cover cookie auth

Refactored TestAuthHeaderComposedSchemeHelperHandlesPreprefixedTokens into
a table-driven test that runs the same three runtime assertions (Bearer
applied, no double-prefix, blank returns empty) for both composed and
cookie auth types. The cookie branch was previously only covered by source
inspection, leaving a gap where a wrong scheme argument in the cookie
AccessToken path could silently emit a bare token.

Also dropped Format: "Bearer {token}" from the runtime test spec. The
composed and cookie branches in AuthHeader() do not consult Auth.Format —
the Bearer prefix comes from HeaderPrefix()'s default — and keeping
Format in the spec implied otherwise.

* test(cli): drop fragile findReturnEnd slice in favor of whole-file checks

The findReturnEnd helper sliced the emitted config.go up to the first
"return " in each if-block to bound the NotContains check. That heuristic
silently shortens the slice if the emitted block ever contains the literal
text "return " before the actual return statement (e.g. an AuthSource
string value), letting a regression slip through.

Replaced with whole-file NotContains for the raw-return literals plus
positive Contains for the ensureAuthScheme wrapper call. The raw return
literal should never appear in correctly emitted composed/cookie config,
so the file-wide check is both simpler and tighter, and the positive
assertion catches the wrong-helper-argument case the previous test was
trying to defend against.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Files touched

Diff

commit ca4898a396731026e1f9e33497d966ebf9d2165d
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat May 16 01:15:25 2026 -0700

    fix(cli): apply Bearer prefix in composed and cookie AuthHeader paths (#1500)
    
    * fix(cli): apply Bearer prefix in composed and cookie AuthHeader paths
    
    The generator's composed and cookie auth branches in `AuthHeader()` returned
    the raw env-var token or chrome-imported AccessToken without applying the
    spec's HeaderPrefix, so any CLI configured via env-var fallback hit 401 on
    every call (`Authorization: <raw>` instead of `Authorization: Bearer <raw>`).
    The `auth login --chrome` path masked this because the token exchanged
    through Clerk already had the prefix; the env-var path documented in the
    SKILL did not.
    
    Composed and cookie branches now wrap returned tokens in a new
    `ensureAuthScheme(scheme, token)` helper that applies `Auth.HeaderPrefix`
    (defaulting to `Bearer`) and skips the prefix when the value already
    carries it case-insensitively. This handles users who pre-prefixed their
    env var to work around the bug without double-prefixing.
    
    bearer_token/api_key/oauth2 branches are untouched.
    
    Closes #1419
    
    * test(cli): extend ensureAuthScheme runtime test to cover cookie auth
    
    Refactored TestAuthHeaderComposedSchemeHelperHandlesPreprefixedTokens into
    a table-driven test that runs the same three runtime assertions (Bearer
    applied, no double-prefix, blank returns empty) for both composed and
    cookie auth types. The cookie branch was previously only covered by source
    inspection, leaving a gap where a wrong scheme argument in the cookie
    AccessToken path could silently emit a bare token.
    
    Also dropped Format: "Bearer {token}" from the runtime test spec. The
    composed and cookie branches in AuthHeader() do not consult Auth.Format —
    the Bearer prefix comes from HeaderPrefix()'s default — and keeping
    Format in the spec implied otherwise.
    
    * test(cli): drop fragile findReturnEnd slice in favor of whole-file checks
    
    The findReturnEnd helper sliced the emitted config.go up to the first
    "return " in each if-block to bound the NotContains check. That heuristic
    silently shortens the slice if the emitted block ever contains the literal
    text "return " before the actual return statement (e.g. an AuthSource
    string value), letting a regression slip through.
    
    Replaced with whole-file NotContains for the raw-return literals plus
    positive Contains for the ensureAuthScheme wrapper call. The raw return
    literal should never appear in correctly emitted composed/cookie config,
    so the file-wide check is both simpler and tighter, and the positive
    assertion catches the wrong-helper-argument case the previous test was
    trying to defend against.
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/templates/config.go.tmpl |  33 +++++--
 internal/generator/templates_test.go        | 140 ++++++++++++++++++++++++++++
 2 files changed, 167 insertions(+), 6 deletions(-)

diff --git a/internal/generator/templates/config.go.tmpl b/internal/generator/templates/config.go.tmpl
index d92f4dec..f15749df 100644
--- a/internal/generator/templates/config.go.tmpl
+++ b/internal/generator/templates/config.go.tmpl
@@ -445,20 +445,20 @@ func (c *Config) AuthHeader() string {
 {{- range .Auth.EnvVarSpecs}}
 	if c.{{resolveEnvVarField .Name}} != "" {
 		c.AuthSource = "env:{{.Name}}"
-		return c.{{resolveEnvVarField .Name}}
+		return ensureAuthScheme("{{$.Auth.HeaderPrefix}}", c.{{resolveEnvVarField .Name}})
 	}
 {{- end}}
 {{- else}}
 	{{- with $canonicalEnvVar}}
 	if c.{{resolveEnvVarField .Name}} != "" {
 		c.AuthSource = "env:{{.Name}}"
-		return c.{{resolveEnvVarField .Name}}
+		return ensureAuthScheme("{{$.Auth.HeaderPrefix}}", c.{{resolveEnvVarField .Name}})
 	}
 	{{- end}}
 {{- end}}
 	if c.AccessToken != "" {
 		c.AuthSource = "browser"
-		return c.AccessToken
+		return ensureAuthScheme("{{.Auth.HeaderPrefix}}", c.AccessToken)
 	}
 	return ""
 {{- else if eq .Auth.Type "composed"}}
@@ -467,20 +467,20 @@ func (c *Config) AuthHeader() string {
 {{- range .Auth.EnvVarSpecs}}
 	if c.{{resolveEnvVarField .Name}} != "" {
 		c.AuthSource = "env:{{.Name}}"
-		return c.{{resolveEnvVarField .Name}}
+		return ensureAuthScheme("{{$.Auth.HeaderPrefix}}", c.{{resolveEnvVarField .Name}})
 	}
 {{- end}}
 {{- else}}
 	{{- with $canonicalEnvVar}}
 	if c.{{resolveEnvVarField .Name}} != "" {
 		c.AuthSource = "env:{{.Name}}"
-		return c.{{resolveEnvVarField .Name}}
+		return ensureAuthScheme("{{$.Auth.HeaderPrefix}}", c.{{resolveEnvVarField .Name}})
 	}
 	{{- end}}
 {{- end}}
 	if c.AccessToken != "" {
 		c.AuthSource = "chrome-composed"
-		return c.AccessToken
+		return ensureAuthScheme("{{.Auth.HeaderPrefix}}", c.AccessToken)
 	}
 	return ""
 {{- else}}
@@ -501,6 +501,27 @@ func applyAuthFormat(format string, replacements map[string]string) string {
 	return format
 }
 
+{{- if or (eq .Auth.Type "composed") (eq .Auth.Type "cookie")}}
+
+// ensureAuthScheme returns "<scheme> <token>" but skips the prefix when the
+// token already carries it case-insensitively, so a user who exports the
+// env var with the scheme already attached doesn't end up double-prefixed.
+// Empty scheme returns the token as-is.
+func ensureAuthScheme(scheme, token string) string {
+	if token == "" {
+		return ""
+	}
+	if scheme == "" {
+		return token
+	}
+	schemeWithSpace := scheme + " "
+	if len(token) >= len(schemeWithSpace) && strings.EqualFold(token[:len(schemeWithSpace)], schemeWithSpace) {
+		return token
+	}
+	return schemeWithSpace + token
+}
+{{- end}}
+
 {{- if .HasAuthCommand}}
 
 func (c *Config) SaveTokens(clientID, clientSecret, accessToken, refreshToken string, expiry time.Time) error {
diff --git a/internal/generator/templates_test.go b/internal/generator/templates_test.go
index cd491382..3485ca4b 100644
--- a/internal/generator/templates_test.go
+++ b/internal/generator/templates_test.go
@@ -249,3 +249,143 @@ func TestAuthHeaderTokenEnvVarsDoNotEmitDuplicateMapKeys(t *testing.T) {
 		})
 	}
 }
+
+// TestAuthHeaderComposedAndCookieApplySchemePrefix pins the fix for #1419:
+// composed and cookie auth types must apply the spec's HeaderPrefix (defaulting
+// to "Bearer ") when emitting AuthHeader(), so env-var and chrome-composed
+// token paths don't return raw tokens that the upstream API will reject as
+// "Authorization: <raw>" instead of "Authorization: Bearer <raw>".
+func TestAuthHeaderComposedAndCookieApplySchemePrefix(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name       string
+		authType   string
+		envVar     string
+		envField   string
+		authSource string
+	}{
+		{
+			name:       "composed-single-env-var",
+			authType:   "composed",
+			envVar:     "SUNO_TOKEN",
+			envField:   "SunoToken",
+			authSource: "chrome-composed",
+		},
+		{
+			name:       "cookie-single-env-var",
+			authType:   "cookie",
+			envVar:     "NOTION_TOKEN",
+			envField:   "NotionToken",
+			authSource: "browser",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			apiSpec := minimalSpec(tt.name)
+			apiSpec.Auth = spec.AuthConfig{
+				Type:    tt.authType,
+				Header:  "Authorization",
+				EnvVars: []string{tt.envVar},
+			}
+
+			outputDir := filepath.Join(t.TempDir(), tt.name+"-pp-cli")
+			require.NoError(t, New(apiSpec, outputDir).Generate())
+
+			configSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "config", "config.go"))
+			require.NoError(t, err)
+			content := string(configSrc)
+
+			// The raw-return literal must appear nowhere in the emitted config — every
+			// composed/cookie return path now flows through ensureAuthScheme. Whole-file
+			// NotContains avoids depending on a fragile slice boundary.
+			require.NotContains(t, content, "return c."+tt.envField+"\n",
+				"%s auth must not return raw env-var token without scheme prefix:\n%s", tt.authType, content)
+			require.NotContains(t, content, "return c.AccessToken\n",
+				"%s auth must not return raw AccessToken without scheme prefix:\n%s", tt.authType, content)
+
+			// The replacement ensureAuthScheme call must be the wrapper for both paths.
+			require.Contains(t, content,
+				`return ensureAuthScheme("Bearer", c.`+tt.envField+")",
+				"%s auth env-var path should wrap return in ensureAuthScheme:\n%s", tt.authType, content)
+			require.Contains(t, content,
+				`return ensureAuthScheme("Bearer", c.AccessToken)`,
+				"%s auth AccessToken path should wrap return in ensureAuthScheme:\n%s", tt.authType, content)
+
+			// Confirm AuthSource label preserved (no behavior regression on which branch we took).
+			require.Contains(t, content, `c.AuthSource = "`+tt.authSource+`"`)
+
+			// Generated config must compile and pass its own tests.
+			runGoCommand(t, outputDir, "test", "./internal/config")
+		})
+	}
+}
+
+// TestAuthHeaderSchemeHelperHandlesPreprefixedTokens compiles a composed-auth
+// and a cookie-auth CLI and exercises the emitted ensureAuthScheme helper
+// through AuthHeader() to confirm the positive (Bearer prefix applied) and
+// negative (no double prefix when the user pre-prefixes the env var) cases
+// for both auth types. The Bearer prefix here comes from HeaderPrefix()'s
+// default (the composed and cookie branches do not consult Auth.Format), so
+// the test specs intentionally omit Format.
+func TestAuthHeaderSchemeHelperHandlesPreprefixedTokens(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		authType string
+		envVar   string
+		envField string
+	}{
+		{name: "composed", authType: "composed", envVar: "FOO_TOKEN", envField: "FooToken"},
+		{name: "cookie", authType: "cookie", envVar: "BAR_TOKEN", envField: "BarToken"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			apiSpec := minimalSpec(tt.name + "-auth-runtime")
+			apiSpec.Auth = spec.AuthConfig{
+				Type:    tt.authType,
+				Header:  "Authorization",
+				EnvVars: []string{tt.envVar},
+			}
+
+			outputDir := filepath.Join(t.TempDir(), tt.name+"-auth-runtime-pp-cli")
+			require.NoError(t, New(apiSpec, outputDir).Generate())
+
+			testFile := filepath.Join(outputDir, "internal", "config", "auth_scheme_runtime_test.go")
+			testSrc := `package config
+
+import "testing"
+
+func TestEnsureAuthSchemeAppliesBearerPrefix(t *testing.T) {
+	c := &Config{` + tt.envField + `: "eyJxxx"}
+	if got := c.AuthHeader(); got != "Bearer eyJxxx" {
+		t.Fatalf("expected Bearer-prefixed header, got %q", got)
+	}
+}
+
+func TestEnsureAuthSchemeDoesNotDoublePrefix(t *testing.T) {
+	c := &Config{` + tt.envField + `: "Bearer eyJxxx"}
+	if got := c.AuthHeader(); got != "Bearer eyJxxx" {
+		t.Fatalf("expected single Bearer prefix, got %q", got)
+	}
+}
+
+func TestEnsureAuthSchemeBlankReturnsEmpty(t *testing.T) {
+	c := &Config{}
+	if got := c.AuthHeader(); got != "" {
+		t.Fatalf("expected empty header for blank config, got %q", got)
+	}
+}
+`
+			require.NoError(t, os.WriteFile(testFile, []byte(testSrc), 0o644))
+			runGoCommand(t, outputDir, "test", "./internal/config")
+		})
+	}
+}

← f3f70435 fix(cli): refresh auth on redirects via CheckRedirect (#1497  ·  back to Cli Printing Press  ·  fix(skills): surface novel-feature hand-code scope at Phase 793d5b27 →