[object Object]

← back to Cli Printing Press

fix(cli): credit auth_protocol on structural OAuth surface, not "Bearer " literal (#1176)

e8c03cffc5241963ce803a30ef29c541ae3f958d · 2026-05-12 01:47:07 -0700 · Trevin Chow

scoreAuthScheme gated bearer/oauth2/openidconnect schemes on the literal
"Bearer " string appearing in client/config/auth source. That made the
dimension brittle: a cosmetic polish pass could lift a CLI from 3/10 to
8/10 just by adding an unused `bearerPrefix = "Bearer "` const, without
changing any runtime behaviour.

Instead, look for real OAuth machinery — a generated config with a
RefreshToken rotation field, or a hand-written internal/oauth/ helper
package — and credit bearer-style schemes once either signal is present.
The credit only applies when the scheme is actually bearer-style, so
basic-auth scoring is unchanged.

Closes #941

Files touched

Diff

commit e8c03cffc5241963ce803a30ef29c541ae3f958d
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue May 12 01:47:07 2026 -0700

    fix(cli): credit auth_protocol on structural OAuth surface, not "Bearer " literal (#1176)
    
    scoreAuthScheme gated bearer/oauth2/openidconnect schemes on the literal
    "Bearer " string appearing in client/config/auth source. That made the
    dimension brittle: a cosmetic polish pass could lift a CLI from 3/10 to
    8/10 just by adding an unused `bearerPrefix = "Bearer "` const, without
    changing any runtime behaviour.
    
    Instead, look for real OAuth machinery — a generated config with a
    RefreshToken rotation field, or a hand-written internal/oauth/ helper
    package — and credit bearer-style schemes once either signal is present.
    The credit only applies when the scheme is actually bearer-style, so
    basic-auth scoring is unchanged.
    
    Closes #941
---
 internal/pipeline/scorecard.go            | 40 +++++++++++--
 internal/pipeline/scorecard_tier2_test.go | 97 ++++++++++++++++++++++++++++++-
 2 files changed, 131 insertions(+), 6 deletions(-)

diff --git a/internal/pipeline/scorecard.go b/internal/pipeline/scorecard.go
index b2813303..51c139d7 100644
--- a/internal/pipeline/scorecard.go
+++ b/internal/pipeline/scorecard.go
@@ -1762,6 +1762,7 @@ func evaluateAuthProtocol(dir string, spec *openAPISpecInfo) dimensionScore {
 		return dimensionScore{scored: true}
 	}
 
+	hasStructuralOAuth := hasStructuralOAuthSurface(dir, configContent)
 	referencedSchemes := referencedSecuritySchemes(spec.SecurityRequirements)
 	totalScore := 0
 	scoredSets := 0
@@ -1773,7 +1774,7 @@ func evaluateAuthProtocol(dir string, spec *openAPISpecInfo) dimensionScore {
 		bestScore := -1
 		scoreable := false
 		for _, alternative := range requirementSet.Alternatives {
-			score, ok := scoreAuthAlternative(clientContent, configContent, authContent, spec.SecuritySchemes, alternative, referencedSchemes)
+			score, ok := scoreAuthAlternative(clientContent, configContent, authContent, hasStructuralOAuth, spec.SecuritySchemes, alternative, referencedSchemes)
 			if !ok {
 				continue
 			}
@@ -1856,7 +1857,7 @@ func referencedSecuritySchemes(requirementSets []securityRequirementSet) map[str
 	return referenced
 }
 
-func scoreAuthAlternative(clientContent, configContent, authContent string, schemes map[string]openAPISecurityScheme, alternative []string, referencedSchemes map[string]bool) (int, bool) {
+func scoreAuthAlternative(clientContent, configContent, authContent string, hasStructuralOAuth bool, schemes map[string]openAPISecurityScheme, alternative []string, referencedSchemes map[string]bool) (int, bool) {
 	if len(alternative) == 0 {
 		return 0, false
 	}
@@ -1876,7 +1877,7 @@ func scoreAuthAlternative(clientContent, configContent, authContent string, sche
 		if composedHeaders && isAPIKeyHeaderScheme(scheme) {
 			score, scoreable = scoreComposedHeaderScheme(clientContent, scheme)
 		} else {
-			score, scoreable = scoreAuthScheme(clientContent, configContent, authContent, scheme)
+			score, scoreable = scoreAuthScheme(clientContent, configContent, authContent, hasStructuralOAuth, scheme)
 		}
 		if !scoreable {
 			continue
@@ -1897,7 +1898,7 @@ func scoreAuthAlternative(clientContent, configContent, authContent string, sche
 	return score, true
 }
 
-func scoreAuthScheme(clientContent, configContent, authContent string, scheme openAPISecurityScheme) (int, bool) {
+func scoreAuthScheme(clientContent, configContent, authContent string, hasStructuralOAuth bool, scheme openAPISecurityScheme) (int, bool) {
 	nameLower := strings.ToLower(scheme.Key)
 	headerName := "Authorization"
 	authHeaderMatched := false
@@ -1905,6 +1906,7 @@ func scoreAuthScheme(clientContent, configContent, authContent string, scheme op
 	queryMatched := false
 	envMatched := false
 	scoreable := false
+	bearerStyle := false
 
 	if strings.EqualFold(scheme.Type, "apikey") && scheme.In == "header" && strings.TrimSpace(scheme.HeaderName) != "" {
 		headerName = scheme.HeaderName
@@ -1918,6 +1920,7 @@ func scoreAuthScheme(clientContent, configContent, authContent string, scheme op
 		}
 	case strings.Contains(nameLower, "bearer") || (scheme.Type == "http" && scheme.Scheme == "bearer"):
 		scoreable = true
+		bearerStyle = true
 		bearerLiteral := scheme.Prefix
 		if strings.TrimSpace(bearerLiteral) == "" {
 			bearerLiteral = "Bearer"
@@ -1942,6 +1945,7 @@ func scoreAuthScheme(clientContent, configContent, authContent string, scheme op
 		}
 	case strings.EqualFold(scheme.Type, "oauth2"), strings.EqualFold(scheme.Type, "openidconnect"):
 		scoreable = true
+		bearerStyle = true
 		if authPrefixLiteralPresent("Bearer", clientContent, configContent, authContent) {
 			authHeaderMatched = true
 		}
@@ -1950,6 +1954,18 @@ func scoreAuthScheme(clientContent, configContent, authContent string, scheme op
 		return 0, false
 	}
 
+	// Bearer-style schemes (http/bearer, oauth2, openidconnect) are otherwise
+	// scored by grepping for the "Bearer " literal and the spec's scheme key as
+	// an env-var needle, both of which a cosmetic polish pass can fake by adding
+	// an unused const. Real OAuth machinery (refresh-token rotation in config or
+	// a dedicated oauth helper package) credits both signals at once because the
+	// CLI demonstrably exchanges tokens and reads OAuth env vars (CLIENT_ID,
+	// REFRESH_TOKEN) that sanitizeEnvName never matches.
+	if bearerStyle && hasStructuralOAuth {
+		authHeaderMatched = true
+		envMatched = true
+	}
+
 	// AuthProtocol pattern: generated clients use Header.Set/Add with the expected header name.
 	if headerAssignmentPresent(clientContent, headerName) {
 		headerNameMatched = true
@@ -2099,6 +2115,22 @@ func headerAssignmentPresent(clientContent, headerName string) bool {
 		strings.Contains(clientContent, `Header.Add("`+headerName+`"`)
 }
 
+// refreshTokenFieldRe word-anchors RefreshToken so cosmetic names like
+// NoRefreshToken or RefreshTokenError don't satisfy the structural check.
+var refreshTokenFieldRe = regexp.MustCompile(`\bRefreshToken\b`)
+
+// hasStructuralOAuthSurface returns true when the printed CLI ships real
+// OAuth machinery rather than a literal "Bearer " const that grep can find.
+// Either signal is sufficient — a generated config.go with a RefreshToken
+// rotation field, or a hand-written internal/oauth/ helper package.
+func hasStructuralOAuthSurface(dir, configContent string) bool {
+	if refreshTokenFieldRe.MatchString(configContent) {
+		return true
+	}
+	info, err := os.Stat(filepath.Join(dir, "internal", "oauth"))
+	return err == nil && info.IsDir()
+}
+
 func authPrefixLiteralPresent(prefix string, contents ...string) bool {
 	// AuthProtocol pattern: literal auth prefixes are emitted as quoted strings.
 	doubleQuoted := `"` + prefix + ` "`
diff --git a/internal/pipeline/scorecard_tier2_test.go b/internal/pipeline/scorecard_tier2_test.go
index 58e5e7ab..c4d14311 100644
--- a/internal/pipeline/scorecard_tier2_test.go
+++ b/internal/pipeline/scorecard_tier2_test.go
@@ -2358,9 +2358,102 @@ func TestScoreAuthScheme_BearerPrefixOverride(t *testing.T) {
 	withoutPrefix := withPrefix
 	withoutPrefix.Prefix = ""
 
-	scoreWith, _ := scoreAuthScheme(clientStub, configWithToken, "", withPrefix)
-	scoreWithout, _ := scoreAuthScheme(clientStub, configWithToken, "", withoutPrefix)
+	scoreWith, _ := scoreAuthScheme(clientStub, configWithToken, "", false, withPrefix)
+	scoreWithout, _ := scoreAuthScheme(clientStub, configWithToken, "", false, withoutPrefix)
 
 	assert.Greater(t, scoreWith, scoreWithout,
 		"AuthProtocol score with configured prefix must exceed the empty-prefix default when generated code uses the prefix literal")
 }
+
+// TestScoreAuthScheme_StructuralOAuthSurface pins that bearer-style schemes
+// get credit for real OAuth machinery (refresh-token rotation in config, or a
+// dedicated internal/oauth helper package) even when the literal "Bearer "
+// string is absent from source — preventing the score-by-literal regression
+// where a polish pass could add an unused const to lift the score.
+func TestScoreAuthScheme_StructuralOAuthSurface(t *testing.T) {
+	bearerScheme := openAPISecurityScheme{Key: "bearerAuth", Type: "http", Scheme: "bearer"}
+	oauth2Scheme := openAPISecurityScheme{Key: "oauth2Auth", Type: "oauth2"}
+	openIDScheme := openAPISecurityScheme{Key: "oidcAuth", Type: "openidconnect"}
+	basicScheme := openAPISecurityScheme{Key: "basicAuth", Type: "http", Scheme: "basic"}
+
+	clientHeaderOnly := `req.Header.Set("Authorization", token)`
+	configRefresh := `type Config struct { AccessToken string; RefreshToken string }`
+	configNoOAuth := `type Config struct { Token string }`
+
+	t.Run("bearer scheme without literal credited when RefreshToken in config", func(t *testing.T) {
+		score, scoreable := scoreAuthScheme(clientHeaderOnly, configRefresh, "", true, bearerScheme)
+		assert.True(t, scoreable)
+		assert.GreaterOrEqual(t, score, 9, "real OAuth surface should lift score above the literal-grep ceiling")
+	})
+
+	t.Run("bearer scheme without literal stays low when no structural OAuth", func(t *testing.T) {
+		score, scoreable := scoreAuthScheme(clientHeaderOnly, configNoOAuth, "", false, bearerScheme)
+		assert.True(t, scoreable)
+		assert.Less(t, score, 7, "absent literal AND absent OAuth surface must not score as wired auth")
+	})
+
+	t.Run("oauth2 scheme without literal credited when structural OAuth present", func(t *testing.T) {
+		score, scoreable := scoreAuthScheme(clientHeaderOnly, configRefresh, "", true, oauth2Scheme)
+		assert.True(t, scoreable)
+		assert.GreaterOrEqual(t, score, 9)
+	})
+
+	t.Run("openidconnect scheme shares the bearer-style credit path", func(t *testing.T) {
+		// openidconnect lives on the same switch case as oauth2, so a future
+		// split must not silently drop structural credit from this arm.
+		score, scoreable := scoreAuthScheme(clientHeaderOnly, configRefresh, "", true, openIDScheme)
+		assert.True(t, scoreable)
+		assert.GreaterOrEqual(t, score, 9)
+	})
+
+	t.Run("basic scheme not lifted by OAuth surface", func(t *testing.T) {
+		// Counter-check: structural OAuth signal must not bleed into non-bearer
+		// schemes. A Basic scheme without its literal should stay at the
+		// header-name credit ceiling regardless of nearby OAuth machinery.
+		withOAuth, _ := scoreAuthScheme(clientHeaderOnly, configRefresh, "", true, basicScheme)
+		withoutOAuth, _ := scoreAuthScheme(clientHeaderOnly, configRefresh, "", false, basicScheme)
+		assert.Equal(t, withOAuth, withoutOAuth, "OAuth surface must not credit basic-scheme scoring")
+	})
+}
+
+// TestHasStructuralOAuthSurface pins the helper's recognition signals:
+// either a RefreshToken field in generated config.go, or a hand-written
+// internal/oauth helper package on disk.
+func TestHasStructuralOAuthSurface(t *testing.T) {
+	t.Run("refresh-token field in config", func(t *testing.T) {
+		dir := t.TempDir()
+		got := hasStructuralOAuthSurface(dir, "type Config struct { RefreshToken string }")
+		assert.True(t, got)
+	})
+
+	t.Run("internal/oauth package on disk", func(t *testing.T) {
+		dir := t.TempDir()
+		err := os.MkdirAll(filepath.Join(dir, "internal", "oauth"), 0o755)
+		assert.NoError(t, err)
+		assert.True(t, hasStructuralOAuthSurface(dir, "type Config struct { Token string }"))
+	})
+
+	t.Run("neither signal present", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.False(t, hasStructuralOAuthSurface(dir, "type Config struct { Token string }"))
+	})
+
+	t.Run("internal/oauth as a regular file does not count", func(t *testing.T) {
+		dir := t.TempDir()
+		err := os.MkdirAll(filepath.Join(dir, "internal"), 0o755)
+		assert.NoError(t, err)
+		err = os.WriteFile(filepath.Join(dir, "internal", "oauth"), []byte("not a package"), 0o644)
+		assert.NoError(t, err)
+		assert.False(t, hasStructuralOAuthSurface(dir, "type Config struct { Token string }"))
+	})
+
+	t.Run("word-anchored RefreshToken rejects same-word neighbours", func(t *testing.T) {
+		// Cosmetic identifiers that contain "RefreshToken" as a substring
+		// (NoRefreshToken, RefreshTokenError) must not trip the structural
+		// check — otherwise the polish pass this fix defeats just renames
+		// its decoy.
+		dir := t.TempDir()
+		assert.False(t, hasStructuralOAuthSurface(dir, "type Config struct { NoRefreshToken bool }"))
+		assert.False(t, hasStructuralOAuthSurface(dir, "type RefreshTokenError struct{}"))
+	})
+}

← 84138f91 fix(cli): handle PascalCase .NET-shape API responses in sync  ·  back to Cli Printing Press  ·  fix(cli): route scorer subcommand --spec URLs through genera 5e3667df →