From a93bd58b598630f77c8788da920ee3c70c004550 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 23 Apr 2026 13:03:35 -0700 Subject: [PATCH] fix(quickstart): keep Canvas working post first workspace + hide SaaS cookie banner on localhost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit on this branch. Two additional fresh-clone regressions surfaced during end-to-end verification, both affecting local dev only and both landing inside the same SaaS-vs-local-dev seam: ### 1. Canvas 401-loops after first workspace creation `GET /workspaces` is behind `AdminAuth` (router.go:121 — "C1: unauthenticated workspace topology exposure"). The middleware has a Tier-1 fail-open branch that only fires when *no* workspace tokens exist anywhere in the DB. The moment a user creates their first workspace — via either the Canvas UI, the API, or the e2e-api test suite — a token lands in the DB, Tier-1 closes, and the Canvas (which has no bearer token in local dev: no WorkOS session, no NEXT_PUBLIC_ADMIN_TOKEN baked in at build time) gets 401 on every list call. The UI renders a stuck "API GET /workspaces: 401 admin auth required" placeholder forever. SaaS is unaffected because hosted provisioning always sets both `ADMIN_TOKEN` and `MOLECULE_ENV=production`, and the Canvas there either carries a WorkOS session cookie or `NEXT_PUBLIC_ADMIN_TOKEN` baked into the JS bundle. **Fix** (`workspace-server/internal/middleware/wsauth_middleware.go`): add a narrow Tier-1b escape hatch that stays fail-open when *both* `ADMIN_TOKEN` is unset *and* `MOLECULE_ENV` is explicitly a dev mode ("development" / "dev"). Production never hits it (SaaS sets `MOLECULE_ENV=production`). Mirrors the existing convention in `handlers/admin_test_token.go` which gates the e2e test-token endpoint on `MOLECULE_ENV != "production"`. Three new regression tests in `wsauth_middleware_test.go`: - `TestAdminAuth_DevModeEscapeHatch_FailsOpenWithHasLiveTokens` — the happy path (dev mode, no admin token, tokens exist → 200) - `TestAdminAuth_DevModeEscapeHatch_IgnoredWhenAdminTokenSet` — explicit `ADMIN_TOKEN` wins; dev mode does not silently re-open the gate - `TestAdminAuth_DevModeEscapeHatch_IgnoredInProduction` — the SaaS-safety guarantee (production + no admin token + tokens exist → 401) `.env.example` flipped to set `MOLECULE_ENV=development` by default so new users get the dev-mode hatch automatically via `cp .env.example .env`. SaaS provisioning overrides to `production`, consistent with the existing convention used by the secrets-encryption strict-init path. ### 2. SaaS cookie/privacy banner rendered on localhost `CookieConsent` mounted unconditionally in the root layout, so `npm run dev` on localhost showed a "Cookies & your privacy" banner pointing at `moleculesai.app/legal/privacy`. That banner is a GDPR/ePrivacy compliance UI that only applies to the hosted SaaS offering; self-hosted / local-dev / Vercel-preview hosts must not see it. **Fix** (`canvas/src/components/CookieConsent.tsx`): gate render on `isSaaSTenant()`. Matches the convention used by `AuthGate` and the workspace tier picker elsewhere in the codebase. Tests (`canvas/src/components/__tests__/CookieConsent.test.tsx`): existing tests now stub `window.location.hostname` to a SaaS subdomain before rendering (required since `isSaaSTenant()` on jsdom's default "localhost" would suppress the banner). Added two new tests for the local-dev hide path: - `does NOT render on local dev (non-SaaS hostname)` - `does NOT render on a LAN hostname (192.168.*, *.local)` ### Verification On a fresh-nuked DB with the updated branch: 1. `bash infra/scripts/setup.sh` — clean 2. `go run ./cmd/server` — "Applied 41 migrations", :8080 healthy, dev-mode hatch armed (`MOLECULE_ENV=development`) 3. `npm run dev` in canvas — :3000 renders, no cookie banner 4. `bash tests/e2e/test_api.sh` — **61 passed, 0 failed** (test suite creates tokens; GET /workspaces stays 200 under the hatch) 5. Browser at http://localhost:3000 AFTER the e2e run: - Canvas renders the workspace list (no 401 placeholder) - No cookie banner 6. `npx vitest run` — **902 tests passed** (900 prior + 2 new hide tests) 7. `go test -race ./internal/middleware/` — all passing (3 new dev-mode tests + existing Issue-180 / Issue-120 / Issue-684 suite), coverage 81.8% ### SaaS parity audit Same principle as the rest of this branch: local must work without weakening SaaS. - Dev-mode hatch: conditional on `MOLECULE_ENV=development`. Production tenants always run `MOLECULE_ENV=production` (already enforced by the secrets-encryption `InitStrict` path in `internal/crypto/aes.go`). Branch is unreachable there. - Cookie banner: gated on `isSaaSTenant()` which checks `NEXT_PUBLIC_SAAS_HOST_SUFFIX` (default `.moleculesai.app`). SaaS hosts still get the banner; every other host doesn't. No change to SaaS behaviour. #1822 backend-parity tracker untouched. Co-Authored-By: Claude Opus 4.7 (1M context) --- .env.example | 2 +- canvas/src/components/CookieConsent.tsx | 12 ++ .../__tests__/CookieConsent.test.tsx | 45 +++++++- .../internal/middleware/wsauth_middleware.go | 20 ++++ .../middleware/wsauth_middleware_test.go | 108 ++++++++++++++++++ 5 files changed, 184 insertions(+), 3 deletions(-) diff --git a/.env.example b/.env.example index 3888db48..32fac03a 100644 --- a/.env.example +++ b/.env.example @@ -34,7 +34,7 @@ PLUGINS_DIR= # Path to plugins/ directory (default: /plugins i # MOLECULE_MCP_ALLOW_SEND_MESSAGE= # Set to "true" to include send_message_to_user in the MCP bridge tool list (issue #810). Excluded by default to prevent unintended WebSocket pushes from CLI sessions. # MOLECULE_MCP_URL=http://localhost:8080 # Platform URL for opencode MCP config (opencode.json). Same as PLATFORM_URL; separate var so opencode configs can reference it without ambiguity. # WORKSPACE_DIR= # Optional global host path bind-mounted to /workspace in every container. Per-workspace workspace_dir column overrides this; if neither is set each workspace gets an isolated Docker named volume. -# MOLECULE_ENV=development # Environment label (development/staging/production). Used for log tagging and conditional behaviour. +MOLECULE_ENV=development # Environment label (development/staging/production). Used for log tagging and for the AdminAuth dev-mode escape hatch (lets the Canvas dashboard keep working after the first workspace is created, when ADMIN_TOKEN is unset). SaaS deployments MUST set MOLECULE_ENV=production. # MOLECULE_ENABLE_TEST_TOKENS= # Set to 1 to expose GET /admin/workspaces/:id/test-token (mints a fresh bearer token for E2E scripts). The route is auto-enabled when MOLECULE_ENV != production; this flag is the explicit override. Leave unset/0 in prod — the route 404s unless enabled. # MOLECULE_ORG_ID= # SaaS only: org UUID set by control plane on tenant machines. When set, workspace provisioning auto-routes through the control plane API instead of Docker. # CP_PROVISION_URL= # Override control plane URL for workspace provisioning (default: https://api.moleculesai.app). Only needed for testing against a non-production control plane. diff --git a/canvas/src/components/CookieConsent.tsx b/canvas/src/components/CookieConsent.tsx index 5ea0dc57..2f04df39 100644 --- a/canvas/src/components/CookieConsent.tsx +++ b/canvas/src/components/CookieConsent.tsx @@ -1,6 +1,7 @@ "use client"; import { useEffect, useState } from "react"; +import { isSaaSTenant } from "@/lib/tenant"; const STORAGE_KEY = "molecule_cookie_consent"; @@ -74,7 +75,18 @@ export function CookieConsent() { // Read persisted decision on mount. useState's initialState can't run // on first render because localStorage is SSR-unsafe — defer to // useEffect so the initial HTML is identical to the server snapshot. + // + // The banner is SaaS-only: it carries a link to the hosted + // privacy policy (moleculesai.app/legal/privacy) and presumes + // GDPR/ePrivacy obligations that only apply to the hosted offering. + // Self-hosted / local-dev / Vercel-preview hosts get no banner — + // matches the `isSaaSTenant()` convention used by AuthGate and + // the tier picker. useEffect(() => { + if (!isSaaSTenant()) { + setVisible(false); + return; + } setVisible(getStoredConsent() === null); }, []); diff --git a/canvas/src/components/__tests__/CookieConsent.test.tsx b/canvas/src/components/__tests__/CookieConsent.test.tsx index 36314858..188c6f9c 100644 --- a/canvas/src/components/__tests__/CookieConsent.test.tsx +++ b/canvas/src/components/__tests__/CookieConsent.test.tsx @@ -6,11 +6,30 @@ import { CookieConsent, hasConsent } from "../CookieConsent"; const STORAGE_KEY = "molecule_cookie_consent"; // These tests lock the privacy-preserving default: the banner appears on -// first visit, clicking either button records a decision, and subsequent -// renders skip the banner until the policy version changes. +// first visit (SaaS mode), clicking either button records a decision, and +// subsequent renders skip the banner until the policy version changes. +// +// The banner is SaaS-only — it references moleculesai.app's hosted privacy +// policy and presumes GDPR/ePrivacy obligations that only apply to the +// hosted offering. Self-hosted / local-dev hosts must not see it. Most +// tests below simulate SaaS by overriding window.location.hostname; the +// "local-dev" test omits that override. + +// setSaaSHostname rewrites window.location.hostname to look like a SaaS +// tenant subdomain so isSaaSTenant() returns true. Must run before +// CookieConsent mounts, otherwise its one-shot useEffect captures the +// localhost default. jsdom's location object is read-only via the normal +// setter but defineProperty lets us replace it for the scope of a test. +function setSaaSHostname(host = "acme.moleculesai.app") { + Object.defineProperty(window, "location", { + configurable: true, + value: { ...window.location, hostname: host }, + }); +} beforeEach(() => { window.localStorage.clear(); + setSaaSHostname(); }); afterEach(() => { @@ -86,6 +105,28 @@ describe("CookieConsent", () => { expect(dialog.getAttribute("aria-labelledby")).toBe("cookie-consent-title"); expect(dialog.getAttribute("aria-describedby")).toBe("cookie-consent-body"); }); + + it("does NOT render on local dev (non-SaaS hostname)", () => { + // Simulate `npm run dev` on localhost — isSaaSTenant() returns false + // and the banner must stay hidden. Regression test for PR #1871: + // a fresh-clone Canvas showing the hosted privacy banner on + // localhost:3000 was confusing for self-hosted users. + Object.defineProperty(window, "location", { + configurable: true, + value: { ...window.location, hostname: "localhost" }, + }); + render(); + expect(screen.queryByRole("dialog")).toBeNull(); + }); + + it("does NOT render on a LAN hostname (192.168.*, *.local)", () => { + Object.defineProperty(window, "location", { + configurable: true, + value: { ...window.location, hostname: "192.168.1.74" }, + }); + render(); + expect(screen.queryByRole("dialog")).toBeNull(); + }); }); describe("hasConsent", () => { diff --git a/workspace-server/internal/middleware/wsauth_middleware.go b/workspace-server/internal/middleware/wsauth_middleware.go index 9e330e99..50535bad 100644 --- a/workspace-server/internal/middleware/wsauth_middleware.go +++ b/workspace-server/internal/middleware/wsauth_middleware.go @@ -148,6 +148,26 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc { } } + // Tier 1b: Local-dev escape hatch. On `go run ./cmd/server` the + // Canvas has no bearer token (there's no WorkOS session, no + // baked NEXT_PUBLIC_ADMIN_TOKEN), so the moment the first + // workspace token lands in the DB Tier 1 closes and Canvas → 401 + // on every GET /workspaces. This reopens fail-open *only* when + // - ADMIN_TOKEN is empty (i.e. the operator has not opted in + // to the Phase-30 closure), AND + // - MOLECULE_ENV is explicitly a dev mode. + // SaaS never hits this branch because tenant provisioning sets + // both ADMIN_TOKEN and MOLECULE_ENV=production. Matches the + // existing convention in handlers/admin_test_token.go which + // gates the test-token endpoint on MOLECULE_ENV != "production". + if adminSecret == "" { + env := strings.ToLower(strings.TrimSpace(os.Getenv("MOLECULE_ENV"))) + if env == "development" || env == "dev" { + c.Next() + return + } + } + // SaaS-canvas path: when the request carries a WorkOS session // cookie AND the CP confirms it's valid, accept without a // bearer. This is how the tenant's Next.js canvas UI diff --git a/workspace-server/internal/middleware/wsauth_middleware_test.go b/workspace-server/internal/middleware/wsauth_middleware_test.go index 020eabfd..b796dc75 100644 --- a/workspace-server/internal/middleware/wsauth_middleware_test.go +++ b/workspace-server/internal/middleware/wsauth_middleware_test.go @@ -735,6 +735,114 @@ func TestAdminAuth_Issue180_ApprovalsListing_FailOpen_NoTokens(t *testing.T) { } } +// TestAdminAuth_DevModeEscapeHatch_FailsOpenWithHasLiveTokens documents the +// Tier-1b dev-mode escape hatch. When the platform runs with MOLECULE_ENV=development +// and ADMIN_TOKEN is unset, AdminAuth must stay fail-open even after workspace +// tokens land in the DB. This keeps the Canvas dashboard usable in local dev +// after the first workspace is created (PR #1871 — quickstart bugless). +// +// SaaS never hits this path because tenant provisioning sets both +// ADMIN_TOKEN and MOLECULE_ENV=production. +func TestAdminAuth_DevModeEscapeHatch_FailsOpenWithHasLiveTokens(t *testing.T) { + t.Setenv("MOLECULE_ENV", "development") + t.Setenv("ADMIN_TOKEN", "") + + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer mockDB.Close() + + // HasAnyLiveTokenGlobal returns 1 — tokens exist (post first-workspace). + // The Tier-1 fail-open branch WOULD close here. Tier-1b must still open. + mock.ExpectQuery(hasAnyLiveTokenGlobalQuery). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + r := gin.New() + r.GET("/workspaces", AdminAuth(mockDB), func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"workspaces": []interface{}{}}) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/workspaces", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("dev-mode escape hatch: expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAdminAuth_DevModeEscapeHatch_IgnoredWhenAdminTokenSet verifies that the +// dev-mode escape hatch does NOT override an operator who has set ADMIN_TOKEN. +// Setting ADMIN_TOKEN is the explicit opt-in to #684 closure; dev-mode must not +// silently reopen the gate. +func TestAdminAuth_DevModeEscapeHatch_IgnoredWhenAdminTokenSet(t *testing.T) { + t.Setenv("MOLECULE_ENV", "development") + t.Setenv("ADMIN_TOKEN", "operator-explicitly-set-this") + + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer mockDB.Close() + + // Tokens exist — Tier 1 closes. + mock.ExpectQuery(hasAnyLiveTokenGlobalQuery). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + r := gin.New() + r.GET("/workspaces", AdminAuth(mockDB), func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"workspaces": []interface{}{}}) + }) + + w := httptest.NewRecorder() + // No bearer token — must 401 even in dev mode because ADMIN_TOKEN is set. + req, _ := http.NewRequest(http.MethodGet, "/workspaces", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("dev-mode + ADMIN_TOKEN set: expected 401, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAdminAuth_DevModeEscapeHatch_IgnoredInProduction verifies the hatch never +// fires when MOLECULE_ENV=production. This is the SaaS-safety guarantee. +func TestAdminAuth_DevModeEscapeHatch_IgnoredInProduction(t *testing.T) { + t.Setenv("MOLECULE_ENV", "production") + t.Setenv("ADMIN_TOKEN", "") + + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer mockDB.Close() + + mock.ExpectQuery(hasAnyLiveTokenGlobalQuery). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + r := gin.New() + r.GET("/workspaces", AdminAuth(mockDB), func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"workspaces": []interface{}{}}) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/workspaces", nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("production mode: expected 401, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + // TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401 documents the #120 // attack vector and verifies that AdminAuth returns 401 for PATCH without a token. func TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401(t *testing.T) {