[object Object]

← back to Cli Printing Press

fix(cli): filter auth login --chrome cookies by spec auth.cookies allowlist (#1518)

954174014101d225f83acae04854a7cb4f234936 · 2026-05-16 11:10:14 -0700 · Trevin Chow

* fix(cli): filter auth login --chrome cookies by spec auth.cookies allowlist

The composed-auth branch of `auth login --chrome` already kept only the
cookies named in spec.auth.cookies before composing the Authorization
header. The non-composed cookie-auth branch ignored that allowlist and
saved every cookie the target domain had set into `access_token`. The
runtime then sent the whole blob (CSRF tokens, WAF cookies, anti-bot
fingerprints alongside the real credential) on every authenticated
request, routinely tripping upstream WAFs and shadowing the credential
in the Cookie header.

Apply the same allowlist filter in the non-composed branch: parse the
extracted blob, keep only the names declared in spec.auth.cookies, and
rejoin before SaveTokens. A missing required cookie now errors with a
re-login prompt instead of silently saving a bad blob. Specs that
declare no allowlist (legacy cookie-auth CLIs) keep their current
behavior unchanged.

Closes #1292

* fix(cli): filter cookie blob in refreshStoredBrowserCookies too

Greptile flagged that the login-side allowlist filter could be silently
undone by `auth refresh` on CLIs that combine `auth.type: cookie`,
`auth.cookies: [name]`, and `auth.requires_browser_session: true`. The
refresh helper runs unattended and was saving the raw extracted blob,
overwriting the filtered token that login had just persisted.

Apply the same parse-filter-rejoin block to the non-composed branch of
refreshStoredBrowserCookies. Specs without an allowlist still take the
existing code path. Added a subtest that pins the refresh-path filter on
a requires_browser_session spec.

Refs #1292

---------

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

Files touched

Diff

commit 954174014101d225f83acae04854a7cb4f234936
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat May 16 11:10:14 2026 -0700

    fix(cli): filter auth login --chrome cookies by spec auth.cookies allowlist (#1518)
    
    * fix(cli): filter auth login --chrome cookies by spec auth.cookies allowlist
    
    The composed-auth branch of `auth login --chrome` already kept only the
    cookies named in spec.auth.cookies before composing the Authorization
    header. The non-composed cookie-auth branch ignored that allowlist and
    saved every cookie the target domain had set into `access_token`. The
    runtime then sent the whole blob (CSRF tokens, WAF cookies, anti-bot
    fingerprints alongside the real credential) on every authenticated
    request, routinely tripping upstream WAFs and shadowing the credential
    in the Cookie header.
    
    Apply the same allowlist filter in the non-composed branch: parse the
    extracted blob, keep only the names declared in spec.auth.cookies, and
    rejoin before SaveTokens. A missing required cookie now errors with a
    re-login prompt instead of silently saving a bad blob. Specs that
    declare no allowlist (legacy cookie-auth CLIs) keep their current
    behavior unchanged.
    
    Closes #1292
    
    * fix(cli): filter cookie blob in refreshStoredBrowserCookies too
    
    Greptile flagged that the login-side allowlist filter could be silently
    undone by `auth refresh` on CLIs that combine `auth.type: cookie`,
    `auth.cookies: [name]`, and `auth.requires_browser_session: true`. The
    refresh helper runs unattended and was saving the raw extracted blob,
    overwriting the filtered token that login had just persisted.
    
    Apply the same parse-filter-rejoin block to the non-composed branch of
    refreshStoredBrowserCookies. Specs without an allowlist still take the
    existing code path. Added a subtest that pins the refresh-path filter on
    a requires_browser_session spec.
    
    Refs #1292
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/generator_test.go              | 129 ++++++++++++++++++++++
 internal/generator/templates/auth_browser.go.tmpl |  43 +++++++-
 2 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 5bd95d38..e7335a11 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -6502,6 +6502,135 @@ func TestGenerate_CookieAuthWindowsCompatibility(t *testing.T) {
 	assert.Contains(t, content, "cookie-scoop-cli")
 }
 
+// TestGenerate_CookieAuthFiltersAllowlistOnLogin pins that `auth login --chrome`
+// stores only the cookies named in spec.auth.cookies when an allowlist is
+// declared, and falls back to the unfiltered blob when the allowlist is
+// empty (legacy specs).
+func TestGenerate_CookieAuthFiltersAllowlistOnLogin(t *testing.T) {
+	t.Parallel()
+
+	t.Run("allowlist declared filters extracted cookies", func(t *testing.T) {
+		t.Parallel()
+		apiSpec := &spec.APISpec{
+			Name:    "filterapp",
+			Version: "0.1.0",
+			BaseURL: "https://app.example.com",
+			Auth: spec.AuthConfig{
+				Type:         "cookie",
+				Header:       "Cookie",
+				In:           "cookie",
+				CookieDomain: ".example.com",
+				Cookies:      []string{"session_id"},
+				EnvVars:      []string{"FILTERAPP_COOKIES"},
+			},
+			Config: spec.ConfigSpec{Format: "toml"},
+			Resources: map[string]spec.Resource{
+				"items": {
+					Description: "Manage items",
+					Endpoints: map[string]spec.Endpoint{
+						"list": {Method: "GET", Path: "/api/items", Description: "List items"},
+					},
+				},
+			},
+		}
+
+		outputDir := filepath.Join(t.TempDir(), "filterapp-pp-cli")
+		require.NoError(t, New(apiSpec, outputDir).Generate())
+
+		auth, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "auth.go"))
+		require.NoError(t, err)
+		content := string(auth)
+
+		assert.Contains(t, content, `requiredCookies := []string{"session_id"}`)
+		assert.Contains(t, content, "cookieMap := parseCookieString(cookies)")
+		assert.Contains(t, content, `cookies = strings.Join(kept, "; ")`)
+		assert.Contains(t, content, `cookie %q not found for %s`)
+
+		runGoCommand(t, outputDir, "build", "./...")
+	})
+
+	t.Run("empty allowlist preserves legacy behavior", func(t *testing.T) {
+		t.Parallel()
+		apiSpec := &spec.APISpec{
+			Name:    "legacycookieapp",
+			Version: "0.1.0",
+			BaseURL: "https://app.example.com",
+			Auth: spec.AuthConfig{
+				Type:         "cookie",
+				Header:       "Cookie",
+				In:           "cookie",
+				CookieDomain: ".example.com",
+				EnvVars:      []string{"LEGACYCOOKIEAPP_COOKIES"},
+			},
+			Config: spec.ConfigSpec{Format: "toml"},
+			Resources: map[string]spec.Resource{
+				"items": {
+					Description: "Manage items",
+					Endpoints: map[string]spec.Endpoint{
+						"list": {Method: "GET", Path: "/api/items", Description: "List items"},
+					},
+				},
+			},
+		}
+
+		outputDir := filepath.Join(t.TempDir(), "legacycookieapp-pp-cli")
+		require.NoError(t, New(apiSpec, outputDir).Generate())
+
+		auth, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "auth.go"))
+		require.NoError(t, err)
+		content := string(auth)
+
+		assert.NotContains(t, content, "requiredCookies := []string{")
+		assert.NotContains(t, content, `cookies = strings.Join(kept, "; ")`)
+	})
+
+	t.Run("refresh path also filters by allowlist", func(t *testing.T) {
+		t.Parallel()
+		apiSpec := &spec.APISpec{
+			Name:    "refreshfilterapp",
+			Version: "0.1.0",
+			BaseURL: "https://app.example.com",
+			Auth: spec.AuthConfig{
+				Type:                           "cookie",
+				Header:                         "Cookie",
+				In:                             "cookie",
+				CookieDomain:                   ".example.com",
+				Cookies:                        []string{"session_id"},
+				EnvVars:                        []string{"REFRESHFILTERAPP_COOKIES"},
+				RequiresBrowserSession:         true,
+				BrowserSessionValidationPath:   "/api/items",
+				BrowserSessionValidationMethod: "GET",
+			},
+			Config: spec.ConfigSpec{Format: "toml"},
+			Resources: map[string]spec.Resource{
+				"items": {
+					Description: "Manage items",
+					Endpoints: map[string]spec.Endpoint{
+						"list": {Method: "GET", Path: "/api/items", Description: "List items"},
+					},
+				},
+			},
+		}
+
+		outputDir := filepath.Join(t.TempDir(), "refreshfilterapp-pp-cli")
+		require.NoError(t, New(apiSpec, outputDir).Generate())
+
+		auth, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "auth.go"))
+		require.NoError(t, err)
+		content := string(auth)
+
+		require.Contains(t, content, "func refreshStoredBrowserCookies")
+		_, refresh, _ := strings.Cut(content, "func refreshStoredBrowserCookies")
+		// Without the fix, the refresh body has SaveTokens(cookies) with no
+		// prior filter — the second SaveTokens call would then sit before any
+		// allowlist parsing in the function.
+		assert.Contains(t, refresh, `requiredCookies := []string{"session_id"}`)
+		assert.Contains(t, refresh, `cookies = strings.Join(kept, "; ")`)
+
+		runGoCommand(t, outputDir, "build", "./...")
+	})
+}
+
 func TestGenerate_UserAgentOverrideGatedByBrowserTransport(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/generator/templates/auth_browser.go.tmpl b/internal/generator/templates/auth_browser.go.tmpl
index 82d43b56..d92432a9 100644
--- a/internal/generator/templates/auth_browser.go.tmpl
+++ b/internal/generator/templates/auth_browser.go.tmpl
@@ -270,7 +270,32 @@ profile by name when the installed backend supports it.`,
 			fmt.Fprintf(w, "Session saved to %s\n", cfg.Path)
 			return nil
 {{- else}}
-			// Step 4: Save to config
+{{- if .Auth.Cookies}}
+			// Unfiltered, the persisted blob carries every cookie the target
+			// domain has set (CSRF, WAF, anti-bot fingerprints, etc.) into the
+			// Cookie header on every request, which routinely trips upstream
+			// WAFs and shadows the real credential.
+			cookieMap := parseCookieString(cookies)
+			requiredCookies := []string{ {{- range $i, $c := .Auth.Cookies}}{{if $i}}, {{end}}"{{$c}}"{{- end}} }
+			kept := make([]string, 0, len(requiredCookies))
+			for _, name := range requiredCookies {
+				v, ok := cookieMap[name]
+				if !ok {
+					loginURL := "https://" + strings.TrimPrefix(domain, ".")
+					fmt.Fprintf(w, "\n%s Cookie %q not found for %s.\n", red("ERROR"), name, domain)
+					fmt.Fprintln(w, "")
+					fmt.Fprintln(w, "Log in to your account:")
+					fmt.Fprintf(w, "\n  %s\n\n", loginURL)
+					fmt.Fprintln(w, "Then run this command again:")
+					fmt.Fprintf(w, "\n  {{.Name}}-pp-cli auth login --chrome\n")
+					return authErr(fmt.Errorf("cookie %q not found for %s", name, domain))
+				}
+				kept = append(kept, name+"="+v)
+			}
+			cookies = strings.Join(kept, "; ")
+{{- end}}
+
+			// Step 5: Save to config
 			cfg, err := config.Load(flags.configPath)
 			if err != nil {
 				return configErr(err)
@@ -595,6 +620,22 @@ func refreshStoredBrowserCookies(cfg *config.Config, w io.Writer) error {
 	}
 	fmt.Fprintf(w, "Updated stored browser auth from %d cookies.\n", len(requiredCookies))
 {{- else}}
+{{- if .Auth.Cookies}}
+	// Refresh runs unattended, so silently saving the raw blob here would
+	// overwrite the filtered token that login just persisted. Apply the same
+	// allowlist filter as newAuthLoginCmd's non-composed branch.
+	cookieMap := parseCookieString(cookies)
+	requiredCookies := []string{ {{- range $i, $c := .Auth.Cookies}}{{if $i}}, {{end}}"{{$c}}"{{- end}} }
+	kept := make([]string, 0, len(requiredCookies))
+	for _, name := range requiredCookies {
+		v, ok := cookieMap[name]
+		if !ok {
+			return fmt.Errorf("cookie %q not found for %s", name, domain)
+		}
+		kept = append(kept, name+"="+v)
+	}
+	cookies = strings.Join(kept, "; ")
+{{- end}}
 	if err := cfg.SaveTokens("", "", cookies, "", time.Time{}); err != nil {
 		return configErr(fmt.Errorf("saving cookies: %w", err))
 	}

← 34ca8149 feat(cli): add --auth-preference flag for catalog-driven sch  ·  back to Cli Printing Press  ·  fix(cli): drop MCP code-orch handler pre-marshal that base64 ff562375 →