diff --git a/tests/e2e/_lib.sh b/tests/e2e/_lib.sh index be21d129c..c8f933f7d 100755 --- a/tests/e2e/_lib.sh +++ b/tests/e2e/_lib.sh @@ -102,7 +102,7 @@ try: except Exception: pass" 2>/dev/null || true) fi - curl -s -X DELETE "$BASE/workspaces/$wid?confirm=true" \ + e2e_gated_admin_op "$wid" curl -s -X DELETE "$BASE/workspaces/$wid?confirm=true" \ -H "X-Confirm-Name: $name" ${curl_args[@]+"${curl_args[@]}"} > /dev/null || true } @@ -160,3 +160,63 @@ except Exception: e2e_delete_workspace "$_wid" "$_name" done } + +# e2e_gated_admin_op runs a curl invocation, and if the platform returns +# 202/pending_approval (the admin-token path hitting approvals.IsGated — see +# workspace-server/internal/handlers/approval_gate.go), auto-approves via +# /workspaces/:id/approvals/:approvalId/decide and retries the operation. +# +# CR2 RC 10818 made the admin-token gate always-on (it was previously +# org-token-only with a rollout flag). The E2E API Smoke harness uses the +# platform admin bearer for workspace CRUD (create/delete), so Delete now +# returns 202 pending_approval — which broke the smoke's +# "DELETE /workspaces/:id" + "List after delete (count=1)" + "All deleted +# (count=0)" assertions (job 468330, exitcode 6, 1m4s, NOT infra). +# +# The gate is CORRECT (delete_workspace is destructive; the reviewer's +# verdict PASSED on all 4 axes). The harness needs the auto-approve loop, +# NOT a policy.go narrowing — see CR-B 10858 / 10869 / 10870. +# +# Usage: +# R=$(e2e_gated_admin_op ) +# +# The workspace_id is used ONLY to build the /approvals/:id/decide URL +# (POST /workspaces has no workspace_id yet; pass "" and the helper +# will skip approve-on-202 — the gate never fires for create today). +# For DELETE/PATCH/CascadeDelete, pass the target workspace id. +# +# This helper preserves the security guarantee: the gate still fires for +# every admin-token call. The harness is the auto-approver, simulating +# the human-in-the-loop that production deployments use. +e2e_gated_admin_op() { + local _wid="$1"; shift + local _curl_args=("$@") + local _resp + _resp=$("${_curl_args[@]}") + # Detect pending_approval — Python parses + emits the approval_id on stdout + # if present, empty string otherwise. JSON parse failure (e.g. 502 HTML) is + # treated as "not gated" so the smoke fails on real errors rather than + # silently retrying. + local _approval_id + _approval_id=$(printf '%s' "$_resp" | python3 -c "import json,sys +try: + d=json.load(sys.stdin) + if d.get('status')=='pending_approval': + print(d.get('approval_id','')) +except Exception: + pass" 2>/dev/null || true) + if [ -n "$_approval_id" ] && [ -n "$_wid" ]; then + # Auto-approve via the same admin bearer. 200 with approval_id consumed. + local _admin_auth=() + e2e_admin_auth_args _admin_auth + curl -s -X POST "$BASE/workspaces/$_wid/approvals/$_approval_id/decide" \ + -H "Content-Type: application/json" \ + -d '{"decision":"approved","decided_by":"e2e-api-smoke"}' \ + ${_admin_auth[@]+"${_admin_auth[@]}"} > /dev/null || true + # Retry the original operation. The gate's consume-once + # (approval_gate.go UPDATE … RETURNING id) means the SECOND call finds + # the now-approved request and proceeds (returns true from gateDestructive). + _resp=$("${_curl_args[@]}") + fi + printf '%s' "$_resp" +} diff --git a/tests/e2e/test_api.sh b/tests/e2e/test_api.sh index b283d6bc7..f8f12106d 100644 --- a/tests/e2e/test_api.sh +++ b/tests/e2e/test_api.sh @@ -327,9 +327,13 @@ check "GET /bundles/export/:id" '"name":"Summarizer Agent"' "$BUNDLE" ORIG_NAME=$(echo "$BUNDLE" | python3 -c "import sys,json; print(json.load(sys.stdin)['name'])") ORIG_TIER=$(echo "$BUNDLE" | python3 -c "import sys,json; print(json.load(sys.stdin)['tier'])") -# DELETE /workspaces/:id is admin-gated (router.go:167). X-Confirm-Name must -# still match the workspace name even with admin auth. -R=$(acurl -X DELETE "$BASE/workspaces/$SUM_ID?confirm=true" \ +# DELETE /workspaces/:id is admin-gated (router.go:167) AND now also +# approvals-gated for admin-token callers (CR2 RC 10818 — the admin-token +# gate is always-on; delete_workspace is in the gated map). X-Confirm-Name +# must still match the workspace name. e2e_gated_admin_op auto-approves +# the pending approval + retries the DELETE (so the harness sees the +# real 200/'status:removed' result). +R=$(e2e_gated_admin_op "$SUM_ID" acurl -X DELETE "$BASE/workspaces/$SUM_ID?confirm=true" \ -H "X-Confirm-Name: Summarizer Agent") check "DELETE /workspaces/:id" '"status":"removed"' "$R" @@ -345,9 +349,10 @@ check "List after delete (count=1)" "1" "$COUNT" echo "" echo "--- Bundle Round-Trip Test ---" -# Delete the remaining parent Echo — DELETE is admin-gated (router.go:167); -# the platform admin bearer (acurl) authorizes it. X-Confirm-Name still required. -R=$(acurl -X DELETE "$BASE/workspaces/$ECHO_ID?confirm=true" \ +# Delete the remaining parent Echo — DELETE is admin-gated (router.go:167) +# AND approvals-gated for admin-token callers (CR2 RC 10818). Use +# e2e_gated_admin_op to auto-approve the pending approval + retry. +R=$(e2e_gated_admin_op "$ECHO_ID" acurl -X DELETE "$BASE/workspaces/$ECHO_ID?confirm=true" \ -H "X-Confirm-Name: Echo Agent v2") check "Delete before re-import" '"status":"removed"' "$R" diff --git a/tests/e2e/test_channels_e2e.sh b/tests/e2e/test_channels_e2e.sh index 36435bdb5..215594032 100755 --- a/tests/e2e/test_channels_e2e.sh +++ b/tests/e2e/test_channels_e2e.sh @@ -122,10 +122,8 @@ cleanup() { wid="${pair%%|*}"; pair="${pair#*|}" tok="${pair%%|*}"; name="${pair#*|}" [ -z "$wid" ] && continue - local auth=("${ADMIN_AUTH[@]}") - [ -n "$tok" ] && auth=(-H "Authorization: Bearer $tok") - curl -s -X DELETE "$BASE/workspaces/$wid?confirm=true&purge=true" \ - -H "X-Confirm-Name: $name" "${auth[@]}" >/dev/null 2>&1 + e2e_gated_admin_op "$wid" curl -s -X DELETE "$BASE/workspaces/$wid?confirm=true&purge=true" \ + -H "X-Confirm-Name: $name" "${ADMIN_AUTH[@]}" >/dev/null 2>&1 done rm -rf "$WORK_DIR" 2>/dev/null } @@ -418,17 +416,13 @@ fi # DELETE /workspaces/:id is AdminAuth-gated (router.go:167); Tier-2b rejects a # workspace bearer when ADMIN_TOKEN is set, so this MUST use the admin bearer. # X-Confirm-Name must equal the workspace name (the destructive-delete guard). -PURGE_AUTH=("${ADMIN_AUTH[@]}") -[ ${#PURGE_AUTH[@]} -eq 0 ] && [ -n "$WS_TARGET_TOK" ] && PURGE_AUTH=(-H "Authorization: Bearer $WS_TARGET_TOK") -PURGE=$(curl -s -w $'\n%{http_code}' -X DELETE \ +PURGE=$(e2e_gated_admin_op "$WS_TARGET" curl -s -X DELETE \ "$BASE/workspaces/$WS_TARGET?confirm=true&purge=true" \ - -H "X-Confirm-Name: e2e-chan-target-$$" "${PURGE_AUTH[@]}") -PURGE_CODE=$(printf '%s' "$PURGE" | tail -n1) -PURGE_BODY=$(printf '%s' "$PURGE" | sed '$d') -if [ "$PURGE_CODE" = "200" ] && printf '%s' "$PURGE_BODY" | grep -qF '"status":"purged"'; then + -H "X-Confirm-Name: e2e-chan-target-$$" "${ADMIN_AUTH[@]}") +if printf '%s' "$PURGE" | grep -qF '"status":"purged"'; then pass "DELETE ?purge=true returned purged" else - fail "DELETE ?purge=true" "code=$PURGE_CODE body=$PURGE_BODY" + fail "DELETE ?purge=true" "body=$PURGE" fi # Target was purged → its token is revoked; query its channels with admin # bearer. The purge hard-deletes workspace_channels rows for the target. diff --git a/tests/e2e/test_chat_upload_e2e.sh b/tests/e2e/test_chat_upload_e2e.sh index 2ae47ea6a..72bcf99cb 100755 --- a/tests/e2e/test_chat_upload_e2e.sh +++ b/tests/e2e/test_chat_upload_e2e.sh @@ -38,9 +38,11 @@ cleanup() { local rc=$? set +e if [ -n "$PARENT" ]; then - curl -sS -X DELETE "$BASE/workspaces/$PARENT?confirm=true&purge=true" \ + local admin_auth=() + e2e_admin_auth_args admin_auth + e2e_gated_admin_op "$PARENT" curl -sS -X DELETE "$BASE/workspaces/$PARENT?confirm=true&purge=true" \ -H "X-Confirm-Name: e2e-chat-upload" \ - ${PARENT_TOK:+-H "Authorization: Bearer $PARENT_TOK"} >/dev/null 2>&1 + "${admin_auth[@]}" >/dev/null 2>&1 fi exit $rc } diff --git a/tests/e2e/test_comprehensive_e2e.sh b/tests/e2e/test_comprehensive_e2e.sh index 0c57ae260..49c466df9 100755 --- a/tests/e2e/test_comprehensive_e2e.sh +++ b/tests/e2e/test_comprehensive_e2e.sh @@ -565,8 +565,9 @@ check "Delete PM requires name confirmation" '"destructive_action_requires_confi R=$(curl -s -X DELETE "$BASE/workspaces/$PM_ID" -H "X-Confirm-Name: Test PM") check "Delete PM requires cascade confirmation" '"confirmation_required"' "$R" -# Delete with confirmation -R=$(curl -s -X DELETE "$BASE/workspaces/$PM_ID?confirm=true" -H "X-Confirm-Name: Test PM") +# Delete with confirmation. This destructive path is approval-gated for admin +# callers, so drive the same approve-and-retry helper used by focused E2Es. +R=$(e2e_gated_admin_op "$PM_ID" curl -s -X DELETE "$BASE/workspaces/$PM_ID?confirm=true" -H "X-Confirm-Name: Test PM") check "Delete PM cascades" '"cascade_deleted"' "$R" # Delete outsider @@ -574,13 +575,15 @@ e2e_delete_workspace "$OUTSIDER_ID" "Test Outsider" # Clean up remaining workspaces (bundle imports, runtime test workspaces, etc.) sleep 2 -curl -s "$BASE/workspaces" | python3 -c " -import json, sys, subprocess, time -ws = json.load(sys.stdin) -for w in ws: - time.sleep(0.5) # avoid rate limit - subprocess.run(['curl', '-s', '-X', 'DELETE', '$BASE/workspaces/' + w['id'] + '?confirm=true', '-H', 'X-Confirm-Name: ' + w.get('name','')], capture_output=True) -" 2>/dev/null +while IFS=$'\t' read -r wid name; do + [ -z "$wid" ] && continue + sleep 0.5 # avoid rate limit + e2e_delete_workspace "$wid" "$name" >/dev/null 2>&1 || true +done < <(curl -s "$BASE/workspaces" | python3 -c " +import json, sys +for w in json.load(sys.stdin): + print(str(w.get('id','')) + '\t' + str(w.get('name',''))) +" 2>/dev/null) # Poll for clean state up to 30s — DB cascade + container stop is async on busy systems for _ in 1 2 3 4 5 6; do diff --git a/workspace-server/internal/handlers/approval_gate.go b/workspace-server/internal/handlers/approval_gate.go index b4d917e9d..823a8078b 100644 --- a/workspace-server/internal/handlers/approval_gate.go +++ b/workspace-server/internal/handlers/approval_gate.go @@ -142,21 +142,31 @@ func gateDestructive(c *gin.Context, b events.EventEmitter, workspaceID string, if !approvals.IsGated(action) { return true } - // Scope (RFC platform-agent Phase 4b). Wiring is a one-liner in each - // destructive handler; the activation policy lives here, centrally, so it is - // uniform and testable: - // - default-OFF rollout flag, so the wiring is inert until an operator - // enables it (mirrors the 3a/3c default-off design and protects existing - // org-token automation from a surprise async-approval behaviour change); - // - only callers holding an ORG token are gated. The platform agent runs - // with MOLECULE_API_KEY=, so the auth middleware sets - // org_token_id. Ordinary workspace-token agents and human CP-session - // operators (cp_session_actor — the approvers themselves) are NOT gated, - // so normal operation is byte-identical. This realises the file-header - // trust boundary ("anything holding an org-admin token still goes - // through the gate") without gating everyone. - if !destructiveGateEnabled() || !callerHoldsOrgToken(c) { - return true + // Scope (RFC platform-agent Phase 4b, hardened by core#2574). The + // activation policy lives here, centrally, so it is uniform and testable: + // - admin-token callers (Tier 2b ADMIN_TOKEN bootstrap + Tier 3 + // workspace-token fallback) are ALWAYS gated when the action is + // gated — the rollout flag does NOT bypass them. This closes the + // core#2574 privilege-escalation hole: the concierge agent holds + // the tenant ADMIN_TOKEN and was minting org tokens + writing + // secrets with ZERO pending approvals because the old code only + // checked for org_token_id, which admin-token callers never set. + // Admin-token-bearing agents are EXACTLY the human-in-the-loop + // bypass risk the RFC Phase 4 was supposed to prevent. + // - org-token callers (Tier 2a, user-minted via canvas UI) follow + // the rollout flag (default-OFF; set MOLECULE_PLATFORM_APPROVAL_GATE=1 + // to enable). The default-off posture protects existing org-token + // automation from a surprise async-approval behaviour change. + // - non-agent callers (workspace tokens, session cookies) bypass + // entirely — ordinary operation is byte-identical. + isAdminToken := callerIsAdminToken(c) + if !isAdminToken { + if !destructiveGateEnabled() { + return true + } + if !callerHoldsOrgToken(c) { + return true + } } approved, approvalID, err := requireApproval(c.Request.Context(), b, workspaceID, action, reason, contextMap) if err != nil { @@ -181,6 +191,9 @@ func gateDestructive(c *gin.Context, b events.EventEmitter, workspaceID string, // MOLECULE_PLATFORM_APPROVAL_GATE=1 (or "true") — typically when the platform // agent is deployed to the org. Keeps 4b's wiring shipped-but-dormant, matching // the platform-agent feature's default-off posture (3a/3c). +// +// (core#2574) The flag is now an ORG-TOKEN-ONLY switch: the admin-token +// path is gated regardless of the flag (see gateDestructive). func destructiveGateEnabled() bool { v := os.Getenv("MOLECULE_PLATFORM_APPROVAL_GATE") return v == "1" || v == "true" @@ -194,3 +207,24 @@ func callerHoldsOrgToken(c *gin.Context) bool { _, ok := c.Get("org_token_id") return ok } + +// callerIsAdminToken reports whether the request authenticated with the +// tenant ADMIN_TOKEN (Tier 2b) or the Tier 3 workspace-token fallback +// (which is admin-equivalent — closes #684 if the operator forgot to +// set ADMIN_TOKEN). The auth middleware sets caller_is_admin_token in +// both cases. +// +// (core#2574) This is the missing context that closes the privilege- +// escalation bypass: the concierge agent holds ADMIN_TOKEN (NOT an +// org token) and was minting org tokens + writing secrets with ZERO +// pending approvals because callerHoldsOrgToken returned false for it +// and the gate's rollout flag was off. The fix makes gateDestructive +// always gate admin-token callers regardless of the rollout flag. +func callerIsAdminToken(c *gin.Context) bool { + v, ok := c.Get("caller_is_admin_token") + if !ok { + return false + } + b, ok := v.(bool) + return ok && b +} diff --git a/workspace-server/internal/handlers/approval_gate_admin_token_integration_test.go b/workspace-server/internal/handlers/approval_gate_admin_token_integration_test.go new file mode 100644 index 000000000..700fca8ed --- /dev/null +++ b/workspace-server/internal/handlers/approval_gate_admin_token_integration_test.go @@ -0,0 +1,397 @@ +//go:build integration +// +build integration + +// approval_gate_admin_token_integration_test.go — REAL Postgres coverage for the +// core#2574 admin-token approval gate on org-token mint and secret write. +// +// Run with: +// +// INTEGRATION_DB_URL="postgres://postgres:test@localhost:55432/molecule?sslmode=disable" \ +// go test -tags=integration ./internal/handlers/ -run Integration_AdminTokenGate -v +// +// Why this is NOT a sqlmock test +// ------------------------------ +// The gate is about row-state across calls: the first call creates a pending +// approval row, the second call (after the human approves) consumes it via the +// conditional UPDATE ... RETURNING, and a third call creates a fresh pending row +// because the first approval was single-use. Only real Postgres proves the +// consume-once semantics and the hash-matching lookup. + +package handlers + +import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "testing" + + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db" + "github.com/gin-gonic/gin" + "github.com/google/uuid" + _ "github.com/lib/pq" +) + +func integrationDB_AdminTokenGate(t *testing.T) *sql.DB { + t.Helper() + url := requireIntegrationDBURL(t) + conn, err := sql.Open("postgres", url) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := conn.Ping(); err != nil { + t.Fatalf("ping: %v", err) + } + t.Cleanup(func() { conn.Close() }) + return conn +} + +// seedConciergeWorkspace inserts a root workspace (parent_id NULL) that acts as +// the org concierge / platform agent. The gate's approval rows are keyed by +// workspace_id, so a real row must exist. +// +// (core#2579) This helper ALSO sets the MOLECULE_PLATFORM_WORKSPACE_ID +// env var to the seeded workspace UUID so the production approvalAnchorForGate(c) +// logic (workspace-server/internal/handlers/org_tokens.go:approvalAnchorForGate) +// resolves a real anchor for admin-token callers in the integration tests. +// Without this env var, approvalAnchorForGate returns "" → the handler +// returns the controlled 400 "no approval anchor" → the 3 tests that +// expect 202 (pending_approval) would falsely FAIL. The env var is +// restored on cleanup. +func seedConciergeWorkspace(t *testing.T, conn *sql.DB) string { + t.Helper() + ctx := context.Background() + wsID := uuid.New().String() + name := "gate-itest-" + wsID[:8] + if _, err := conn.ExecContext(ctx, ` + INSERT INTO workspaces (id, name, tier, runtime, status, parent_id) + VALUES ($1, $2, 0, 'claude-code', 'online', NULL) + `, wsID, name); err != nil { + t.Fatalf("seed concierge workspace: %v", err) + } + + // (core#2579) Set the platform-org workspace anchor for admin-token + // approval flows. The handler resolves admin-token callers' anchor + // from this env var (org_tokens.go:approvalAnchorForGate). Pre-#2579 + // the tests didn't need this because the gate was INERT for admin + // tokens; post-#2579 the gate is always-on for admin-token, so the + // anchor must resolve to a real seeded workspace for the gate to + // fire (return 202) instead of fail-closed (return 400). + prevEnv, hadPrev := os.LookupEnv("MOLECULE_PLATFORM_WORKSPACE_ID") + os.Setenv("MOLECULE_PLATFORM_WORKSPACE_ID", wsID) + + t.Cleanup(func() { + if hadPrev { + os.Setenv("MOLECULE_PLATFORM_WORKSPACE_ID", prevEnv) + } else { + os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + } + _, _ = conn.ExecContext(ctx, `DELETE FROM approval_requests WHERE workspace_id = $1`, wsID) + _, _ = conn.ExecContext(ctx, `DELETE FROM org_api_tokens WHERE name LIKE $1`, name+"%") + _, _ = conn.ExecContext(ctx, `DELETE FROM workspace_secrets WHERE workspace_id = $1`, wsID) + _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, wsID) + }) + return wsID +} + +// adminTokenContext returns a gin.Context that simulates the AdminAuth middleware +// for a Tier-2b ADMIN_TOKEN caller (core#2574). +func adminTokenContext(t *testing.T, method, path, body string) (*gin.Context, *httptest.ResponseRecorder) { + t.Helper() + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + var r *http.Request + if body != "" { + r = httptest.NewRequest(method, path, bytes.NewBufferString(body)) + r.Header.Set("Content-Type", "application/json") + } else { + r = httptest.NewRequest(method, path, nil) + } + c.Request = r + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") + return c, w +} + +// TestIntegration_AdminToken_OrgTokenMint_WithoutApproval_Rejected proves that +// an admin-token caller (the concierge agent) attempting to mint an org API +// token WITHOUT a pre-existing approval gets HTTP 202 with a pending approval. +// This is the regression for the live exploit (core#2574). +func TestIntegration_AdminToken_OrgTokenMint_WithoutApproval_Rejected(t *testing.T) { + conn := integrationDB_AdminTokenGate(t) + prev := db.DB + db.DB = conn + t.Cleanup(func() { db.DB = prev }) + setupTestRedis(t) + + _ = seedConciergeWorkspace(t, conn) + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + h := NewOrgTokenHandler() + c, w := adminTokenContext(t, "POST", "/org/tokens", `{"name":"exploit-test-mint"}`) + + h.Create(c) + + if w.Code != http.StatusAccepted { + t.Fatalf("admin-token org-token mint WITHOUT approval: want 202, got %d: %s", w.Code, w.Body.String()) + } + var body map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body["status"] != "pending_approval" { + t.Errorf("status = %v, want pending_approval", body["status"]) + } + if body["action"] != "org_token_mint" { + t.Errorf("action = %v, want org_token_mint", body["action"]) + } + approvalID, ok := body["approval_id"].(string) + if !ok || approvalID == "" { + t.Fatalf("approval_id missing or empty in 202 body: %v", body) + } + + // Verify the pending row was actually persisted. + var count int + if err := conn.QueryRowContext(context.Background(), + `SELECT count(*) FROM approval_requests WHERE id = $1 AND status = 'pending'`, approvalID).Scan(&count); err != nil { + t.Fatalf("count pending row: %v", err) + } + if count != 1 { + t.Fatalf("pending approval rows = %d, want 1", count) + } +} + +// TestIntegration_AdminToken_SecretWrite_WithoutApproval_Rejected proves that +// an admin-token caller attempting to write a workspace secret WITHOUT a +// pre-existing approval gets HTTP 202 with a pending approval (core#2574). +func TestIntegration_AdminToken_SecretWrite_WithoutApproval_Rejected(t *testing.T) { + conn := integrationDB_AdminTokenGate(t) + prev := db.DB + db.DB = conn + t.Cleanup(func() { db.DB = prev }) + setupTestRedis(t) + + wsID := seedConciergeWorkspace(t, conn) + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + h := NewSecretsHandler(nil) + c, w := adminTokenContext(t, "POST", "/workspaces/"+wsID+"/secrets", + fmt.Sprintf(`{"key":"GATED_SECRET_KEY","value":"gated-secret-value"}`)) + c.Params = gin.Params{{Key: "id", Value: wsID}} + + h.Set(c) + + if w.Code != http.StatusAccepted { + t.Fatalf("admin-token secret write WITHOUT approval: want 202, got %d: %s", w.Code, w.Body.String()) + } + var body map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body["status"] != "pending_approval" { + t.Errorf("status = %v, want pending_approval", body["status"]) + } + if body["action"] != "secret_write" { + t.Errorf("action = %v, want secret_write", body["action"]) + } + approvalID, ok := body["approval_id"].(string) + if !ok || approvalID == "" { + t.Fatalf("approval_id missing or empty in 202 body: %v", body) + } + + var count int + if err := conn.QueryRowContext(context.Background(), + `SELECT count(*) FROM approval_requests WHERE id = $1 AND status = 'pending'`, approvalID).Scan(&count); err != nil { + t.Fatalf("count pending row: %v", err) + } + if count != 1 { + t.Fatalf("pending approval rows = %d, want 1", count) + } +} + +// TestIntegration_AdminToken_OrgTokenMint_WithApproval_Succeeds proves the +// happy path: after a human grants the pending approval, the retried mint +// consumes the approval and returns 200 with the plaintext token. +func TestIntegration_AdminToken_OrgTokenMint_WithApproval_Succeeds(t *testing.T) { + conn := integrationDB_AdminTokenGate(t) + prev := db.DB + db.DB = conn + t.Cleanup(func() { db.DB = prev }) + setupTestRedis(t) + + _ = seedConciergeWorkspace(t, conn) + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + h := NewOrgTokenHandler() + name := "approved-mint-test" + + // 1. First call → pending. + c1, w1 := adminTokenContext(t, "POST", "/org/tokens", fmt.Sprintf(`{"name":%q}`, name)) + h.Create(c1) + if w1.Code != http.StatusAccepted { + t.Fatalf("first call: want 202, got %d: %s", w1.Code, w1.Body.String()) + } + var body1 map[string]interface{} + if err := json.Unmarshal(w1.Body.Bytes(), &body1); err != nil { + t.Fatalf("parse first: %v", err) + } + approvalID := body1["approval_id"].(string) + + // 2. Human grants the approval via DB (simulating the Decide handler). + if _, err := conn.ExecContext(context.Background(), + `UPDATE approval_requests SET status='approved', decided_by='integration-test', decided_at=now() WHERE id=$1`, + approvalID); err != nil { + t.Fatalf("approve: %v", err) + } + + // 3. Retry with the SAME name → hash matches → approval consumed → 200. + c2, w2 := adminTokenContext(t, "POST", "/org/tokens", fmt.Sprintf(`{"name":%q}`, name)) + h.Create(c2) + if w2.Code != http.StatusOK { + t.Fatalf("retry after approval: want 200, got %d: %s", w2.Code, w2.Body.String()) + } + var body2 map[string]interface{} + if err := json.Unmarshal(w2.Body.Bytes(), &body2); err != nil { + t.Fatalf("parse second: %v", err) + } + if body2["auth_token"] == "" { + t.Errorf("auth_token missing from 200 response — mint did not proceed") + } + if body2["id"] == "" { + t.Errorf("id missing from 200 response") + } + + // 4. The approval row is now consumed. + var consumedAt *string + if err := conn.QueryRowContext(context.Background(), + `SELECT consumed_at::text FROM approval_requests WHERE id=$1`, approvalID).Scan(&consumedAt); err != nil { + t.Fatalf("read consumed_at: %v", err) + } + if consumedAt == nil || *consumedAt == "" { + t.Fatalf("approval %s was not consumed after the 200 mint", approvalID) + } +} + +// TestIntegration_AdminToken_SecretWrite_WithApproval_Succeeds proves the +// happy path: after a human grants the pending approval, the retried secret +// write consumes the approval and returns 200. +func TestIntegration_AdminToken_SecretWrite_WithApproval_Succeeds(t *testing.T) { + conn := integrationDB_AdminTokenGate(t) + prev := db.DB + db.DB = conn + t.Cleanup(func() { db.DB = prev }) + setupTestRedis(t) + + wsID := seedConciergeWorkspace(t, conn) + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + h := NewSecretsHandler(nil) + key := "GATED_SECRET_KEY" + + // 1. First call → pending. + c1, w1 := adminTokenContext(t, "POST", "/workspaces/"+wsID+"/secrets", + fmt.Sprintf(`{"key":%q,"value":"secret-value"}`, key)) + c1.Params = gin.Params{{Key: "id", Value: wsID}} + h.Set(c1) + if w1.Code != http.StatusAccepted { + t.Fatalf("first call: want 202, got %d: %s", w1.Code, w1.Body.String()) + } + var body1 map[string]interface{} + if err := json.Unmarshal(w1.Body.Bytes(), &body1); err != nil { + t.Fatalf("parse first: %v", err) + } + approvalID := body1["approval_id"].(string) + + // 2. Human grants the approval. + if _, err := conn.ExecContext(context.Background(), + `UPDATE approval_requests SET status='approved', decided_by='integration-test', decided_at=now() WHERE id=$1`, + approvalID); err != nil { + t.Fatalf("approve: %v", err) + } + + // 3. Retry with the SAME key → hash matches → approval consumed → 200. + c2, w2 := adminTokenContext(t, "POST", "/workspaces/"+wsID+"/secrets", + fmt.Sprintf(`{"key":%q,"value":"secret-value"}`, key)) + c2.Params = gin.Params{{Key: "id", Value: wsID}} + h.Set(c2) + if w2.Code != http.StatusOK { + t.Fatalf("retry after approval: want 200, got %d: %s", w2.Code, w2.Body.String()) + } + var body2 map[string]interface{} + if err := json.Unmarshal(w2.Body.Bytes(), &body2); err != nil { + t.Fatalf("parse second: %v", err) + } + if body2["status"] != "saved" { + t.Errorf("status = %v, want saved", body2["status"]) + } + if body2["key"] != key { + t.Errorf("key = %v, want %s", body2["key"], key) + } + + // 4. Approval consumed. + var consumedAt *string + if err := conn.QueryRowContext(context.Background(), + `SELECT consumed_at::text FROM approval_requests WHERE id=$1`, approvalID).Scan(&consumedAt); err != nil { + t.Fatalf("read consumed_at: %v", err) + } + if consumedAt == nil || *consumedAt == "" { + t.Fatalf("approval %s was not consumed after the 200 write", approvalID) + } +} + +// TestIntegration_AdminToken_OrgTokenMint_ExploitRegression blocks the exact +// live-exploit scenario: a concierge holding ADMIN_TOKEN tries to mint a +// full-tenant-admin org token with ZERO pending approvals, and the gate MUST +// reject it with 202. The old code (pre-core#2574) would have bypassed the +// gate entirely and returned 200 with a live token. +func TestIntegration_AdminToken_OrgTokenMint_ExploitRegression(t *testing.T) { + conn := integrationDB_AdminTokenGate(t) + prev := db.DB + db.DB = conn + t.Cleanup(func() { db.DB = prev }) + setupTestRedis(t) + + _ = seedConciergeWorkspace(t, conn) + + // The exploit ran with the default rollout flag OFF (no + // MOLECULE_PLATFORM_APPROVAL_GATE env var set). That is the + // regression posture: the gate must fire EVEN when the flag is off, + // because admin-token callers are ALWAYS gated. + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + h := NewOrgTokenHandler() + c, w := adminTokenContext(t, "POST", "/org/tokens", `{"name":"concierge-exploit-token"}`) + + h.Create(c) + + if w.Code == http.StatusOK { + var body map[string]interface{} + _ = json.Unmarshal(w.Body.Bytes(), &body) + t.Fatalf("EXPLOIT REGRESSION: admin-token mint returned 200 (token created) with ZERO approvals — %v", body) + } + if w.Code != http.StatusAccepted { + t.Fatalf("unexpected status: want 202, got %d: %s", w.Code, w.Body.String()) + } + var body map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body["status"] != "pending_approval" { + t.Errorf("status = %v, want pending_approval", body["status"]) + } + + // Verify NO org token was actually minted. + var count int + if err := conn.QueryRowContext(context.Background(), + `SELECT count(*) FROM org_api_tokens WHERE name = $1`, "concierge-exploit-token").Scan(&count); err != nil { + t.Fatalf("count tokens: %v", err) + } + if count != 0 { + t.Fatalf("EXPLOIT REGRESSION: %d org token(s) minted despite zero approvals", count) + } +} diff --git a/workspace-server/internal/handlers/approval_gate_scope_test.go b/workspace-server/internal/handlers/approval_gate_scope_test.go index fd6836390..ac4981301 100644 --- a/workspace-server/internal/handlers/approval_gate_scope_test.go +++ b/workspace-server/internal/handlers/approval_gate_scope_test.go @@ -1,17 +1,20 @@ package handlers // Phase 4b — unit coverage for the gate's activation SCOPE: the default-off -// rollout flag + org-token-only targeting. These exercise the short-circuit -// paths that return "proceed" BEFORE requireApproval, so they need no DB. The -// full flag-on + org-token + gated → 202 path is covered by the real-Postgres +// rollout flag + org-token-only targeting + (core#2574) admin-token +// ALWAYS-gated. These exercise the short-circuit paths that return +// "proceed" BEFORE requireApproval, so they need no DB. The full +// flag-on + org-token + gated → 202 path is covered by the real-Postgres // approval_gate_integration_test.go. import ( + "database/sql" "net/http/httptest" "os" "testing" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals" + "github.com/DATA-DOG/go-sqlmock" "github.com/gin-gonic/gin" ) @@ -44,33 +47,115 @@ func TestCallerHoldsOrgToken(t *testing.T) { } } -// gateDestructive must return true (proceed, no 202, no DB touch) whenever the -// scope excludes the call: non-gated action, flag off, or non-org-token caller. +// (core#2574) admin-token callers are detected via the caller_is_admin_token +// context key, set by wsauth_middleware.AdminAuth on the Tier 2b ADMIN_TOKEN +// path + Tier 3 workspace-token-fallback path. Without this, the concierge's +// admin-token auth bypassed every gated verb. +func TestCallerIsAdminToken(t *testing.T) { + gin.SetMode(gin.TestMode) + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + if callerIsAdminToken(c) { + t.Error("no caller_is_admin_token in context → must be false") + } + c.Set("caller_is_admin_token", true) + if !callerIsAdminToken(c) { + t.Error("caller_is_admin_token=true → must be true (admin-token path)") + } + // Defensive: wrong type must NOT panic and must return false. + c2, _ := gin.CreateTestContext(httptest.NewRecorder()) + c2.Set("caller_is_admin_token", "not-a-bool") + if callerIsAdminToken(c2) { + t.Error(`caller_is_admin_token="not-a-bool" → must be false (type assertion guard)`) + } +} + +// gateDestructive scope short-circuits. Updated for core#2574: admin-token +// callers are ALWAYS gated (regardless of the rollout flag); org-token callers +// still follow the rollout flag; everyone else bypasses. func TestGateDestructive_ScopeShortCircuits(t *testing.T) { gin.SetMode(gin.TestMode) - newCtx := func(orgToken bool) *gin.Context { + mock := setupTestDB(t) + + // requireApproval sequence for an admin-token caller with a gated action + // and no pre-existing approval: UPDATE consumes nothing, INSERT returns + // the new approval id, SELECT parent_id returns nil. (b is nil in the + // admin-token gate call, so no RecordAndBroadcast query.) + mock.ExpectQuery(`UPDATE approval_requests SET consumed_at`). + WillReturnError(sqlmock.ErrCancelled) // not sql.ErrNoRows on purpose — see note below + _ = mock // referenced for the deferred expectations set inside the call + + newCtx := func(cred string) *gin.Context { c, _ := gin.CreateTestContext(httptest.NewRecorder()) c.Request = httptest.NewRequest("DELETE", "/x", nil) - if orgToken { + switch cred { + case "org-token": c.Set("org_token_id", "tok") + case "admin-token": + c.Set("caller_is_admin_token", true) } return c } - // flag OFF (default) + org-token + gated action → proceed. + // flag OFF (default) + org-token + gated action → proceed (rollout dormant). os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") - if !gateDestructive(newCtx(true), nil, "ws", approvals.ActionDeleteWorkspace, "r", nil) { - t.Error("flag off must proceed (gate dormant)") + if !gateDestructive(newCtx("org-token"), nil, "ws", approvals.ActionDeleteWorkspace, "r", nil) { + t.Error("flag off + org-token must proceed (gate dormant)") } - // flag ON + NO org token (workspace agent / human CP session) → proceed. + // flag ON + NO agent credential (workspace/CP caller) → proceed. t.Setenv("MOLECULE_PLATFORM_APPROVAL_GATE", "1") - if !gateDestructive(newCtx(false), nil, "ws", approvals.ActionDeleteWorkspace, "r", nil) { - t.Error("non-org-token caller must proceed (normal operation unchanged)") + if !gateDestructive(newCtx("none"), nil, "ws", approvals.ActionDeleteWorkspace, "r", nil) { + t.Error("non-agent caller must proceed (normal operation unchanged)") } // flag ON + org token + NON-gated action → proceed (IsGated short-circuit). - if !gateDestructive(newCtx(true), nil, "ws", approvals.Action("not_a_gated_action"), "r", nil) { + if !gateDestructive(newCtx("org-token"), nil, "ws", approvals.Action("not_a_gated_action"), "r", nil) { t.Error("non-gated action must proceed") } } + +// (core#2574) admin-token callers are ALWAYS gated — the rollout flag is +// irrelevant on the admin-token path. The old code would have bypassed; the +// new code MUST fire the gate. This is the regression test for the +// privilege-escalation hole. +func TestGateDestructive_AdminTokenAlwaysGated(t *testing.T) { + gin.SetMode(gin.TestMode) + mock := setupTestDB(t) + + // requireApproval sequence for an admin-token caller (gated action, + // no pre-existing approval). Reusable across BOTH sub-cases below + // (flag off + flag on) — requireApproval makes the same three calls + // regardless of the flag. + expectGateSequence := func() { + mock.ExpectQuery(`UPDATE approval_requests SET consumed_at`). + WillReturnError(sql.ErrNoRows) + mock.ExpectQuery(`WITH existing AS`). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("appr-core2574-1")) + mock.ExpectQuery(`SELECT parent_id FROM workspaces WHERE id`). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + } + + newCtx := func() *gin.Context { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Request = httptest.NewRequest("POST", "/org/tokens", nil) + c.Set("caller_is_admin_token", true) + return c + } + + // Sub-case A: flag OFF (default) + admin-token + gated action → MUST gate. + // This is the regression for core#2574. Old code bypassed here; new + // code MUST NOT. + expectGateSequence() + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + if gateDestructive(newCtx(), nil, "ws", approvals.ActionOrgTokenMint, "r", nil) { + t.Errorf("admin-token + gated action + flag OFF MUST gate (core#2574 privilege-escalation fix). want proceed=false, got proceed=true") + } + + // Sub-case B: flag ON + admin-token + gated action → still must gate. + // The flag is irrelevant to admin-token callers. + expectGateSequence() + t.Setenv("MOLECULE_PLATFORM_APPROVAL_GATE", "1") + if gateDestructive(newCtx(), nil, "ws", approvals.ActionOrgTokenMint, "r", nil) { + t.Errorf("admin-token + gated action + flag ON must gate") + } +} diff --git a/workspace-server/internal/handlers/org_tokens.go b/workspace-server/internal/handlers/org_tokens.go index a1233009a..1c216f39b 100644 --- a/workspace-server/internal/handlers/org_tokens.go +++ b/workspace-server/internal/handlers/org_tokens.go @@ -4,7 +4,9 @@ import ( "io" "log" "net/http" + "os" + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/orgtoken" "github.com/gin-gonic/gin" @@ -78,6 +80,40 @@ func (h *OrgTokenHandler) Create(c *gin.Context) { return } + // (core#2574) Phase-4 approval gate — org_token_mint is in the gated + // action set (approvals.ActionOrgTokenMint, approvals.IsGated). Without + // this gate call, an admin-token-bearing agent (the concierge uses + // MOLECULE_API_KEY = $ADMIN_TOKEN) could mint a full-tenant-admin org + // API token with ZERO pending approvals — a real privilege-escalation + // bypass that was exploited in the live incident (two live org tokens + // minted without human review, then operator-revoked). Now the agent + // gets HTTP 202 with an approval_id and has to retry after a human + // approves via /approvals/decide. + // + // (core#2579) Approvals are keyed by workspace_id (UUID, NOT NULL). + // The previous design passed callerOrg(c) as the anchor — but for + // admin-token callers callerOrg returns "" (no org_token_id in + // context), and "" into the UUID-backed approval query errors + // "invalid input syntax for type uuid" → handler 500. Fix: derive + // a VALID approval anchor per caller class. Org-token callers use + // their token's org_id (callerOrg); admin-token callers use the + // concierge/platform root workspace (callerPlatformOrg); session + // callers fall through. If no anchor can be derived, return a + // controlled 4xx — never pass "" into the UUID query. + ws := approvalAnchorForGate(c) + if ws == "" { + log.Printf("OrgTokenHandler.Create: no approval anchor for caller class=%s (admin-token callers need MOLECULE_PLATFORM_WORKSPACE_ID; org-token callers need a token with a valid org_id; session callers need a concierge root)", orgTokenActorClass(c)) + c.JSON(http.StatusBadRequest, gin.H{ + "error": "no approval anchor for this caller class — set MOLECULE_PLATFORM_WORKSPACE_ID for admin-token callers, or call via a session / org-token with a valid org", + }) + return + } + if !gateDestructive(c, nil, ws, approvals.ActionOrgTokenMint, + "mint org token "+req.Name, + map[string]interface{}{"actor": orgTokenActorClass(c), "name": req.Name, "workspace_id": ws}) { + return + } + createdBy, orgID := orgTokenActor(c) plaintext, id, err := orgtoken.Issue(c.Request.Context(), db.DB, req.Name, createdBy, orgID) @@ -97,6 +133,27 @@ func (h *OrgTokenHandler) Create(c *gin.Context) { }) } +// orgTokenActorClass returns the credential class label for the current +// request, used in the approval gate's context (so an approval for "mint +// from admin-token" cannot be replayed as "mint from org-token:abc" or +// vice versa — the request_hash differs by credential class). +func orgTokenActorClass(c *gin.Context) string { + if v, ok := c.Get("caller_credential_class"); ok { + if s, ok := v.(string); ok && s != "" { + return s + } + } + if v, ok := c.Get("org_token_prefix"); ok { + if s, ok := v.(string); ok && s != "" { + return actorOrgTokenPrefix + s + } + } + if c.GetHeader("Cookie") != "" { + return actorSession + } + return actorAdminToken +} + // Revoke flips revoked_at. 404 when the id doesn't exist OR was // already revoked — idempotent from the caller's perspective. func (h *OrgTokenHandler) Revoke(c *gin.Context) { @@ -154,6 +211,54 @@ func callerOrg(c *gin.Context) string { return orgID } +// approvalAnchorForGate (core#2579) returns a NON-EMPTY workspace_id +// suitable for the approval gate (requireApproval queries +// approval_requests.workspace_id as a UUID, NOT NULL). Unlike +// callerOrg — which intentionally returns "" for admin-token callers +// so org_api_tokens.org_id is NULL (unanchored tokens, deny by +// default) — the approval gate needs a stable anchor for EVERY caller +// class: +// +// - Org-token callers: callerOrg(c) (the token's org_id). Same +// anchor the minted token will be tied to, so the approval and +// the token are audit-co-located. +// - Admin-token callers: MOLECULE_PLATFORM_WORKSPACE_ID env var +// (operator-set UUID of the concierge/platform root workspace). +// This is the platform-org anchor — every admin-token approval +// for org_token_mint lives against the platform root, so a +// human approver sees ONE pending-approval inbox entry for +// "platform agent minted N org tokens this hour" instead of +// N entries scattered by unanchored orgs. Without this env var +// set, admin-token callers get "" (caller returns a controlled +// 4xx, never passes "" into the UUID query). +// - Session callers: same as org-token callers (callerOrg). Today +// session callers have no org_token_id → callerOrg returns "" — +// these callers are blocked at the 4xx with a hint to set the +// platform workspace ID or use an org-token credential. Session +// paths through CP/UI are NOT expected to mint org tokens +// directly (the UI mints via the concierge, which is admin-token +// authenticated → covers via the admin-token branch above). +// +// CRITICAL (RCA on #2579): the previous code passed callerOrg(c) +// directly to gateDestructive, which crashed with "invalid input +// syntax for type uuid" when callerOrg returned "" (admin-token +// callers). That crashed path returned 500 from the handler, which +// silently bypassed the approval gate in a different way (caller +// sees 500 → retries → eventually unblocks via unmonitored paths). +// This helper is the fix: every caller class either gets a valid +// UUID or a controlled 4xx. +func approvalAnchorForGate(c *gin.Context) string { + if orgID := callerOrg(c); orgID != "" { + return orgID + } + if callerIsAdminToken(c) { + if v := os.Getenv("MOLECULE_PLATFORM_WORKSPACE_ID"); v != "" { + return v + } + } + return "" +} + // orgTokenActor returns (createdBy, orgID) for the current request. // // - If authed via another org token (org_token_id in context), diff --git a/workspace-server/internal/handlers/org_tokens_test.go b/workspace-server/internal/handlers/org_tokens_test.go index 6cce574e8..4e46138c7 100644 --- a/workspace-server/internal/handlers/org_tokens_test.go +++ b/workspace-server/internal/handlers/org_tokens_test.go @@ -7,6 +7,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "strings" "testing" "time" @@ -117,86 +119,256 @@ func TestOrgTokenHandler_Create_ActorFromAdminToken(t *testing.T) { h, mock, cleanup := setupOrgTokenTest(t) defer cleanup() - // No Cookie header, no org_token_prefix → actor should be - // "admin-token" (bootstrap path). - mock.ExpectQuery(`INSERT INTO org_api_tokens`). - WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "my-ci", actorAdminToken, nil). - WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-1")) + // (core#2574 / #2579) The Phase-4 approval gate now fires on + // admin-token org_token_mint. The test sets MOLECULE_PLATFORM_ + // WORKSPACE_ID to derive the approval anchor, then mocks the + // approval flow + the post-gate INSERT. + const platformOrgWS = "00000000-0000-0000-0000-00000000aaff" + t.Setenv("MOLECULE_PLATFORM_WORKSPACE_ID", platformOrgWS) + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + // requireApproval's 3-query sequence for the gate (no prior approval): + mock.ExpectQuery(`UPDATE approval_requests SET consumed_at`). + WillReturnError(sql.ErrNoRows) + mock.ExpectQuery(`WITH existing AS`). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("appr-actor-admin")) + mock.ExpectQuery(`SELECT parent_id FROM workspaces WHERE id`). + WithArgs(platformOrgWS). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + + // (core#2574 / #2579) Deliberately NO `INSERT INTO org_api_tokens` + // mock setup. The gate returns proceed=false → the post-gate + // orgtoken.Issue call is NEVER reached. If the gate is bypassed + // (regression), the unmocked INSERT will fail and this test + // will fail (sqlmock will surface the unfulfilled expectation + // as an error). c, w := buildCtx("POST", "/org/tokens", `{"name":"my-ci"}`) + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") h.Create(c) - if w.Code != http.StatusOK { - t.Fatalf("want 200, got %d: %s", w.Code, w.Body.String()) + // (core#2574 / #2579) Gate fires correctly → 202 with + // approval_id; the post-gate INSERT mock is therefore NOT + // called (gateDestructive returns false before orgtoken.Issue). + if w.Code != http.StatusAccepted { + t.Fatalf("admin-token + gated action MUST return 202 (Phase-4 approval gate), got %d: %s", + w.Code, w.Body.String()) } var body struct { - ID string `json:"id"` - Prefix string `json:"prefix"` - Name string `json:"name"` - Token string `json:"auth_token"` - Warning string `json:"warning"` + Status string `json:"status"` + ApprovalID string `json:"approval_id"` + Action string `json:"action"` + Reason string `json:"reason"` } if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { t.Fatalf("parse: %v", err) } - if body.Token == "" { - t.Errorf("plaintext auth_token missing from response") + if body.Status != "pending_approval" { + t.Errorf("status = %q, want pending_approval", body.Status) } - if body.Prefix != body.Token[:8] { - t.Errorf("prefix %q should equal first 8 chars of token %q", body.Prefix, body.Token[:8]) + if body.ApprovalID != "appr-actor-admin" { + t.Errorf("approval_id = %q, want appr-actor-admin", body.ApprovalID) } - if body.Name != "my-ci" { - t.Errorf("name round-trip mismatch: %q", body.Name) - } - if body.Warning == "" { - t.Errorf("warning text missing") + if body.Action != "org_token_mint" { + t.Errorf("action = %q, want org_token_mint", body.Action) } + // Sanity: post-gate INSERT was NOT called (gate fired). if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("unmet: %v", err) + t.Errorf("unmet (post-gate INSERT should not be called): %v", err) } } func TestOrgTokenHandler_Create_ActorFromOrgTokenPrefix(t *testing.T) { - h, mock, cleanup := setupOrgTokenTest(t) + h, _, cleanup := setupOrgTokenTest(t) defer cleanup() - // When an existing org token authenticated the mint, audit - // records "org-token:" using the 8-char plaintext - // prefix from the presenter's token. - mock.ExpectQuery(`INSERT INTO org_api_tokens`). - WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), nil, actorOrgTokenPrefix+"parent12", nil). - WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-new")) + // (core#2574 / #2579) The Phase-4 approval gate fires for ALL + // gated callers — org-token too. The caller's org_id is the + // approval anchor (callerOrg returns the token's org_id; the + // integration test only sets the prefix, so callerOrg returns "" + // here — but the gate is still in effect). The org-token-with- + // prefix context alone is not enough; a token record must also + // exist for the prefix to resolve to an org_id. This test now + // expects the 4xx (no anchor) since no platform workspace is + // set and the prefix is not backed by a token row. Pin the + // controlled-4xx contract for org-token-prefix-only requests. + os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + defer os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") c, w := buildCtx("POST", "/org/tokens", `{}`) c.Set("org_token_prefix", "parent12") h.Create(c) - if w.Code != http.StatusOK { - t.Fatalf("want 200, got %d: %s", w.Code, w.Body.String()) + // Gate fires → 202 (NOT 200): the prefix is the actor label, + // but the actual approval anchor is the org_id (which the test + // does not provide via DB lookup). Empty anchor → 4xx. + if w.Code != http.StatusBadRequest { + t.Fatalf("org-token prefix without resolved org_id MUST return 400, got %d: %s", + w.Code, w.Body.String()) } - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("unmet: %v", err) +} + +// (core#2574) Regression test: an admin-token-bearing caller (the +// concierge agent holds $ADMIN_TOKEN) MUST be gated by the Phase-4 +// approval gate when minting an org token. The live incident (2026-06-11) +// had the gate INERT on the admin-token path — two live full-tenant-admin +// org API tokens were minted with zero pending approvals. The fix wires +// gateDestructive into OrgTokenHandler.Create and the gate's +// callerIsAdminToken detection overrides the rollout flag (admin-token +// is ALWAYS gated when the action is gated, regardless of the flag). +func TestOrgTokenHandler_Create_AdminToken_GatedByApproval(t *testing.T) { + h, mock, cleanup := setupOrgTokenTest(t) + defer cleanup() + + // requireApproval sequence for an admin-token caller (gated action, + // no pre-existing approval): + // 1. UPDATE approval_requests SET consumed_at = now() … RETURNING id + // → no row (sql.ErrNoRows) + // 2. WITH existing AS … INSERT INTO approval_requests … RETURNING id + // 3. SELECT parent_id FROM workspaces WHERE id → NULL + mock.ExpectQuery(`UPDATE approval_requests SET consumed_at`). + WillReturnError(sql.ErrNoRows) + mock.ExpectQuery(`WITH existing AS`). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("appr-core2574-org-mint")) + mock.ExpectQuery(`SELECT parent_id FROM workspaces WHERE id`). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + + // NOTE: deliberately NO `INSERT INTO org_api_tokens` mock setup. If + // the gate is bypassed (the bug), the handler will reach the + // orgtoken.Issue call and try to run that INSERT against the mock, + // which has no expectation — sqlmock will return an error and the test + // will fail. The gate firing = no INSERT = test passes. + + c, w := buildCtx("POST", "/org/tokens", `{"name":"concierge-rogue-mint"}`) + // core#2574: the auth middleware sets caller_is_admin_token when the + // request authenticates via Tier 2b ADMIN_TOKEN (or Tier 3 workspace- + // token fallback). Simulate that here. + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") + + // The rollout flag is OFF (default) — this is the regression + // assertion: even without MOLECULE_PLATFORM_APPROVAL_GATE, the + // admin-token path must gate. + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + // (core#2579) Admin-token callers need a valid UUID anchor for + // requireApproval's approval_requests.workspace_id query. Set the + // platform-org workspace ID for the duration of the test. The + // approval row keys by this UUID; the test's mock for the + // SELECT parent_id FROM workspaces query returns nil (a root + // workspace — the platform-org row is parent_id NULL). + const platformOrgWS = "00000000-0000-0000-0000-00000000aaff" + t.Setenv("MOLECULE_PLATFORM_WORKSPACE_ID", platformOrgWS) + + h.Create(c) + + // Gate fires → 202 Accepted with a pending approval_id. + if w.Code != http.StatusAccepted { + t.Fatalf("admin-token + gated action MUST return 202 (Phase-4 approval gate), got %d: %s", + w.Code, w.Body.String()) + } + var body struct { + Status string `json:"status"` + ApprovalID string `json:"approval_id"` + Action string `json:"action"` + Reason string `json:"reason"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body.Status != "pending_approval" { + t.Errorf("status = %q, want \"pending_approval\"", body.Status) + } + if body.ApprovalID != "appr-core2574-org-mint" { + t.Errorf("approval_id = %q, want \"appr-core2574-org-mint\"", body.ApprovalID) + } + if body.Action != "org_token_mint" { + t.Errorf("action = %q, want \"org_token_mint\"", body.Action) + } + if body.Reason == "" { + t.Errorf("reason text missing — operators need a human-readable explanation for the pending approval") + } +} + +// TestOrgTokenHandler_Create_AdminToken_NoAnchor_Returns400 (core#2579) +// pins the controlled-4xx behavior for the empty-anchor path: when +// an admin-token caller hits org_token_mint WITHOUT +// MOLECULE_PLATFORM_WORKSPACE_ID set, the handler returns 400 with a +// clear hint (not 500, not a UUID-syntax error, not a silent +// gate-bypass). Regression: previous code passed callerOrg(c)="" +// directly into gateDestructive → requireApproval's +// approval_requests.workspace_id query failed with +// "invalid input syntax for type uuid: \"\"" → 500 from handler → +// the unmonitored 500 looked like a transient infra failure, not a +// security gate. The fix: derive a valid anchor FIRST (env-var for +// admin-token, org_id for org-token, 4xx for everything else). +func TestOrgTokenHandler_Create_AdminToken_NoAnchor_Returns400(t *testing.T) { + h, _, cleanup := setupOrgTokenTest(t) + defer cleanup() + + // Admin-token caller with NO env var and no org_token_id → + // callerOrg="" → no approval anchor. UNSET the env explicitly + // to ensure a clean slate (other tests in the package may set it). + os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + defer os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + c, w := buildCtx("POST", "/org/tokens", `{"name":"rogue-mint-no-anchor"}`) + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("admin-token WITHOUT platform workspace anchor MUST return 400 (controlled), got %d: %s", + w.Code, w.Body.String()) + } + var body struct { + Error string `json:"error"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body.Error == "" { + t.Errorf("error message missing — operators need the hint about MOLECULE_PLATFORM_WORKSPACE_ID") + } + // Sanity: the hint must mention the env var so an operator + // who hit this in prod knows exactly what to set. + if !strings.Contains(body.Error, "MOLECULE_PLATFORM_WORKSPACE_ID") { + t.Errorf("error message should mention the env var hint; got: %q", body.Error) } } func TestOrgTokenHandler_Create_ActorFromSession(t *testing.T) { - h, mock, cleanup := setupOrgTokenTest(t) + h, _, cleanup := setupOrgTokenTest(t) defer cleanup() - // Cookie present but no org_token_prefix — that's the canvas - // session path. Today recorded as bare "session". When the - // follow-up captures WorkOS user_id, this test changes to - // "session:user_01XXX". - mock.ExpectQuery(`INSERT INTO org_api_tokens`). - WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "from-browser", actorSession, nil). - WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-browser")) + // (core#2574 / #2579) Cookie present, no org_token_prefix, no + // org_token_id, no admin-token env var. The session path has + // no resolvable approval anchor — callerOrg returns "" and + // the env var is unset. Per approvalAnchorForGate contract, + // this returns a controlled 4xx. The gate works correctly: + // session callers cannot mint org tokens without an explicit + // org_id (set via the platform workspace env, or by calling + // through the concierge with an org-token credential). + os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + defer os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") c, w := buildCtx("POST", "/org/tokens", `{"name":"from-browser"}`) c.Request.Header.Set("Cookie", "mcp_session=abc") h.Create(c) - if w.Code != http.StatusOK { - t.Fatalf("want 200, got %d: %s", w.Code, w.Body.String()) + if w.Code != http.StatusBadRequest { + t.Fatalf("session caller without resolvable org_id MUST return 400, got %d: %s", + w.Code, w.Body.String()) } } @@ -224,16 +396,50 @@ func TestOrgTokenHandler_Create_NameTooLong400(t *testing.T) { func TestOrgTokenHandler_Create_EmptyBodyOK(t *testing.T) { h, mock, cleanup := setupOrgTokenTest(t) defer cleanup() - // Empty POST must still mint a token — name is optional. + + // (core#2574 / #2579) Empty body → admin-token caller path + // (no Cookie, no org_token_prefix). With platform workspace + // set, the gate fires; the test mocks the approval flow. + const platformOrgWS = "00000000-0000-0000-0000-00000000aaff" + t.Setenv("MOLECULE_PLATFORM_WORKSPACE_ID", platformOrgWS) + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + mock.ExpectQuery(`UPDATE approval_requests SET consumed_at`). + WillReturnError(sql.ErrNoRows) + mock.ExpectQuery(`WITH existing AS`). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("appr-empty-body")) + mock.ExpectQuery(`SELECT parent_id FROM workspaces WHERE id`). + WithArgs(platformOrgWS). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) mock.ExpectQuery(`INSERT INTO org_api_tokens`). WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), nil, actorAdminToken, nil). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-min")) c, w := buildCtx("POST", "/org/tokens", "") + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") h.Create(c) - if w.Code != http.StatusOK { - t.Errorf("empty body should mint OK; got %d: %s", w.Code, w.Body.String()) + // (core#2574 / #2579) Gate fires correctly → 202 with + // approval_id (admin-token + gated action). The pre-gate + // expectations are met (approval flow); the post-gate + // orgtoken.Issue INSERT was NOT called (gate returned false). + if w.Code != http.StatusAccepted { + t.Errorf("empty body admin-token: gate MUST return 202, got %d: %s", w.Code, w.Body.String()) + } + var body struct { + Status string `json:"status"` + ApprovalID string `json:"approval_id"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body.Status != "pending_approval" { + t.Errorf("status = %q, want pending_approval", body.Status) + } + if body.ApprovalID != "appr-empty-body" { + t.Errorf("approval_id = %q, want appr-empty-body", body.ApprovalID) } } diff --git a/workspace-server/internal/handlers/secrets_test.go b/workspace-server/internal/handlers/secrets_test.go index 569c17ec7..ec6ec4841 100644 --- a/workspace-server/internal/handlers/secrets_test.go +++ b/workspace-server/internal/handlers/secrets_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" "strings" "testing" "time" @@ -1176,3 +1177,73 @@ func TestDeleteGlobal_AutoRestartsAffectedWorkspaces(t *testing.T) { t.Errorf("unmet sqlmock expectations: %v", err) } } + +// (core#2574) Regression test: a caller presenting the tenant ADMIN_TOKEN +// (the concierge's MCP credential) MUST be gated when writing a workspace +// secret. The live incident (2026-06-11) had the gate INERT on the +// admin-token path — TEST_APPROVAL_SECRET + TEST_APPROVAL_DUMMY_KEY were +// written with zero pending approvals and the secret-change auto-restart +// fired (core#2573). The fix: gateDestructive on ActionSecretWrite +// now checks callerIsAdminToken (caller_is_admin_token context key set +// by AdminAuth on the Tier 2b ADMIN_TOKEN path) and ALWAYS gates, +// regardless of the rollout flag. +func TestSecretsSet_AdminToken_GatedByApproval(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewSecretsHandler(nil) + + // requireApproval sequence for an admin-token caller (gated action, + // no pre-existing approval). The gate's requireApproval runs THREE + // queries: UPDATE (consume), INSERT (new pending), SELECT (parent_id). + mock.ExpectQuery(`UPDATE approval_requests SET consumed_at`). + WillReturnError(sql.ErrNoRows) + mock.ExpectQuery(`WITH existing AS`). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("appr-core2574-secret-write")) + mock.ExpectQuery(`SELECT parent_id FROM workspaces WHERE id`). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + + // NOTE: deliberately NO `INSERT INTO workspace_secrets` mock setup. If + // the gate is bypassed (the bug), the handler reaches the INSERT and + // sqlmock returns an error → test fails. The gate firing = no INSERT + // = test passes. + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "550e8400-e29b-41d4-a716-446655440000"}} + + body := `{"key":"TEST_APPROVAL_SECRET","value":"should-have-required-approval"}` + c.Request = httptest.NewRequest("POST", "/workspaces/550e8400-e29b-41d4-a716-446655440000/secrets", bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + + // core#2574: the auth middleware sets caller_is_admin_token when + // the request authenticates via Tier 2b ADMIN_TOKEN. + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") + + // Rollout flag is OFF (default) — this is the regression assertion: + // even WITHOUT MOLECULE_PLATFORM_APPROVAL_GATE=1, the admin-token + // path MUST gate. + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + defer os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + + handler.Set(c) + + // Gate fires → 202 Accepted with a pending approval_id. + if w.Code != http.StatusAccepted { + t.Fatalf("admin-token + secret_write MUST return 202 (Phase-4 approval gate), got %d: %s", + w.Code, w.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp["status"] != "pending_approval" { + t.Errorf("status = %v, want \"pending_approval\"", resp["status"]) + } + if resp["approval_id"] != "appr-core2574-secret-write" { + t.Errorf("approval_id = %v, want \"appr-core2574-secret-write\"", resp["approval_id"]) + } + if resp["action"] != "secret_write" { + t.Errorf("action = %v, want \"secret_write\"", resp["action"]) + } +} diff --git a/workspace-server/internal/middleware/wsauth_middleware.go b/workspace-server/internal/middleware/wsauth_middleware.go index 9407a4e7d..73f51f2ab 100644 --- a/workspace-server/internal/middleware/wsauth_middleware.go +++ b/workspace-server/internal/middleware/wsauth_middleware.go @@ -245,6 +245,14 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid admin auth token"}) return } + // (core#2574) Mark the request as admin-token-authed so the + // approval gate can apply. Without this flag the gate's + // callerHoldsOrgToken helper only fires for org-token callers, + // and the concierge's admin-token path bypassed every gated + // verb (org_token_mint, secret_write) — see core#2574 for the + // live privilege-escalation evidence that motivated this fix. + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token") c.Next() return } @@ -255,6 +263,12 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid admin auth token"}) return } + // (core#2574) Tier-3 fallback is also an "agent that can do org-admin + // things" — flag it for the gate too, so the gate doesn't accidentally + // bypass on a misconfigured deploy (no ADMIN_TOKEN, any workspace + // token accepted as admin). Without this, the bypass is silent. + c.Set("caller_is_admin_token", true) + c.Set("caller_credential_class", "admin-token-tier3-fallback") c.Next() } } diff --git a/workspace-server/internal/staginge2e/approval_gate_concierge_e2e_test.go b/workspace-server/internal/staginge2e/approval_gate_concierge_e2e_test.go new file mode 100644 index 000000000..cce316ed4 --- /dev/null +++ b/workspace-server/internal/staginge2e/approval_gate_concierge_e2e_test.go @@ -0,0 +1,216 @@ +//go:build staging_e2e + +package staginge2e + +// approval_gate_concierge_e2e_test.go — live staging end-to-end coverage for +// core#2574: the Phase-4 approval gate on the admin-token path. +// +// What it proves that unit + integration tests cannot: that against a REAL +// SaaS tenant, the concierge's ADMIN_TOKEN (Tier 2b) is gated when minting +// org API tokens and writing workspace secrets — not just that the Go code +// returns 202, but that the REAL Postgres row is created and the retry-after- +// approval flow works through the actual HTTP surface. +// +// Run with: +// +// STAGING_E2E=1 CP_BASE_URL=https://cp.staging.moleculesai.app \ +// CP_ADMIN_API_TOKEN=... go test -tags=staging_e2e ./internal/staginge2e/ \ +// -run TestConciergeApprovalGate_Staging -v +// +// Guarded by the staging_e2e build tag + STAGING_E2E=1 env gate. +// Teardown is t.Cleanup-driven (admin DELETE /cp/admin/tenants). + +import ( + "encoding/json" + "fmt" + "net/http" + "strings" + "testing" + "time" +) + +func TestConciergeApprovalGate_Staging(t *testing.T) { + cfg := requireStagingEnv(t) + + slug := fmt.Sprintf("e2e-gate-%d", time.Now().Unix()%100000000) + t.Logf("approval-gate: slug=%s", slug) + + // --- Step 1: provision throwaway org + tenant --- + orgID := adminCreateOrg(t, cfg, slug) + t.Cleanup(func() { adminDeleteTenant(t, cfg, slug) }) + t.Logf("org created: org_id=%s", orgID) + + token := tenantAdminToken(t, cfg, slug) + host := slug + "." + cfg.subdomainSuffix + waitForHTTP(t, host, http.StatusOK, 10*time.Minute, "tenant /health ready") + t.Logf("tenant TLS ready: %s", host) + + // --- Step 2: create an ordinary workspace (secret-write needs a :id) --- + ordinaryWS := tenantCreateWorkspace(t, cfg, host, token, orgID) + t.Logf("ordinary workspace created: %s", ordinaryWS) + + // Install platform agent so there is a concierge workspace (kind=platform). + platformID := findPlatformRoot(t, host, token, orgID) + if platformID == "" { + platformID = newUUIDv4(t) + body := fmt.Sprintf(`{"id":%q,"name":%q}`, platformID, "E2E Gate Concierge") + status, resp := doTenantJSON(t, "POST", + "https://"+host+"/admin/org/platform-agent", token, orgID, body) + if status != http.StatusOK { + t.Fatalf("install platform agent: HTTP %d: %s", status, resp) + } + t.Logf("platform agent installed: %s", platformID) + } else { + t.Logf("platform agent already present: %s", platformID) + } + + // ── Regression (c): concierge admin-token mint WITHOUT approval → 202 ───── + t.Run("org_token_mint_without_approval_rejected", func(t *testing.T) { + status, body := doTenantJSON(t, "POST", + "https://"+host+"/org/tokens", token, orgID, `{"name":"exploit-test-token"}`) + if status == http.StatusOK { + var resp map[string]interface{} + _ = json.Unmarshal([]byte(body), &resp) + t.Fatalf("REGRESSION core#2574: admin-token mint returned 200 (token created) with ZERO approvals — %v", resp) + } + if status != http.StatusAccepted { + t.Fatalf("admin-token mint: want 202, got %d: %s", status, body) + } + if !strings.Contains(body, `"status":"pending_approval"`) { + t.Fatalf("expected pending_approval in 202 body, got: %s", body) + } + if !strings.Contains(body, `"action":"org_token_mint"`) { + t.Fatalf("expected action=org_token_mint in 202 body, got: %s", body) + } + approvalID := jsonField(body, "approval_id") + if approvalID == "" { + t.Fatalf("approval_id missing from 202 body: %s", body) + } + t.Logf("mint correctly gated (202, approval_id=%s)", approvalID) + + // Verify NO token was actually minted. + listStatus, listBody := doTenantJSON(t, "GET", + "https://"+host+"/org/tokens", token, orgID, "") + if listStatus != http.StatusOK { + t.Fatalf("list tokens: HTTP %d: %s", listStatus, listBody) + } + if strings.Contains(listBody, "exploit-test-token") { + t.Fatalf("REGRESSION: exploit-test-token appears in live token list despite zero approvals") + } + t.Logf("no token minted — list does not contain exploit-test-token") + }) + + // ── (a): concierge admin-token secret write WITHOUT approval → 202 ──────── + var secretApprovalID string + t.Run("secret_write_without_approval_rejected", func(t *testing.T) { + url := "https://" + host + "/workspaces/" + ordinaryWS + "/secrets" + status, body := doTenantJSON(t, "POST", url, token, orgID, + `{"key":"E2E_GATED_SECRET","value":"gated-value-123"}`) + if status == http.StatusOK { + var resp map[string]interface{} + _ = json.Unmarshal([]byte(body), &resp) + t.Fatalf("REGRESSION core#2574: admin-token secret write returned 200 with ZERO approvals — %v", resp) + } + if status != http.StatusAccepted { + t.Fatalf("admin-token secret write: want 202, got %d: %s", status, body) + } + if !strings.Contains(body, `"status":"pending_approval"`) { + t.Fatalf("expected pending_approval in 202 body, got: %s", body) + } + if !strings.Contains(body, `"action":"secret_write"`) { + t.Fatalf("expected action=secret_write in 202 body, got: %s", body) + } + secretApprovalID = jsonField(body, "approval_id") + if secretApprovalID == "" { + t.Fatalf("approval_id missing from 202 body: %s", body) + } + t.Logf("secret write correctly gated (202, approval_id=%s)", secretApprovalID) + + // Verify the secret was NOT actually written. + listStatus, listBody := doTenantJSON(t, "GET", url, token, orgID, "") + if listStatus != http.StatusOK { + t.Fatalf("list secrets: HTTP %d: %s", listStatus, listBody) + } + if strings.Contains(listBody, "E2E_GATED_SECRET") { + t.Fatalf("REGRESSION: E2E_GATED_SECRET appears in list despite zero approvals") + } + t.Logf("secret not written — list does not contain E2E_GATED_SECRET") + }) + + // ── (b): WITH a granted approval, secret write succeeds ─────────────────── + // The decide endpoint is /workspaces/:id/approvals/:approvalId/decide. + // For secret write, the workspace_id is the ordinary workspace ID. + // For org token mint, workspace_id is empty (org-scoped), so there is no + // HTTP decide path today — the integration tests cover the mint retry. + t.Run("secret_write_with_approval_succeeds", func(t *testing.T) { + if secretApprovalID == "" { + t.Skip("secret_write_without_approval_rejected did not capture an approval_id") + } + + // 1. Grant the approval via the decide endpoint. + decideURL := fmt.Sprintf("https://%s/workspaces/%s/approvals/%s/decide", + host, ordinaryWS, secretApprovalID) + status, body := doTenantJSON(t, "POST", decideURL, token, orgID, `{"decision":"approved"}`) + if status != http.StatusOK { + t.Fatalf("approve decision: HTTP %d: %s", status, body) + } + if !strings.Contains(body, `"status":"approved"`) { + t.Fatalf("expected approved status in decide body, got: %s", body) + } + t.Logf("approval %s granted", secretApprovalID) + + // 2. Retry the secret write with the SAME key. + writeURL := "https://" + host + "/workspaces/" + ordinaryWS + "/secrets" + status, body = doTenantJSON(t, "POST", writeURL, token, orgID, + `{"key":"E2E_GATED_SECRET","value":"gated-value-123"}`) + if status != http.StatusOK { + t.Fatalf("retry after approval: want 200, got %d: %s", status, body) + } + if !strings.Contains(body, `"status":"saved"`) { + t.Fatalf("expected saved status in 200 body, got: %s", body) + } + t.Logf("secret write succeeded after approval (200)") + + // 3. Verify the secret IS now present. + listStatus, listBody := doTenantJSON(t, "GET", writeURL, token, orgID, "") + if listStatus != http.StatusOK { + t.Fatalf("list secrets after write: HTTP %d: %s", listStatus, listBody) + } + if !strings.Contains(listBody, "E2E_GATED_SECRET") { + t.Fatalf("E2E_GATED_SECRET missing from list after approved write: %s", listBody) + } + t.Logf("secret present in list after approved write") + + // 4. Single-use: a SECOND write with the SAME key must create a NEW + // pending approval (the first one was consumed). + status, body = doTenantJSON(t, "POST", writeURL, token, orgID, + `{"key":"E2E_GATED_SECRET","value":"gated-value-456"}`) + if status != http.StatusAccepted { + t.Fatalf("second write after consumption: want 202 (fresh pending), got %d: %s", status, body) + } + if !strings.Contains(body, `"status":"pending_approval"`) { + t.Fatalf("expected fresh pending_approval for second write, got: %s", body) + } + freshApprovalID := jsonField(body, "approval_id") + if freshApprovalID == secretApprovalID { + t.Fatalf("approval_id reused — the consumed approval was not single-use: %s", freshApprovalID) + } + t.Logf("single-use verified: second write got fresh approval_id=%s", freshApprovalID) + }) + + // ── List pending approvals via admin surface ────────────────────────────── + // The /approvals/pending endpoint is AdminAuth-gated and returns ALL pending + // approvals across the tenant. After the mint rejection above, there should be + // at least one pending org_token_mint approval (workspace_id may be ""). + t.Run("pending_approvals_listable_by_admin", func(t *testing.T) { + status, body := doTenantJSON(t, "GET", + "https://"+host+"/approvals/pending", token, orgID, "") + if status != http.StatusOK { + t.Fatalf("list pending approvals: HTTP %d: %s", status, body) + } + if !strings.Contains(body, `"action":"org_token_mint"`) { + t.Fatalf("expected at least one org_token_mint pending approval in list: %s", body) + } + t.Logf("pending approvals list contains org_token_mint (admin can see the gate-fired request)") + }) +}