← back to Cli Printing Press
fix(cli): gate refreshAccessToken emission on Auth.TokenURL non-empty (#1244)
5c359ba6813f20cbcf75d9910bfb70f7aa75935a · 2026-05-12 12:33:28 -0700 · Trevin Chow
Closes #1200. Bearer-only printed CLIs (api_key, bearer_token, OAuth2
specs lacking a TokenURL) were emitting a `refreshAccessToken()` method
that silently returned nil on every call because the runtime tokenURL
was empty. `authHeader()` invoked it whenever an access token expired,
hiding the real failure: users hit the OAuth refresh wall with no error
and `auth status` cheerfully reported `has_refresh_token: ...`.
Tighten both template gates in client.go.tmpl (method definition + call
site) from `{{- if .HasAuthCommand}}` to
`{{- if and .HasAuthCommand (ne .Auth.TokenURL "")}}` so neither emits
for specs without a real token-refresh endpoint. With the outer gate
now guaranteeing TokenURL non-empty, drop the redundant inner
`{{- if .Auth.TokenURL}}` block and the unreachable runtime
`if tokenURL == "" { return nil }` guard.
Add `TestRefreshAccessToken_GatedByTokenURL` with four subtests covering
bearer_token, api_key, oauth2 authorization_code (TokenURL set), and
oauth2 with AuthorizationURL but no TokenURL. The last case pins the
gate on TokenURL specifically so it can't be silently re-keyed on
Auth.Type. Existing `TestTokenScaffoldingFollowsAuthSurface` drops its
client-side scaffolding assertions, which now follow the tighter gate.
Goldens regenerated for `generate-golden-api`, `generate-tier-routing-api`,
and `generate-golden-api-oauth2-cc`: pure subtraction of the dead
method body and call site for the first two, and removal of the
now-unreachable runtime guard inside the function body for the cc case.
Files touched
M internal/generator/auth_none_dead_code_test.goA internal/generator/client_refresh_token_gate_test.goM internal/generator/templates/client.go.tmplM testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.goM testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.goM testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
Diff
commit 5c359ba6813f20cbcf75d9910bfb70f7aa75935a
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue May 12 12:33:28 2026 -0700
fix(cli): gate refreshAccessToken emission on Auth.TokenURL non-empty (#1244)
Closes #1200. Bearer-only printed CLIs (api_key, bearer_token, OAuth2
specs lacking a TokenURL) were emitting a `refreshAccessToken()` method
that silently returned nil on every call because the runtime tokenURL
was empty. `authHeader()` invoked it whenever an access token expired,
hiding the real failure: users hit the OAuth refresh wall with no error
and `auth status` cheerfully reported `has_refresh_token: ...`.
Tighten both template gates in client.go.tmpl (method definition + call
site) from `{{- if .HasAuthCommand}}` to
`{{- if and .HasAuthCommand (ne .Auth.TokenURL "")}}` so neither emits
for specs without a real token-refresh endpoint. With the outer gate
now guaranteeing TokenURL non-empty, drop the redundant inner
`{{- if .Auth.TokenURL}}` block and the unreachable runtime
`if tokenURL == "" { return nil }` guard.
Add `TestRefreshAccessToken_GatedByTokenURL` with four subtests covering
bearer_token, api_key, oauth2 authorization_code (TokenURL set), and
oauth2 with AuthorizationURL but no TokenURL. The last case pins the
gate on TokenURL specifically so it can't be silently re-keyed on
Auth.Type. Existing `TestTokenScaffoldingFollowsAuthSurface` drops its
client-side scaffolding assertions, which now follow the tighter gate.
Goldens regenerated for `generate-golden-api`, `generate-tier-routing-api`,
and `generate-golden-api-oauth2-cc`: pure subtraction of the dead
method body and call site for the first two, and removal of the
now-unreachable runtime guard inside the function body for the cc case.
---
internal/generator/auth_none_dead_code_test.go | 51 ++++------
.../generator/client_refresh_token_gate_test.go | 106 +++++++++++++++++++++
internal/generator/templates/client.go.tmpl | 9 +-
.../internal/client/client.go | 3 -
.../internal/client/client.go | 13 ---
.../tier-routing-golden/internal/client/client.go | 13 ---
6 files changed, 129 insertions(+), 66 deletions(-)
diff --git a/internal/generator/auth_none_dead_code_test.go b/internal/generator/auth_none_dead_code_test.go
index ff42d7fd..fa95fda8 100644
--- a/internal/generator/auth_none_dead_code_test.go
+++ b/internal/generator/auth_none_dead_code_test.go
@@ -10,36 +10,32 @@ import (
"github.com/stretchr/testify/require"
)
-// Symbols that exist solely to support an `auth` subcommand. They must
-// disappear from generated config.go / client.go for `auth.type: "none"`
+// Config-side symbols that exist solely to support an `auth` subcommand.
+// They must disappear from generated config.go for `auth.type: "none"`
// CLIs (no caller can populate them) and stay for any non-none auth flow.
-var (
- configTokenScaffolding = []string{
- "AccessToken",
- "RefreshToken",
- "TokenExpiry",
- "ClientID",
- "ClientSecret",
- "SaveTokens",
- "ClearTokens",
- }
- clientTokenScaffolding = []string{
- "refreshAccessToken",
- "c.Config.AccessToken",
- "c.Config.RefreshToken",
- "c.Config.TokenExpiry",
- }
-)
+//
+// Client-side refresh plumbing (refreshAccessToken et al.) follows a
+// tighter gate (shouldEmitAuth + Auth.TokenURL != "") and is covered by
+// TestRefreshAccessToken_GatedByTokenURL in client_refresh_token_gate_test.go.
+var configTokenScaffolding = []string{
+ "AccessToken",
+ "RefreshToken",
+ "TokenExpiry",
+ "ClientID",
+ "ClientSecret",
+ "SaveTokens",
+ "ClearTokens",
+}
// TestTokenScaffoldingFollowsAuthSurface pins all three branches of
// shouldEmitAuth(): Auth.Type != "none", Auth.AuthorizationURL != "", and a
// graphql_persisted_query traffic-analysis hint each independently keep the
-// OAuth-shape token scaffolding emitting; only when all three are absent do
-// the symbols disappear (otherwise they would be dead code with no caller on
-// the CLI surface). The cases also verify that the generated CLI still
-// compiles with the gating in place — gated imports falling out of sync with
-// gated function bodies would otherwise pass the symbol-absence assertions
-// but ship a non-buildable CLI.
+// OAuth-shape token scaffolding emitting in config.go; only when all three
+// are absent do the symbols disappear (otherwise they would be dead code
+// with no caller on the CLI surface). The cases also verify that the
+// generated CLI still compiles with the gating in place — gated imports
+// falling out of sync with gated function bodies would otherwise pass the
+// symbol-absence assertions but ship a non-buildable CLI.
func TestTokenScaffoldingFollowsAuthSurface(t *testing.T) {
t.Parallel()
@@ -99,11 +95,6 @@ func TestTokenScaffoldingFollowsAuthSurface(t *testing.T) {
tt.expect(t, configSrc, sym)
}
- clientSrc := readGeneratedFile(t, outputDir, "internal", "client", "client.go")
- for _, sym := range clientTokenScaffolding {
- tt.expect(t, clientSrc, sym)
- }
-
// Catch import-gating mistakes: an orphan reference to a gated
// symbol (e.g., a stray time.Time{} after TokenExpiry is removed)
// would pass the substring asserts but produce non-buildable
diff --git a/internal/generator/client_refresh_token_gate_test.go b/internal/generator/client_refresh_token_gate_test.go
new file mode 100644
index 00000000..faf123a1
--- /dev/null
+++ b/internal/generator/client_refresh_token_gate_test.go
@@ -0,0 +1,106 @@
+package generator
+
+import (
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// Guards #1200: refreshAccessToken() and its call site must only be emitted
+// when the spec actually carries an OAuth token-refresh endpoint
+// (Auth.TokenURL != ""). For bearer-only CLIs (api_key, bearer_token with
+// no TokenURL) the method becomes a guaranteed no-op that silently swallows
+// every refresh call, leaving users to hit a 1-hour token expiry wall with
+// no error and `auth status` cheerfully reporting `has_refresh_token: ...`.
+//
+// Both prongs must hold: the method definition and the do()-path call site
+// share the same gate, so dropping one without the other leaves either an
+// orphan symbol or an undefined-symbol compile error.
+func TestRefreshAccessToken_GatedByTokenURL(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ auth spec.AuthConfig
+ want bool
+ }{
+ {
+ name: "bearer_only_omits_refresh",
+ auth: spec.AuthConfig{
+ Type: "bearer_token",
+ Header: "Authorization",
+ Format: "Bearer {token}",
+ EnvVars: []string{"BEARER_ONLY_TOKEN"},
+ },
+ want: false,
+ },
+ {
+ name: "api_key_omits_refresh",
+ auth: spec.AuthConfig{
+ Type: "api_key",
+ Header: "Authorization",
+ Format: "Bearer {token}",
+ EnvVars: []string{"API_KEY_TOKEN"},
+ },
+ want: false,
+ },
+ {
+ name: "oauth2_authorization_code_emits_refresh",
+ auth: spec.AuthConfig{
+ Type: "oauth2",
+ Header: "Authorization",
+ Format: "Bearer {token}",
+ EnvVars: []string{"AUTHCODE_TOKEN"},
+ AuthorizationURL: "https://example.com/oauth/authorize",
+ TokenURL: "https://example.com/oauth/token",
+ },
+ want: true,
+ },
+ {
+ // Pins the gate on TokenURL specifically — an oauth2 spec
+ // with an AuthorizationURL but no TokenURL must still drop
+ // refresh plumbing. Without this case the gate could be
+ // silently re-keyed on Auth.Type ("oauth2") and the
+ // authorization_code-emits-refresh case alone would still pass.
+ name: "oauth2_without_token_url_omits_refresh",
+ auth: spec.AuthConfig{
+ Type: "oauth2",
+ Header: "Authorization",
+ Format: "Bearer {token}",
+ EnvVars: []string{"NO_TOKEN_URL_TOKEN"},
+ AuthorizationURL: "https://example.com/oauth/authorize",
+ },
+ want: false,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec(tt.name)
+ apiSpec.Auth = tt.auth
+
+ outputDir := filepath.Join(t.TempDir(), tt.name+"-pp-cli")
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ clientSrc := readGeneratedFile(t, outputDir, "internal", "client", "client.go")
+
+ methodPresent := strings.Contains(clientSrc, "func (c *Client) refreshAccessToken()")
+ callPresent := strings.Contains(clientSrc, "c.refreshAccessToken()")
+ assert.Equal(t, tt.want, methodPresent,
+ "refreshAccessToken method definition emission must follow Auth.TokenURL non-empty")
+ assert.Equal(t, tt.want, callPresent,
+ "refreshAccessToken call-site emission must follow Auth.TokenURL non-empty")
+
+ // Catch import-gating mistakes: an orphan reference to a gated
+ // symbol would pass the substring asserts but produce
+ // non-buildable generated source. Skipped under -short.
+ runGoCommand(t, outputDir, "build", "./...")
+ })
+ }
+}
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index 34e0b973..6c1cb41c 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -1229,7 +1229,7 @@ func (c *Client) authHeader() (string, error) {
}
c.ccMu.Unlock()
}
-{{- else if .HasAuthCommand}}
+{{- else if and .HasAuthCommand (ne .Auth.TokenURL "")}}
if c.Config.AccessToken != "" && !c.Config.TokenExpiry.IsZero() && time.Now().After(c.Config.TokenExpiry) && c.Config.RefreshToken != "" {
if err := c.refreshAccessToken(); err != nil {
return "", err
@@ -1327,13 +1327,12 @@ func (c *Client) mintClientCredentials(clientID, clientSecret string) error {
}
{{- end}}
-{{- if .HasAuthCommand}}
+{{- if and .HasAuthCommand (ne .Auth.TokenURL "")}}
func (c *Client) refreshAccessToken() error {
if c.Config == nil {
return nil
}
-{{- if .Auth.TokenURL}}
if c.Config.RefreshToken == "" {
return nil
}
@@ -1342,9 +1341,6 @@ func (c *Client) refreshAccessToken() error {
if tokenURL == "" {
tokenURL = "{{.Auth.TokenURL}}"
}
- if tokenURL == "" {
- return nil
- }
params := url.Values{
"grant_type": {"refresh_token"},
@@ -1391,7 +1387,6 @@ func (c *Client) refreshAccessToken() error {
if err := c.Config.SaveTokens(c.Config.ClientID, c.Config.ClientSecret, tokenResp.AccessToken, refreshToken, expiry); err != nil {
return fmt.Errorf("saving refreshed token: %w", err)
}
-{{- end}}
return nil
}
diff --git a/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go b/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go
index 8c443678..4134cd5d 100644
--- a/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go
+++ b/testdata/golden/expected/generate-golden-api-oauth2-cc/printing-press-oauth2-cc/internal/client/client.go
@@ -490,9 +490,6 @@ func (c *Client) refreshAccessToken() error {
if tokenURL == "" {
tokenURL = "https://api.cc.example/oauth/token"
}
- if tokenURL == "" {
- return nil
- }
params := url.Values{
"grant_type": {"refresh_token"},
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go
index 5630e4ff..52ed9525 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/client/client.go
@@ -368,22 +368,9 @@ func (c *Client) authHeader() (string, error) {
if c.Config == nil {
return "", nil
}
- if c.Config.AccessToken != "" && !c.Config.TokenExpiry.IsZero() && time.Now().After(c.Config.TokenExpiry) && c.Config.RefreshToken != "" {
- if err := c.refreshAccessToken(); err != nil {
- return "", err
- }
- }
return c.Config.AuthHeader(), nil
}
-func (c *Client) refreshAccessToken() error {
- if c.Config == nil {
- return nil
- }
-
- return nil
-}
-
// sanitizeJSONResponse strips known JSONP/XSSI prefixes and UTF-8 BOM from
// response bodies so that downstream JSON parsing succeeds. For clean JSON
// responses these checks are no-ops.
diff --git a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
index f5c5ddc4..c9d1c95f 100644
--- a/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
+++ b/testdata/golden/expected/generate-tier-routing-api/tier-routing-golden/internal/client/client.go
@@ -481,22 +481,9 @@ func (c *Client) authHeader() (string, error) {
if c.Config == nil {
return "", nil
}
- if c.Config.AccessToken != "" && !c.Config.TokenExpiry.IsZero() && time.Now().After(c.Config.TokenExpiry) && c.Config.RefreshToken != "" {
- if err := c.refreshAccessToken(); err != nil {
- return "", err
- }
- }
return c.Config.AuthHeader(), nil
}
-func (c *Client) refreshAccessToken() error {
- if c.Config == nil {
- return nil
- }
-
- return nil
-}
-
// sanitizeJSONResponse strips known JSONP/XSSI prefixes and UTF-8 BOM from
// response bodies so that downstream JSON parsing succeeds. For clean JSON
// responses these checks are no-ops.
← c05d3bc7 fix(cli): always quote SQL identifiers emitted by safeSQLNam
·
back to Cli Printing Press
·
fix(cli): prioritize bearer + apply root-security filter in 09d72e68 →