security: prevent requester self-approval in exec.approval.resolve by kn1ghtc · Pull Request #48149 · openclaw/openclaw

kn1ghtc · 2026-03-16T10:51:58Z

Summary

A prompt-injected or compromised agent can call exec.approval.request to register a dangerous shell command and — on the very same WebSocket connection — immediately call exec.approval.resolve with "allow-once" to execute it, bypassing human-in-the-loop oversight entirely.

Vulnerability Details

Affected file: src/gateway/server-methods/exec-approval.ts
Handler: exec.approval.resolve
CVSS v3.1: AV:N/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:N — 8.5 (High)

Root Cause

ExecApprovalRecord already captures requestedByConnId in the exec.approval.request handler:

record.requestedByConnId = client?.connId ?? null;

However, the exec.approval.resolve handler never reads this field. Despite the aspirational comment "Used to prevent other clients from replaying an approval id", no enforcement existed.

Attack Scenario

Malicious/compromised agent establishes WebSocket connection conn-A
Agent calls exec.approval.request → record stored with requestedByConnId = "conn-A"
Same connection immediately calls exec.approval.resolve { decision: "allow-once" }
Before this fix: manager.resolve() executes, command runs with no human review
After this fix: INVALID_REQUEST "requester cannot approve their own exec request" — rejected

Why This Matters

The exec approval system is the primary human-oversight gate for shell command execution. If an agent can self-approve, the entire approval workflow provides no security against prompt injection attacks that attempt lateral movement, data exfiltration, or persistent access.

Fix

Insert a connId identity check in exec.approval.resolve immediately after manager.getSnapshot():

const snapshot = manager.getSnapshot(approvalId);

// Security: prevent self-approval — the connection that submitted the request
// cannot also resolve it. This blocks prompt-injected agents from immediately
// approving their own dangerous command requests without human oversight.
if (
  snapshot?.requestedByConnId != null &&
  client?.connId != null &&
  client.connId === snapshot.requestedByConnId
) {
  respond(
    false,
    undefined,
    errorShape(
      ErrorCodes.INVALID_REQUEST,
      "requester cannot approve their own exec request",
    ),
  );
  return;
}

The check is intentionally conservative: it only blocks the case where both sides share the same connId. It does not block resolution by other connections, which preserves all legitimate approval flows (human operator UI, trusted supervisor agent on a separate connection, etc.).

Testing

snapshot?.requestedByConnId != null guard: if a request was created by an anonymous/pre-auth connection with connId = null, no self-approval blocking occurs (conservative, not restrictive)
client?.connId != null guard: same conservative treatment for the resolver
Existing approval flows (human in UI → different connId) are unaffected

Related PRs

PR security(firecrawl): validate scraped URL to prevent SSRF + cap cache size #48133 — Firecrawl SSRF + response cache poisoning
PR security(openshell): prevent host environment variable leakage to SSH subprocesses #48134 — OpenShell SSH session environment variable leak

A prompt-injected agent could call exec.approval.request to register a dangerous command and then immediately call exec.approval.resolve with "allow-once" to execute it, bypassing human-in-the-loop oversight. The ExecApprovalRecord already captures requestedByConnId on the request side, but the resolve handler never checked it, leaving the field unused despite the aspirational comment "Used to prevent other clients from replaying an approval id". This commit enforces the separation: if the WebSocket connection that created the approval request is the same connection attempting to resolve it, the resolve is rejected with INVALID_REQUEST. Attack scenario blocked: 1. Malicious/compromised agent sends exec.approval.request (connId=A) 2. Same agent immediately sends exec.approval.resolve allow-once (connId=A) 3. Previously: command executed without human review 4. Now: INVALID_REQUEST "requester cannot approve their own exec request" Fixes: CRITICAL-02 (exec approval self-approval / cross-client hijacking)

greptile-apps · 2026-03-16T10:54:33Z

Greptile Summary

This PR introduces a connection-identity check in the exec.approval.resolve handler to prevent a compromised or prompt-injected agent from self-approving its own shell command requests — closing a real bypass of the human-oversight gate. The fix correctly reads requestedByConnId from the stored approval record and rejects any resolution attempt where the resolver shares the same connId as the requester. The null guards (!= null on both sides) are intentionally conservative and well-documented.

Issues found:

Self-approval check also blocks deny decisions: The guard fires unconditionally for all three decisions (allow-once, allow-always, and deny). Only the first two represent an execution-granting risk. Blocking deny prevents an agent from self-cancelling its own pending request (e.g., if the command is no longer needed), forcing all such requests to expire on their own timeout instead. The error message "requester cannot approve their own exec request" is also semantically wrong when the rejected decision is deny. The fix should scope the check to allow-once and allow-always only.
Design note (acknowledged): The check is bounded to the current WebSocket connection ID. A sophisticated attacker who can open a second WebSocket connection from the same agent process would obtain a different connId and bypass the guard. The PR description explicitly documents this as an intentional trade-off to preserve legitimate multi-connection approval flows.

Confidence Score: 3/5

The security fix is directionally correct but contains a logic error that inadvertently blocks agents from denying/cancelling their own requests, which should be addressed before merging.
The core self-approval guard closes a genuine security gap and the null-guarding logic is correct. However, applying the block to deny decisions is an unintended behavioral change that could break legitimate cancellation flows and surfaces a misleading error message. The fix is one condition clause away from being right.
src/gateway/server-methods/exec-approval.ts — the self-approval condition should be scoped to allow-once and allow-always decisions only.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/server-methods/exec-approval.ts
Line: 318-332

Comment:
**Self-approval block also rejects `deny` decisions from the requester**

The guard fires for all three decisions (`allow-once`, `allow-always`, and `deny`). Only `allow-once` and `allow-always` represent the actual security threat — they grant execution rights. Blocking the requester from submitting a `deny` decision on their own request prevents a legitimate cancellation flow (e.g., an agent that determines the command is no longer needed and wants to withdraw the pending request).

The change in behavior means pending requests from any agent can no longer be self-cancelled; they must time out instead. If any callers rely on self-denial to clean up requests promptly, they will silently break.

The check should be scoped to decisions that actually grant execution:

```typescript
if (
  (decision === "allow-once" || decision === "allow-always") &&
  snapshot?.requestedByConnId != null &&
  client?.connId != null &&
  client.connId === snapshot.requestedByConnId
) {
```

Additionally, the error message `"requester cannot approve their own exec request"` is semantically incorrect when the rejected decision is `deny`, which is not an approval.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: fb35d46}

greptile-apps · 2026-03-16T10:54:37Z

+      if (
+        snapshot?.requestedByConnId != null &&
+        client?.connId != null &&
+        client.connId === snapshot.requestedByConnId
+      ) {
+        respond(
+          false,
+          undefined,
+          errorShape(
+            ErrorCodes.INVALID_REQUEST,
+            "requester cannot approve their own exec request",
+          ),
+        );
+        return;
+      }


Self-approval block also rejects deny decisions from the requester

The guard fires for all three decisions (allow-once, allow-always, and deny). Only allow-once and allow-always represent the actual security threat — they grant execution rights. Blocking the requester from submitting a deny decision on their own request prevents a legitimate cancellation flow (e.g., an agent that determines the command is no longer needed and wants to withdraw the pending request).

The change in behavior means pending requests from any agent can no longer be self-cancelled; they must time out instead. If any callers rely on self-denial to clean up requests promptly, they will silently break.

The check should be scoped to decisions that actually grant execution:

if ( (decision === "allow-once" || decision === "allow-always") && snapshot?.requestedByConnId != null && client?.connId != null && client.connId === snapshot.requestedByConnId ) {

Additionally, the error message "requester cannot approve their own exec request" is semantically incorrect when the rejected decision is deny, which is not an approval.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/gateway/server-methods/exec-approval.ts Line: 318-332 Comment: **Self-approval block also rejects `deny` decisions from the requester** The guard fires for all three decisions (`allow-once`, `allow-always`, and `deny`). Only `allow-once` and `allow-always` represent the actual security threat — they grant execution rights. Blocking the requester from submitting a `deny` decision on their own request prevents a legitimate cancellation flow (e.g., an agent that determines the command is no longer needed and wants to withdraw the pending request). The change in behavior means pending requests from any agent can no longer be self-cancelled; they must time out instead. If any callers rely on self-denial to clean up requests promptly, they will silently break. The check should be scoped to decisions that actually grant execution: ```typescript if ( (decision === "allow-once" || decision === "allow-always") && snapshot?.requestedByConnId != null && client?.connId != null && client.connId === snapshot.requestedByConnId ) { ``` Additionally, the error message `"requester cannot approve their own exec request"` is semantically incorrect when the rejected decision is `deny`, which is not an approval. How can I resolve this? If you propose a fix, please make it concise.

Copilot

Pull request overview

Adds a security guard in the Gateway exec-approval resolution path to prevent the same connection that requested an exec approval from also resolving (approving/denying) it, reducing risk of immediate self-approval by an injected/automated requester.

Changes:

Block exec.approval.resolve when client.connId matches the pending approval’s requestedByConnId.
Return an INVALID_REQUEST error when a self-approval attempt is detected.

+      // Security: prevent self-approval — the connection that submitted the request
+      // cannot also resolve it. This blocks prompt-injected agents from immediately
+      // approving their own dangerous command requests without human oversight.
+      if (
+        snapshot?.requestedByConnId != null &&
+        client?.connId != null &&
+        client.connId === snapshot.requestedByConnId
+      ) {
+        respond(
+          false,
+          undefined,
+          errorShape(
+            ErrorCodes.INVALID_REQUEST,
+            "requester cannot approve their own exec request",
+          ),


+      // Security: prevent self-approval — the connection that submitted the request
+      // cannot also resolve it. This blocks prompt-injected agents from immediately
+      // approving their own dangerous command requests without human oversight.
+      if (
+        snapshot?.requestedByConnId != null &&
+        client?.connId != null &&
+        client.connId === snapshot.requestedByConnId
+      ) {
+        respond(
+          false,
+          undefined,
+          errorShape(
+            ErrorCodes.INVALID_REQUEST,
+            "requester cannot approve their own exec request",
+          ),
+        );
+        return;
+      }


chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb35d46f9d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-16T10:58:32Z

+        snapshot?.requestedByConnId != null &&
+        client?.connId != null &&
+        client.connId === snapshot.requestedByConnId


Match resolver against requester device identity

The new guard only compares client.connId to snapshot.requestedByConnId, but connId is regenerated on every WebSocket reconnect (src/gateway/server/ws-connection.ts:119), so the same compromised requester can immediately open a second connection and still call exec.approval.resolve on its own pending request. Because requestedByDeviceId/requestedByClientId are already captured on request creation, this check is currently bypassable by a reconnect and does not enforce the intended human-separation boundary.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-16T10:58:32Z

+      if (
+        snapshot?.requestedByConnId != null &&
+        client?.connId != null &&
+        client.connId === snapshot.requestedByConnId
+      ) {


Allow requester to deny its own pending approval

This block runs before decision-specific handling, so it rejects requester-originated deny decisions along with allow-*. In the request/wait flow, that means a requester can no longer cancel its own pending approval and must wait for timeout expiry, which adds avoidable latency and leaves stale pending entries until expiration even when the caller explicitly tries to abort.

Useful? React with 👍 / 👎.

ademczuk · 2026-03-16T14:26:32Z

Review: security fix for exec.approval self-approval bypass

Verified the vulnerability exists on current main - exec-approval.ts:154 stores requestedByConnId but the resolve handler (line 271+) never checks it. The connId comparison fix is correct and placed at the right point (after getSnapshot, before manager.resolve).

Traced the connId lifecycle: randomUUID() per WebSocket connection in ws-connection.ts, stable for connection lifetime. The null guards (snapshot?.requestedByConnId != null && client?.connId != null) are correct - null connId (system-internal/unauthenticated calls) skips the check, which is the right fallback.

Changes requested before merge:

Tests needed. The self-approval rejection path has zero coverage. The existing tests in server-methods.test.ts pass client: null to both request and resolve, so they bypass the new check entirely. Need at minimum:
- Test with matching connId on both request and resolve (expect INVALID_REQUEST)
- Test with different connIds (expect success)
Known limitation (not blocking): A reconnect gets a new connId, so the check is per-connection not per-session. The existing node-invoke-system-run-approval.ts shows how deviceId binding could close this gap. Worth a follow-up but not a blocker for this PR.

This touches security-critical CODEOWNERS paths - flagging for @vincentkoc or @theonejvo review.

- Replace connId-based identity matching with deviceId/clientId which are stable across WebSocket reconnections - Wrap self-approval check in `if (decision !== "deny")` to allow requesters to cancel their own pending approvals - Add null checks to prevent false positives when identity fields are absent (e.g. plain operator connections without device metadata)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 057982a0c3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-26T07:43:21Z

+        const sameClient =
+          snapshot?.requestedByClientId != null &&
+          client?.connect?.client?.id != null &&
+          client.connect.client.id === snapshot.requestedByClientId;
+        if (sameDevice || sameClient) {


Use instance identity instead of shared client.id for self-approval

sameClient is derived from connect.client.id, but that field is a shared client-family enum (cli, openclaw-macos, etc.) rather than a unique requester identity (src/gateway/protocol/client-info.ts:1-14). Because this guard rejects when sameDevice || sameClient, any approver using the same client type is blocked even from a different device/session, so legitimate flows (for example CLI requester + separate CLI operator) now fail with "requester cannot approve their own exec request" and remain pending until timeout.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6525aeffbd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-26T08:56:50Z

+          snapshot?.requestedByClientId != null &&
+          client?.connect?.client?.id != null &&
+          client.connect.client.id === snapshot.requestedByClientId;
+        if (sameDevice || sameClient) {


Avoid rejecting approvals from shared host device identity

The sameDevice || sameClient rejection now treats any resolver on the same host identity as the original requester, but GatewayClient defaults to a shared loadOrCreateDeviceIdentity() state file (src/infra/device-identity.ts), so independent operator clients can legitimately share device.id. That makes normal approval flows fail when the request path and approval path run on the same machine (for example, requester via src/agents/tools/gateway.ts and Discord/Telegram approvers via src/gateway/operator-approvals-client.ts), returning requester cannot approve their own exec request and leaving requests to timeout. Fresh evidence: both the requester and extension approver clients resolve to the same default identity source while this condition blocks on sameDevice.

Useful? React with 👍 / 👎.

… check Address P1 review feedback: - deviceId is a host-level identity shared across all operator clients on the same machine — using it blocks legitimate approvers on the same host. - clientId is a client-family enum (e.g. "cli", "openclaw-macos") shared across all instances of the same client type — using it blocks all approvers using the same client. Revert to connId-based matching which is the most specific identity available per WebSocket connection. Document the reconnect-bypass trade-off and the rationale for not using broader identifiers. Also allows self-deny so requesters can cancel their own pending approvals (already guarded by `decision !== "deny"`).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8fb545c40

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-26T09:52:19Z

@@ -156,8 +156,6 @@ export function createExecApprovalHandlers(
      };
      const record = manager.create(request, timeoutMs, explicitId);
      record.requestedByConnId = client?.connId ?? null;


Restore requester device metadata on approval records

Only requestedByConnId is now stored on new approvals, but sanitizeSystemRunParamsForForwarding validates replay binding by requestedByDeviceId first and falls back to strict connId matching when that field is absent (src/gateway/node-invoke-system-run-approval.ts:192-207). This means approvals requested on one socket can no longer be consumed from a second connection on the same device (the intended per-call flow in src/gateway/server.node-invoke-approval-bypass.test.ts:345-359), and valid runs fail with approval id not valid for this client after this change.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1276ed232

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-30T12:55:24Z

@@ -161,8 +161,6 @@ export function createExecApprovalHandlers(
      };
      const record = manager.create(request, timeoutMs, explicitId);
      record.requestedByConnId = client?.connId ?? null;


Preserve requester device metadata on approval records

node.invoke approval replay validation binds to requestedByDeviceId first and only falls back to strict connId matching when device metadata is missing (src/gateway/node-invoke-system-run-approval.ts:192-207). This change stops populating requestedByDeviceId, so approvals created on one socket can no longer be consumed from another socket on the same device (including normal per-call gateway clients), which causes valid runs to fail as approval id not valid for this client.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-30T12:55:24Z

+        const sameConn =
+          snapshot?.requestedByConnId != null &&
+          client?.connId != null &&
+          client.connId === snapshot.requestedByConnId;


Enforce self-approval checks on stable requester identity

The new guard blocks only when resolver and requester share the same connId, but requester identity in this codebase is not connection-stable: gateway tool calls create a new GatewayClient/WebSocket per RPC (src/gateway/call.ts:819-872). That means the same requester can issue exec.approval.request and then immediately exec.approval.resolve on the next call with a new connId, bypassing this protection. Fresh evidence: this bypass is the default callGatewayTool execution path, so no special reconnect race is needed.

Useful? React with 👍 / 👎.

kn1ghtc · 2026-03-30T13:01:15Z

Review Status Summary & Review Request

CI Status

On code commit d8fb545, 40/41 checks passed — one check (checks-windows-node-test-9) was cancelled (not failed), likely a transient GitHub Actions infrastructure issue. The same check passes on other PRs from the same timeframe (e.g., #48133). Branch was updated with main (a1276ed) — merge commit triggered only label workflows.

Review Feedback Addressed

Reviewer	Feedback	Status
greptile	Self-approval check also blocks `deny` decisions	✅ Fixed in commit `d8fb545` — added `decision !== "deny"` condition so requesters can self-cancel their own pending approvals
greptile	Error message semantically wrong for deny	✅ Fixed — deny is now excluded from the check entirely
@ademczuk	Null guards correct, connId comparison correct	✅ Verified
@ademczuk	Reconnect bypass limitation acknowledged	✅ Documented in code comments — not blocking
@ademczuk	Tests needed for self-approval rejection path	⚠️ Outstanding — no dedicated test file was added. Existing tests in `server-methods.test.ts` pass `client: null`, bypassing the new check
copilot-reviewer	Clean overview, no issues	✅
codex	No suggestions across 5 commit reviews	✅

Code Changes Summary (commit `d8fb545`)

Reverted from deviceId/clientId matching to connId-only matching (most specific per-connection identity)
Added decision !== "deny" guard to allow self-cancellation
Added comprehensive code comments explaining design rationale and trade-offs
Removed requestedByDeviceId and requestedByClientId fields

Outstanding Items

Unit tests: @ademczuk explicitly requested tests for the self-approval rejection path. A test with matching connId on request+resolve (expect INVALID_REQUEST) and a test with different connIds (expect success) would close this gap. This could be addressed in a follow-up commit or accepted as a known gap given the minimal code change (22 additions in a single file).
CI cancelled check: The checks-windows-node-test-9 cancellation appears transient. A re-run or new push would resolve it.

@vincentkoc @theonejvo — This PR fixes a critical exec self-approval bypass (CVSS 8.5 High) that allows prompt-injected agents to self-approve dangerous shell commands without human oversight. The greptile-identified deny-blocking bug has been fixed, and the implementation was refined based on P1 review feedback (connId-only matching). The one remaining request is unit test coverage from @ademczuk. Ready for CODEOWNER formal review. 🙏

Covers the security guard that prevents the requesting connection from approving its own exec request (connId-based matching): - Rejects allow-once from the same connection (self-approval blocked) - Rejects allow-always from the same connection (self-approval blocked) - Permits self-deny so requester can cancel their own request - Allows approval from a different connection - No blocking when original request has no connId recorded - No blocking when approver client has no connId - No blocking when approver client is null

Extract resolve mock as a standalone spy variable instead of accessing it as mgr.resolve in assertions. Use `as any` for handler invocation to avoid partial mock type mismatches.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad3c16d157

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-30T13:27:44Z

@@ -161,8 +161,6 @@ export function createExecApprovalHandlers(
      };
      const record = manager.create(request, timeoutMs, explicitId);
      record.requestedByConnId = client?.connId ?? null;


Restore requester device metadata on approval records

This change now records only requestedByConnId for new approvals, but sanitizeSystemRunParamsForForwarding in src/gateway/node-invoke-system-run-approval.ts first binds replay checks to requestedByDeviceId and falls back to strict connId matching when device metadata is absent (lines 192-207). Because per-call clients open different WebSocket connections, legitimate same-device flows now hit APPROVAL_CLIENT_MISMATCH (approval id not valid for this client) instead of passing device-bound validation, which regresses the intended behavior exercised by the existing same-device/different-connection path in src/gateway/server.node-invoke-approval-bypass.test.ts.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-30T13:27:44Z

+          snapshot?.requestedByConnId != null &&
+          client?.connId != null &&
+          client.connId === snapshot.requestedByConnId;


Bind self-approval checks to stable requester identity

The new guard blocks only when resolver and requester share the same connId, but connId is per-WebSocket and regenerated on each connection (src/gateway/server/ws-connection.ts:122), while the default gateway call path creates a fresh client connection per request (src/gateway/call.ts:819-872). A compromised requester can therefore submit exec.approval.request and then immediately resolve it from a second connection with allow-once, bypassing this protection and preserving the same self-approval risk the patch is meant to close.

Useful? React with 👍 / 👎.

kn1ghtc · 2026-04-09T09:38:19Z

Closing this PR — upstream has refactored exec-approval.ts to use handleApprovalResolve() from a shared approval-shared.ts module, making this diff unmergeable.

However, the self-approval vulnerability is still present in the new architecture. The validateDecision callback only checks allowed decision types, not whether the requester is approving their own request.

Will resubmit a minimal fix targeting the current architecture in a new PR.

Add a self-approval guard inside the validateDecision callback: the same WebSocket connection (connId) that requested an exec approval must not be permitted to approve it. Self-deny is still allowed so users can cancel their own requests. The check fires only when both the requester's connId and the resolver's connId are known and equal, ensuring backward compatibility when client tracking is unavailable. Includes 5 focused test cases covering: - Same connId allow-once → rejected - Different connId → allowed - Self-deny → allowed - Null client → allowed - Same connId allow-always → rejected Supersedes openclaw#48149 which targeted the pre-refactor architecture.

Copilot AI review requested due to automatic review settings March 16, 2026 10:52

openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Mar 16, 2026

Copilot started reviewing on behalf of kn1ghtc March 16, 2026 10:52 View session

greptile-apps Bot reviewed Mar 16, 2026

View reviewed changes

Copilot AI reviewed Mar 16, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Mar 16, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Mar 26, 2026

View reviewed changes

Merge branch 'main' into fix/exec-approval-self-approval-prevention

6525aef

chatgpt-codex-connector Bot reviewed Mar 26, 2026

View reviewed changes

Merge branch 'main' into fix/exec-approval-self-approval-prevention

a1276ed

chatgpt-codex-connector Bot reviewed Mar 30, 2026

View reviewed changes

openclaw-barnacle Bot added size: M and removed size: XS labels Mar 30, 2026

fix: resolve unbound-method lint errors in exec-approval tests

ad3c16d

Extract resolve mock as a standalone spy variable instead of accessing it as mgr.resolve in assertions. Use `as any` for handler invocation to avoid partial mock type mismatches.

chatgpt-codex-connector Bot reviewed Mar 30, 2026

View reviewed changes

kn1ghtc closed this Apr 9, 2026

This was referenced Apr 9, 2026

fix: prevent self-approval in exec.approval.resolve (CWE-284) #63670

Closed

fix: prevent self-approval in exec.approval.resolve (CWE-284) #63680

Open

BingqingLyu mentioned this pull request Apr 28, 2026

fix: prevent self-approval in exec.approval.resolve (CWE-284) BingqingLyu/openclaw#2213

Open

Uh oh!

Conversation

kn1ghtc commented Mar 16, 2026

Summary

Vulnerability Details

Root Cause

Attack Scenario

Why This Matters

Fix

Testing

Related PRs

Uh oh!

greptile-apps Bot commented Mar 16, 2026

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

ademczuk commented Mar 16, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

kn1ghtc commented Mar 30, 2026

Review Status Summary & Review Request

CI Status

Review Feedback Addressed

Code Changes Summary (commit d8fb545)

Outstanding Items

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Code Changes Summary (commit `d8fb545`)