feat(api): add outbound require_review to the protection resource

.claude-plugin/marketplace.json

@@ -7,7 +7,7 @@
   },
   "metadata": {
     "description": "e2a plugins for Claude Code — open-source email API for applications and AI agents",
-    "version": "0.9.6"
+    "version": "0.9.7"
   },
   "plugins": [
     {

.cursor-plugin/marketplace.json

@@ -6,7 +6,7 @@
   },
   "metadata": {
     "description": "e2a — open-source email API for applications and AI agents (MCP configuration and canonical docs).",
-    "version": "0.9.6"
+    "version": "0.9.7"
   },
   "plugins": [
     {

api/openapi.yaml

@@ -304,6 +304,8 @@ components:
           type: string
         outbound_policy_action:
           type: string
+        outbound_require_review:
+          type: boolean
         outbound_scan:
           type: string
         outbound_scan_block_threshold:
@@ -350,6 +352,7 @@ components:
         - inbound_policy_action
         - outbound_policy
         - outbound_policy_action
+        - outbound_require_review
         - inbound_scan
         - inbound_scan_review_threshold
         - inbound_scan_block_threshold
@@ -3828,7 +3831,7 @@ components:
         inbound:
           $ref: "#/components/schemas/ProtectionDirectionRequest"
         outbound:
-          $ref: "#/components/schemas/ProtectionDirectionRequest"
+          $ref: "#/components/schemas/ProtectionOutboundRequest"
       required:
         - inbound
         - outbound
@@ -3843,7 +3846,7 @@ components:
         inbound:
           $ref: "#/components/schemas/ProtectionDirectionView"
         outbound:
-          $ref: "#/components/schemas/ProtectionDirectionView"
+          $ref: "#/components/schemas/ProtectionOutboundView"
       required:
         - inbound
         - outbound
@@ -4029,6 +4032,38 @@ components:
           type: integer
       type: object
       x-stability-level: beta
+    ProtectionOutboundRequest:
+      additionalProperties: false
+      properties:
+        gate:
+          $ref: "#/components/schemas/ProtectionGateRequest"
+        require_review:
+          default: false
+          description: When true, hold every outbound send for review regardless of the gate policy, allowlist, or non-match action.
+          type: boolean
+        scan:
+          $ref: "#/components/schemas/ProtectionScanRequest"
+      required:
+        - gate
+        - scan
+      type: object
+      x-stability-level: beta
+    ProtectionOutboundView:
+      additionalProperties: true
+      properties:
+        gate:
+          $ref: "#/components/schemas/ProtectionGateView"
+        require_review:
+          default: false
+          description: When true, hold every outbound send for review regardless of the gate policy, allowlist, or non-match action. A content scan can still block a message that crosses the scan block threshold.
+          type: boolean
+        scan:
+          $ref: "#/components/schemas/ProtectionScanView"
+      required:
+        - gate
+        - scan
+      type: object
+      x-stability-level: beta
     ProtectionScanRequest:
       additionalProperties: false
       properties:

cli/src/__tests__/protection.test.ts

@@ -52,6 +52,7 @@ describe("protection commands", () => {
     const put = mockReplaceProtection.mock.calls[0][1];
     expect(put.outbound.gate.action).toBe("flag");
     expect(put.outbound.scan.sensitivity).toBe("off");
+    expect(put.outbound.requireReview).toBe(false);
     // Untouched knobs survive: gate policy/allowlist, inbound, holds.
     expect(put.outbound.gate.policy).toBe("allowlist");
     expect(put.outbound.gate.allowlist).toEqual(["trusted@x.com"]);
@@ -100,6 +101,8 @@ describe("protection commands", () => {
     const put = mockReplaceProtection.mock.calls[0][1];
     expect(put.outbound.gate.action).toBe("review");
     expect(put.outbound.scan.sensitivity).toBe("medium");
+    // #989: the switch that actually holds every send, independent of the gate.
+    expect(put.outbound.requireReview).toBe(true);
   });
 
   it("NEVER writes when the read fails — a transient GET error must not reset the doc", async () => {
@@ -129,7 +132,7 @@ describe("protection commands", () => {
     await protectionGet("bot@agents.e2a.dev", {});
 
     const output = mockStdout.mock.calls.map((c: unknown[]) => c[0]).join("");
-    expect(output).toContain("outbound: gate=allowlist/review scan=medium");
+    expect(output).toContain("outbound: gate=allowlist/review scan=medium require_review=off");
     expect(output).toContain("inbound:  gate=open/review scan=high");
     expect(output).toContain("holds:    ttl=3600s on_expiry=approve");
     expect(output).toContain("notifications=enabled");

cli/src/commands/protection.ts

@@ -2,6 +2,7 @@ import type {
   ProtectionConfigView,
   ProtectionConfigRequest,
   ProtectionDirectionView,
+  ProtectionOutboundView,
 } from "@e2a/sdk/v1";
 import { createClient } from "../sdk.js";
 import { EXIT, fail } from "../exit.js";
@@ -25,7 +26,7 @@ function summarize(config: ProtectionConfigView): string {
   const dir = (d: ProtectionDirectionView) =>
     `gate=${d.gate.policy ?? "open"}/${d.gate.action ?? "flag"} scan=${d.scan.sensitivity ?? "off"}`;
   return (
-    `outbound: ${dir(config.outbound)}\n` +
+    `outbound: ${dir(config.outbound)} require_review=${config.outbound.requireReview ? "on" : "off"}\n` +
     `inbound:  ${dir(config.inbound)}\n` +
     `holds:    ttl=${config.holds.ttlSeconds ?? 604800}s on_expiry=${config.holds.onExpiry ?? "reject"} notifications=${config.holds.suppressNotifications ? "suppressed" : "enabled"}\n`
   );
@@ -63,6 +64,18 @@ function applyReview(direction: ProtectionDirectionView, mode: "on" | "off"): vo
   }
 }
 
+/**
+ * Outbound review also flips require_review (#989). Without it, "hold for
+ * review" only fires on recipients that fail the gate, so under the default
+ * "open" policy the action never runs and the switch would look on while
+ * holding nothing; the scan below was the old workaround for that. With
+ * require_review the gate holds every send outright.
+ */
+function applyOutboundReview(direction: ProtectionOutboundView, mode: "on" | "off"): void {
+  applyReview(direction, mode);
+  direction.requireReview = mode === "on";
+}
+
 export async function protectionSet(
   email: string | undefined,
   opts: ProtectionSetOptions,
@@ -86,7 +99,7 @@ export async function protectionSet(
   // flow). A thrown GET propagates and the PUT below is never reached.
   const config = await client.agents.getProtection(email);
 
-  if (opts.outboundReview) applyReview(config.outbound, opts.outboundReview as "on" | "off");
+  if (opts.outboundReview) applyOutboundReview(config.outbound, opts.outboundReview as "on" | "off");
   if (opts.inboundReview) applyReview(config.inbound, opts.inboundReview as "on" | "off");
   if (opts.suppressNotifications !== undefined) {
     config.holds.suppressNotifications = opts.suppressNotifications === "on";

docs/api.md

@@ -746,7 +746,9 @@ or on the deployment's shared domain (see `GET /v1/info`).
 - `GET/PUT /v1/agents/{email}/protection` — **(beta)** read / wholesale-replace the
   agent's protection posture: inbound/outbound trust gate, content-scan
   sensitivity, and the hold-queue mechanism (TTL + expiration action). Setting the
-  outbound gate to `review` (or enabling the scan) is what turns on HITL holds.
+  outbound gate to `review`, enabling the scan, or setting `outbound.require_review`
+  is what turns on HITL holds; `require_review` holds every outbound send for
+  review regardless of the gate policy, allowlist, or non-match action.
   Account scope only. Beta — shape may change before it is declared stable.
 - `POST /v1/agents/{email}/test` — send a platform test email to the agent's own
   address to confirm inbound delivery.

internal/agent/require_review_test.go

@@ -0,0 +1,75 @@
+package agent_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/jackc/pgx/v5"
+
+	"github.com/tokencanopy/e2a/internal/identity"
+	"github.com/tokencanopy/e2a/internal/outbound"
+)
+
+// TestDeliverOutbound_RequireReviewHoldsEverySend is the #989 regression: with
+// require_review set on a permissive open gate — every recipient matches, so no
+// non-match ever fires — the send is still held for review. Before the fix the
+// only config that held everything was an allowlist with an empty list, which
+// reached the same state by making "nothing matched" mean "we decided to hold".
+func TestDeliverOutbound_RequireReviewHoldsEverySend(t *testing.T) {
+	api, store, _, _ := setupAsyncAPI(t)
+	ctx := context.Background()
+	user, ag := selfAgent(t, store, "requirereview")
+
+	if _, err := store.UpdateAgentProtection(ctx, ag.ID, user.ID, identity.ProtectionConfig{
+		InboundGatePolicy:       "open",
+		InboundGateAction:       "flag",
+		InboundScanSensitivity:  identity.SensitivityOff,
+		OutboundGatePolicy:      "open", // permissive: every recipient matches
+		OutboundGateAction:      "flag", // a real non-match would only annotate
+		OutboundRequireReview:   true,
+		OutboundScanSensitivity: identity.SensitivityOff,
+		HITLTTLSeconds:          3600,
+		HITLExpirationAction:    "approve",
+	}); err != nil {
+		t.Fatalf("UpdateAgentProtection: %v", err)
+	}
+	ag, err := store.GetAgentByID(ctx, ag.ID)
+	if err != nil {
+		t.Fatalf("GetAgentByID: %v", err)
+	}
+
+	res, oerr := api.DeliverOutbound(ctx, user, ag, outbound.SendRequest{
+		To: []string{"alice@external.test"}, Subject: "hold every send", Body: "b",
+	}, "send", "", nil, nil)
+	if oerr != nil {
+		t.Fatalf("DeliverOutbound: %+v", oerr)
+	}
+	if res == nil || !res.Held {
+		t.Fatalf("result = %+v, want a held result", res)
+	}
+
+	var status, reason string
+	if err := store.WithTx(ctx, func(tx pgx.Tx) error {
+		return tx.QueryRow(ctx, `SELECT status, COALESCE(review_reason, '') FROM messages WHERE id=$1`, res.PendingMessageID).Scan(&status, &reason)
+	}); err != nil {
+		t.Fatalf("read held row: %v", err)
+	}
+	if status != identity.MessageStatusPendingReview {
+		t.Errorf("held row status = %q, want %q", status, identity.MessageStatusPendingReview)
+	}
+	if reason != identity.ReviewReasonRecipientGate {
+		t.Errorf("held row review_reason = %q, want %q", reason, identity.ReviewReasonRecipientGate)
+	}
+
+	// The gate audit row records the action that actually applied (review), not
+	// the configured non-match action (flag).
+	var action string
+	if err := store.WithTx(ctx, func(tx pgx.Tx) error {
+		return tx.QueryRow(ctx, `SELECT action FROM protection_events WHERE message_id=$1 AND source='gate'`, res.PendingMessageID).Scan(&action)
+	}); err != nil {
+		t.Fatalf("read gate audit row: %v", err)
+	}
+	if action != "review" {
+		t.Errorf("gate audit action = %q, want review", action)
+	}
+}

internal/agent/screening.go

@@ -21,6 +21,7 @@ import (
 // email.blocked. Mirrors relay.inboundScreenResult on the egress side.
 type outboundVerdict struct {
 	Applied       piguard.Action // most-severe of gate + scan
+	gateAction    piguard.Action // the gate's own action (for its audit row)
 	scanAction    piguard.Action // the scan's own action (for its audit row)
 	ReviewReason  string         // recipient_gate | outbound_scan (drives denorm + event)
 	ScanScore     *float64
@@ -77,6 +78,15 @@ func allRecipients(req outbound.SendRequest) []string {
 	return out
 }
 
+// firstSendRecipient anchors the audit row for a require_review hold, where no
+// recipient tripped the gate but the send still needs a subject address.
+func firstSendRecipient(req outbound.SendRequest) string {
+	if recips := allRecipients(req); len(recips) > 0 {
+		return recips[0]
+	}
+	return ""
+}
+
 func domainOf(addr string) string {
 	if i := strings.LastIndex(addr, "@"); i >= 0 {
 		return strings.TrimSpace(addr[i+1:])
@@ -172,11 +182,23 @@ func (a *API) screenOutbound(ctx context.Context, agent *identity.AgentIdentity,
 	var v outboundVerdict
 
 	gateAction := piguard.ActionAllow
-	if flagged, addr := recipientGate(agent, req); flagged {
-		gateAction = piguard.Action(agent.OutboundPolicyAction)
+	switch {
+	case agent.OutboundRequireReview:
+		// require_review short-circuits the recipient match (#989): every send
+		// is held for review whatever the policy, allowlist, or non-match action
+		// says, so "hold everything" no longer depends on matching nothing. The
+		// first recipient anchors the audit row since none actually tripped.
+		gateAction = piguard.ActionReview
 		v.gateFlagged = true
-		v.GateAddr = addr
+		v.GateAddr = firstSendRecipient(req)
+	default:
+		if flagged, addr := recipientGate(agent, req); flagged {
+			gateAction = piguard.Action(agent.OutboundPolicyAction)
+			v.gateFlagged = true
+			v.GateAddr = addr
+		}
 	}
+	v.gateAction = gateAction
 
 	scanAction := piguard.ActionAllow
 	if identity.ContentScanEnabled() && agent.OutboundScan == identity.ScanOn && a.screen != nil {
@@ -221,7 +243,11 @@ func (a *API) screenOutbound(ctx context.Context, agent *identity.AgentIdentity,
 		v.ReviewReason = identity.ReviewReasonOutboundScan
 	}
 	if v.Reason == "" && v.gateFlagged {
-		v.Reason = "recipient not permitted by outbound policy"
+		if agent.OutboundRequireReview {
+			v.Reason = "require_review holds every outbound send"
+		} else {
+			v.Reason = "recipient not permitted by outbound policy"
+		}
 	}
 	return v
 }
@@ -239,7 +265,7 @@ func (v outboundVerdict) screeningEvents(messageID string, agent *identity.Agent
 			Direction:   "outbound",
 			Source:      identity.ScreeningSourceGate,
 			Reason:      identity.ReviewReasonRecipientGate,
-			Action:      agent.OutboundPolicyAction,
+			Action:      string(v.gateAction),
 			SubjectAddr: v.GateAddr,
 		})
 	}

internal/agent/screening_test.go

@@ -109,6 +109,74 @@ func TestScreenOutbound_OpenAllowsBenign(t *testing.T) {
 	}
 }
 
+// TestScreenOutbound_RequireReview: require_review holds every send for review
+// regardless of the gate — an open policy with action=flag, an allowlist that
+// explicitly matches the recipient, and a non-match action of block all resolve
+// to review. Before #989, "hold everything" was only expressible by making the
+// allowlist empty so nothing matched.
+func TestScreenOutbound_RequireReview(t *testing.T) {
+	a := testScreenAPI()
+	cases := []struct {
+		name      string
+		policy    string
+		allowlist []string
+		action    string
+	}{
+		{"open gate never flags but is held anyway", identity.OutboundPolicyOpen, nil, "flag"},
+		{"matching allowlist recipient is still held", identity.OutboundPolicyAllowlist, []string{"ok@friend.com"}, "flag"},
+		{"block non-match action is overridden to review", identity.OutboundPolicyAllowlist, nil, "block"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ag := &identity.AgentIdentity{
+				Domain: "bot.example.com", ID: "bot@bot.example.com",
+				OutboundPolicy: tc.policy, OutboundAllowlist: tc.allowlist,
+				OutboundPolicyAction: tc.action, OutboundRequireReview: true,
+				OutboundScan: identity.ScanOff,
+			}
+			v := a.screenOutbound(context.Background(), ag, outbound.SendRequest{
+				To: []string{"ok@friend.com"}, Subject: "hi", Body: "benign hello",
+			})
+			if v.Applied != piguard.ActionReview {
+				t.Errorf("applied = %q, want review", v.Applied)
+			}
+			if !v.gateFlagged || v.ReviewReason != identity.ReviewReasonRecipientGate {
+				t.Errorf("gateFlagged=%v reason=%q, want a recipient_gate hold", v.gateFlagged, v.ReviewReason)
+			}
+			if v.GateAddr != "ok@friend.com" {
+				t.Errorf("gate addr = %q, want the first recipient", v.GateAddr)
+			}
+		})
+	}
+}
+
+// TestScreenOutbound_RequireReviewOffKeepsGateSemantics pins the boundary the
+// switch implicates: with require_review unset, an open gate with action=review
+// still holds nothing (the #989 gap, now expressible the other way), and the
+// empty-allowlist composition still holds every send.
+func TestScreenOutbound_RequireReviewOffKeepsGateSemantics(t *testing.T) {
+	a := testScreenAPI()
+	req := outbound.SendRequest{To: []string{"anyone@anywhere.com"}, Subject: "hi", Body: "benign hello"}
+
+	open := &identity.AgentIdentity{
+		Domain: "bot.example.com", ID: "bot@bot.example.com",
+		OutboundPolicy: identity.OutboundPolicyOpen, OutboundPolicyAction: "review",
+		OutboundScan: identity.ScanOff,
+	}
+	if v := a.screenOutbound(context.Background(), open, req); v.Applied != piguard.ActionAllow {
+		t.Errorf("open policy + review + require_review off: applied = %q, want allow", v.Applied)
+	}
+
+	emptyAllowlist := &identity.AgentIdentity{
+		Domain: "bot.example.com", ID: "bot@bot.example.com",
+		OutboundPolicy: identity.OutboundPolicyAllowlist, OutboundAllowlist: []string{},
+		OutboundPolicyAction: "review", OutboundScan: identity.ScanOff,
+	}
+	if v := a.screenOutbound(context.Background(), emptyAllowlist, req); v.Applied != piguard.ActionReview {
+		t.Errorf("empty allowlist + review must keep holding, applied = %q", v.Applied)
+	}
+}
+
 // TestScreenOutbound_Scan: outbound_scan=on flags an injection payload (Unicode
 // Tags smuggling) and combines via MoreSevere with the gate.
 func TestScreenOutbound_Scan(t *testing.T) {