[object Object]

← back to Cli Printing Press

fix(cli): doctor preserves configured User-Agent when API uses UA as auth credential (#1361)

3bb6d37bfbe94e675051bc6c453dd32ad9985e07 · 2026-05-15 20:51:33 -0400 · tallyberner

* fix(cli): doctor preserves configured User-Agent when API uses UA as auth credential

When an API spec declares Auth.Header == "User-Agent" with Auth.In ==
"header" (e.g. weather.gov's userAgent securityScheme), the doctor
template's credential-probe block emitted two consecutive assignments
to the same map key:

    authHeaders["User-Agent"] = authHeader              // configured UA
    authHeaders["User-Agent"] = "<name>-pp-cli"       // hardcoded fallback

The fallback unconditionally clobbered the operator's configured UA, so
the probe tested the wrong identity. For NWS, the credential probe hit
api.weather.gov with the hardcoded "<name>-pp-cli" instead of the
operator's contact-info UA, defeating the entire point of NWS's UA-based
auth.

Root cause: doctor.go.tmpl gated the fallback only on
.UsesBrowserManagedUserAgent and .HasRequiredHeader "User-Agent". It
didn't notice that the line above had already set
authHeaders["User-Agent"] = authHeader when the auth header itself is
User-Agent.

Fix: add a third skip condition. Compute authUsesUA = Auth.In is
"header" (or empty default) AND lower(Auth.Header) == "user-agent";
skip the fallback when true.

Tests pin both directions: the UA-as-auth case must NOT emit the
hardcoded fallback, and the bearer-auth case (Auth.Header is
Authorization) must STILL emit it so the probe identifies itself when
User-Agent isn't the credential.

Surfaced by pp-nws-storm Codex review 2026-05-13.

* Update internal/generator/doctor_auth_status_test.go

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* fix(cli): gofmt internal/generator/doctor_auth_status_test.go

go-lint was failing on a misaligned struct literal in
TestDoctorPreservesUserAgentWhenUsedAsAPIKey. gofmt -w aligns the
Type/Header fields. No behavior change.

---------

Co-authored-by: Dior Vass <dior@diorconstruction.com>
Co-authored-by: Trevin Chow <trevin@trevinchow.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Files touched

Diff

commit 3bb6d37bfbe94e675051bc6c453dd32ad9985e07
Author: tallyberner <127001603+tallyberner@users.noreply.github.com>
Date:   Fri May 15 20:51:33 2026 -0400

    fix(cli): doctor preserves configured User-Agent when API uses UA as auth credential (#1361)
    
    * fix(cli): doctor preserves configured User-Agent when API uses UA as auth credential
    
    When an API spec declares Auth.Header == "User-Agent" with Auth.In ==
    "header" (e.g. weather.gov's userAgent securityScheme), the doctor
    template's credential-probe block emitted two consecutive assignments
    to the same map key:
    
        authHeaders["User-Agent"] = authHeader              // configured UA
        authHeaders["User-Agent"] = "<name>-pp-cli"       // hardcoded fallback
    
    The fallback unconditionally clobbered the operator's configured UA, so
    the probe tested the wrong identity. For NWS, the credential probe hit
    api.weather.gov with the hardcoded "<name>-pp-cli" instead of the
    operator's contact-info UA, defeating the entire point of NWS's UA-based
    auth.
    
    Root cause: doctor.go.tmpl gated the fallback only on
    .UsesBrowserManagedUserAgent and .HasRequiredHeader "User-Agent". It
    didn't notice that the line above had already set
    authHeaders["User-Agent"] = authHeader when the auth header itself is
    User-Agent.
    
    Fix: add a third skip condition. Compute authUsesUA = Auth.In is
    "header" (or empty default) AND lower(Auth.Header) == "user-agent";
    skip the fallback when true.
    
    Tests pin both directions: the UA-as-auth case must NOT emit the
    hardcoded fallback, and the bearer-auth case (Auth.Header is
    Authorization) must STILL emit it so the probe identifies itself when
    User-Agent isn't the credential.
    
    Surfaced by pp-nws-storm Codex review 2026-05-13.
    
    * Update internal/generator/doctor_auth_status_test.go
    
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    
    * fix(cli): gofmt internal/generator/doctor_auth_status_test.go
    
    go-lint was failing on a misaligned struct literal in
    TestDoctorPreservesUserAgentWhenUsedAsAPIKey. gofmt -w aligns the
    Type/Header fields. No behavior change.
    
    ---------
    
    Co-authored-by: Dior Vass <dior@diorconstruction.com>
    Co-authored-by: Trevin Chow <trevin@trevinchow.com>
    Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/doctor_auth_status_test.go | 93 +++++++++++++++++++++++++++
 internal/generator/templates/doctor.go.tmpl   | 15 ++++-
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/internal/generator/doctor_auth_status_test.go b/internal/generator/doctor_auth_status_test.go
index 31da8585..b01c8d15 100644
--- a/internal/generator/doctor_auth_status_test.go
+++ b/internal/generator/doctor_auth_status_test.go
@@ -89,3 +89,96 @@ func TestDoctorOAuth2PerCallRequiredEnvVarDefersToConfigAuth(t *testing.T) {
 			}`,
 		"per_call+Required env-var check must route missing-required through the authConfigured else chain, not as an unconditional append")
 }
+
+// TestDoctorPreservesConfiguredUserAgentWhenAuthHeaderIsUserAgent pins the
+// fix for the "User-Agent IS the auth credential" case. When the API spec
+// declares Auth.Header == "User-Agent" + Auth.In == "header" (e.g. the
+// weather.gov userAgent securityScheme), the credential-probe code path
+// must keep the user's configured UA on authHeaders["User-Agent"]; the
+// hardcoded "<name>-pp-cli" fallback must NOT emit, because it would
+// overwrite the operator's identity and make the probe test the wrong UA.
+func TestDoctorPreservesConfiguredUserAgentWhenAuthHeaderIsUserAgent(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("ua-auth-fixture")
+	apiSpec.Auth = spec.AuthConfig{
+		Type:       "api_key",
+		Header:     "User-Agent",
+		In:         "header",
+		VerifyPath: "/alerts/active",
+		EnvVars:    []string{"UA_AUTH_FIXTURE_USER_AGENT"},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "ua-auth-fixture-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	doctor := string(doctorSrc)
+
+	// The configured UA must be set on the probe.
+	require.Contains(t, doctor, `authHeaders["User-Agent"] = authHeader`,
+		"doctor probe must assign the configured UA (via authHeader) to authHeaders[\"User-Agent\"]")
+
+	// The hardcoded fallback must NOT clobber the configured UA.
+	require.NotContains(t, doctor, `authHeaders["User-Agent"] = "ua-auth-fixture-pp-cli"`,
+		"doctor must not overwrite authHeaders[\"User-Agent\"] with the hardcoded fallback when Auth.Header itself is User-Agent")
+}
+
+// TestDoctorEmitsHardcodedUserAgentForBearerAuthSpecs guards the converse:
+// when Auth.Header is Authorization (the common bearer case), the
+// hardcoded User-Agent fallback must still emit so the probe identifies
+// itself. This pins that the UA-preservation fix is scoped to the
+// UA-as-auth case and does not regress the default path.
+func TestDoctorEmitsHardcodedUserAgentForBearerAuthSpecs(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("bearer-auth-fixture")
+	apiSpec.Auth = spec.AuthConfig{
+		Type:       "bearer_token",
+		Header:     "Authorization",
+		In:         "header",
+		VerifyPath: "/me",
+		EnvVars:    []string{"BEARER_AUTH_FIXTURE_TOKEN"},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "bearer-auth-fixture-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	doctor := string(doctorSrc)
+
+	require.Contains(t, doctor, `authHeaders["User-Agent"] = "bearer-auth-fixture-pp-cli"`,
+		"doctor must continue to emit the hardcoded UA fallback for bearer-auth specs where the API does not use User-Agent itself as the credential")
+}
+
+// TestDoctorPreservesConfiguredUserAgentWhenAuthInIsEmpty pins the
+// default-Auth.In behaviour: when Auth.In is empty, the doctor template
+// treats it as the header-auth path (the query-auth branch is the
+// special case). The UA-preservation gate must trip on this default
+// too — otherwise a spec that omits Auth.In would silently regress.
+func TestDoctorPreservesConfiguredUserAgentWhenAuthInIsEmpty(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("ua-auth-empty-in")
+	apiSpec.Auth = spec.AuthConfig{
+		Type:   "api_key",
+		Header: "User-Agent",
+		// Auth.In intentionally left empty to exercise the default.
+		VerifyPath: "/alerts/active",
+		EnvVars:    []string{"UA_AUTH_EMPTY_IN_USER_AGENT"},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "ua-auth-empty-in-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	doctor := string(doctorSrc)
+
+	require.Contains(t, doctor, `authHeaders["User-Agent"] = authHeader`,
+		"doctor probe must assign the configured UA when Auth.In is empty (defaults to header)")
+	require.NotContains(t, doctor, `authHeaders["User-Agent"] = "ua-auth-empty-in-pp-cli"`,
+		"doctor must not emit the hardcoded UA fallback when Auth.Header is User-Agent, even with Auth.In empty")
+}
diff --git a/internal/generator/templates/doctor.go.tmpl b/internal/generator/templates/doctor.go.tmpl
index 2e5aedd9..f0024ac1 100644
--- a/internal/generator/templates/doctor.go.tmpl
+++ b/internal/generator/templates/doctor.go.tmpl
@@ -429,7 +429,20 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 {{- range .RequiredHeaders}}
 						authHeaders["{{.Name}}"] = "{{.Value}}"
 {{- end}}
-{{- if and (not .UsesBrowserManagedUserAgent) (not (.HasRequiredHeader "User-Agent"))}}
+{{- /*
+	Emit the hardcoded User-Agent fallback only when no other code path has
+	already populated authHeaders["User-Agent"]. Three cases skip the fallback:
+	  1. UsesBrowserManagedUserAgent — the browser transport sets the UA.
+	  2. HasRequiredHeader "User-Agent" — the RequiredHeaders range above set it.
+	  3. Auth.In is "header" (or empty default) AND Auth.Header is "User-Agent" —
+	     the API uses the User-Agent header itself as the auth credential
+	     (NWS-style), so the "authHeaders[Auth.Header] = authHeader" line above
+	     already set it to the configured value. The hardcoded fallback would
+	     overwrite the operator's configured UA and make the credential probe
+	     test the wrong identity.
+*/}}
+{{- $authUsesUA := and .Auth (or (eq .Auth.In "") (eq .Auth.In "header")) (eq (lower .Auth.Header) "user-agent")}}
+{{- if and (not .UsesBrowserManagedUserAgent) (not (.HasRequiredHeader "User-Agent")) (not $authUsesUA)}}
 						authHeaders["User-Agent"] = "{{.Name}}-pp-cli"
 {{- end}}
 						_, authErr := c.GetWithHeaders(verifyPath, authParams, authHeaders)

← 2d0ad5ae fix(ci): switch queue update_method to merge so fork PRs can  ·  back to Cli Printing Press  ·  fix(cli): expand pm_stale timestamp-field list for non-updat 55f5b26b →