fix(plugins): restrict bundled plugin dir resolution to trusted package roots#73275
fix(plugins): restrict bundled plugin dir resolution to trusted package roots#73275pgondhi987 merged 20 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR hardens Confidence Score: 5/5Safe to merge — the fix is minimal, logically sound, and directly backed by a regression test. No P0 or P1 findings. The security hardening is correct: No files require special attention. Reviews (2): Last reviewed commit: "fix: address PR review feedback" | Re-trigger Greptile |
|
@greptile review |
|
Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Keep open. This PR is protected by MEMBER author association and the Best possible solution: Keep this PR open for explicit maintainer review. If accepted, land the bundled plugin directory trust hardening with focused regression coverage after reviewing override/source-checkout compatibility, packaged and non-standard launcher behavior, and the Signal installer fetch/HTTPS handling; otherwise close it manually with a maintainer rationale. What I checked:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against b79e617ad12c. |
|
Addressed in Quoted comment from @aisle-research-bot:
|
|
Addressed in Quoted comment from @clawsweeper:
|
|
Addressed in Quoted comment from @aisle-research-bot:
|
|
Addressed in Quoted comment from @clawsweeper:
|
|
Addressed in Quoted comment from @aisle-research-bot:
|
|
Addressed in Quoted comment from @clawsweeper:
|
|
Addressed in Quoted comment from @clawsweeper:
|
|
Addressed in Quoted comment from @clawsweeper:
|
|
Addressed in Quoted comment from @aisle-research-bot:
|
|
Addressed in Quoted comment from @aisle-research-bot:
|
|
Addressed in Quoted comment from @aisle-research-bot:
|
…ge roots (openclaw#73275) * fix: address issue * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address codex review feedback * fix: address codex review feedback * fix: address codex review feedback * fix: address PR review feedback * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address review feedback * docs: add changelog entry for PR merge
Summary
resolveBundledPluginsDirincludedcwdin the package-root candidate list even in non-source-checkout (production) mode. An attacker-controlled working directory shaped like an OpenClaw source checkout could override the bundled plugin root and supply a maliciousextensions/memory-core/runtime-api.js, reachable viadoctor.memory.*gateway RPC paths.argv1doesn't resolve the real package root (single-binary, custom launcher), the fallback pathargv1 → cwd → moduleUrlgave attacker-controlled directories priority over the real module root.cwdis now excluded from the resolution candidate list whenpreferSourceCheckoutis false (production mode). The override-path fallback also addsmoduleUrlalongsideargv1as a trusted candidate.cwdremains trusted there for local dev.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
resolveBundledPluginsDirincluded{ cwd: process.cwd() }unconditionally in the non-preferSourceCheckoutpath.resolveOpenClawPackageRootSyncaccepts any directory that contains apackage.jsonwithname: "openclaw", so a crafted working directory satisfies the check.cwdwas excluded from the candidate list in production mode.argv1does not resolve to the real package root.Regression Test Plan (if applicable)
src/plugins/bundled-dir.test.tsargv1is a non-package path (/usr/bin/env) andcwdis a crafted OpenClaw-shaped directory,resolveBundledPluginsDirmust not return the cwd-rooted extensions tree.User-visible / Behavior Changes
None in standard installed use. Developers running
openclawdirectly from a source checkout via a packaged binary without vitest or tsx are unaffected becausemoduleUrlresolution picks up the same root.Diagram (if applicable)
Security Impact (required)
argv1andmoduleUrl, both of which reflect the actual installed or running package, not the process working directory.Repro + Verification
Environment
Steps
mkdir /tmp/evil && echo '{"name":"openclaw"}' > /tmp/evil/package.json && mkdir -p /tmp/evil/.git /tmp/evil/src /tmp/evil/extensions/memory-core && echo "throw new Error('pwned')" > /tmp/evil/extensions/memory-core/runtime-api.jsargv1pointing to a non-package path (e.g., via a single-binary launcher) andcwd=/tmp/evil.doctor.memory.*RPC path.Expected
/tmp/evil/extensionsis never used.Actual (before fix)
/tmp/evil/extensionswas selected as the bundled plugin dir.Evidence
does not resolve bundled plugins from cwd when argv1 is not a package rootinsrc/plugins/bundled-dir.test.tscovers the exact attack preconditions and fails on the pre-fix code.Human Verification (required)
cwdRootfrom candidates whenpreferSourceCheckoutis false; override-fallback path addsmoduleUrlas a second trusted candidate.preferSourceCheckout=true(vitest/tsx) retainscwd; deduplication filter handles identical roots;isSourceCheckoutRootguard still excludes source checkouts from the override fallback.Review Conversations
Compatibility / Migration
Risks and Mitigations
argv1andmoduleUrlboth fail to resolve the package root could lose bundled plugin resolution fallback viacwd.bundled-dir.ts) still provide coverage for edge cases.