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 003eec7..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,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`). `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`. **Config-free commands**: `version` and `completion` skip config file loading and work even if the config is missing or corrupted. @@ -154,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 5ff8ac1..7d82199 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 @@ -706,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.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..1457cf0 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" @@ -12,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" @@ -33,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" @@ -54,8 +66,18 @@ func TestVersionCmd_JSON(t *testing.T) { } func TestSetVersionInfo(t *testing.T) { + restoreVersionInfo(t) + 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) { + restoreVersionInfo(t) + + 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/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 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..af13430 --- /dev/null +++ b/cli/pkg/client/useragent.go @@ -0,0 +1,63 @@ +package client + +import ( + "fmt" + "net/http" + "runtime" + "strings" + "sync/atomic" +) + +// 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) { + ua := formatUserAgent(v) + currentUserAgent.Store(&ua) +} + +// UserAgent returns the User-Agent header value of every request: +// "stackctl/ (/)". +func UserAgent() string { + if ua := currentUserAgent.Load(); ua != nil { + return *ua + } + return devUserAgent +} + +// 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..f901657 --- /dev/null +++ b/cli/pkg/client/useragent_test.go @@ -0,0 +1,198 @@ +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 := currentUserAgent.Load() + t.Cleanup(func() { currentUserAgent.Store(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(c *Client) error + // handler answers the request (nil: 200 with an empty JSON object). + 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"). + setup func(c *Client) + }{ + { + name: "API request", + run: func(c *Client) error { return c.Get("/api/v1/stack-instances", nil) }, + wantCalls: 1, + }, + { + name: "session request", + run: func(c *Client) error { return c.Logout("r1") }, + wantCalls: 1, + }, + { + name: "WebSocket upgrade", + run: func(c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + handler: func(_ *testing.T, 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(c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + 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) + 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(c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + 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) + 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(c *Client) error { + conn, err := c.dialWS(context.Background(), "/ws", nil) + if err == nil { + conn.Close() + } + return err + }, + 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")) + 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(t, 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(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 961de39..afd7589 100644 --- a/cli/pkg/client/websocket.go +++ b/cli/pkg/client/websocket.go @@ -265,7 +265,7 @@ func (c *Client) dialWS(ctx context.Context, path string, warnWriter io.Writer) return nil, err } - header := http.Header{} + header := baseWSHeader() if c.APIKey != "" { header.Set("X-API-Key", c.APIKey) } else if token != "" { @@ -293,7 +293,8 @@ 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. + conn2, resp2, err2 := dialer.DialContext(ctx, fallbackURL, baseWSHeader()) if err2 != nil { if hsErr := handshakeError(resp2, err2, false); hsErr != nil { return nil, hsErr @@ -308,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