fix(uninstall): refuse to remove current working directory during cleanup by xydigit-sj · Pull Request #90813 · openclaw/openclaw

xydigit-sj · 2026-06-06T02:25:51Z

Summary

openclaw uninstall --all --yes --non-interactive could delete the caller's current working directory when the resolved workspace or state path matched process.cwd() or contained it. The removePath helper already rejected empty strings, filesystem root, and the home directory, but it did not guard against the active working directory.

Changes:

src/commands/cleanup-utils.ts: isUnsafeRemovalTarget now refuses targets that are or contain process.cwd().
src/commands/cleanup-utils.test.ts: regression tests proving removePath rejects cwd and a directory containing cwd with ok: false and logs an error.
docs/install/uninstall.md: moves --dry-run --all ahead of the non-interactive stanza and adds a caution note.

Verification

node scripts/run-vitest.mjs src/commands/cleanup-utils.test.ts --run passes (10/10).
node scripts/run-oxlint.mjs src/commands/cleanup-utils.ts src/commands/cleanup-utils.test.ts passes.
pnpm exec oxfmt --write src/commands/cleanup-utils.ts src/commands/cleanup-utils.test.ts docs/install/uninstall.md applied.
Crabbox local-container command-level proof on PR head 8cfb48d7d9e6c264ce0a407bea3efeb604f5ecf7 passed: built the CLI on Node.js v24.16.0, ran openclaw uninstall --all --yes --non-interactive from inside a configured workspace, observed the unsafe-path refusal, and verified the workspace still existed.

Real behavior proof

Behavior addressed: openclaw uninstall --all --yes --non-interactive could delete the current working directory (#90806).

Real environment tested: Local OpenClaw source checkout on branch fix/uninstall-cwd-guard, Linux, Node.js 22.

Exact steps or command run after this patch:

node --import tsx -e '
import { removePath } from "./src/commands/cleanup-utils.js";
import path from "node:path";
import fs from "node:fs/promises";
import os from "node:os";

const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-uninstall-cwd-"));
const originalCwd = process.cwd();
process.chdir(tmpDir);

const runtime = {
  log: (msg) => console.log("LOG:", msg),
  error: (msg) => console.log("ERROR:", msg),
};

const result = await removePath(process.cwd(), runtime, { dryRun: false });

console.log("result.ok:", result.ok);
console.log("result.skipped:", result.skipped);

const stillExists = await fs.access(tmpDir).then(() => true).catch(() => false);
console.log("cwd still exists:", stillExists);

process.chdir(originalCwd);
await fs.rm(tmpDir, { recursive: true, force: true });
'

Evidence after fix: Terminal capture from running the patched removePath against the current working directory:

ERROR: Refusing to remove unsafe path: /tmp/openclaw-uninstall-cwd-MhMtZz
result.ok: false
result.skipped: undefined
cwd still exists: true

Observed result after fix:

removePath(process.cwd()) returns { ok: false } instead of deleting the directory.
An error is logged: Refusing to remove unsafe path: ....
The temporary cwd directory still exists after the call.

Additional maintainer proof: Command-level uninstall proof in Crabbox local-container.

Real environment tested: Crabbox local-container via Podman, node:24-bookworm, Node.js v24.16.0, PR head 8cfb48d7d9e6c264ce0a407bea3efeb604f5ecf7.

Exact steps or command run after this patch:

node scripts/crabbox-wrapper.mjs run \
  --provider local-container \
  --local-container-runtime podman \
  --local-container-image node:24-bookworm \
  --fresh-pr openclaw/openclaw#90813 \
  --idle-timeout 30m \
  --ttl 60m \
  --timing-json \
  --shell -- 'set -euo pipefail; repo=$(pwd); echo PROOF_NODE=$(node -v); echo PROOF_HEAD=$(git rev-parse HEAD); git merge-base --is-ancestor 8cfb48d7d9e6c264ce0a407bea3efeb604f5ecf7 HEAD; env CI=1 corepack pnpm install --frozen-lockfile >/tmp/openclaw-install.log; env CI=1 corepack pnpm build >/tmp/openclaw-build.log; tmp=$(mktemp -d); home=$tmp/home; state=$tmp/state; workspace=$tmp/workspace; nested=$workspace/nested; mkdir -p "$home" "$state" "$nested"; printf "{\"agents\":{\"defaults\":{\"workspace\":\"%s\"}}}\n" "$workspace" > "$state/openclaw.json"; echo PROOF_WORKSPACE=$workspace; echo PROOF_CWD=$nested; set +e; output=$(cd "$nested" && env HOME="$home" OPENCLAW_STATE_DIR="$state" OPENCLAW_CONFIG_PATH="$state/openclaw.json" OPENCLAW_WORKSPACE_DIR="$workspace" node "$repo/dist/entry.js" uninstall --all --yes --non-interactive 2>&1); status=$?; set -e; echo PROOF_EXIT=$status; printf "%s\n" "$output"; printf "%s\n" "$output" | grep -F "Refusing to remove unsafe path: $workspace"; test -d "$workspace"; test -d "$nested"; echo PROOF_WORKSPACE_STILL_EXISTS=1; rm -rf "$tmp"'

Evidence after fix: Terminal capture from running the built CLI against a workspace that contains the command's current working directory:

PROOF_NODE=v24.16.0
PROOF_HEAD=8cfb48d7d9e6c264ce0a407bea3efeb604f5ecf7
PROOF_WORKSPACE=/tmp/tmp.oniUmE1Nc8/workspace
PROOF_CWD=/tmp/tmp.oniUmE1Nc8/workspace/nested
PROOF_EXIT=0
Recommended first: openclaw backup create
Gateway service disabled.
Removed /tmp/tmp.oniUmE1Nc8/state
Refusing to remove unsafe path: /tmp/tmp.oniUmE1Nc8/workspace
CLI still installed. Remove via npm/pnpm if desired.
Refusing to remove unsafe path: /tmp/tmp.oniUmE1Nc8/workspace
PROOF_WORKSPACE_STILL_EXISTS=1

Observed result after fix:

openclaw uninstall --all --yes --non-interactive refuses to remove the configured workspace when the command runs from a nested directory inside that workspace.
The command logs Refusing to remove unsafe path: ....
The workspace and nested cwd still exist after the command.

What was not tested: A packaged global npm install. The proof built this PR's CLI from source in an isolated container and exercised the same uninstall --all --yes --non-interactive command path.

…anup

clawsweeper · 2026-06-06T02:27:45Z

Codex review: needs real behavior proof before merge. Reviewed June 6, 2026, 1:09 AM ET / 05:09 UTC.

Summary
The PR tightens shared cleanup removal to refuse the current working directory or an ancestor, adds regression tests, and updates uninstall docs to recommend dry-run before non-interactive removal.

PR surface: Source +3, Tests +35, Docs +6. Total +44 across 3 files.

Reproducibility: yes. from source inspection, though not live-executed in this read-only review. Current main's removePath reaches fs.rm after only empty/root/home guards, and uninstall passes resolved state/workspace paths into that shared cleanup path.

Review metrics: 1 noteworthy metric.

Real proof coverage: 1 of 2 guard cases shown. The supplied terminal proof covers cwd itself, while the newer ancestor-of-cwd guard is currently proven only by a unit test.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦞 diamond lobster
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted terminal output from final head showing removePath refuses a parent directory containing process.cwd(), or equivalent live uninstall proof from a throwaway setup.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: Terminal proof shows removePath refuses cwd, but final head also adds ancestor-of-cwd refusal and that path is not shown in real output yet. 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.

Risk before merge

[P1] Final-head real behavior proof covers refusing cwd itself but not the ancestor-of-cwd guard added later, so maintainers should request updated terminal/live proof or provide their own before merge.

Maintainer options:

Decide the mitigation before merge
Land this shared cleanup guard once final-head proof covers the cwd and ancestor cases, then close the linked data-loss issue as fixed by the merged PR.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No ClawSweeper repair job is needed; maintainers should wait for updated final-head real behavior proof or provide their own before merging.

Security
Cleared: The diff tightens a destructive filesystem guard and updates docs/tests; it does not add supply-chain, secret-handling, or new code-execution surface.

Review details

Best possible solution:

Land this shared cleanup guard once final-head proof covers the cwd and ancestor cases, then close the linked data-loss issue as fixed by the merged PR.

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

Yes from source inspection, though not live-executed in this read-only review. Current main's removePath reaches fs.rm after only empty/root/home guards, and uninstall passes resolved state/workspace paths into that shared cleanup path.

Is this the best way to solve the issue?

Yes, this is the best code boundary: guarding removePath protects uninstall, reset, and shared cleanup callers instead of patching one CLI stanza. The remaining issue is proof freshness for the second commit, not the implementation shape.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Terminal proof shows removePath refuses cwd, but final head also adds ancestor-of-cwd refusal and that path is not shown in real output yet. 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.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

P0: The PR addresses a destructive uninstall path that could delete the caller's current working directory, matching the data-loss emergency rubric.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Terminal proof shows removePath refuses cwd, but final head also adds ancestor-of-cwd refusal and that path is not shown in real output yet. 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

PR surface:

Source +3, Tests +35, Docs +6. Total +44 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	0	+3
Tests	1	35	0	+35
Docs	1	7	1	+6
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	45	1	+44

What I checked:

Current main removal guard: Current main rejects empty, filesystem root, and home directory removal targets, then returns false; cwd and ancestor-of-cwd targets are not protected before fs.rm. (src/commands/cleanup-utils.ts:73, ab7c92282522)
Uninstall entry point: The uninstall flow resolves state/config/workspace paths and calls removeStateAndLinkedPaths and removeWorkspaceDirs for selected state/workspace scopes, so the shared removePath guard is on the destructive path. (src/commands/uninstall.ts:202, ab7c92282522)
PR head implementation: The head commit adds isPathWithin(path.resolve(process.cwd()), resolved) before the fs.rm branch, covering both cwd itself and removal targets that contain cwd. (src/commands/cleanup-utils.ts:86, 8cfb48d7d9e6)
Regression coverage: The PR adds tests for refusing removePath(process.cwd()) and refusing a parent directory containing the current working directory. (src/commands/cleanup-utils.test.ts:206, 8cfb48d7d9e6)
Dependency contract: The inspected @openclaw/fs-safe 0.3.0 path implementation returns true for equality and nested targets, matching the PR's use of isPathWithin(cwd, target).
CI evidence: Public check-runs for head 8cfb48d show the relevant command/core runtime lanes completed successfully; skipped matrix jobs and neutral CodeQL were not treated as patch defects. (8cfb48d7d9e6)

Likely related people:

Shakker: git blame on current main attributes cleanup-utils and uninstall destructive cleanup paths to commit 5a0f9cb; the history is shallow/grafted, so this is the strongest current-main owner signal available. (role: introduced current cleanup surface; confidence: medium; commits: 5a0f9cb03cd1; files: src/commands/cleanup-utils.ts, src/commands/uninstall.ts)
sallyom: The PR timeline shows assignment to sallyom, and the second head commit adds the ancestor-of-cwd guard and test on the same cleanup helper. (role: recent follow-up owner; confidence: high; commits: 8cfb48d7d9e6; files: src/commands/cleanup-utils.ts, src/commands/cleanup-utils.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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 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.

…anup (openclaw#90813) * fix(uninstall): refuse to remove current working directory during cleanup * fix(uninstall): guard cleanup ancestors of cwd --------- Co-authored-by: sallyom <somalley@redhat.com>

fix(uninstall): refuse to remove current working directory during cle…

b154e83

…anup

openclaw-barnacle Bot added docs Improvements or additions to documentation commands Command implementations size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 6, 2026

This was referenced Jun 6, 2026

🦞 OpenClaw 生态日报 2026-06-06 jasonalang/big_model_radar#47

Open

🦞 OpenClaw 生态日报 2026-06-06 litang9/big_model_radar#22

Open

sallyom self-assigned this Jun 6, 2026

fix(uninstall): guard cleanup ancestors of cwd

8cfb48d

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 6, 2026

sallyom merged commit 743051d into openclaw:main Jun 6, 2026
184 of 190 checks passed

github-actions Bot mentioned this pull request Jun 6, 2026

📡 Upstream Digest — 2026-06-06 06:48 UTC curtismercier/openclaw-mods#1023

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(uninstall): refuse to remove current working directory during cleanup#90813

fix(uninstall): refuse to remove current working directory during cleanup#90813
sallyom merged 2 commits into
openclaw:mainfrom
xydigit-sj:fix/uninstall-cwd-guard

xydigit-sj commented Jun 6, 2026 •

edited by sallyom

Loading

Uh oh!

clawsweeper Bot commented Jun 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

xydigit-sj commented Jun 6, 2026 • edited by sallyom 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 Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xydigit-sj commented Jun 6, 2026 •

edited by sallyom

Loading

clawsweeper Bot commented Jun 6, 2026 •

edited

Loading