From 46890f9f7e7ec3098f7cbdaf7bf6d2ce6f70938a Mon Sep 17 00:00:00 2001 From: omattsson Date: Fri, 23 Oct 2026 18:13:12 +0200 Subject: [PATCH 1/3] feat(client): send a stackctl User-Agent header (#170) Every request from cli/pkg/client, the session requests in auth.go and both WebSocket dials (also the ?token= fallback) now send "User-Agent: stackctl/ (/)". cmd.SetVersionInfo passes the build version to client.SetVersion, so the header matches `stackctl version`; a build without ldflags sends "stackctl/dev". Characters outside the RFC 9110 token set become "_". The header is set where each request is built, not in a transport, because cmd/stack.go replaces HTTPClient. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 + README.md | 2 + cli/cmd/version.go | 2 + cli/cmd/version_test.go | 14 +++ cli/pkg/client/auth.go | 1 + cli/pkg/client/client.go | 1 + cli/pkg/client/useragent.go | 56 +++++++++ cli/pkg/client/useragent_test.go | 195 +++++++++++++++++++++++++++++++ cli/pkg/client/websocket.go | 6 +- 9 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 cli/pkg/client/useragent.go create mode 100644 cli/pkg/client/useragent_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 003eec7..09afd2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -125,6 +125,8 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl **Warning headers**: `send()` writes each `Warning` response header (for example a `299` deprecation notice) to `Client.WarnWriter` (stderr when nil) as `Warning: `, for every command and also on error responses. The quoted warn-text is unescaped and sanitized like server error messages; one header can hold several comma-separated warnings (warn-dates are skipped). The same text is written once per client (retries do not repeat it; the dedupe set holds at most 64 texts), and writes are serialized under `warnMu`. `sanitizeServerMessage` replaces control characters (`unicode.IsControl`: C0, DEL, C1) with a space and drops bidi controls (U+200E, U+200F, U+202A–U+202E, U+2066–U+2069). +**User-Agent**: every request sends `User-Agent: stackctl/ (/)` (`pkg/client/useragent.go`). `cmd.SetVersionInfo` calls `client.SetVersion`, so it matches `stackctl version` (empty: `dev`). Set it where the request is built (`setUserAgent` in `send()`, `sendSession()`, `dialWS()` and its `?token=` fallback), not in a transport: `stack.go` replaces `HTTPClient`. Tests that call `SetVersion` must not run in parallel. + **Plugins** (`cmd/plugins.go`): for a plugin, `Execute` sets `TraverseChildren`, so the global flags before the plugin name are parsed by stackctl and not passed to the plugin; `pluginEnv` exports them (`STACKCTL_OUTPUT`, `STACKCTL_QUIET`, `STACKCTL_NO_COLOR` + `NO_COLOR`, `STACKCTL_DEBUG`, `STACKCTL_INSECURE`, `STACKCTL_CONFIG_DIR`, plus the context values). Arguments after the plugin name pass unchanged. Contract in `EXTENDING.md`. **Config-free commands**: `version` and `completion` skip config file loading and work even if the config is missing or corrupted. diff --git a/README.md b/README.md index 5ff8ac1..0d78a36 100644 --- a/README.md +++ b/README.md @@ -325,6 +325,8 @@ With `--id`, a connection that ends for another reason before every listed insta stackctl writes any `Warning` header from the API (for example a deprecation notice) to stderr as `Warning: `, for every command. +Every request, also the WebSocket upgrade, sends `User-Agent: stackctl/ (/)`, for example `stackctl/1.4.0 (linux/amd64)`. The version is the one of `stackctl version`. + ### Templates ```bash diff --git a/cli/cmd/version.go b/cli/cmd/version.go index 3302244..378f02f 100644 --- a/cli/cmd/version.go +++ b/cli/cmd/version.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" + "github.com/omattsson/stackctl/cli/pkg/client" "github.com/omattsson/stackctl/cli/pkg/output" "github.com/spf13/cobra" ) @@ -18,6 +19,7 @@ func SetVersionInfo(version, commit, date string) { buildVersion = version buildCommit = commit buildDate = date + client.SetVersion(version) } var versionCmd = &cobra.Command{ diff --git a/cli/cmd/version_test.go b/cli/cmd/version_test.go index b054978..c48701c 100644 --- a/cli/cmd/version_test.go +++ b/cli/cmd/version_test.go @@ -3,8 +3,10 @@ package cmd import ( "bytes" "encoding/json" + "runtime" "testing" + "github.com/omattsson/stackctl/cli/pkg/client" "github.com/omattsson/stackctl/cli/pkg/output" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -54,8 +56,20 @@ func TestVersionCmd_JSON(t *testing.T) { } func TestSetVersionInfo(t *testing.T) { + prevVersion, prevCommit, prevDate := buildVersion, buildCommit, buildDate + t.Cleanup(func() { SetVersionInfo(prevVersion, prevCommit, prevDate) }) + SetVersionInfo("3.0.0", "xyz789", "2025-12-25") assert.Equal(t, "3.0.0", buildVersion) assert.Equal(t, "xyz789", buildCommit) assert.Equal(t, "2025-12-25", buildDate) } + +func TestSetVersionInfo_SetsUserAgent(t *testing.T) { + prevVersion, prevCommit, prevDate := buildVersion, buildCommit, buildDate + t.Cleanup(func() { SetVersionInfo(prevVersion, prevCommit, prevDate) }) + + SetVersionInfo("3.1.4", "abc", "2026-01-01") + assert.Equal(t, "3.1.4", buildVersion) + assert.Equal(t, "stackctl/3.1.4 ("+runtime.GOOS+"/"+runtime.GOARCH+")", client.UserAgent()) +} diff --git a/cli/pkg/client/auth.go b/cli/pkg/client/auth.go index f6380b4..4763abc 100644 --- a/cli/pkg/client/auth.go +++ b/cli/pkg/client/auth.go @@ -387,6 +387,7 @@ func (c *Client) sendSession(method, path, bearer, refreshToken string) (*http.R return nil, fmt.Errorf("creating request: %w", err) } req.Header.Set("Accept", "application/json") + setUserAgent(req.Header) if bearer != "" { req.Header.Set("Authorization", "Bearer "+bearer) } diff --git a/cli/pkg/client/client.go b/cli/pkg/client/client.go index ea714f8..640654c 100644 --- a/cli/pkg/client/client.go +++ b/cli/pkg/client/client.go @@ -259,6 +259,7 @@ func (c *Client) send(method, path string, body interface{}) (*http.Response, er req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", "application/json") + setUserAgent(req.Header) // API key takes precedence over JWT token if c.APIKey != "" { diff --git a/cli/pkg/client/useragent.go b/cli/pkg/client/useragent.go new file mode 100644 index 0000000..5181881 --- /dev/null +++ b/cli/pkg/client/useragent.go @@ -0,0 +1,56 @@ +package client + +import ( + "fmt" + "net/http" + "runtime" + "strings" + "sync/atomic" +) + +// version is the stackctl version in the User-Agent header. main sets it +// once at startup through cmd.SetVersionInfo and SetVersion. +var version atomic.Value // string + +// SetVersion sets the stackctl version that every request reports in its +// User-Agent header. An empty version means "dev". +func SetVersion(v string) { + version.Store(v) +} + +// UserAgent returns the User-Agent header value of every request: +// "stackctl/ (/)". +func UserAgent() string { + v, _ := version.Load().(string) + return formatUserAgent(v) +} + +// formatUserAgent builds the header value. Every byte of v that is not an +// RFC 9110 tchar becomes "_", so a bad version (a space, a newline) cannot +// break the request or split the product token. +func formatUserAgent(v string) string { + if v == "" { + v = "dev" + } + b := []byte(v) + for i, c := range b { + if !isTchar(c) { + b[i] = '_' + } + } + return fmt.Sprintf("stackctl/%s (%s/%s)", b, runtime.GOOS, runtime.GOARCH) +} + +// isTchar reports whether c is an RFC 9110 token character. +func isTchar(c byte) bool { + switch { + case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9': + return true + } + return strings.IndexByte("!#$%&'*+-.^_`|~", c) >= 0 +} + +// setUserAgent sets the User-Agent header on h. +func setUserAgent(h http.Header) { + h.Set("User-Agent", UserAgent()) +} diff --git a/cli/pkg/client/useragent_test.go b/cli/pkg/client/useragent_test.go new file mode 100644 index 0000000..add8837 --- /dev/null +++ b/cli/pkg/client/useragent_test.go @@ -0,0 +1,195 @@ +package client + +import ( + "context" + "net/http" + "net/http/httptest" + "runtime" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func wantUserAgent(v string) string { + return "stackctl/" + v + " (" + runtime.GOOS + "/" + runtime.GOARCH + ")" +} + +func TestFormatUserAgent(t *testing.T) { + t.Parallel() + tests := []struct { + name string + version string + want string + }{ + {name: "empty version means dev", version: "", want: wantUserAgent("dev")}, + {name: "dev", version: "dev", want: wantUserAgent("dev")}, + {name: "release", version: "1.4.0", want: wantUserAgent("1.4.0")}, + {name: "git describe", version: "v1.4.0-3-gabc123-dirty", want: wantUserAgent("v1.4.0-3-gabc123-dirty")}, + {name: "all tchar symbols kept", version: "1!#$%&'*+-.^_`|~", want: wantUserAgent("1!#$%&'*+-.^_`|~")}, + {name: "space", version: "1.4.0 beta", want: wantUserAgent("1.4.0_beta")}, + {name: "newline", version: "1.4.0\r\nX-Evil: 1", want: wantUserAgent("1.4.0__X-Evil__1")}, + {name: "separators", version: "1/2(3)", want: wantUserAgent("1_2_3_")}, + {name: "non-ASCII bytes", version: "1.0é", want: wantUserAgent("1.0__")}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, formatUserAgent(tt.version)) + }) + } +} + +// TestUserAgent_Requests checks that every request kind sends the +// User-Agent of SetVersion. It changes the package version, so it does not +// run in parallel: Go runs the parallel tests after it, and it restores the +// version before it returns. +func TestUserAgent_Requests(t *testing.T) { + prev, _ := version.Load().(string) + t.Cleanup(func() { SetVersion(prev) }) + + SetVersion("") + assert.Equal(t, wantUserAgent("dev"), UserAgent(), "no version means dev") + + SetVersion("v9.8.7") + want := wantUserAgent("v9.8.7") + require.Equal(t, want, UserAgent()) + + tests := []struct { + name string + // run sends the request to the server URL. + run func(t *testing.T, c *Client) error + // handler answers the request (nil: 200 with an empty JSON object). + handler func(w http.ResponseWriter, r *http.Request) + // wantCalls is the number of requests that must carry the header. + wantCalls int + // setup sets the credentials (nil: Token "jwt-1"). + setup func(c *Client) + }{ + { + name: "API request", + run: func(_ *testing.T, c *Client) error { return c.Get("/api/v1/stack-instances", nil) }, + wantCalls: 1, + }, + { + name: "session request", + run: func(_ *testing.T, c *Client) error { return c.Logout("r1") }, + wantCalls: 1, + }, + { + name: "WebSocket upgrade", + run: func(_ *testing.T, c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + handler: func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err == nil { + conn.Close() + } + }, + wantCalls: 1, + }, + { + name: "WebSocket query-param fallback after 401", + run: func(_ *testing.T, c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + handler: func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("token") == "" { + assert.Equal(t, "Bearer jwt-1", r.Header.Get("Authorization"), "first dial uses the header") + w.WriteHeader(http.StatusUnauthorized) + return + } + assert.Empty(t, r.Header.Get("Authorization"), "fallback sends the token in the URL only") + assert.Empty(t, r.Header.Get("X-API-Key"), "fallback sends no API key") + conn, err := upgrader.Upgrade(w, r, nil) + if err == nil { + conn.Close() + } + }, + wantCalls: 2, + }, + { + name: "WebSocket upgrade with API key", + setup: func(c *Client) { c.APIKey = "key-1" }, + run: func(_ *testing.T, c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + handler: func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "key-1", r.Header.Get("X-API-Key")) + assert.Empty(t, r.Header.Get("Authorization")) + conn, err := upgrader.Upgrade(w, r, nil) + if err == nil { + conn.Close() + } + }, + wantCalls: 1, + }, + { + name: "WebSocket API key refused, JWT fallback", + setup: func(c *Client) { c.APIKey = "key-1"; c.Token = "jwt-1" }, + run: func(_ *testing.T, c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + handler: func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("token") == "" { + assert.Equal(t, "key-1", r.Header.Get("X-API-Key"), "first dial uses the API key") + assert.Empty(t, r.Header.Get("Authorization")) + w.WriteHeader(http.StatusUnauthorized) + return + } + assert.Equal(t, "jwt-1", r.URL.Query().Get("token")) + assert.Empty(t, r.Header.Get("X-API-Key"), "fallback sends no API key") + assert.Empty(t, r.Header.Get("Authorization")) + conn, err := upgrader.Upgrade(w, r, nil) + if err == nil { + conn.Close() + } + }, + wantCalls: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if assert.Equal(t, want, r.Header.Get("User-Agent"), "%s %s", r.Method, r.URL) { + calls.Add(1) + } + if tt.handler != nil { + tt.handler(w, r) + return + } + writeJSON(w, http.StatusOK, map[string]string{}) + })) + defer srv.Close() + + c := New(srv.URL) + if tt.setup != nil { + tt.setup(c) + } else { + c.Token = "jwt-1" + } + require.NoError(t, tt.run(t, c)) + srv.Close() // wait for the handlers before reading calls + assert.Equal(t, int32(tt.wantCalls), calls.Load()) + }) + } +} diff --git a/cli/pkg/client/websocket.go b/cli/pkg/client/websocket.go index 961de39..e1ebb52 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -266,6 +266,7 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) } header := http.Header{} + setUserAgent(header) if c.APIKey != "" { header.Set("X-API-Key", c.APIKey) } else if token != "" { @@ -293,7 +294,10 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) // transparently switches to the JWT path (documented precedence). if resp != nil && resp.StatusCode == http.StatusUnauthorized && token != "" { fallbackURL := appendQueryToken(wsURL, token) - conn2, resp2, err2 := dialer.DialContext(ctx, fallbackURL, nil) + // The fallback carries the token in the URL only, never in a header. + fallbackHeader := http.Header{} + setUserAgent(fallbackHeader) + conn2, resp2, err2 := dialer.DialContext(ctx, fallbackURL, fallbackHeader) if err2 != nil { if hsErr := handshakeError(resp2, err2, false); hsErr != nil { return nil, hsErr From 3c6df82a9eedcf0ef0150a71cf797bc89a7d1254 Mon Sep 17 00:00:00 2001 From: omattsson Date: Fri, 23 Oct 2026 18:24:16 +0200 Subject: [PATCH 2/3] build(go): bump Go to 1.26.10 for GO-2026-6809 Go 1.26.9 is affected by GO-2026-6809 (crypto/x509 trusts compromised intermediate certificates). govulncheck reaches it through the TLS WebSocket dial in client.dialWS. CI reads the Go version from cli/go.mod. Co-Authored-By: Claude Opus 5.5 --- cli/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/go.mod b/cli/go.mod index 8b02083..1376e72 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -1,6 +1,6 @@ module github.com/omattsson/stackctl/cli -go 1.26.9 +go 1.26.10 require ( github.com/gorilla/websocket v1.5.3 From fe2e44663ac7d0f920cde02dc2ff16e3e082d347 Mon Sep 17 00:00:00 2001 From: omattsson Date: Fri, 23 Oct 2026 19:55:49 +0200 Subject: [PATCH 3/3] refactor(client): build the User-Agent once, share the WebSocket base header Review fixes for #171: - SetVersion builds the header value once (atomic.Pointer[string]); UserAgent() only loads it. - Both WebSocket dials take their headers from baseWSHeader(), so the ?token= fallback cannot miss a header that the first dial sends. - All version tests restore the build globals, so TestVersionCmd_JSON no longer leaks "2.0.0" into the client User-Agent of later cmd tests. - The test server handlers report failures on their subtest. - CLAUDE.md documents the t.Parallel exception for tests that change package-global state; the docs name Go 1.26.10. Co-Authored-By: Claude Opus 5.5 --- .github/copilot-instructions.md | 2 +- CLAUDE.md | 6 +++--- README.md | 2 +- cli/cmd/version_test.go | 16 ++++++++++---- cli/pkg/client/useragent.go | 19 ++++++++++------ cli/pkg/client/useragent_test.go | 37 +++++++++++++++++--------------- cli/pkg/client/websocket.go | 16 +++++++++----- 7 files changed, 61 insertions(+), 37 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 36e86c4..bf15316 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -7,7 +7,7 @@ stackctl is a Go CLI tool (Cobra) for managing Kubernetes stack deployments. It ## Architecture - All code lives under `cli/` — the Go module root (`cli/go.mod`). Run Go commands from `cli/`. -- Go version: see `cli/go.mod` (1.26.9 or later). CI uses `go-version-file: cli/go.mod`. +- Go version: see `cli/go.mod` (1.26.10 or later). CI uses `go-version-file: cli/go.mod`. - Commands: `cli/cmd/` (one file per command group, subcommands registered in `init()`) - HTTP client: `cli/pkg/client/client.go` (REST), `cli/pkg/client/auth.go` (session renewal), `cli/pkg/client/websocket.go` (deployment log streaming) - Types: `cli/pkg/types/types.go` (client-side structs matching API responses) diff --git a/CLAUDE.md b/CLAUDE.md index 09afd2d..b697b8d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ Go CLI tool (Cobra + yaml.v3) for managing Kubernetes stack deployments. Pure API client — talks to the [k8s-stack-manager](https://github.com/omattsson/k8s-stack-manager) backend. No backend logic, no frontend, no database, no direct K8s interaction. -Go version: see `cli/go.mod` (1.26.9 or later); the Dockerfile builder image may be newer. +Go version: see `cli/go.mod` (1.26.10 or later); the Dockerfile builder image may be newer. ## Project Structure @@ -125,7 +125,7 @@ cli/ # Go module root (github.com/omattsson/stackctl/cl **Warning headers**: `send()` writes each `Warning` response header (for example a `299` deprecation notice) to `Client.WarnWriter` (stderr when nil) as `Warning: `, for every command and also on error responses. The quoted warn-text is unescaped and sanitized like server error messages; one header can hold several comma-separated warnings (warn-dates are skipped). The same text is written once per client (retries do not repeat it; the dedupe set holds at most 64 texts), and writes are serialized under `warnMu`. `sanitizeServerMessage` replaces control characters (`unicode.IsControl`: C0, DEL, C1) with a space and drops bidi controls (U+200E, U+200F, U+202A–U+202E, U+2066–U+2069). -**User-Agent**: every request sends `User-Agent: stackctl/ (/)` (`pkg/client/useragent.go`). `cmd.SetVersionInfo` calls `client.SetVersion`, so it matches `stackctl version` (empty: `dev`). Set it where the request is built (`setUserAgent` in `send()`, `sendSession()`, `dialWS()` and its `?token=` fallback), not in a transport: `stack.go` replaces `HTTPClient`. Tests that call `SetVersion` must not run in parallel. +**User-Agent**: every request sends `User-Agent: stackctl/ (/)` (`pkg/client/useragent.go`). `cmd.SetVersionInfo` calls `client.SetVersion`, so it matches `stackctl version` (empty: `dev`). `SetVersion` builds the header value once; `UserAgent()` only loads it. Set it where the request is built (`setUserAgent` in `send()` and `sendSession()`; `baseWSHeader()` for both WebSocket dials, also the `?token=` fallback), not in a transport: `stack.go` replaces `HTTPClient`. **Plugins** (`cmd/plugins.go`): for a plugin, `Execute` sets `TraverseChildren`, so the global flags before the plugin name are parsed by stackctl and not passed to the plugin; `pluginEnv` exports them (`STACKCTL_OUTPUT`, `STACKCTL_QUIET`, `STACKCTL_NO_COLOR` + `NO_COLOR`, `STACKCTL_DEBUG`, `STACKCTL_INSECURE`, `STACKCTL_CONFIG_DIR`, plus the context values). Arguments after the plugin name pass unchanged. Contract in `EXTENDING.md`. @@ -156,7 +156,7 @@ contexts: ## Testing Conventions - `testify/assert`, table-driven tests -- `pkg/` tests call `t.Parallel()` on the parent test and on subtests +- `pkg/` tests call `t.Parallel()` on the parent test and on subtests. Exception: a test that changes package-global state (for example `client.SetVersion`, see `TestUserAgent_Requests`) must not call `t.Parallel()`, nor its subtests; it restores the state with `t.Cleanup`. Go runs the parallel top-level tests only after the serial ones - `cmd/` tests that touch package globals (`cfg`, `printer`, `flagAPIURL` and other flag variables) must not call `t.Parallel()`. Pure-function tests in `cmd/` (for example `resolve_test.go`) may - No `tt := tt` needed (Go 1.22+ loop variable semantics) - Test helpers: `setupStackTestCmd` (`cmd/stack_test.go`), `setupTestCmd` (`cmd/config_test.go`) diff --git a/README.md b/README.md index 0d78a36..7d82199 100644 --- a/README.md +++ b/README.md @@ -708,7 +708,7 @@ stackctl completion powershell | Out-String | Invoke-Expression ### Prerequisites -- Go version: see `cli/go.mod` (1.26.9 or later) +- Go version: see `cli/go.mod` (1.26.10 or later) - A running [k8s-stack-manager](https://github.com/omattsson/k8s-stack-manager) backend for integration/e2e tests (`make dev` in that repo) ### Getting Started diff --git a/cli/cmd/version_test.go b/cli/cmd/version_test.go index c48701c..1457cf0 100644 --- a/cli/cmd/version_test.go +++ b/cli/cmd/version_test.go @@ -14,7 +14,16 @@ import ( // Version tests mutate package-level globals, so they cannot run in parallel. +// restoreVersionInfo restores the build globals and the client User-Agent +// version when t ends. Every test that changes the build globals calls it. +func restoreVersionInfo(t *testing.T) { + t.Helper() + prevVersion, prevCommit, prevDate := buildVersion, buildCommit, buildDate + t.Cleanup(func() { SetVersionInfo(prevVersion, prevCommit, prevDate) }) +} + func TestVersionCmd_Table(t *testing.T) { + restoreVersionInfo(t) buildVersion = "1.2.3" buildCommit = "abc123" buildDate = "2025-01-01" @@ -35,6 +44,7 @@ func TestVersionCmd_Table(t *testing.T) { } func TestVersionCmd_JSON(t *testing.T) { + restoreVersionInfo(t) buildVersion = "2.0.0" buildCommit = "def456" buildDate = "2025-06-15" @@ -56,8 +66,7 @@ func TestVersionCmd_JSON(t *testing.T) { } func TestSetVersionInfo(t *testing.T) { - prevVersion, prevCommit, prevDate := buildVersion, buildCommit, buildDate - t.Cleanup(func() { SetVersionInfo(prevVersion, prevCommit, prevDate) }) + restoreVersionInfo(t) SetVersionInfo("3.0.0", "xyz789", "2025-12-25") assert.Equal(t, "3.0.0", buildVersion) @@ -66,8 +75,7 @@ func TestSetVersionInfo(t *testing.T) { } func TestSetVersionInfo_SetsUserAgent(t *testing.T) { - prevVersion, prevCommit, prevDate := buildVersion, buildCommit, buildDate - t.Cleanup(func() { SetVersionInfo(prevVersion, prevCommit, prevDate) }) + restoreVersionInfo(t) SetVersionInfo("3.1.4", "abc", "2026-01-01") assert.Equal(t, "3.1.4", buildVersion) diff --git a/cli/pkg/client/useragent.go b/cli/pkg/client/useragent.go index 5181881..af13430 100644 --- a/cli/pkg/client/useragent.go +++ b/cli/pkg/client/useragent.go @@ -8,21 +8,28 @@ import ( "sync/atomic" ) -// version is the stackctl version in the User-Agent header. main sets it -// once at startup through cmd.SetVersionInfo and SetVersion. -var version atomic.Value // string +// currentUserAgent is the User-Agent header value of every request. +// SetVersion builds it once; every request only loads it. main sets it at +// startup through cmd.SetVersionInfo; nil means "dev". +var currentUserAgent atomic.Pointer[string] + +// devUserAgent is the User-Agent when SetVersion was never called. +var devUserAgent = formatUserAgent("") // SetVersion sets the stackctl version that every request reports in its // User-Agent header. An empty version means "dev". func SetVersion(v string) { - version.Store(v) + ua := formatUserAgent(v) + currentUserAgent.Store(&ua) } // UserAgent returns the User-Agent header value of every request: // "stackctl/ (/)". func UserAgent() string { - v, _ := version.Load().(string) - return formatUserAgent(v) + if ua := currentUserAgent.Load(); ua != nil { + return *ua + } + return devUserAgent } // formatUserAgent builds the header value. Every byte of v that is not an diff --git a/cli/pkg/client/useragent_test.go b/cli/pkg/client/useragent_test.go index add8837..f901657 100644 --- a/cli/pkg/client/useragent_test.go +++ b/cli/pkg/client/useragent_test.go @@ -46,8 +46,8 @@ func TestFormatUserAgent(t *testing.T) { // run in parallel: Go runs the parallel tests after it, and it restores the // version before it returns. func TestUserAgent_Requests(t *testing.T) { - prev, _ := version.Load().(string) - t.Cleanup(func() { SetVersion(prev) }) + prev := currentUserAgent.Load() + t.Cleanup(func() { currentUserAgent.Store(prev) }) SetVersion("") assert.Equal(t, wantUserAgent("dev"), UserAgent(), "no version means dev") @@ -59,9 +59,9 @@ func TestUserAgent_Requests(t *testing.T) { tests := []struct { name string // run sends the request to the server URL. - run func(t *testing.T, c *Client) error + run func(c *Client) error // handler answers the request (nil: 200 with an empty JSON object). - handler func(w http.ResponseWriter, r *http.Request) + handler func(t *testing.T, w http.ResponseWriter, r *http.Request) // wantCalls is the number of requests that must carry the header. wantCalls int // setup sets the credentials (nil: Token "jwt-1"). @@ -69,24 +69,24 @@ func TestUserAgent_Requests(t *testing.T) { }{ { name: "API request", - run: func(_ *testing.T, c *Client) error { return c.Get("/api/v1/stack-instances", nil) }, + run: func(c *Client) error { return c.Get("/api/v1/stack-instances", nil) }, wantCalls: 1, }, { name: "session request", - run: func(_ *testing.T, c *Client) error { return c.Logout("r1") }, + run: func(c *Client) error { return c.Logout("r1") }, wantCalls: 1, }, { name: "WebSocket upgrade", - run: func(_ *testing.T, c *Client) error { + run: func(c *Client) error { conn, err := c.dialWS(context.Background(), "/ws", nil) if err == nil { conn.Close() } return err }, - handler: func(w http.ResponseWriter, r *http.Request) { + handler: func(_ *testing.T, w http.ResponseWriter, r *http.Request) { conn, err := upgrader.Upgrade(w, r, nil) if err == nil { conn.Close() @@ -96,14 +96,14 @@ func TestUserAgent_Requests(t *testing.T) { }, { name: "WebSocket query-param fallback after 401", - run: func(_ *testing.T, c *Client) error { + run: func(c *Client) error { conn, err := c.dialWS(context.Background(), "/ws", nil) if err == nil { conn.Close() } return err }, - handler: func(w http.ResponseWriter, r *http.Request) { + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) { if r.URL.Query().Get("token") == "" { assert.Equal(t, "Bearer jwt-1", r.Header.Get("Authorization"), "first dial uses the header") w.WriteHeader(http.StatusUnauthorized) @@ -121,14 +121,14 @@ func TestUserAgent_Requests(t *testing.T) { { name: "WebSocket upgrade with API key", setup: func(c *Client) { c.APIKey = "key-1" }, - run: func(_ *testing.T, c *Client) error { + run: func(c *Client) error { conn, err := c.dialWS(context.Background(), "/ws", nil) if err == nil { conn.Close() } return err }, - handler: func(w http.ResponseWriter, r *http.Request) { + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) { assert.Equal(t, "key-1", r.Header.Get("X-API-Key")) assert.Empty(t, r.Header.Get("Authorization")) conn, err := upgrader.Upgrade(w, r, nil) @@ -141,14 +141,14 @@ func TestUserAgent_Requests(t *testing.T) { { name: "WebSocket API key refused, JWT fallback", setup: func(c *Client) { c.APIKey = "key-1"; c.Token = "jwt-1" }, - run: func(_ *testing.T, c *Client) error { + run: func(c *Client) error { conn, err := c.dialWS(context.Background(), "/ws", nil) if err == nil { conn.Close() } return err }, - handler: func(w http.ResponseWriter, r *http.Request) { + handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) { if r.URL.Query().Get("token") == "" { assert.Equal(t, "key-1", r.Header.Get("X-API-Key"), "first dial uses the API key") assert.Empty(t, r.Header.Get("Authorization")) @@ -174,7 +174,7 @@ func TestUserAgent_Requests(t *testing.T) { calls.Add(1) } if tt.handler != nil { - tt.handler(w, r) + tt.handler(t, w, r) return } writeJSON(w, http.StatusOK, map[string]string{}) @@ -187,8 +187,11 @@ func TestUserAgent_Requests(t *testing.T) { } else { c.Token = "jwt-1" } - require.NoError(t, tt.run(t, c)) - srv.Close() // wait for the handlers before reading calls + require.NoError(t, tt.run(c)) + // Close waits for the HTTP handlers, but not for a handler after + // a WebSocket Upgrade (hijacked connection): do not assert in a + // handler after Upgrade. calls is counted before the handler. + srv.Close() assert.Equal(t, int32(tt.wantCalls), calls.Load()) }) } diff --git a/cli/pkg/client/websocket.go b/cli/pkg/client/websocket.go index e1ebb52..afd7589 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -265,8 +265,7 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) return nil, err } - header := http.Header{} - setUserAgent(header) + header := baseWSHeader() if c.APIKey != "" { header.Set("X-API-Key", c.APIKey) } else if token != "" { @@ -295,9 +294,7 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) if resp != nil && resp.StatusCode == http.StatusUnauthorized && token != "" { fallbackURL := appendQueryToken(wsURL, token) // The fallback carries the token in the URL only, never in a header. - fallbackHeader := http.Header{} - setUserAgent(fallbackHeader) - conn2, resp2, err2 := dialer.DialContext(ctx, fallbackURL, fallbackHeader) + conn2, resp2, err2 := dialer.DialContext(ctx, fallbackURL, baseWSHeader()) if err2 != nil { if hsErr := handshakeError(resp2, err2, false); hsErr != nil { return nil, hsErr @@ -312,6 +309,15 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) return nil, fmt.Errorf("connecting to WebSocket: %w", err) } +// baseWSHeader returns the headers of both WebSocket dials (the header-auth +// dial and its ?token= fallback), without any credentials. Add a header for +// every upgrade request here, so the fallback cannot miss it. +func baseWSHeader() http.Header { + header := http.Header{} + setUserAgent(header) + return header +} + // wsAuthRequiredMessage is the 401 message of the k8s-stack-manager // WebSocket handler when the request has no JWT. A server that accepts API // keys on /ws (k8s-stack-manager v0.8.0 or later) answers a bad key with