[codex] Bind exec approval trust to realpaths by steipete · Pull Request #82825 · openclaw/openclaw

steipete · 2026-05-17T01:27:23Z

Summary

Introduce a canonical exec approval trust path that prefers executable realpaths while keeping display/candidate paths separate.
Apply that trust path to path-shaped allowlists, safe-bin trust, skill auto-allow, Allow Always persistence, and approval audit metadata.
Add regression coverage for symlink retargeting across the affected exec approval paths.

Fixes #45595.
Supersedes #73543.

Verification

pnpm test src/infra/exec-allowlist-matching.test.ts src/infra/exec-approvals.test.ts src/infra/exec-approvals-safe-bins.test.ts src/infra/exec-approvals-allow-always.test.ts src/infra/exec-command-resolution.test.ts src/agents/bash-tools.exec-host-gateway.test.ts src/node-host/invoke-system-run.test.ts -- --reporter=verbose
pnpm test src/infra/exec-safe-bin-runtime-policy.test.ts src/infra/exec-approvals-safe-bins.test.ts src/infra/exec-approvals-analysis.test.ts src/infra/exec-approvals-config.test.ts -- --reporter=verbose
/Users/steipete/Projects/agent-scripts/skills/codex-review/scripts/codex-review --mode branch
pnpm check:changed (Testbox tbx_01krswr5mdcjzha7e01n0vhx8t, Actions run 25979318476, exit 0)
git diff --check

Real behavior proof

Behavior addressed: path-shaped exec approvals and trusted-bin decisions now bind to the executable realpath instead of the mutable symlink path.
Real environment tested: local OpenClaw checkout at ac12b39 using production exec approval modules via Node/tsx, with real temp executable files and symlinks on macOS.
Exact steps or command run after this patch: ran a Node/tsx OpenClaw production-module probe that created executable safe-bin symlinks, evaluated allowlist matching, retargeted the symlink, and re-evaluated safe-bin trust.
Evidence after fix: terminal output from the real production-module probe:

openclaw realpath proof root=/tmp/openclaw-realpath-proof-...
allowlist symlink pattern match=false
allowlist realpath pattern match=true
safe-bin old target trusted before retarget=true
allowlist old realpath after retarget=false
allowlist new realpath after retarget=true
safe-bin old target trusted after retarget=false
safe-bin new target trusted after retarget=true
non-executable safe-bin link trusts sibling executable dir=false

Observed result after fix: the symlink-path allowlist did not match, the realpath allowlist did match, old executable targets stopped being trusted after symlink retargeting, the new target was trusted, and a non-executable safe-bin symlink did not expand trust to a sibling executable directory.
What was not tested: native approval UI button flow; this proof covers the runtime policy decision path before UI delivery.

clawsweeper · 2026-05-17T01:28:28Z

Codex review: found issues before merge.

Summary
The PR makes exec approval allowlists, safe-bin trust, skill auto-allow, Allow Always persistence, and approval audit metadata use executable realpaths, with symlink-retargeting regression coverage and a changelog entry.

Reproducibility: yes. at source level. Current main computes resolvedRealPath but path-shaped allowlist matching still uses resolvedPath, and the linked Homebrew/asdf-style symlink scenario follows that code path.

Real behavior proof
Sufficient (terminal): The PR body includes copied terminal output from an after-fix production-module probe with real temp executables and symlinks, which is sufficient for the non-UI policy path it claims to test.

Next step before merge
Manual maintainer/security review is needed because the PR changes exec approval trust policy, has a protected maintainer label, and current head has blocking security-sensitive findings.

Security
Needs attention: The patch improves realpath binding but currently introduces security-sensitive approval bypass risks in Allow Always and derived safe-bin trust.

Review findings

[P1] Preserve interpreter detection before persisting realpaths — src/infra/exec-approvals-allowlist.ts:1146-1150
[P1] Gate derived safe-bin target dirs before trusting them — src/infra/exec-safe-bin-trust.ts:175

Review details

Best possible solution:

Land a revised realpath-canonicalization change that keeps interpreter and inline-eval detection keyed to executable identity and only trusts derived safe-bin target dirs after the same writable-dir handling as explicit trust.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. Current main computes resolvedRealPath but path-shaped allowlist matching still uses resolvedPath, and the linked Homebrew/asdf-style symlink scenario follows that code path.

Is this the best way to solve the issue?

No. Realpath-bound trust is the right direction, but this PR is not safe to merge until it preserves interpreter/inline-eval guards and handles writable derived safe-bin target dirs.

Full review comments:

[P1] Preserve interpreter detection before persisting realpaths — src/infra/exec-approvals-allowlist.ts:1146-1150
collectAllowAlwaysPatterns now checks isInterpreterLikeAllowlistPattern against the trust realpath. Common symlinks such as python3 -> .../python3.12 no longer match the fixed interpreter basename set, so inline-eval commands can be persisted as Allow Always entries instead of being blocked.
Confidence: 0.9
[P1] Gate derived safe-bin target dirs before trusting them — src/infra/exec-safe-bin-trust.ts:175
buildTrustedSafeBinDirs now adds realpath target dirs for configured safe-bin symlinks, but runtime warnings still inspect only explicit trusted dirs. A trusted symlink that points into a writable target directory can silently make replacements in that directory trusted.
Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.87

Security concerns:

[high] Allow Always interpreter guard can be bypassed — src/infra/exec-approvals-allowlist.ts:1146
Using the realpath basename for interpreter detection can miss versioned interpreter targets such as python3.12, allowing inline-eval commands to persist durable approvals.
Confidence: 0.9
[high] Derived safe-bin target dirs skip writable-dir checks — src/infra/exec-safe-bin-trust.ts:175
The patch trusts symlink target dirs without applying the same writable-dir warning or rejection path used for explicit trusted dirs, weakening safe-bin trust boundaries.
Confidence: 0.86

What I checked:

Current-main source reproduces the reported mismatch: Current main records both resolvedPath and resolvedRealPath, but path-shaped allowlist matching still compares patterns against resolvedPath, which matches the linked symlink-vs-realpath bug report. (src/infra/exec-command-resolution.ts:368, c4f20b656eac)
PR proof is real runtime evidence: The PR body includes copied terminal output from a Node/tsx production-module probe using real temp executables and symlinks, showing after-fix realpath allowlist and safe-bin retarget decisions; it does not cover the native approval UI flow. (ac12b39ff400)
Allow Always interpreter guard regression: PR head feeds the trust realpath into candidatePath before calling isInterpreterLikeAllowlistPattern; the existing interpreter helper recognizes fixed basenames, so versioned realpaths such as python3.12 can skip the inline-eval persistence guard. (src/infra/exec-approvals-allowlist.ts:1146, ac12b39ff400)
Derived safe-bin target dirs skip writable-dir handling: PR head derives realpath target directories from configured safe-bin symlinks and adds them to the trusted set, while runtime policy still warns only for explicitly configured safeBinTrustedDirs. (src/infra/exec-safe-bin-trust.ts:175, ac12b39ff400)
Live PR state and protected label: The live PR is open, mergeable, and labeled maintainer and P1, so it should stay open for explicit maintainer handling rather than conservative cleanup close. (ac12b39ff400)
Related item context: The linked bug remains open with source-repro/security-review/product-decision labels, and the older realpath-only PR remains open as a narrower predecessor superseded by this broader PR.

Likely related people:

steipete: Recent history shows repeated merged work on exec approvals, inline-eval routing, safe-bin trust, and command-resolution refactors in the touched files, in addition to authoring this PR. (role: feature-history owner and current PR author; confidence: high; commits: bd0e10a2f6, 3f7e6eebc2, b67e600bff; files: src/infra/exec-approvals-allowlist.ts, src/infra/exec-command-resolution.ts, src/infra/exec-safe-bin-trust.ts)
pgondhi987: Recent commits changed safe-bin argument validation and shell-wrapper allowlist parsing in the same exec-approval allowlist path touched by this PR. (role: recent adjacent contributor; confidence: medium; commits: 9ac4272b35, 50f4440c96, fc065b2693; files: src/infra/exec-approvals-allowlist.ts, src/node-host/invoke-system-run.ts)
hkochar: Recent work touched safe-bin trusted-dir normalization and case-insensitive comparison, adjacent to this PR's derived target-dir trust changes. (role: recent adjacent contributor; confidence: medium; commits: 2d10a2c76f; files: src/infra/exec-safe-bin-trust.ts)

Remaining risk / open question:

Existing symlink-path approvals and docs compatibility still need an explicit migration or doctor-repair decision if realpath-only matching lands.
The PR's runtime proof does not cover the native approval UI flow, though it does cover the policy decision path.

Codex review notes: model gpt-5.5, reasoning high; reviewed against c4f20b656eac.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: caaa942845

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-17T01:55:43Z

    const patternMatches = hasPathSelector(pattern)
-      ? matchesExecAllowlistPattern(pattern, resolvedPath)
+      ? matchesExecAllowlistPattern(pattern, trustPath)
      : pattern !== "*" && matchesExecutableBasenamePattern(pattern, resolution);


Preserve compatibility for existing path allowlist entries

Matching path-shaped allowlist patterns only against trustPath means existing entries saved as symlink paths (for example /opt/homebrew/bin/rg) will stop matching immediately after this upgrade, even when the binary target has not changed. In environments that already persisted those patterns via prior allow-always decisions or manual edits, this becomes a behavior regression (unexpected allowlist miss prompts) rather than a security hardening-only change. Consider matching both candidate and trust paths during a transition window, or migrating stored path patterns to their current realpaths when loading approvals.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 856c5a3f0f

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-17T02:16:38Z

+  const candidatePath = resolveExecutionTargetTrustPath(segment.resolution, params.cwd);
  if (!candidatePath) {
    return;
  }


Keep interpreter guard off trust-path basename in allow-always

collectAllowAlwaysPatterns now feeds the trust realpath into isInterpreterLikeAllowlistPattern, which can hide interpreter identity when symlinks resolve to versioned binaries (for example python3 -> .../python3.12). Because isInterpreterLikeAllowlistPattern only recognizes fixed interpreter names, this path returns false and the function can persist an allow-always entry for interpreter commands that were previously blocked, reintroducing interpreter/inline-eval bypass risk on common package-manager layouts.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fd389029f8

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-17T02:22:19Z

+  const targetDirs = resolveTrustedSafeBinTargetDirs(entries, safeBins);
+  return new Set([...resolveTrustedSafeBinDirs(entries), ...targetDirs]);


Check writability before trusting derived safe-bin target dirs

buildTrustedSafeBinDirs now auto-adds realpath target directories for configured safe bins, but runtime warnings still only inspect explicitly configured safeBinTrustedDirs (resolveExecSafeBinRuntimePolicy calls listWritableExplicitTrustedSafeBinDirs only). This means a writable symlink target directory can become trusted without any warning, so replacing the target binary (for a permitted safe bin like jq) can bypass approval checks silently. Please gate derived target dirs behind the same writability checks (or avoid trusting them automatically) before adding them to the trusted set.

Useful? React with 👍 / 👎.

openclaw-barnacle Bot added agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels May 17, 2026

clawsweeper Bot added the P1 High-priority user-facing bug, regression, or broken workflow. label May 17, 2026

steipete force-pushed the codex/exec-trust-realpath branch 3 times, most recently from 92384ce to caaa942 Compare May 17, 2026 01:50

chatgpt-codex-connector Bot reviewed May 17, 2026

View reviewed changes

steipete force-pushed the codex/exec-trust-realpath branch 4 times, most recently from 47aebe5 to 856c5a3 Compare May 17, 2026 02:11

chatgpt-codex-connector Bot reviewed May 17, 2026

View reviewed changes

steipete force-pushed the codex/exec-trust-realpath branch from 856c5a3 to fd38902 Compare May 17, 2026 02:17

openclaw-barnacle Bot added size: L and removed size: M labels May 17, 2026

chatgpt-codex-connector Bot reviewed May 17, 2026

View reviewed changes

steipete force-pushed the codex/exec-trust-realpath branch from fd38902 to d4165c6 Compare May 17, 2026 02:28

fix(exec): bind approval trust to realpaths

ac12b39

steipete force-pushed the codex/exec-trust-realpath branch from d4165c6 to ac12b39 Compare May 17, 2026 02:32

clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. impact:security Security boundary, credential, authz, sandbox, or sensitive-data risk. labels May 17, 2026

github-actions Bot mentioned this pull request May 17, 2026

🦞 OpenClaw 生态日报 2026-05-17 ivanweng2077/big_model_radar#55

Open

steipete merged commit 524185a into main May 17, 2026
124 of 126 checks passed

steipete deleted the codex/exec-trust-realpath branch May 17, 2026 02:41

steipete mentioned this pull request May 17, 2026

fix(exec/approvals): match executable realpath against allowlist patterns (#45595) #73543

Closed

woodygreen pushed a commit to woodygreen/openclaw that referenced this pull request May 18, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

b73a6fc

clawsweeper Bot mentioned this pull request May 20, 2026

Remove skill prelude exec allowlist #84570

Merged

galiniliev pushed a commit to galiniliev/openclaw that referenced this pull request May 20, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

2a8c5e7

clawsweeper Bot mentioned this pull request May 23, 2026

fix(security): add ReDoS guard to exec approval argPattern matching #82950

Open

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

e7fbe8a

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

c5b7f0f

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

24a14f1

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

752d7fd

galiniliev pushed a commit to galiniliev/openclaw that referenced this pull request May 25, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

ec24ed1

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

663f2c2

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

7f6fae6

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

0ff57a5

jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

4f717a2

SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

528e4a5

sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026

fix(exec): bind approval trust to realpaths (openclaw#82825)

1702613

clawsweeper Bot mentioned this pull request Jun 17, 2026

[codex] fix: allow trusted .openclaw symlink #72713

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[codex] Bind exec approval trust to realpaths#82825

[codex] Bind exec approval trust to realpaths#82825
steipete merged 1 commit into
mainfrom
codex/exec-trust-realpath

steipete commented May 17, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 17, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 17, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 17, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		const targetDirs = resolveTrustedSafeBinTargetDirs(entries, safeBins);
		return new Set([...resolveTrustedSafeBinDirs(entries), ...targetDirs]);

Uh oh!

Uh oh!

Conversation

steipete commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Real behavior proof

Uh oh!

clawsweeper Bot commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 17, 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 May 17, 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 May 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

steipete commented May 17, 2026 •

edited

Loading

clawsweeper Bot commented May 17, 2026 •

edited

Loading