[object Object]

← back to Cli Printing Press

fix(cli): classify doctor HTTP 403 as scope-limited WARN, not invalid FAIL (#1033)

b1dded7d7db1962c2d3f6909c5f863b74367a25d · 2026-05-11 00:11:32 -0700 · Trevin Chow

* fix(cli): classify doctor HTTP 403 as scope-limited WARN, not invalid FAIL

The doctor template lumped HTTP 401 and 403 into a single "invalid (HTTP
%d) — check your credentials" branch. A 403 means the credential was
accepted but lacks scope for the verify_path endpoint — telling users to
re-check the credential value wastes their time when the real fix is
adjusting the dashboard API key scope.

Split the case so 401 keeps the existing invalid wording (FAIL) and 403
emits "scope-limited (HTTP 403) — credentials are valid but lack
permission for this endpoint. Check your dashboard's API key scope."
(WARN). Adds a matching indicator-switch case so the human-readable
output renders WARN instead of falling through to FAIL. scope-limited
contains no substring from doctorExitForFailOn's worstError set, so
--fail-on=error continues to ignore it — which is the desired behavior.

Closes #895

* test(cli): update stale 401/403 doctor verdict comment

The comment in TestGeneratedDoctor_AuthVerifyPathProbesEndpoint said
"401/403 keeps the strict 'invalid' verdict" but the prior commit splits
those: 403 now maps to scope-limited. The assertion only checks the 401
wording, which still passes, so the test wasn't lying — only the comment
was. Point at the new test that covers the 403 branch.

Files touched

Diff

commit b1dded7d7db1962c2d3f6909c5f863b74367a25d
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Mon May 11 00:11:32 2026 -0700

    fix(cli): classify doctor HTTP 403 as scope-limited WARN, not invalid FAIL (#1033)
    
    * fix(cli): classify doctor HTTP 403 as scope-limited WARN, not invalid FAIL
    
    The doctor template lumped HTTP 401 and 403 into a single "invalid (HTTP
    %d) — check your credentials" branch. A 403 means the credential was
    accepted but lacks scope for the verify_path endpoint — telling users to
    re-check the credential value wastes their time when the real fix is
    adjusting the dashboard API key scope.
    
    Split the case so 401 keeps the existing invalid wording (FAIL) and 403
    emits "scope-limited (HTTP 403) — credentials are valid but lack
    permission for this endpoint. Check your dashboard's API key scope."
    (WARN). Adds a matching indicator-switch case so the human-readable
    output renders WARN instead of falling through to FAIL. scope-limited
    contains no substring from doctorExitForFailOn's worstError set, so
    --fail-on=error continues to ignore it — which is the desired behavior.
    
    Closes #895
    
    * test(cli): update stale 401/403 doctor verdict comment
    
    The comment in TestGeneratedDoctor_AuthVerifyPathProbesEndpoint said
    "401/403 keeps the strict 'invalid' verdict" but the prior commit splits
    those: 403 now maps to scope-limited. The assertion only checks the 401
    wording, which still passes, so the test wasn't lying — only the comment
    was. Point at the new test that covers the 403 branch.
---
 internal/generator/auth_status_test.go             | 30 ++++++++++++++++++++++
 internal/generator/generator_test.go               |  4 ++-
 internal/generator/templates/doctor.go.tmpl        |  6 ++++-
 .../internal/cli/doctor.go                         |  2 ++
 .../printing-press-golden/internal/cli/doctor.go   |  2 ++
 .../tier-routing-golden/internal/cli/doctor.go     |  2 ++
 6 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/internal/generator/auth_status_test.go b/internal/generator/auth_status_test.go
index 26006a48..33e8693b 100644
--- a/internal/generator/auth_status_test.go
+++ b/internal/generator/auth_status_test.go
@@ -51,6 +51,36 @@ func TestDoctorWithVerifyPathCanClaimCredentialsValid(t *testing.T) {
 	runGoCommand(t, outputDir, "build", "./...")
 }
 
+func TestDoctorClassifiesHTTP401AsInvalidAnd403AsScopeLimited(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("doctor-scope")
+	apiSpec.Auth.VerifyPath = "/account"
+
+	outputDir := filepath.Join(t.TempDir(), "doctor-scope-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	doctorSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	src := string(doctorSrc)
+
+	require.Contains(t, src, "case authAPIErr.StatusCode == 401:",
+		"doctor must keep a dedicated 401 branch so HTTP 401 reports as invalid credentials")
+	require.Contains(t, src, `"invalid (HTTP %d) — check your credentials"`,
+		"doctor's 401 message must continue to direct users to check the credential value")
+
+	require.Contains(t, src, "case authAPIErr.StatusCode == 403:",
+		"doctor must split 401 and 403 so 403 (scope-limited) is not misreported as invalid")
+	require.Contains(t, src, `"scope-limited (HTTP %d) — credentials are valid but lack permission for this endpoint. Check your dashboard's API key scope."`,
+		"doctor's 403 message must surface as scope-limited and point at dashboard scope, not the credential value")
+
+	require.NotContains(t, src, "authAPIErr.StatusCode == 401 || authAPIErr.StatusCode == 403",
+		"doctor must not collapse 401 and 403 into a single invalid branch")
+
+	require.Contains(t, src, `case strings.Contains(s, "scope-limited"):`,
+		"doctor's human-readable indicator switch must classify scope-limited as WARN, not FAIL")
+}
+
 func TestAuthStatusReportsCredentialsPresentNotVerified(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 184f755c..357ad261 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -4799,7 +4799,9 @@ func TestGeneratedDoctor_AuthVerifyPathProbesEndpoint(t *testing.T) {
 	assert.Contains(t, content, `verifyPath := "/me?fields=id"`)
 	assert.Contains(t, content, `c.GetWithHeaders(verifyPath`)
 	assert.NotContains(t, content, `&http.Client{`)
-	// When verify_path is set, 401/403 keeps the strict "invalid" verdict
+	// When verify_path is set, HTTP 401 keeps the strict "invalid" verdict.
+	// 403 is handled separately as scope-limited; see
+	// TestDoctorClassifiesHTTP401AsInvalidAnd403AsScopeLimited.
 	assert.Contains(t, content, `"invalid (HTTP %d) — check your credentials"`)
 	// And does NOT emit the inconclusive fallback wording
 	assert.NotContains(t, content, "inconclusive (HTTP %d from base URL")
diff --git a/internal/generator/templates/doctor.go.tmpl b/internal/generator/templates/doctor.go.tmpl
index 02be3b72..2e5aedd9 100644
--- a/internal/generator/templates/doctor.go.tmpl
+++ b/internal/generator/templates/doctor.go.tmpl
@@ -439,8 +439,10 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 							report["credentials"] = "valid"
 						case errors.As(authErr, &authAPIErr):
 							switch {
-							case authAPIErr.StatusCode == 401 || authAPIErr.StatusCode == 403:
+							case authAPIErr.StatusCode == 401:
 								report["credentials"] = fmt.Sprintf("invalid (HTTP %d) — check your credentials", authAPIErr.StatusCode)
+							case authAPIErr.StatusCode == 403:
+								report["credentials"] = fmt.Sprintf("scope-limited (HTTP %d) — credentials are valid but lack permission for this endpoint. Check your dashboard's API key scope.", authAPIErr.StatusCode)
 							default:
 								// Non-auth HTTP error (404, 500, etc.) — don't blame credentials
 								report["credentials"] = fmt.Sprintf("ok (HTTP %d from %s, but auth was accepted)", authAPIErr.StatusCode, verifyPath)
@@ -502,6 +504,8 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 				case strings.HasPrefix(s, "optional"):
 					// Optional-auth CLI with no key set — informational, not a failure.
 					indicator = yellow("INFO")
+				case strings.Contains(s, "scope-limited"):
+					indicator = yellow("WARN")
 				case strings.Contains(s, "error") || strings.Contains(s, "not configured") || strings.Contains(s, "unreachable") || strings.Contains(s, "invalid") || strings.Contains(s, "missing"):
 					indicator = red("FAIL")
 				case s == "not required":
diff --git a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/doctor.go b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/doctor.go
index 6e68e94d..aa98206a 100644
--- a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/doctor.go
+++ b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/doctor.go
@@ -279,6 +279,8 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 				case strings.HasPrefix(s, "optional"):
 					// Optional-auth CLI with no key set — informational, not a failure.
 					indicator = yellow("INFO")
+				case strings.Contains(s, "scope-limited"):
+					indicator = yellow("WARN")
 				case strings.Contains(s, "error") || strings.Contains(s, "not configured") || strings.Contains(s, "unreachable") || strings.Contains(s, "invalid") || strings.Contains(s, "missing"):
 					indicator = red("FAIL")
 				case s == "not required":
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/doctor.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/doctor.go
index 6172b3fd..6a62a438 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/doctor.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/doctor.go
@@ -227,6 +227,8 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 				case strings.HasPrefix(s, "optional"):
 					// Optional-auth CLI with no key set — informational, not a failure.
 					indicator = yellow("INFO")
+				case strings.Contains(s, "scope-limited"):
+					indicator = yellow("WARN")
 				case strings.Contains(s, "error") || strings.Contains(s, "not configured") || strings.Contains(s, "unreachable") || strings.Contains(s, "invalid") || strings.Contains(s, "missing"):
 					indicator = red("FAIL")
 				case s == "not required":
diff --git a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/doctor.go b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/doctor.go
index d57149cd..ea9b544e 100644
--- a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/doctor.go
+++ b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/cli/doctor.go
@@ -249,6 +249,8 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 				case strings.HasPrefix(s, "optional"):
 					// Optional-auth CLI with no key set — informational, not a failure.
 					indicator = yellow("INFO")
+				case strings.Contains(s, "scope-limited"):
+					indicator = yellow("WARN")
 				case strings.Contains(s, "error") || strings.Contains(s, "not configured") || strings.Contains(s, "unreachable") || strings.Contains(s, "invalid") || strings.Contains(s, "missing"):
 					indicator = red("FAIL")
 				case s == "not required":

← 3e56bedf fix(generator): preserve multi-spec server prefixes (#861)  ·  back to Cli Printing Press  ·  test(cli): pin doctor authConfigured short-circuit for OAuth 9e604a95 →