Skip to content

fix(init,remove): refuse to write or delete through a symlinked project directory (PHARN-02) - #196

Merged
PrzemekGalarowicz merged 2 commits into
mainfrom
claude/bold-archimedes-5czyyn
Sep 24, 2026
Merged

PrzemekGalarowicz merged 2 commits into
mainfrom
claude/bold-archimedes-5czyyn

Conversation

@PrzemekGalarowicz

Copy link
Copy Markdown
Contributor

What this changes

safeJoin only checks the path as a string, and cpSync/rmSync follow symlinked parent directories. As a result, when a project directory was a symlink pointing outside the project, init and remove acted outside it. Reproduced cases:

  • .claude/commands symlinked: init wrote 11 pharn-*.md files outside the project, with no prompt.
  • pharn -> ../team-shared/pharn: init overwrote an external file and created 421 files outside the project.
  • pharn -> ../shared: pharn remove a11y deleted ../shared/pharn-review/a11y, including a user file inside it.

The fix:

  • init: before the first write, installCapabilities checks every file path the install will write (the install manifest plus .claude/settings.json) with findSymlinkComponent. If any path goes through a symlink, it refuses the whole install and names each symlinked directory. update and add already behaved this way.
  • remove: it exits 1 without deleting anything when a target directory's path goes through a symlink. In the picker, the whole selection is checked before the confirm prompt.
  • Behavior change: a project whose own features/ is a symlink used to get every file except features/README.md. Its install is now refused as a whole.
  • Docs: README "Safety model" and CLAUDE.md updated.

Built with /pharn-dev-ship; stage artifacts are in .dev/features/dest-symlink-guard-init-remove/. Results:

  • validate: exit 0
  • regress: no-regressions
  • verify: PASS

Type of change

  • feat — new stack option, wizard step, or command capability
  • fix — bug fix
  • docs — docs-only change
  • chore / refactor — tooling or internal restructure, no behavior change

Area(s) touched

lib/install-capabilities | commands/remove | docs (README.md, CLAUDE.md)

Checklist

  • Read the existing file(s) before editing; followed the ESM .js-extension import convention.
  • Updated the matching tests/*.test.ts. 15 of the new cases fail on the old source.
  • Updated the relevant docs.
  • Preserved the security invariants in src/lib/validate.ts.

Quality gates

  • npm run check passes locally (format, lint, typecheck, 1292/1292 tests; non-root user, node 22).
  • npm run build succeeds (left to CI).
  • npm run test:coverage passes (left to CI).

Notes for the reviewer

The /pharn-dev-review findings below are advisory and not addressed here:

  • the refusal comes after the overwrite prompt, so a user can answer "yes" and still be refused (nothing is written either way);
  • the features/ behavior change described above;
  • no CHANGELOG entry.

A symlink created between the check and the write (a TOCTOU race) is still not covered, the same gap update has.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc


Generated by Claude Code

…ct directory (PHARN-02)

`safeJoin` is lexical and `cpSync`/`rmSync` follow symlinked ancestors, so a
project whose `.claude/commands` or `pharn/` is a symlink had `init` write the
install OUTSIDE the project (overwriting external files without a prompt), and
`pharn remove a11y` with `pharn -> ../shared` deleted `../shared/pharn-review/a11y`.

- `installCapabilities` pre-flights every destination the install manifest says
  it writes (plus `.claude/settings.json`) with `findSymlinkComponent` and
  refuses the whole install, naming each symlinked component, before the first
  write — the posture `update` and `add` already had.
- `remove` refuses (exit 1) when a target dir's path crosses a symlinked
  component, for the whole picker selection before its confirm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 99ef1e9e-7b6e-45e6-aebf-54dd0a6055ad


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread tests/install-capabilities.test.ts Fixed
…egExp

CodeQL flagged the incomplete escaping in the per-link RegExp.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc
@PrzemekGalarowicz
PrzemekGalarowicz merged commit 31f15ce into main Sep 24, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants