From acb9d81169a07e8940c64bc09ee2f4b228347d7b Mon Sep 17 00:00:00 2001 From: core-devops Date: Sat, 6 Jun 2026 16:18:09 -0700 Subject: [PATCH] feat(approval-gate): wire gateDestructive into live destructive handlers (Phase 4b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Activates the merged 4a approval-gate infra. Scope is centralized in gateDestructive so each handler is a one-liner, and the policy is uniform + testable: - default-OFF rollout flag MOLECULE_PLATFORM_APPROVAL_GATE (mirrors 3a/3c default-off; protects existing org-token automation until the platform agent ships); - ONLY org-token callers are gated. The platform agent runs with MOLECULE_API_KEY= (auth middleware sets org_token_id); ordinary workspace-token agents and human CP-session operators are NOT gated, so normal operation is byte-identical. Realises the file-header trust boundary without gating everyone. Wired: WorkspaceHandler.Delete (delete_workspace) + SecretsHandler.Set (secret_write). requireApproval/gateDestructive now take the events.EventEmitter interface and tolerate a nil emitter (SecretsHandler is broadcaster-less) — the pending approval row is still persisted, only the live canvas push is skipped. 3 new unit tests (flag default-off, org-token detection, scope short-circuits) + existing approval/secret/delete suites green; golangci-lint clean. Deferred (documented follow-on): org_token_mint (OrgTokenHandler is org-scoped, no workspace_id to key the approval) + deprovision_workspace (no workspace-server handler — lives CP-side). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/handlers/approval_gate.go | 61 ++++++++++++--- .../handlers/approval_gate_scope_test.go | 76 +++++++++++++++++++ workspace-server/internal/handlers/secrets.go | 13 ++++ .../internal/handlers/workspace_crud.go | 11 +++ 4 files changed, 152 insertions(+), 9 deletions(-) create mode 100644 workspace-server/internal/handlers/approval_gate_scope_test.go diff --git a/workspace-server/internal/handlers/approval_gate.go b/workspace-server/internal/handlers/approval_gate.go index 1ed487207..b4d917e9d 100644 --- a/workspace-server/internal/handlers/approval_gate.go +++ b/workspace-server/internal/handlers/approval_gate.go @@ -29,6 +29,7 @@ import ( "fmt" "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" @@ -50,7 +51,7 @@ func approvalRequestHash(workspaceID, action string, contextMap map[string]inter // requireApproval returns (approved=true, consumedID) when a matching approval // exists and was just consumed; otherwise it creates/reuses a pending approval // and returns (false, pendingID). A non-nil error is a server error. -func requireApproval(ctx context.Context, b *events.Broadcaster, workspaceID string, action approvals.Action, reason string, contextMap map[string]interface{}) (bool, string, error) { +func requireApproval(ctx context.Context, b events.EventEmitter, workspaceID string, action approvals.Action, reason string, contextMap map[string]interface{}) (bool, string, error) { hash := approvalRequestHash(workspaceID, string(action), contextMap) // 1. Atomically consume an approved + unconsumed request, if one exists. @@ -103,18 +104,25 @@ func requireApproval(ctx context.Context, b *events.Broadcaster, workspaceID str // Broadcast to the canvas (the user-facing signal). For a platform agent the // parent_id is NULL, so the requested-event on its own workspace IS the user // prompt; ordinary workspaces also escalate to their parent. - if bErr := b.RecordAndBroadcast(ctx, string(events.EventApprovalRequested), workspaceID, map[string]interface{}{ - "approval_id": approvalID, - "action": string(action), - "reason": reason, - }); bErr != nil { - log.Printf("approval_gate: broadcast requested failed (ws=%s): %v", workspaceID, bErr) + // + // b may be nil: stateless handlers (e.g. org-token mint — OrgTokenHandler is + // an empty struct with no broadcaster) still gate; they just can't push a + // live canvas event. The pending approval row is persisted regardless, so + // the request is never lost — only the notification is skipped. + if b != nil { + if bErr := b.RecordAndBroadcast(ctx, string(events.EventApprovalRequested), workspaceID, map[string]interface{}{ + "approval_id": approvalID, + "action": string(action), + "reason": reason, + }); bErr != nil { + log.Printf("approval_gate: broadcast requested failed (ws=%s): %v", workspaceID, bErr) + } } var parentID *string if pErr := db.DB.QueryRowContext(ctx, `SELECT parent_id FROM workspaces WHERE id = $1`, workspaceID).Scan(&parentID); pErr != nil { log.Printf("approval_gate: parent lookup failed (ws=%s): %v", workspaceID, pErr) } - if parentID != nil { + if parentID != nil && b != nil { if bErr := b.RecordAndBroadcast(ctx, string(events.EventApprovalEscalated), *parentID, map[string]interface{}{ "approval_id": approvalID, "from_workspace_id": workspaceID, @@ -130,10 +138,26 @@ func requireApproval(ctx context.Context, b *events.Broadcaster, workspaceID str // gateDestructive runs requireApproval for a gated action and, when approval is // still pending, writes the 202 response and returns false (caller must stop). // Returns true when the caller may proceed (action consumed an approval). -func gateDestructive(c *gin.Context, b *events.Broadcaster, workspaceID string, action approvals.Action, reason string, contextMap map[string]interface{}) bool { +func gateDestructive(c *gin.Context, b events.EventEmitter, workspaceID string, action approvals.Action, reason string, contextMap map[string]interface{}) bool { 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 + } approved, approvalID, err := requireApproval(c.Request.Context(), b, workspaceID, action, reason, contextMap) if err != nil { log.Printf("gateDestructive: %v (ws=%s action=%s)", err, workspaceID, action) @@ -151,3 +175,22 @@ func gateDestructive(c *gin.Context, b *events.Broadcaster, workspaceID string, } return true } + +// destructiveGateEnabled is the default-off rollout flag for the org-level +// destructive-op approval gate. Inert until an operator sets +// 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). +func destructiveGateEnabled() bool { + v := os.Getenv("MOLECULE_PLATFORM_APPROVAL_GATE") + return v == "1" || v == "true" +} + +// callerHoldsOrgToken reports whether the request authenticated with an org +// token (the auth middleware sets org_token_id, see middleware/wsauth_middleware.go). +// The platform agent uses an org-admin token; ordinary workspace-token agents +// and human CP sessions do not, so they bypass the gate entirely. +func callerHoldsOrgToken(c *gin.Context) bool { + _, ok := c.Get("org_token_id") + return ok +} diff --git a/workspace-server/internal/handlers/approval_gate_scope_test.go b/workspace-server/internal/handlers/approval_gate_scope_test.go new file mode 100644 index 000000000..fd6836390 --- /dev/null +++ b/workspace-server/internal/handlers/approval_gate_scope_test.go @@ -0,0 +1,76 @@ +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 +// approval_gate_integration_test.go. + +import ( + "net/http/httptest" + "os" + "testing" + + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals" + "github.com/gin-gonic/gin" +) + +func TestDestructiveGateEnabled_DefaultOff(t *testing.T) { + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + if destructiveGateEnabled() { + t.Fatal("gate must be OFF by default (no env)") + } + for _, v := range []string{"1", "true"} { + t.Setenv("MOLECULE_PLATFORM_APPROVAL_GATE", v) + if !destructiveGateEnabled() { + t.Errorf("%q must enable the gate", v) + } + } + t.Setenv("MOLECULE_PLATFORM_APPROVAL_GATE", "0") + if destructiveGateEnabled() { + t.Error(`"0" must keep the gate off`) + } +} + +func TestCallerHoldsOrgToken(t *testing.T) { + gin.SetMode(gin.TestMode) + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + if callerHoldsOrgToken(c) { + t.Error("no org_token_id in context → must be false (workspace/CP caller)") + } + c.Set("org_token_id", "tok-abc") + if !callerHoldsOrgToken(c) { + t.Error("org_token_id set → must be true (platform-agent / org-admin caller)") + } +} + +// 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. +func TestGateDestructive_ScopeShortCircuits(t *testing.T) { + gin.SetMode(gin.TestMode) + newCtx := func(orgToken bool) *gin.Context { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Request = httptest.NewRequest("DELETE", "/x", nil) + if orgToken { + c.Set("org_token_id", "tok") + } + return c + } + + // flag OFF (default) + org-token + gated action → proceed. + os.Unsetenv("MOLECULE_PLATFORM_APPROVAL_GATE") + if !gateDestructive(newCtx(true), nil, "ws", approvals.ActionDeleteWorkspace, "r", nil) { + t.Error("flag off must proceed (gate dormant)") + } + + // flag ON + NO org token (workspace agent / human CP session) → 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)") + } + + // 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) { + t.Error("non-gated action must proceed") + } +} diff --git a/workspace-server/internal/handlers/secrets.go b/workspace-server/internal/handlers/secrets.go index d6c716fdd..aac7a7f75 100644 --- a/workspace-server/internal/handlers/secrets.go +++ b/workspace-server/internal/handlers/secrets.go @@ -9,6 +9,7 @@ import ( "regexp" "strings" + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/approvals" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/audit" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/crypto" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db" @@ -320,6 +321,18 @@ func (h *SecretsHandler) Set(c *gin.Context) { return } + // RFC platform-agent Phase 4b: gate org-token (platform-agent) secret writes + // behind human approval. The context includes the key so an approval for one + // secret cannot authorise writing another. No-op for ordinary callers and + // when the rollout flag is off (scoping lives in gateDestructive). + // SecretsHandler has no broadcaster, so pass nil — requireApproval persists + // the pending row regardless; only the live canvas push is skipped. + if !gateDestructive(c, nil, workspaceID, approvals.ActionSecretWrite, + "write secret "+body.Key, + map[string]interface{}{"workspace_id": workspaceID, "key": body.Key}) { + return + } + // Encrypt the value (AES-256-GCM if SECRETS_ENCRYPTION_KEY is set, plaintext otherwise) encrypted, err := crypto.Encrypt([]byte(body.Value)) if err != nil { diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index 1c79ee2b8..4faaf1a8d 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -16,6 +16,7 @@ import ( "strings" "time" + "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/events" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/models" @@ -356,6 +357,16 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) { return } + // RFC platform-agent Phase 4b: gate org-token (platform-agent) deletes behind + // human approval. No-op for ordinary workspace/CP-session callers and when + // the rollout flag is off (scoping lives in gateDestructive). Placed after + // the synchronous X-Confirm-Name guard, before any destruction. + if !gateDestructive(c, h.broadcaster, id, approvals.ActionDeleteWorkspace, + "delete workspace "+workspaceName, + map[string]interface{}{"workspace_id": id, "name": workspaceName}) { + return + } + // Check for children rows, err := db.DB.QueryContext(ctx, `SELECT id, name FROM workspaces WHERE parent_id = $1 AND status != 'removed'`, id) -- 2.52.0