fix: jail plugin-controlled paths to the plugin root

[SebTardif]

What Problem This Solves

plugin-inspector inspect --no-openclaw and check resolve plugin-owned path strings (package.json main / module / exports, OpenClaw entrypoints, sourceRoot, openclaw.defaultCheckoutPath) with path.resolve and then existsSync / readFile. There is no jail to the plugin root.

On Windows, path.resolve(pluginDir, "//evil.example/share/x.js") becomes a UNC path (\\evil.example\share\x.js). existsSync on that specifier can contact a remote SMB share and leak NTLM material from a plugin the operator thought was offline and credential-free. Absolute paths and ../ also read host files outside the plugin.

invalidPackageFileSpec already rejects / and .. for package.json files. This change applies the same idea after resolve: reject UNC, absolute, and escaped specifiers, and keep only paths inside the plugin root.

A rejected sourceRoot no longer falls back to scanning the whole checkout. A rejected plugin defaultCheckoutPath no longer falls through to ./openclaw. The report now emits target-openclaw-rejected with a --openclaw / openclawPath migration. README and the copy-ready example no longer document ../openclaw as plugin checkout config. Parent-directory checks use the .. segment, so in-root names like ..generated/index.js stay valid. Only --openclaw / options.openclawPath is the operator override. Jail helpers stay internal; they are lexical path checks, not a public symlink-confinement API.

Evidence

Live node of the patched helpers. A parent escape is rejected. An in-root ..generated name is kept. A bad sourceRoot throws instead of scanning a replacement tree. A plugin-manifest ../fake-openclaw is rejected. The same path as configuredPath still resolves as the operator route.

$ node /tmp/proof-pi76-live.mjs
dotted_name true
parent_escape null
sourceRoot_threw true
sourceRoot_error sourceRoot "../shared-plugin" is outside the plugin root; refuse to scan a replacement tree
candidates_with_rootDir []
candidates_operator [ '../target' ]
plugin_manifest_status rejected
operator_status ok
inspectPluginRoot_plugin_status rejected
inspectPluginRoot_operator_status ok
DONE

$ node --input-type=module -e 'inspectPluginRoot({ pluginRoot, ... })'
status rejected
message plugin defaultCheckoutPath "../openclaw" is outside the plugin root; pass --openclaw / openclawPath to compare against a sibling checkout
warning plugin defaultCheckoutPath "../openclaw" is outside the plugin root; pass --openclaw / openclawPath to compare against a sibling checkout

Earlier Windows 11 inspect CLI at f33adf2 still stands: UNC main and ../ entry were not added to sourceFiles.

Real behavior proof

• Behavior or issue addressed: Plugin-controlled path strings no longer resolve to UNC shares, absolute host files, or parent-directory escapes. Invalid sourceRoot fails instead of scanning the checkout. Rejected plugin defaultCheckoutPath is reported instead of silently selecting ./openclaw.
• Real environment tested: macOS (Darwin 25.6.0 arm64), Node v26.8.2, checkout /private/tmp/pi76-p1 on fix/plugin-path-jail.
• Exact steps or command run after this patch: Ran node /tmp/proof-pi76-live.mjs against the patched helpers and against production inspectPluginRoot with a plugin-owned ../fake-openclaw setting, then again with openclawPath: "../fake-openclaw".
• Evidence after fix: terminal output above. Production inspectPluginRoot prints target-openclaw-rejected and the --openclaw / openclawPath migration. inspectPluginRoot_plugin_status rejected, inspectPluginRoot_operator_status ok.
• Observed result after fix: The old documented ../openclaw plugin setting is rejected with a migration message. Only an explicit operator openclawPath reads the sibling checkout. README and the copy-ready example no longer advertise that rejected setting.
• What was not tested: Live SMB authentication against a real remote share. Symlink or Windows junction confinement before I/O. The public contract is now lexical-only and internal.

Crabpot source-mode smoke against this tip (CRABPOT_PLUGIN_INSPECTOR_DIR=/private/tmp/pi76-p1 CRABPOT_PLUGIN_INSPECTOR_CLI=source npm run plugin-inspector:smoke): Status FAIL, 60 fixtures, 83 breakages. The same command against current plugin-inspector main also FAIL, 60 fixtures, 83 breakages, same top findings (agentchat / wecom missing-expected-seam). This PR does not change that consumer baseline.

Summary

Jail plugin-controlled paths to the plugin root. Fail closed on invalid sourceRoot. Report rejected plugin checkout paths instead of substituting another tree. Keep --openclaw / configuredPath as the operator sibling route.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 26, 2026, 4:31 AM ET / 08:31 UTC (Revision 6).

ClawSweeper review

What this changes

The branch rejects plugin-supplied path escapes during inspection, reports rejected OpenClaw checkout settings with an operator migration message, and adds documentation and tests.

Merge readiness

⛔ Blocked before merge - 6 items remain

Current main still resolves plugin-controlled paths without these guards, so this PR remains useful. The earlier line-level findings are addressed, and the supplied live trace supports the lexical checks. Maintainer approval is still needed for the configuration break and the stated symlink and junction limit.

Priority: P2
Reviewed head: b218d202ff46d30abcad704d2e4f1209d99baeb9
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused implementation and live lexical-path trace are useful, but the physical-path security boundary and upgrade behavior remain unresolved.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: Authority-chain proof required: the macOS Node trace exercises production inspectPluginRoot and shows rejection of a plugin ../ checkout while the operator override succeeds. It does not establish that an in-root symlink or junction is rejected before the final filesystem read; no stored-data model changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Authority-chain proof required: the macOS Node trace exercises production inspectPluginRoot and shows rejection of a plugin ../ checkout while the operator override succeeds. It does not establish that an in-root symlink or junction is rejected before the final filesystem read; no stored-data model changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	6 items	Current main still needs the change: Current main resolves fixture source roots with path.join and accepts plugin checkout candidates without the proposed jail. Introduced path guard: The new helper rejects absolute and escaping specifiers before returning a path for inspection; its comment expressly limits the check to lexical paths. Filesystem boundary remains: An accepted entrypoint reaches existsSync. The lexical helper does not resolve symlinks or Windows junctions before that I/O.
Findings	None	None.
Security	Needs attention	Physical paths remain outside the lexical guard: The helper returns an in-root lexical path without checking its filesystem target; the subsequent existence and read paths can follow a symlink or junction beyond the root.

How this fits together

Plugin Inspector reads plugin manifests, package metadata, and optional OpenClaw checkout settings to produce inspection and compatibility reports. These path checks sit between plugin-supplied strings and filesystem reads.

flowchart LR
A[Plugin files and config] --> B[Path checks]
B --> C{Inside plugin root?}
C -->|Yes| D[Filesystem inspection]
C -->|No| E[Rejection diagnostic]
D --> F[Inspection report]
E --> F

Loading

Decision needed

Question	Recommendation
May plugin-controlled paths remain lexically confined while symlinks and junctions can lead outside the plugin root, and may existing external checkout settings fail closed by default?	Require physical confinement: Keep the useful lexical checks, but prevent plugin-controlled symlinks and junctions from reaching outside files before filesystem I/O and verify the upgrade path.

Why: The branch deliberately narrows the security guarantee and changes a previously documented configuration path; accepting both effects requires repository-owner judgment.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the macOS Node trace exercises production inspectPluginRoot and shows rejection of a plugin ../ checkout while the operator override succeeds. It does not establish that an in-root symlink or junction is rejected before the final filesystem read; no stored-data model changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve security concern: Physical paths remain outside the lexical guard - The helper returns an in-root lexical path without checking its filesystem target; the subsequent existence and read paths can follow a symlink or junction beyond the root.
• Resolve merge risk (P1) - Existing configurations that place sourceRoot or defaultCheckoutPath outside the plugin root stop working; sibling-checkout users must move selection to the operator override.
• Resolve merge risk (P1) - An in-root symlink or Windows junction can still lead filesystem reads outside the plugin root. The proposed lexical boundary has no final-I/O proof for that nearest forbidden path.
• Complete next step (P2) - Obtain an owner decision on physical-path confinement and the configuration break, then provide final-I/O proof for the chosen security boundary before merge.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [medium] Physical paths remain outside the lexical guard — src/path-utils.js:44

Agent review details

Security

Needs attention: The guard blocks lexical escapes, but an accepted symlink or junction can still direct a filesystem read outside the plugin root.

Review metrics

Metric	Value	Why it matters
Code and test delta	production +127/−10 lines; tests +345/−0 lines	Most growth is focused regression coverage for the new path boundary.

Merge-risk options

Maintainer options:

1. Complete the boundary before merge (recommended)
   Cover symlink and junction escapes before final I/O and verify both existing-config upgrades and the explicit operator route.
2. Accept lexical scope
   A repository owner can explicitly accept the remaining physical-path exposure and documented configuration break.
3. Pause for contract design
   Keep the PR open while deciding whether a compatible strict mode better serves existing users.

Technical review

Best possible solution:

Approve an explicit filesystem boundary and upgrade contract, then retain the narrow path guard with a tested migration route and final-I/O coverage appropriate to that boundary.

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

Yes for the reported lexical escapes: the PR supplies a live production-entrypoint trace and focused tests. The remaining symlink or junction path was not exercised.

Is this the best way to solve the issue?

Unclear. Lexical rejection is a focused improvement, but it does not establish the full filesystem boundary suggested by the security goal, and the compatibility cost needs owner acceptance.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 6b7bad3d0c1d.

Labels

Label changes:

No label changes.

Label justifications:

• P2: This is a bounded inspector security improvement with a material configuration decision before merge.
• merge-risk: 🚨 compatibility: Previously accepted external source and checkout settings now fail closed and require operator migration.
• merge-risk: 🚨 security-boundary: The new lexical guard still permits an in-root symlink or junction to reach outside files.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the macOS Node trace exercises production inspectPluginRoot and shows rejection of a plugin ../ checkout while the operator override succeeds. It does not establish that an in-root symlink or junction is rejected before the final filesystem read; no stored-data model changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

• [medium] Physical paths remain outside the lexical guard — src/path-utils.js:44
  The helper returns an in-root lexical path without checking its filesystem target; the subsequent existence and read paths can follow a symlink or junction beyond the root.
  Confidence: 0.91

What I checked:

• Current main still needs the change: Current main resolves fixture source roots with path.join and accepts plugin checkout candidates without the proposed jail. (src/config.js:124, 6b7bad3d0c1d)
• Introduced path guard: The new helper rejects absolute and escaping specifiers before returning a path for inspection; its comment expressly limits the check to lexical paths. (src/path-utils.js:37, b218d202ff46)
• Filesystem boundary remains: An accepted entrypoint reaches existsSync. The lexical helper does not resolve symlinks or Windows junctions before that I/O. (src/inspector.js:650, b218d202ff46)
• Upgrade behavior: A plugin-provided external defaultCheckoutPath now returns rejected rather than falling through to checkout discovery; the report directs operators to an explicit override. (src/openclaw-target.js:17, b218d202ff46)
• Production-path proof and prior review: The PR body records a macOS Node run through inspectPluginRoot: plugin ../fake-openclaw was rejected while the operator override succeeded. The contributor also reports the crabpot smoke had the same 60-fixture, 83-breakage result on main and this head. The previous completed review lists no remaining line-level findings. (b218d202ff46)
• Feature history: Current-main history connects Vincent Koc to plugin-root configuration and security sanitizers, and Sebastien Tardif to prior merged inspector runtime work. (src/config.js:120, 6b7bad3d0c1d)

Likely related people:

• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Sebastien Tardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Patrick Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add final-effect proof that the nearest forbidden path through an in-root symlink or junction is stopped before filesystem I/O.
• Obtain owner approval for the external sourceRoot and defaultCheckoutPath upgrade behavior.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)

• reviewed 2026-09-11T03:23:35.506Z sha f33adf2 :: needs real behavior proof before merge. :: [P1] Reject invalid source roots instead of scanning a replacement tree | [P1] Make external checkout rejection explicit before using fallbacks | [P2] Check parent-directory segments rather than a two-dot prefix
• reviewed 2026-09-18T18:20:46.463Z sha abbdc90 :: needs real behavior proof before merge. :: [P1] Keep plugin checkout configuration out of the operator override
• reviewed 2026-09-18T18:46:59.692Z sha e8bd395 :: needs real behavior proof before merge. :: [P1] Make rejected legacy checkout configuration actionable
• reviewed 2026-09-18T18:54:10.049Z sha 9b2b44c :: needs real behavior proof before merge. :: none
• reviewed 2026-09-18T18:59:29.271Z sha b218d20 :: blocked before merge. :: none

[SebTardif]

@clawsweeper re-review

The three named findings are in the tip.

• Invalid sourceRoot now throws instead of scanning the checkout as a replacement tree.
• A rejected plugin defaultCheckoutPath returns status: "rejected" and does not fall through to ./openclaw. Operator configuredPath / --openclaw is still the sibling-checkout route.
• Parent-directory checks use the .. segment, so in-root names like ..generated/index.js stay valid.

Live node /tmp/proof-pi76-live.mjs: dotted_name true, parent_escape null, sourceRoot_threw true, plugin_manifest_status rejected, operator_status ok.

Owner decision still needed: should the public jail helpers guarantee realpath/symlink confinement before I/O, or stay lexical with that limit documented? This PR does not claim the stronger contract.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[SebTardif]

@clawsweeper re-review

P1 keep-plugin-checkout-out-of-operator-override is in the tip.

inspectCompatibilityFixtureSet now passes only options.openclawPath as configuredPath. Plugin defaultCheckoutPath stays on the manifest path and is rejected. Production inspectPluginRoot with a plugin-owned ../fake-openclaw setting returns rejected. The same tree with openclawPath: "../fake-openclaw" returns ok.

Merged current main (5dd7e9c) to clear the dirty merge state.

Live node /tmp/proof-pi76-live.mjs: inspectPluginRoot_plugin_status rejected, inspectPluginRoot_operator_status ok.

Owner decision still needed on whether the public jail helpers guarantee realpath/symlink confinement. This PR does not claim that contract.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/35382315969
• Updated: 2026-09-18T18:49:13.685Z

[SebTardif]

@clawsweeper re-review

P1 make-rejected-legacy-checkout-actionable is in the tip.

Rejected plugin defaultCheckoutPath now carries an explicit --openclaw / openclawPath diagnostic. inspectPluginRoot reports target-openclaw-rejected instead of a generic unavailable log. README and examples/plugin-inspector.config.json no longer tell operators to put ../openclaw in plugin checkout config.

Live production inspectPluginRoot with the old documented ../openclaw setting:

status rejected
message plugin defaultCheckoutPath "../openclaw" is outside the plugin root; pass --openclaw / openclawPath to compare against a sibling checkout
warning plugin defaultCheckoutPath "../openclaw" is outside the plugin root; pass --openclaw / openclawPath to compare against a sibling checkout

Owner decision still needed on whether the public jail helpers stay lexical or guarantee realpath/symlink confinement.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[SebTardif]

@clawsweeper re-review

Findings were already none on 9b2b44c. This tip follows the recommended jail contract: resolveJailedPluginPath and isWithinPluginRoot are no longer exported from the advanced public API. README states they are lexical path checks and do not follow or reject symlinks or Windows junctions.

Crabpot source-mode smoke against this tip: FAIL, 60 fixtures, 83 breakages. The same smoke against plugin-inspector main is also FAIL, 60/83, same top agentchat / wecom missing-expected-seam findings. This PR does not change that baseline.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.