From 575789ca3c775c8853e184762ed18e6f2cea209d Mon Sep 17 00:00:00 2001 From: core-devops Date: Thu, 11 Jun 2026 10:38:49 -0700 Subject: [PATCH] fix(org-tokens): verified-session (human) mints skip the approval gate (core#2593) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canvas Settings -> Org API Keys "+ New Key" returns the raw anchor 400 from #2579: the browser authenticates via a CP-VERIFIED WorkOS session (AdminAuth sets cp_session_actor strictly after VerifiedCPSession), but approvalAnchorForGate has no session branch — the #2579 assumption "the UI mints via the concierge" was wrong; the canvas mints directly with the browser session. Design (per core#2593): the gate exists to put a HUMAN between an AGENT and a privileged mint. When the minter IS a human at the dashboard, a pending-approval the same human would approve is a no-op round-trip — verified-session callers now mint directly (audited: created_by records the session actor hash instead of the lossy "session" constant). Agent classes (admin-token, org-token) remain fully gated; the anchor 4xx contract is unchanged for them. SECURITY: the human discriminator is cp_session_actor (set only post-verification), NEVER the raw Cookie header — a bearer-authed agent attaching a junk Cookie cannot bypass the gate. The previously forgeable Cookie checks in orgTokenActor/orgTokenActorClass are replaced with the verified key (they only affected audit labels, but the same sloppy pattern next to a gate-skip would have been a hole). Tests: TestOrgTokenHandler_Create_VerifiedSession_SkipsGate (200, no approval SQL, created_by = actor — ExpectationsWereMet proves the gate did not fire) + ActorFromSession repurposed as the bypass-resistance pin (junk Cookie without verification -> still 400). Full handlers package green; go build -tags=integration clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/handlers/org_tokens.go | 96 ++++++++++++++----- .../internal/handlers/org_tokens_test.go | 62 ++++++++++-- 2 files changed, 124 insertions(+), 34 deletions(-) diff --git a/workspace-server/internal/handlers/org_tokens.go b/workspace-server/internal/handlers/org_tokens.go index 1c216f39b..9db2cfc53 100644 --- a/workspace-server/internal/handlers/org_tokens.go +++ b/workspace-server/internal/handlers/org_tokens.go @@ -48,11 +48,11 @@ type createOrgTokenRequest struct { } type createOrgTokenResponse struct { - ID string `json:"id"` - Prefix string `json:"prefix"` - Name string `json:"name,omitempty"` - Token string `json:"auth_token"` // plaintext — shown ONCE - Warning string `json:"warning"` // UX hint: copy now + ID string `json:"id"` + Prefix string `json:"prefix"` + Name string `json:"name,omitempty"` + Token string `json:"auth_token"` // plaintext — shown ONCE + Warning string `json:"warning"` // UX hint: copy now } // Create mints a new org token. The plaintext is returned exactly @@ -100,18 +100,38 @@ func (h *OrgTokenHandler) Create(c *gin.Context) { // 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 + // (core#2593) Verified-session (human) callers SKIP the approval + // gate by design: the gate exists to put a human between an AGENT + // and a privileged mint — when the minter IS a human at the + // dashboard (CP-verified WorkOS session; AdminAuth only sets + // cp_session_actor after VerifiedCPSession confirms the cookie + // upstream), routing them through a pending-approval that the same + // human would approve is a pure no-op round-trip. Live regression + // this fixes: Settings → Org API Keys → "+ New Key" returned the + // raw anchor 400 because approvalAnchorForGate had no session + // branch (the #2579 assumption "the UI mints via the concierge" + // was wrong — the canvas mints directly with the browser session). + // + // SECURITY: keyed on cp_session_actor (set ONLY post-verification), + // NEVER on the raw Cookie header — an admin-token agent attaching a + // junk Cookie header must NOT bypass the gate (pinned by + // TestOrgTokenHandler_Create_ActorFromSession). + if actor := callerVerifiedSessionActor(c); actor != "" { + log.Printf("orgtoken: mint by verified human session %s — approval gate bypassed by design (core#2593)", actor) + } else { + 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)", 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 an 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) @@ -148,7 +168,9 @@ func orgTokenActorClass(c *gin.Context) string { return actorOrgTokenPrefix + s } } - if c.GetHeader("Cookie") != "" { + // Verified session only — the raw Cookie header is forgeable by + // bearer-authed agents and must not influence classification. + if callerVerifiedSessionActor(c) != "" { return actorSession } return actorAdminToken @@ -181,8 +203,8 @@ func (h *OrgTokenHandler) Revoke(c *gin.Context) { // and in mint/revoke audit logs. Kept as constants so the labels // are greppable across services (log pipelines, audit queries). const ( - actorOrgTokenPrefix = "org-token:" // appended: 8-char plaintext prefix from the UI - actorSession = "session" // WorkOS-session-verified call + actorOrgTokenPrefix = "org-token:" // appended: 8-char plaintext prefix from the UI + actorSession = "session" // WorkOS-session-verified call actorAdminToken = "admin-token" // bootstrap ADMIN_TOKEN env ) @@ -259,6 +281,27 @@ func approvalAnchorForGate(c *gin.Context) string { return "" } +// callerVerifiedSessionActor returns the cp_session_actor identity +// ("session:") when — and ONLY when — the request authenticated +// via a CP-VERIFIED WorkOS session cookie (AdminAuth sets the key +// strictly after VerifiedCPSession confirms the cookie upstream +// against /cp/auth/me). Returns "" for every bearer-authed caller, +// including ones that happen to carry an unverified/junk Cookie +// header. This is the human-vs-agent discriminator for the approval +// gate (core#2593) — do NOT replace it with a raw Cookie check, which +// any agent can forge. +func callerVerifiedSessionActor(c *gin.Context) string { + v, ok := c.Get("cp_session_actor") + if !ok { + return "" + } + s, ok := v.(string) + if !ok { + return "" + } + return s +} + // orgTokenActor returns (createdBy, orgID) for the current request. // // - If authed via another org token (org_token_id in context), @@ -274,11 +317,14 @@ func orgTokenActor(c *gin.Context) (createdBy, orgID string) { return actorOrgTokenPrefix + s, callerOrg(c) } } - // Session-tier auth doesn't stash a WorkOS user_id in the gin - // context today. Until it does, treat session requests as "session" - // with no org anchor. - if c.GetHeader("Cookie") != "" { - return actorSession, "" + // Verified-session callers carry the cp_session_actor identity + // ("session:") — record THAT as created_by so the audit + // trail distinguishes which human session minted the token. + // (Previously this keyed off the raw Cookie header, which both + // misattributed bearer-authed requests carrying incidental + // cookies AND lost the per-session identity.) + if actor := callerVerifiedSessionActor(c); actor != "" { + return actor, "" } return actorAdminToken, "" } diff --git a/workspace-server/internal/handlers/org_tokens_test.go b/workspace-server/internal/handlers/org_tokens_test.go index 4e46138c7..1c6c9612b 100644 --- a/workspace-server/internal/handlers/org_tokens_test.go +++ b/workspace-server/internal/handlers/org_tokens_test.go @@ -12,8 +12,8 @@ import ( "testing" "time" - "github.com/DATA-DOG/go-sqlmock" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db" + "github.com/DATA-DOG/go-sqlmock" "github.com/gin-gonic/gin" ) @@ -349,14 +349,12 @@ func TestOrgTokenHandler_Create_ActorFromSession(t *testing.T) { h, _, cleanup := setupOrgTokenTest(t) defer cleanup() - // (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). + // (core#2593) BYPASS-RESISTANCE: a raw Cookie header WITHOUT a + // CP-verified session (cp_session_actor unset) must NOT be + // treated as a human — otherwise any admin-token agent could + // skip the approval gate by attaching a junk Cookie. Such a + // caller has no org_token_id, no admin-token context and no + // anchor env → controlled 400, gate intact. os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") defer os.Unsetenv("MOLECULE_PLATFORM_WORKSPACE_ID") os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") @@ -372,6 +370,52 @@ func TestOrgTokenHandler_Create_ActorFromSession(t *testing.T) { } } +// TestOrgTokenHandler_Create_VerifiedSession_SkipsGate (core#2593) +// pins the human-mint path: a CP-VERIFIED session caller (AdminAuth +// sets cp_session_actor only after VerifiedCPSession confirms the +// WorkOS cookie upstream) mints WITHOUT an approval round-trip — the +// human at the dashboard IS the approver, so 202-pending would be a +// no-op loop. Live regression: Settings → Org API Keys → "+ New Key" +// returned the anchor 400 because #2579 had no session branch. +// The mock expects ONLY the orgtoken INSERT — any approval-flow SQL +// would fail ExpectationsWereMet, proving the gate did not fire. +func TestOrgTokenHandler_Create_VerifiedSession_SkipsGate(t *testing.T) { + h, mock, cleanup := setupOrgTokenTest(t) + defer cleanup() + + 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") + + const actor = "session:deadbeefcafe1234" + mock.ExpectQuery(`INSERT INTO org_api_tokens`). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "from-dashboard", actor, nil). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-human")) + + c, w := buildCtx("POST", "/org/tokens", `{"name":"from-dashboard"}`) + c.Set("cp_session_actor", actor) + h.Create(c) + + if w.Code != http.StatusOK { + t.Fatalf("verified-session mint MUST return 200 (no approval round-trip), got %d: %s", + w.Code, w.Body.String()) + } + var body struct { + ID string `json:"id"` + Prefix string `json:"prefix"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("parse: %v", err) + } + if body.ID != "tok-human" { + t.Errorf("id = %q, want tok-human", body.ID) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet/extra SQL — approval-flow queries must NOT run for verified sessions: %v", err) + } +} + func TestOrgTokenHandler_Create_NameTooLong400(t *testing.T) { h, _, cleanup := setupOrgTokenTest(t) defer cleanup() -- 2.52.0