From 365f13199e2c79b0ecbb4d5af427880bdd432911 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 19 Apr 2026 01:33:47 -0700 Subject: [PATCH] fix(security): scrub workspace-server token + upstream error logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the pre-launch log-scrub audit: 1. handlers/workspace_provision.go:548 logged `token[:8]` — the exact H1 pattern that panicked on short keys. Even with a length guard, leaking 8 chars of an auth token into centralized logs shortens the search space for anyone who gets log-read access. Now logs only `len(token)` as a liveness signal. 2. provisioner/cp_provisioner.go:101 fell back to logging the raw control-plane response body when the structured {"error":"..."} field was absent. If the CP ever echoed request headers (Authorization) or a portion of user-data back in an error path, the bearer token would end up in our tenant-instance logs. Now logs the byte count only; the structured error remains in place for the happy path. Also caps the read at 64 KiB via io.LimitReader to prevent log-flood DoS from a compromised upstream. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/handlers/workspace_provision.go | 5 ++++- .../internal/provisioner/cp_provisioner.go | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 7290c56c..f023e240 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -545,6 +545,9 @@ func (h *WorkspaceHandler) provisionWorkspaceCP(workspaceID, templatePath string if tokenErr != nil { log.Printf("CPProvisioner: failed to issue token for %s: %v", workspaceID, tokenErr) } else { - log.Printf("CPProvisioner: issued auth token for workspace %s (prefix: %s...)", workspaceID, token[:8]) + // Don't log any prefix of the token. Earlier H1 regression showed + // this slice pattern (token[:8]) panics when a helper returns a + // short value. Length alone is enough to confirm a token issued. + log.Printf("CPProvisioner: issued auth token for workspace %s (len=%d)", workspaceID, len(token)) } } diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index ca224d99..3b1a040c 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -91,14 +91,21 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, } defer resp.Body.Close() - respBody, _ := io.ReadAll(resp.Body) + // Cap body read at 64 KiB — the CP only ever returns small JSON + // responses; an unbounded read could be weaponized into log-flood + // DoS by a compromised upstream. + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) var result cpProvisionResponse json.Unmarshal(respBody, &result) if resp.StatusCode != http.StatusCreated { + // Prefer the structured {"error":"..."} field. Do NOT fall back + // to string(respBody) — our logs ingest errors, and an upstream + // misconfiguration that echoed the Authorization header or + // request body into the response would leak bearer tokens. errMsg := result.Error if errMsg == "" { - errMsg = string(respBody) + errMsg = fmt.Sprintf("", len(respBody)) } return "", fmt.Errorf("cp provisioner: provision failed (%d): %s", resp.StatusCode, errMsg) }