[cristim]

Summary

Closes #517. executeExchange (AWS) and executeAzureExchange quote and run an irreversible exchange in one request. A single user holding execute:ri-exchange bypassed the 4-eyes policy that only the approve path enforced (#501).

With RequireDifferentApprover on, both endpoints now return 403 (direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow). The check runs after the permission, account-scope and constraints gates and before any quote, exchange or idempotency-claim call. A global-config lookup failure fails closed (server error, not "mode off"). With the mode off nothing changes.

• AWS: the constraints block moved into authorizeAWSExchangeExecution (also keeps executeExchange under the cyclomatic limit), which now ends with the 4-eyes check.
• Azure: the check follows authorizeAzureExchangeExecution.

Behaviour to be aware of

There is no direct-execute path with a second approver: no user-facing flow creates a pending manual record from a quote (pending records come from automated runs in manual mode, via /api/ri-exchange/approve). So with 4-eyes on, interactive one-shot exchanges are refused, and exchanges go through the approval flow. Single-operator deployments can turn the mode off in settings. A real second-approver flow for interactive exchanges would be a separate feature. #82 (no audit row for direct executes) is out of scope.

Scope limits (from the independent review, tracked in #679)

This gates the two direct-execute endpoints named in #517. Scheduler reshape in auto mode still executes exchanges unattended and does not consult RequireDifferentApprover (by design of auto mode; whether it should is an owner question in #679). requireDifferentRIExchangeApprover also accepts rows with no recorded creator, unlike purchases (pre-existing, #233; also in #679). So "no exchange executes without a different approver" is not yet true for those two paths.

Tests

GOWORK=off GOTOOLCHAIN=go1.26.6, non-race, under the platform test-suite lock (local go is 1.27.1; I did not confirm the toolchain switch took effect):

• go test ./internal/api: PASS (full package).
• New: TestExecuteExchange_FourEyesRefusesDirectExecute, TestExecuteExchange_FourEyesPolicyLookupErrorFailsClosed, TestExecuteAzureExchange_FourEyesRefusesDirectExecute (403, exact message, zero exchange calls). Mode-off behaviour is covered by the existing execute tests, which use a default (off) global config.
• With the new gate temporarily made a no-op, the Azure test fails (panic: the strict ops-client mock rejects the unexpected call), confirming it fails without the fix. I did not separately record the AWS tests' pre-fix failure.
• Mock/fixture-based; no live AWS or Azure calls.

Not run locally: full race suite (disk hold; CI covers it), frontend.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 3 billable files and costs up to $0.75.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 54 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 82 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 976c3075-b9c1-425c-b888-0ff1d6acbc22

📥 Commits

Reviewing files that changed from the base of the PR and between 72843b5 and 2aa8f1d.

📒 Files selected for processing (3)

• internal/api/handler_ri_exchange.go
• internal/api/handler_ri_exchange_azure_test.go
• internal/api/handler_ri_exchange_test.go

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review of PR 673 at 2aa8f1d

Verdict: APPROVE the diff as scoped (both direct execute endpoints). Two items outside the diff need an explicit decision before #517 is described as "no RI exchange executes without a different approver". CI was still in progress when I wrote this; the verdict covers only the SHA above and needs green CI.

Execution paths enumerated (grep over internal/, cmd/, frontend/ at this SHA)

Path	Reaches an exchange via	4-eyes status
POST /api/ri-exchange/execute (executeExchange)	exchange.ExecuteExchange	Gated by this PR (403 when RequireDifferentApprover on; after permission, scope and constraints, before claimExchangeSubmit and any AWS call)
POST /api/ri-exchange/azure-instances/exchange (executeAzureExchange)	client.ExecuteExchange	Gated by this PR, before the re-quote, guardrails and idempotency claim
POST /api/ri-exchange/approve/{id}, session branch	executeApprovedExchange	requireDifferentRIExchangeApprover (see item B)
approve, email-token branch	executeApprovedExchange	Same check; non-nil creator with no session fails closed (500)
Scheduler reshape, mode "auto" (internal/server/handler_ri_exchange.go -> exchange.RunAutoExchange)	executes unattended	Not gated (item A)
Scheduler reshape, mode "manual"	creates pending rows only	Approval path above
cmd/, MCP, CLI in this repo	no ExecuteExchange caller found	n/a

No other caller of ExecuteExchange exists outside handler_ri_exchange.go. The API-key admin session is covered because the gate reads the global flag, not the caller's role. A config read error fails closed (tested for AWS).

Findings

• A (P2, outside the diff, needs owner decision). With RIExchangeEnabled and RIExchangeMode == "auto", the scheduler executes exchanges with no approval and never consults RequireDifferentApprover. That is an unattended execution path that 4-eyes does not cover. Either state "auto mode is an explicit admin opt-in, exempt" in sec(ri-exchange): direct execute paths run an irreversible exchange with no approval and no four-eyes check #517/the PR body, or file a follow-up to refuse or downgrade auto to manual while 4-eyes is on.
• B (P3, pre-existing fix(purchase): scheduler-created executions have no creator and cannot pass four-eyes #233, not introduced here). requireDifferentRIExchangeApprover returns nil when CreatedByUserID is nil, so under 4-eyes an automated pending row can be approved by any approve-any holder, or by an emailed token with no session at all. Purchases deny NULL creators in this mode; exchanges deliberately do not. Not a regression; mention it in the PR body so the claim is accurate.
• C (P3, tests). (1) Azure config-lookup-error fail-closed is untested (AWS is). (2) The AWS tests do not spy on exchange.ExecuteExchange (a package-level function); they rely on 403 vs the no-credentials 500. That is enough to fail on a regression but does not prove "no AWS call".

Evidence reproduced (macOS, exact SHA in the author's worktree, non-race, under the platform test-suite lock, GOWORK=off GOTOOLCHAIN=go1.26.6, 11 GiB free)

• go test ./internal/api -run 'FourEyes|TestExecuteExchange|TestExecuteAzureExchange|TestApproveRIExchange|RIExchangeApprover' -count=1: ok.
• Mutation (go -overlay, no source touched) turning the gate into a no-op: TestExecuteExchange_FourEyesRefusesDirectExecute fails (expected 403, got 500 "exchange execution failed" from the unmocked AWS call); TestExecuteAzureExchange_FourEyesRefusesDirectExecute fails. So both new tests are non-vacuous.
• Mock-based evidence only; no live AWS/Azure calls.