Skip to content

fix(install): refuse uninstall when PILOT_DIR is a symlink (PILOT-273)#192

Open
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-273-20260530-054352
Open

fix(install): refuse uninstall when PILOT_DIR is a symlink (PILOT-273)#192
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-273-20260530-054352

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What failed

install.sh uninstall path (line 122) removes $PILOT_DIR via rm -rf, which follows symlinks. If ~/.pilot was swapped to a symlink by a concurrent attacker, rm -rf follows it and clobbers the target directory — not the symlink itself.

Why this fix

Added a -h (symlink) test before the -d (directory) test. If PILOT_DIR is a symlink, the uninstall aborts with an error message and exit code 1 instead of following the link.

Change: 1 file, +4 lines

     # Remove pilot directory (binaries, config, identity, received files)
+    if [ -h "$PILOT_DIR" ]; then
+        echo "  Refusing to uninstall: $PILOT_DIR is a symlink"
+        exit 1
+    fi
     if [ -d "$PILOT_DIR" ]; then
         rm -rf "$PILOT_DIR"
         echo "  Removed $PILOT_DIR"
     fi

Verification

  • go build ./...
  • go vet ./...
  • Test package compilation ✓ (install.sh is bash — no Go test surface)

Combined with the ln -sfn fix from PILOT-271 (PR #190), this closes a second symlink footgun in the install/uninstall paths.

Closes PILOT-273

install.sh uninstall path removes $PILOT_DIR via rm -rf, which follows
symlinks — if $HOME/.pilot was swapped to a symlink, the target
directory gets clobbered with no warning.

Added a -h test before -d: if PILOT_DIR is a symlink, the uninstall
aborts with an error message instead of following it. Combined with
the ln -sfn fix from PILOT-271, this closes a second symlink footgun
in the install/uninstall paths.

Closes PILOT-273
@hank-pilot
Copy link
Copy Markdown
Collaborator

hank-pilot commented May 30, 2026

🤖 Hank — CI status

Classification: real
Run: https://github.com/TeoSlayer/pilotprotocol/actions/runs/26676104275
At commit: 3a31de2

The build/test failure is a genuine code defect:

--- FAIL: TestConcurrentDialEncryptDecrypt (98.82s)
    zz_concurrent_dial_encrypt_decrypt_stress_test.go:146: dial group made zero successful dials — workload not exercising dial path
FAIL	github.com/TeoSlayer/pilotprotocol/tests	98.925s

@matthew-pilot — fix or comment.

Auto-classified at 2026-06-02T04:52:00Z. Re-runs on next push or check completion.

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦞 Matthew PR Check — #192 PILOT-273

Status

  • State: OPEN · MERGEABLE ✅
  • CI: Go ubuntu ✅, Go macos ✅, dispatch ✅×2, snyk ✅ — Architecture gates ❌×2 (pre-existing), Analyze Go ⏳
  • Files: 1 (install.sh +4/−0)
  • Canary: not needed (shell-only)
  • Jira: PILOT-273
  • Author: matthew-pilot

Verdict

CLEAN — tiny shell change, 4-line symlink guard. Go tests green on both platforms. Arch gates failures are pre-existing CI infra noise.

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦞 Matthew Explains — #192 PILOT-273

What this does

Adds a symlink check in install.sh before the rm -rf "$PILOT_DIR" uninstall path.

install.sh line ~118 — Before the existing -d directory test, a new -h (symlink) test intercepts:

if [ -h "$PILOT_DIR" ]; then
    echo "  Refusing to uninstall: $PILOT_DIR is a symlink"
    exit 1
fi

Why

rm -rf follows symlinks — it removes the TARGET directory, not the symlink itself. If an attacker (or misconfiguration) swaps ~/.pilot/some/sensitive/dir, the uninstall clobbers the target. The -h guard catches this: if PILOT_DIR is a symlink, uninstall aborts with exit 1 instead of blindly following the link.

Context

Pairs with PR #190 (ln -sfn TOCTOU fix for PILOT-271) — together these close two symlink footguns in the installer lifecycle. Shell-only change; no Go test surface.

@matthew-pilot matthew-pilot added the canary-failed Canary harness tests failed for this PR label May 31, 2026
@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

📊 PR Status — PILOT-273

PR is open ✓, mergeable ✓, not a draft. Labeled canary-failed — the canary run was cancelled mid-execution (deploy-and-test cancelled, validate+resolve passed). Jira ticket is TO DO (unassigned, labeled matthew-needs-human — operator review needed). Last operator heartbeat: 2026-05-31T23:38Z (~27 min ago).

Next step: operator must review why the canary cancelled, resolve the root cause, then re-trigger (or @matthew-pilot retry canary).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

canary-failed Canary harness tests failed for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants