[object Object]

← back to Cli Printing Press

fix(cli): stop doctor from claiming valid creds are invalid (#270)

6a184771b6bcca0c2957bb13ade7515eccdc9eff · 2026-04-24 20:42:47 -0700 · Trevin Chow

Adds Auth.VerifyPath to the spec and updates the doctor template to
probe baseURL+verify_path when it's set. When unset, doctor still
probes the bare base URL but classifies 401/403 as "inconclusive"
rather than "invalid" — many APIs (Meta /v23.0, GitHub-style
versioned APIs, etc.) return 401 from un-routed roots regardless of
token validity, so the old behavior produced confident-but-wrong
FAIL verdicts. The human renderer maps "inconclusive" to WARN; the
new wording does not contain any --fail-on=error substring.

Fixes #267.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Files touched

Diff

commit 6a184771b6bcca0c2957bb13ade7515eccdc9eff
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri Apr 24 20:42:47 2026 -0700

    fix(cli): stop doctor from claiming valid creds are invalid (#270)
    
    Adds Auth.VerifyPath to the spec and updates the doctor template to
    probe baseURL+verify_path when it's set. When unset, doctor still
    probes the bare base URL but classifies 401/403 as "inconclusive"
    rather than "invalid" — many APIs (Meta /v23.0, GitHub-style
    versioned APIs, etc.) return 401 from un-routed roots regardless of
    token validity, so the old behavior produced confident-but-wrong
    FAIL verdicts. The human renderer maps "inconclusive" to WARN; the
    new wording does not contain any --fail-on=error substring.
    
    Fixes #267.
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 internal/generator/generator_test.go        | 98 +++++++++++++++++++++++++++++
 internal/generator/templates/doctor.go.tmpl | 25 ++++++++
 internal/spec/spec.go                       | 11 ++++
 3 files changed, 134 insertions(+)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 6607dd8d..4e061454 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -2037,6 +2037,104 @@ func TestGeneratedDoctor_AuthHintsWithoutKeyURL(t *testing.T) {
 	assert.NotContains(t, content, "auth_key_url")
 }
 
+func TestGeneratedDoctor_AuthVerifyPathProbesEndpoint(t *testing.T) {
+	t.Parallel()
+
+	// Models the Meta Ads case from issue #267: a versioned base URL where
+	// the bare root returns 401 regardless of token validity. The spec sets
+	// auth.verify_path so doctor probes a known-good endpoint instead.
+	apiSpec := &spec.APISpec{
+		Name:    "metadoc",
+		Version: "0.1.0",
+		BaseURL: "https://graph.facebook.com/v23.0",
+		Auth: spec.AuthConfig{
+			Type:       "bearer_token",
+			Header:     "Authorization",
+			Format:     "Bearer {token}",
+			EnvVars:    []string{"META_ADS_API_TOKEN"},
+			VerifyPath: "/me?fields=id",
+		},
+		Config: spec.ConfigSpec{
+			Format: "toml",
+			Path:   "~/.config/metadoc-pp-cli/config.toml",
+		},
+		Resources: map[string]spec.Resource{
+			"accounts": {
+				Description: "Manage ad accounts",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/me/adaccounts", Description: "List accounts"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "metadoc-pp-cli")
+	gen := New(apiSpec, outputDir)
+	require.NoError(t, gen.Generate())
+
+	doctorGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	content := string(doctorGo)
+
+	// Probe should target baseURL + verify_path, not bare baseURL
+	assert.Contains(t, content, `verifyPath := "/me?fields=id"`)
+	assert.Contains(t, content, `http.NewRequest("GET", baseURL+verifyPath, nil)`)
+	// When verify_path is set, 401/403 keeps the strict "invalid" verdict
+	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")
+}
+
+func TestGeneratedDoctor_NoVerifyPathSoftens401(t *testing.T) {
+	t.Parallel()
+
+	// Without auth.verify_path, the doctor probe falls back to the bare base
+	// URL. 401/403 from the base URL must be reported as "inconclusive", not
+	// "invalid", because many APIs return 401 from un-routed roots regardless
+	// of token validity.
+	apiSpec := &spec.APISpec{
+		Name:    "softdoc",
+		Version: "0.1.0",
+		BaseURL: "https://api.example.com",
+		Auth: spec.AuthConfig{
+			Type:    "api_key",
+			Header:  "X-Api-Key",
+			EnvVars: []string{"SOFT_API_KEY"},
+		},
+		Config: spec.ConfigSpec{
+			Format: "toml",
+			Path:   "~/.config/softdoc-pp-cli/config.toml",
+		},
+		Resources: map[string]spec.Resource{
+			"items": {
+				Description: "Manage items",
+				Endpoints: map[string]spec.Endpoint{
+					"list": {Method: "GET", Path: "/items", Description: "List items"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "softdoc-pp-cli")
+	gen := New(apiSpec, outputDir)
+	require.NoError(t, gen.Generate())
+
+	doctorGo, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "doctor.go"))
+	require.NoError(t, err)
+	content := string(doctorGo)
+
+	// Probe should hit the bare base URL (no verify_path appended)
+	assert.Contains(t, content, `http.NewRequest("GET", baseURL, nil)`)
+	assert.NotContains(t, content, "verifyPath := ")
+	// 401/403 fallback must be the soft "inconclusive" verdict
+	assert.Contains(t, content, `"inconclusive (HTTP %d from base URL — set auth.verify_path in spec for a definitive probe)"`)
+	// And must NOT use the strict "invalid" wording
+	assert.NotContains(t, content, `"invalid (HTTP %d) — check your credentials"`)
+	// Renderer must classify "inconclusive" as WARN before the FAIL clause
+	assert.Contains(t, content, `case strings.HasPrefix(s, "inconclusive"):`)
+	assert.Contains(t, content, `indicator = yellow("WARN")`)
+}
+
 func TestGeneratedDoctor_NoAuthShowsNotRequired(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/generator/templates/doctor.go.tmpl b/internal/generator/templates/doctor.go.tmpl
index cbb2dd0f..57fc96fa 100644
--- a/internal/generator/templates/doctor.go.tmpl
+++ b/internal/generator/templates/doctor.go.tmpl
@@ -195,7 +195,15 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 				} else if !apiReachable {
 					report["credentials"] = "skipped (API unreachable)"
 				} else {
+{{- if .Auth.VerifyPath}}
+					verifyPath := "{{.Auth.VerifyPath}}"
+					if !strings.HasPrefix(verifyPath, "/") {
+						verifyPath = "/" + verifyPath
+					}
+					authReq, _ := http.NewRequest("GET", baseURL+verifyPath, nil)
+{{- else}}
 					authReq, _ := http.NewRequest("GET", baseURL, nil)
+{{- end}}
 {{- if and .Auth .Auth.In (eq .Auth.In "query")}}
 					q := authReq.URL.Query()
 					q.Set("{{if .Auth.Header}}{{.Auth.Header}}{{else}}api_key{{end}}", authHeader)
@@ -218,7 +226,18 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
 						case authResp.StatusCode >= 200 && authResp.StatusCode < 300:
 							report["credentials"] = "valid"
 						case authResp.StatusCode == 401 || authResp.StatusCode == 403:
+{{- if .Auth.VerifyPath}}
 							report["credentials"] = fmt.Sprintf("invalid (HTTP %d) — check your credentials", authResp.StatusCode)
+{{- else}}
+							// The probe hit the bare base URL because no auth.verify_path
+							// is configured in the spec. Many APIs return 401/403 from a
+							// bare versioned root regardless of token validity (the path
+							// isn't routed but the gateway still demands credentials).
+							// Don't claim invalid without certainty — set verify_path to
+							// a known-good authenticated GET (e.g. /me, /v1/account, /user)
+							// for a definitive verdict.
+							report["credentials"] = fmt.Sprintf("inconclusive (HTTP %d from base URL — set auth.verify_path in spec for a definitive probe)", authResp.StatusCode)
+{{- end}}
 						default:
 							// Non-auth HTTP error (404, 500, etc.) — don't blame credentials
 							report["credentials"] = fmt.Sprintf("ok (HTTP %d from base URL, but auth was accepted)", authResp.StatusCode)
@@ -269,6 +288,12 @@ 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.HasPrefix(s, "inconclusive"):
+					// The credential probe could not produce a definitive verdict
+					// (typically because the bare base URL returns 401/403 even for
+					// valid tokens). Surface as WARN, not FAIL — the user's actual
+					// commands will reveal a real auth failure if one exists.
+					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/internal/spec/spec.go b/internal/spec/spec.go
index 024d56f5..58a48f65 100644
--- a/internal/spec/spec.go
+++ b/internal/spec/spec.go
@@ -168,6 +168,17 @@ type AuthConfig struct {
 	Cookies          []string `yaml:"cookies,omitempty" json:"cookies,omitempty"`             // named cookies to extract for composed auth (e.g. ["customerId", "authToken"])
 	Inferred         bool     `yaml:"inferred,omitempty" json:"inferred,omitempty"`           // true when auth was inferred from spec description, not declared in securitySchemes
 
+	// VerifyPath is an optional path appended to base_url that the doctor
+	// command probes to validate credentials. Set this to a known-good
+	// authenticated GET endpoint that returns 2xx for any valid token (e.g.
+	// "/me?fields=id" for Meta, "/v1/account" for Stripe, "/user" for GitHub,
+	// "/users/@me" for Discord). When empty, doctor falls back to probing
+	// the bare base URL and classifies 401/403 as "inconclusive" rather than
+	// "invalid", because many versioned API roots return 401 regardless of
+	// token validity (the path isn't a routed endpoint, but the gateway
+	// still demands credentials in a meaningful context).
+	VerifyPath string `yaml:"verify_path,omitempty" json:"verify_path,omitempty"`
+
 	// Browser-session verification fields. Used when a website-facing CLI
 	// depends on browser-derived cookies or clearance state for its required
 	// happy path. The generator emits validation and proof handling, and the

← 24785ee5 fix(cli): avoid duplicate batch store upsert generation (#23  ·  back to Cli Printing Press  ·  fix(cli): dispatch UpsertBatch to typed tables (#268) (#271) 8eea3a0e →