fix(security): enforce Phase-4 approval gate on admin-token path (core#2574) #2579
+61
-1
@@ -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 <workspace_id_for_approve> <curl args...>)
|
||||
#
|
||||
# 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"
|
||||
}
|
||||
|
||||
+11
-6
@@ -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"
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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=<org-admin token>, 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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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:<prefix>" 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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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"])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)")
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user