#21663: fix(gateway): prevent self-approval of timed-out exec requests
gateway
size: XS
Cluster:
Device Pairing and Gateway Fixes
## Summary
- A client with approvals scope could approve its own timed-out `system.run` request, violating separation of duty.
- Added a check comparing `snapshot.requestedByClientId` against the approving client's ID (`opts.client?.connect?.client?.id`) before allowing timed-out approval to proceed.
- Returns `SELF_APPROVAL_NOT_PERMITTED` error when the requesting and approving client are the same.
## Test plan
- [ ] Verify that a client cannot approve its own timed-out exec request (receives "self-approval not permitted" error)
- [ ] Verify that a different client with approvals scope can still approve timed-out exec requests
- [ ] Verify that normal (non-timed-out) approval flow is unaffected
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
Added self-approval check for timed-out `system.run` exec requests to enforce separation of duty. When an approval request times out (decision=null), clients with `operator.approvals` scope could previously approve their own timed-out requests. This PR prevents that by comparing `snapshot.requestedByClientId` against the approving client's ID and returning `SELF_APPROVAL_NOT_PERMITTED` error when they match.
**Key changes:**
- Added check in `src/gateway/node-invoke-system-run-approval.ts:251-258` comparing `approvingClientId` with `snapshot.requestedByClientId`
- Only applies to timed-out approval scenario (normal approval flow unaffected)
- Import order changed (type imports moved to top) - follows TypeScript conventions
<h3>Confidence Score: 5/5</h3>
- This PR is safe to merge - fixes a security vulnerability with minimal scope
- The fix is narrowly scoped to the specific timed-out approval scenario, adds proper separation-of-duty enforcement, and doesn't affect the normal approval flow. The check correctly handles null values and follows existing patterns in the codebase.
- No files require special attention
<sub>Last reviewed commit: 49fad25</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#8683: fix: Exec approval bypass via client-controlled flags in system.run
by coygeek · 2026-02-04
81.4%
#21661: fix(agents): treat approval timeout as denial regardless of askFall...
by AI-Reviewer-QS · 2026-02-20
78.8%
#23708: fix(gateway): auto-approve scope upgrades for loopback clients
by widingmarcus-cyber · 2026-02-22
74.6%
#11873: fix: eliminate TOCTOU race in readExecApprovalsSnapshot
by Yida-Dev · 2026-02-08
73.8%
#14127: fix(exec): return command output when gateway approval is Always Allow
by Siziff · 2026-02-11
73.8%
#22587: fix(gateway): silently auto-approve local paired-device scope upgrades
by abhishekp76 · 2026-02-21
71.8%
#20089: fix(gateway): preserve control-ui scopes when dangerouslyDisableDev...
by vashkartik · 2026-02-18
71.7%
#20435: fix(exec): prioritize user 'always allow' config over tool defaults...
by ChisomUma · 2026-02-18
71.7%
#23361: Gateway: reject scope assertions without identity binding
by bmendonca3 · 2026-02-22
71.6%
#22712: fix(gateway): auto-approve all device pairing for localhost connect...
by NewdlDewdl · 2026-02-21
71.6%