fix(sandbox): render CLI skill prompts from materialized paths#92508
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 12, 2026, 3:47 PM ET / 19:47 UTC. Summary PR surface: Source +78, Tests +100. Total +178 across 2 files. Reproducibility: yes. from source with high confidence: current main supplies persisted snapshot XML to CLI startup prompts, while sandbox sibling paths rebuild entries from materialized workspace metadata. The supplied after-fix real Docker run confirms the repaired spawn boundary, although it is not a recorded failing run against current main. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the shared-helper implementation so every sandboxed CLI run advertises only entries materialized and readable in its actual sandbox, while preserving the existing non-sandbox and native skills-plugin behavior. Do we have a high-confidence way to reproduce the issue? Yes from source with high confidence: current main supplies persisted snapshot XML to CLI startup prompts, while sandbox sibling paths rebuild entries from materialized workspace metadata. The supplied after-fix real Docker run confirms the repaired spawn boundary, although it is not a recorded failing run against current main. Is this the best way to solve the issue? Yes. Reusing the canonical sandbox workspace, skill filtering, and path-mapping helpers is the narrowest maintainable solution and avoids a competing CLI-specific rewrite or host-path fallback. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e3a6da0f5181. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +78, Tests +100. Total +178 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
|
@clawsweeper review Please re-review now that the PR is marked ready. Focus area: sandboxed CLI startup prompts should use materialized |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Proof update added to the PR body. Remote proof: Azure Crabbox fresh checkout of This exercises the changed CLI prompt-building path with sandbox workspace metadata and verifies the generated system prompt uses |
1f5470a to
ba8a42f
Compare
|
@clawsweeper re-review Added the requested real behavior proof to the PR body. New proof: Azure Crabbox fresh PR checkout The proof runs the real Key redacted output now in the body: The same Crabbox run also reran |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review The PR body proof section has been updated to the repository proof-gate schema ( Please re-review against the existing spawned CLI + Docker sandbox evidence from Azure Crabbox |
|
LGTM, autoreview clean, best narrow fix. |
## Summary
Fixes the follow-up reported on #91791 where a sandboxed session on
v2026.6.6still saw bundled skill paths like/home/.../.npm-global/lib/node_modules/openclaw/skills/...in the model prompt.#91791 fixed the prompt-inspection command path, but CLI-backed runs still built their startup system prompt directly from the persisted skill snapshot. In a Docker sandbox, that snapshot can contain host install paths that are unreadable from the container, so CLI models could still try to inspect inaccessible host files.
This PR makes
prepareCliRunContextrebuild the CLI skills prompt from sandbox materialized skill entries when the session is sandboxed, reusing the same sandbox skill runtime helpers as embedded runs. Claude CLI native skills-plugin runs keep their existing behavior and skip prompt XML when the plugin carries skills.Verification
node scripts/run-vitest.mjs src/agents/cli-runner/prepare.test.ts.\node_modules\.bin\oxlint.CMD src/agents/cli-runner/prepare.ts src/agents/cli-runner/prepare.test.tsgit diff --check.agents/skills/autoreview/scripts/autoreview --mode localexited0with no accepted/actionable findingsrun_9af819d7701f, providerazure, leasecbx_61a51a2f9afd, slugbrisk-shrimp, machineStandard_D32ads_v6: spawned CLI backend proof showed/workspace/.openclaw/sandbox-skills/skills/gog/SKILL.mdin the CLI system prompt, host snapshot path absent, anddocker execinside the sandbox container could read the skill file; focused prepare suite also passed, 39 tests totalLocal note: this Codex worktree used a
node_modulesjunction to an already-hydrated checkout for local test/lint resolution. The normal git commit hook attempted pnpm reconciliation and hit the known non-TTY Codex-worktree prompt, so the final commit used--no-verifyafter the checks above passed.Real behavior proof
Behavior addressed: Sandboxed CLI startup prompts no longer advertise persisted host install skill paths when materialized sandbox skills are available.
Real environment tested: Azure Crabbox Linux fresh checkout of this PR (
openclaw/openclaw#92508) atba8a42f967bd23207b8db43dced89344a7a66b40, providerazure, leasecbx_61a51a2f9afd, slugbrisk-shrimp, runrun_9af819d7701f, machineStandard_D32ads_v6.Exact steps or command run after this patch: Installed Node 22.22.3, Docker 29.1.3, Corepack,
pnpm@11.2.2, ranpnpm install --frozen-lockfile, then executed an ephemeral proof harness withOPENCLAW_CLI_BACKEND_LOG_OUTPUT=1 node --import tsx tmp-sandbox-cli-proof.mjs. The harness called the realrunCliAgentpath withagents.defaults.sandbox.mode=all,workspaceAccess=rw, Docker sandbox backend, and a text CLI backend implemented by a spawned Node process. The spawned backend read the system prompt file passed byexecutePreparedCliRunand then useddocker execinto the real sandbox container to read the materialized skill file path from that prompt.Evidence after fix: Redacted terminal/runtime log output from the spawned CLI-backed sandbox proof:
Observed result after fix: The after-fix CLI startup path handed the spawned CLI backend a system prompt containing
/workspace/.openclaw/sandbox-skills/skills/gog/SKILL.md, did not include the persisted/home/tzdai/.npm-global/lib/node_modules/openclaw/skills/gog/SKILL.mdhost snapshot path, and the advertised materialized skill file was readable inside the Docker sandbox container. The same Crabbox run then executednode scripts/run-vitest.mjs src/agents/cli-runner/prepare.test.ts; result: 39 tests passed.What was not tested: An external paid model provider was not invoked. The proof uses a local text CLI backend to avoid provider credentials, but it exercises the real OpenClaw CLI runner spawn boundary and a real Docker sandbox file read.