From 4c0cb487c1499345ff38f1cad25f0a880d6b0f31 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Wed, 22 Apr 2026 17:13:38 -0700 Subject: [PATCH 1/2] fix(cp-provisioner): use CP_ADMIN_API_TOKEN bearer for /cp/admin/* routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptom (prod tenant hongmingwang, 2026-04-22): cp provisioner: console: unexpected 401 GET /workspaces/:id/console → 502 (View Logs broken) Root cause: the tenant's CPProvisioner.authHeaders sent the provision- gate shared secret as the Authorization bearer for every outbound CP call, including /cp/admin/workspaces/:id/console. But CP gates /cp/admin/* with CP_ADMIN_API_TOKEN — a distinct secret so a compromised tenant's provision credentials can't read other tenants' serial console output. Bearer mismatch → 401. Fix: split authHeaders into two methods — - provisionAuthHeaders(): Authorization: Bearer for /cp/workspaces/* (Start, Stop, IsRunning) - adminAuthHeaders(): Authorization: Bearer for /cp/admin/* (GetConsoleOutput and future admin reads) Both still send X-Molecule-Admin-Token for per-tenant identity. When CP_ADMIN_API_TOKEN is unset (dev / self-hosted single-secret setups), cpAdminAPIKey falls back to sharedSecret so nothing regresses. Rollout requirement: the tenant EC2 needs CP_ADMIN_API_TOKEN in its env — this PR wires up the code, but CP's tenant-provision path must inject the value. Filed as follow-up; until then, operators can set it manually on existing tenants. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/provisioner/cp_provisioner.go | 59 ++++++-- .../provisioner/cp_provisioner_test.go | 140 ++++++++++++++++-- 2 files changed, 169 insertions(+), 30 deletions(-) diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index 68606fea..0c0e6c9c 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -18,11 +18,12 @@ import ( // // Auto-activated when MOLECULE_ORG_ID is set (SaaS tenant). type CPProvisioner struct { - baseURL string - orgID string - sharedSecret string // Authorization: Bearer — platform-wide gate - adminToken string // X-Molecule-Admin-Token — per-tenant identity (controlplane #118/#130) - httpClient *http.Client + baseURL string + orgID string + sharedSecret string // Authorization: Bearer — gates /cp/workspaces/* (provision routes) + adminToken string // X-Molecule-Admin-Token — per-tenant identity (controlplane #118/#130) + cpAdminAPIKey string // Authorization: Bearer — gates /cp/admin/* (read-only ops routes; distinct secret from sharedSecret) + httpClient *http.Client } // NewCPProvisioner creates a provisioner that delegates to the control plane. @@ -58,17 +59,26 @@ func NewCPProvisioner() (*CPProvisioner, error) { // bootstrap path). Without it, post-#118 CP rejects every // /cp/workspaces/* call with 401. adminToken := os.Getenv("ADMIN_TOKEN") + // CP_ADMIN_API_TOKEN gates /cp/admin/* (distinct from the provision + // shared secret so a compromised tenant's provision creds can't read + // other tenants' serial console). Falls back to sharedSecret only for + // dev / legacy self-hosted deployments that don't split the two. + cpAdminAPIKey := os.Getenv("CP_ADMIN_API_TOKEN") + if cpAdminAPIKey == "" { + cpAdminAPIKey = sharedSecret + } return &CPProvisioner{ - baseURL: baseURL, - orgID: orgID, - sharedSecret: sharedSecret, - adminToken: adminToken, - httpClient: &http.Client{Timeout: 120 * time.Second}, + baseURL: baseURL, + orgID: orgID, + sharedSecret: sharedSecret, + adminToken: adminToken, + cpAdminAPIKey: cpAdminAPIKey, + httpClient: &http.Client{Timeout: 120 * time.Second}, }, nil } -// authHeaders sets both auth headers on the outbound request: +// provisionAuthHeaders sets the auth headers for /cp/workspaces/* routes: // - Authorization: Bearer — platform gate // - X-Molecule-Admin-Token: — identity gate // @@ -76,7 +86,7 @@ func NewCPProvisioner() (*CPProvisioner, error) { // deployments without a real CP still work (those don't hit a CP that // enforces either gate). In prod both are set by the controlplane // bootstrap, so both headers land on every outbound call. -func (p *CPProvisioner) authHeaders(req *http.Request) { +func (p *CPProvisioner) provisionAuthHeaders(req *http.Request) { if p.sharedSecret != "" { req.Header.Set("Authorization", "Bearer "+p.sharedSecret) } @@ -85,6 +95,23 @@ func (p *CPProvisioner) authHeaders(req *http.Request) { } } +// adminAuthHeaders sets the auth header for /cp/admin/* routes. The CP +// gates this route family with CP_ADMIN_API_TOKEN — a distinct secret +// from the provision-route shared secret so a compromised tenant can't +// read other tenants' serial console via /cp/admin/workspaces/:id/console. +// +// The per-tenant X-Molecule-Admin-Token is still included for parity +// with the provision path (CP may cross-check it for audit attribution +// even on admin calls). +func (p *CPProvisioner) adminAuthHeaders(req *http.Request) { + if p.cpAdminAPIKey != "" { + req.Header.Set("Authorization", "Bearer "+p.cpAdminAPIKey) + } + if p.adminToken != "" { + req.Header.Set("X-Molecule-Admin-Token", p.adminToken) + } +} + type cpProvisionRequest struct { OrgID string `json:"org_id"` WorkspaceID string `json:"workspace_id"` @@ -123,7 +150,7 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, return "", fmt.Errorf("cp provisioner: create request: %w", err) } httpReq.Header.Set("Content-Type", "application/json") - p.authHeaders(httpReq) + p.provisionAuthHeaders(httpReq) resp, err := p.httpClient.Do(httpReq) if err != nil { @@ -158,7 +185,7 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, func (p *CPProvisioner) Stop(ctx context.Context, workspaceID string) error { url := fmt.Sprintf("%s/cp/workspaces/%s?instance_id=%s", p.baseURL, workspaceID, workspaceID) req, _ := http.NewRequestWithContext(ctx, "DELETE", url, nil) - p.authHeaders(req) + p.provisionAuthHeaders(req) resp, err := p.httpClient.Do(req) if err != nil { return fmt.Errorf("cp provisioner: stop: %w", err) @@ -194,7 +221,7 @@ func (p *CPProvisioner) Stop(ctx context.Context, workspaceID string) error { func (p *CPProvisioner) IsRunning(ctx context.Context, workspaceID string) (bool, error) { url := fmt.Sprintf("%s/cp/workspaces/%s/status?instance_id=%s", p.baseURL, workspaceID, workspaceID) req, _ := http.NewRequestWithContext(ctx, "GET", url, nil) - p.authHeaders(req) + p.provisionAuthHeaders(req) resp, err := p.httpClient.Do(req) if err != nil { return true, fmt.Errorf("cp provisioner: status: %w", err) @@ -226,7 +253,7 @@ func (p *CPProvisioner) IsRunning(ctx context.Context, workspaceID string) (bool func (p *CPProvisioner) GetConsoleOutput(ctx context.Context, workspaceID string) (string, error) { url := fmt.Sprintf("%s/cp/admin/workspaces/%s/console", p.baseURL, workspaceID) req, _ := http.NewRequestWithContext(ctx, "GET", url, nil) - p.authHeaders(req) + p.adminAuthHeaders(req) resp, err := p.httpClient.Do(req) if err != nil { return "", fmt.Errorf("cp provisioner: console: %w", err) diff --git a/workspace-server/internal/provisioner/cp_provisioner_test.go b/workspace-server/internal/provisioner/cp_provisioner_test.go index 162e8717..247863e3 100644 --- a/workspace-server/internal/provisioner/cp_provisioner_test.go +++ b/workspace-server/internal/provisioner/cp_provisioner_test.go @@ -40,13 +40,13 @@ func TestNewCPProvisioner_FallsBackToProvisionSharedSecret(t *testing.T) { } } -// TestAuthHeaders_NoopWhenBothEmpty — the self-hosted path that -// doesn't gate /cp/workspaces/* must not add stray auth headers +// TestProvisionAuthHeaders_NoopWhenBothEmpty — the self-hosted path +// that doesn't gate /cp/workspaces/* must not add stray auth headers // (bearer-like content would surprise non-bearer intermediaries). -func TestAuthHeaders_NoopWhenBothEmpty(t *testing.T) { +func TestProvisionAuthHeaders_NoopWhenBothEmpty(t *testing.T) { p := &CPProvisioner{sharedSecret: "", adminToken: ""} req := httptest.NewRequest("GET", "http://x/", nil) - p.authHeaders(req) + p.provisionAuthHeaders(req) if got := req.Header.Get("Authorization"); got != "" { t.Errorf("Authorization set to %q with empty secret; want unset", got) } @@ -55,13 +55,13 @@ func TestAuthHeaders_NoopWhenBothEmpty(t *testing.T) { } } -// TestAuthHeaders_SetsBothWhenBothProvided — happy path for SaaS -// tenants. Both the platform-wide shared secret and the per-tenant +// TestProvisionAuthHeaders_SetsBothWhenBothProvided — happy path for +// SaaS tenants. Both the platform-wide shared secret and the per-tenant // admin_token land on every outbound call. -func TestAuthHeaders_SetsBothWhenBothProvided(t *testing.T) { +func TestProvisionAuthHeaders_SetsBothWhenBothProvided(t *testing.T) { p := &CPProvisioner{sharedSecret: "the-secret", adminToken: "tok-abc"} req := httptest.NewRequest("GET", "http://x/", nil) - p.authHeaders(req) + p.provisionAuthHeaders(req) if got := req.Header.Get("Authorization"); got != "Bearer the-secret" { t.Errorf("Authorization = %q, want %q", got, "Bearer the-secret") } @@ -70,14 +70,14 @@ func TestAuthHeaders_SetsBothWhenBothProvided(t *testing.T) { } } -// TestAuthHeaders_OnlyAdminTokenWhenSecretEmpty — in the transition -// window where the tenant has admin_token but PROVISION_SHARED_SECRET -// isn't set, still send the admin token. CP middleware decides whether -// the shared secret is required. -func TestAuthHeaders_OnlyAdminTokenWhenSecretEmpty(t *testing.T) { +// TestProvisionAuthHeaders_OnlyAdminTokenWhenSecretEmpty — in the +// transition window where the tenant has admin_token but +// PROVISION_SHARED_SECRET isn't set, still send the admin token. CP +// middleware decides whether the shared secret is required. +func TestProvisionAuthHeaders_OnlyAdminTokenWhenSecretEmpty(t *testing.T) { p := &CPProvisioner{sharedSecret: "", adminToken: "tok-abc"} req := httptest.NewRequest("GET", "http://x/", nil) - p.authHeaders(req) + p.provisionAuthHeaders(req) if got := req.Header.Get("Authorization"); got != "" { t.Errorf("Authorization = %q, want unset", got) } @@ -86,6 +86,75 @@ func TestAuthHeaders_OnlyAdminTokenWhenSecretEmpty(t *testing.T) { } } +// TestAdminAuthHeaders_UsesCPAdminAPIKeyNotSharedSecret — /cp/admin/* +// routes are gated by CP_ADMIN_API_TOKEN on the CP side (distinct from +// PROVISION_SHARED_SECRET). The tenant must send the admin key as the +// bearer on these routes or CP returns 401. +func TestAdminAuthHeaders_UsesCPAdminAPIKeyNotSharedSecret(t *testing.T) { + p := &CPProvisioner{ + sharedSecret: "provision-secret", + adminToken: "tok-abc", + cpAdminAPIKey: "admin-api-key", + } + req := httptest.NewRequest("GET", "http://x/", nil) + p.adminAuthHeaders(req) + if got := req.Header.Get("Authorization"); got != "Bearer admin-api-key" { + t.Errorf("Authorization = %q, want %q", got, "Bearer admin-api-key") + } + if got := req.Header.Get("X-Molecule-Admin-Token"); got != "tok-abc" { + t.Errorf("X-Molecule-Admin-Token = %q, want tok-abc", got) + } +} + +// TestAdminAuthHeaders_FallsBackToSharedSecretWhenAdminKeyUnset — +// self-hosted and dev deployments set PROVISION_SHARED_SECRET but not +// CP_ADMIN_API_TOKEN. Fall back so single-secret setups keep working +// (CP in those deployments either accepts both bearers or doesn't gate +// /cp/admin/*). +func TestAdminAuthHeaders_FallsBackToSharedSecretWhenAdminKeyUnset(t *testing.T) { + p := &CPProvisioner{ + sharedSecret: "provision-secret", + adminToken: "tok-abc", + cpAdminAPIKey: "provision-secret", // NewCPProvisioner sets this when env is unset + } + req := httptest.NewRequest("GET", "http://x/", nil) + p.adminAuthHeaders(req) + if got := req.Header.Get("Authorization"); got != "Bearer provision-secret" { + t.Errorf("Authorization = %q, want fallback %q", got, "Bearer provision-secret") + } +} + +// TestNewCPProvisioner_ReadsCPAdminAPIToken — env-to-field wiring. +// When CP_ADMIN_API_TOKEN is set, cpAdminAPIKey picks it up. +func TestNewCPProvisioner_ReadsCPAdminAPIToken(t *testing.T) { + t.Setenv("MOLECULE_ORG_ID", "org-abc") + t.Setenv("MOLECULE_CP_SHARED_SECRET", "shared") + t.Setenv("CP_ADMIN_API_TOKEN", "admin-key") + p, err := NewCPProvisioner() + if err != nil { + t.Fatalf("NewCPProvisioner: %v", err) + } + if p.cpAdminAPIKey != "admin-key" { + t.Errorf("cpAdminAPIKey = %q, want %q", p.cpAdminAPIKey, "admin-key") + } +} + +// TestNewCPProvisioner_CPAdminAPITokenFallsBackToSharedSecret — +// operators that don't split the two secrets (dev / self-hosted) still +// get a working admin bearer via the fallback. +func TestNewCPProvisioner_CPAdminAPITokenFallsBackToSharedSecret(t *testing.T) { + t.Setenv("MOLECULE_ORG_ID", "org-abc") + t.Setenv("MOLECULE_CP_SHARED_SECRET", "shared") + t.Setenv("CP_ADMIN_API_TOKEN", "") + p, err := NewCPProvisioner() + if err != nil { + t.Fatalf("NewCPProvisioner: %v", err) + } + if p.cpAdminAPIKey != "shared" { + t.Errorf("cpAdminAPIKey fallback = %q, want %q", p.cpAdminAPIKey, "shared") + } +} + // TestStart_HappyPath — Start posts to the stubbed CP, passes the // bearer, and parses the returned instance_id. func TestStart_HappyPath(t *testing.T) { @@ -516,3 +585,46 @@ func TestClose_Noop(t *testing.T) { t.Errorf("Close should return nil, got %v", err) } } + +// TestGetConsoleOutput_UsesAdminBearer — regression guard for the +// split-bearer fix. /cp/admin/workspaces/:id/console must send +// Authorization: Bearer , NOT . +// Previously the tenant sent sharedSecret → CP 401 → tenant 502 on +// the "View Logs" UI. Symptom log: "cp provisioner: console: unexpected 401" +// on hongmingwang prod tenant, 2026-04-22. +func TestGetConsoleOutput_UsesAdminBearer(t *testing.T) { + var sawBearer, sawMethod, sawPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawBearer = r.Header.Get("Authorization") + sawMethod = r.Method + sawPath = r.URL.Path + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{"output":"boot log"}`) + })) + defer srv.Close() + + p := &CPProvisioner{ + baseURL: srv.URL, + orgID: "org-1", + sharedSecret: "provision-secret-do-not-use-here", + adminToken: "tok-xyz", + cpAdminAPIKey: "admin-api-key", + httpClient: srv.Client(), + } + out, err := p.GetConsoleOutput(context.Background(), "ws-1") + if err != nil { + t.Fatalf("GetConsoleOutput: %v", err) + } + if out != "boot log" { + t.Errorf("output = %q, want %q", out, "boot log") + } + if sawMethod != "GET" { + t.Errorf("method = %q, want GET", sawMethod) + } + if sawPath != "/cp/admin/workspaces/ws-1/console" { + t.Errorf("path = %q, want /cp/admin/workspaces/ws-1/console", sawPath) + } + if sawBearer != "Bearer admin-api-key" { + t.Errorf("bearer = %q, want Bearer admin-api-key (NOT the provision secret)", sawBearer) + } +} From 72524284d318f9e8233c86d1ff78c812c291d5f9 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Wed, 22 Apr 2026 17:25:39 -0700 Subject: [PATCH 2/2] ci: retrigger after retarget to main