Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# GRILL — dest-symlink-guard-init-remove

Plan: `.dev/features/dest-symlink-guard-init-remove/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live
`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only.

## Findings

```yaml
- type: FINDING
rule_id: 'P0'
severity: important
file: '.dev/features/dest-symlink-guard-init-remove/PLAN.md:62'
problem: "The pre-flight walks the FILE list, so a symlinked leaf that is itself a to-be-created directory is covered only through the files beneath it; an empty upstream dir would not be walked. Acceptable because cpSync only creates dirs for files it copies, but the claim should say 'every written file path'."
evidence: 'for every rel in `collectExpectedInstallPaths(...)` ∪ `{.claude/settings.json}`'
- type: FINDING
rule_id: 'P3'
severity: minor
file: '.dev/features/dest-symlink-guard-init-remove/PLAN.md:30'
problem: 'install-capabilities.ts will import install-manifest.ts (the mirror it is mirrored by); no cycle exists today (install-manifest imports constants/layout/symlink-guard/validate only), but the dependency direction should be noted.'
evidence: '`installCapabilities` pre-flight: for every rel in `collectExpectedInstallPaths(...)`'
- type: FINDING
rule_id: 'P4'
severity: minor
file: '.dev/features/dest-symlink-guard-init-remove/PLAN.md:44'
problem: "SECURITY.md's path-traversal scope line is not in `## Files`; README + CLAUDE.md cover the user-facing claim, so this is optional."
evidence: '`README.md` — "Safety model"'
```

## Summary

Closes the two remaining write/delete sites with the existing shared walk; the TOCTOU residual is
named. Concerns are wording/coverage, not design.

ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build.
80 changes: 80 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# PLAN — dest-symlink-guard-init-remove (PHARN-02: `init` / `remove` must not write or delete through a symlinked project directory)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: before any write, `init` walks every destination path it will write (the install manifest +
`.claude/settings.json`) with `findSymlinkComponent` and refuses the whole install, naming the
symlinked component; `remove` refuses (before any delete, for the whole selection) when a
capability dir's path crosses a symlinked component — the same posture `update` and `add` already take.
- layer(s): the CLI itself (`src/lib/install-capabilities.ts`, `src/commands/remove.ts`)
- constitution_refs: [P0, P1, P2, P3, P5, P7]

## Discovery — verified this run (P6)

Base = `main` @ `2b74a74` (PHARN-01 merged, #195). Reproduced in the review (`repro/init-symlink`,
`repro/init-symlink2`, `repro/remove-symlink`):

- `.claude/commands -> ../../dotfiles/…` → `init` wrote 11 `pharn-*.md` outside the project, no prompt.
- `pharn -> ../team-shared/pharn` → `init` overwrote an external file and created 421 files outside.
- `pharn -> ../shared` → `pharn remove a11y` deleted `../shared/pharn-review/a11y` incl. a user file.

Live code: `installCapabilityDirs` (`cpSync` at `safeJoin(projectRoot, …)`), `copyFilteredDir`, the
docs/license/contracts/core/floor copies and `settings.json` all check only the SOURCE; only
`features/README.md` walks the destination (`destAcceptsWrite`). `deleteCapabilityDir` walks nothing.
`findSymlinkComponent` (`lib/symlink-guard.ts`) is the shared physical walk; `collectExpectedInstallPaths`
(`lib/install-manifest.ts`) enumerates every file `installCapabilities` writes except the user-owned
`.claude/settings.json`.

## Files

- `src/lib/install-capabilities.ts` — `installCapabilities` pre-flight: for every rel in
`collectExpectedInstallPaths(...)` ∪ `{.claude/settings.json}`, `findSymlinkComponent(projectRoot, rel)`;
any hit → throw `ManifestValidationError` naming the component(s) (sorted, capped) before the first
write. ENOTDIR from the walk is not a symlink hit (left to the existing copy behaviour) — layer CLI/lib
- `src/commands/remove.ts` — before deleting (named path, and the picker for the WHOLE selection
before the first delete), `findSymlinkComponent(cwd, capabilityRelDir(...))`; a hit → `logError`
naming the component + exit 1, nothing deleted, config/records untouched — layer CLI/command
- `tests/install-capabilities.test.ts` — symlinked `.claude`, `.claude/commands`, `.claude/hooks`,
`pharn`, `pharn/pharn-review`, capability leaf dir, `.claude/settings.json` → throw, external dir
snapshot unchanged, project unchanged (no partial write)
- `tests/remove.test.ts` — symlinked `pharn` / `pharn/pharn-review` / capability dir → exit 1, external
dir unchanged, config write skipped; picker with one unsafe pick → nothing deleted
- `README.md` — "Safety model": `init` and `remove` also refuse symlinked destination paths (P4)
- `CLAUDE.md` — `install-capabilities` / `remove` paragraphs: destination walk (P4)

## Contracts satisfied

- README "Safety model" / `SECURITY.md` path-traversal scope / `LIMITS.md` containment — now true for
`init` and `remove`, not only `update`/`add`.
- CLAUDE.md "lexical/physical split": `safeJoin` contains the string, `findSymlinkComponent` refuses the
path on disk — applied at the two remaining write/delete sites.

## Evals to write (P1)

- init pre-flight: each symlinked root above → `ManifestValidationError` naming that component; the
link target dir byte-identical; nothing written under the project
- init with a regular (non-symlink) existing tree → unchanged behaviour (existing tests stay green)
- remove named: `pharn` symlink → exit 1, `../shared/pharn-review/a11y/NOTES.md` still present,
`writePharnConfig` not called, records untouched
- remove picker: two picks, one crossing a symlink → exit 1 before ANY delete (the safe one survives too)
- remove of a normal capability → unchanged

## Guarantee audit (P0)

- "`init` writes nothing through a symlinked destination component" → floor: `findSymlinkComponent`
over every manifest rel before the first write (lstat walk). Residual (advisory, named): a symlink
created concurrently between pre-flight and copy (TOCTOU) is not covered — same residual as `update`.
- "`remove` deletes nothing through a symlinked component" → floor: same walk before `rmSync`.

## Trust audit (P2)

- The project tree is local, user-controlled state; the walk reads only `lstat` metadata and emits a path
string (DATA) into the error message. No untrusted remote content drives a new branch.

## Determinism audit (P5)

- Branch = `findSymlinkComponent(...) !== null` (membership over lstat results); the terminal is a named
hard-fail, never a silent skip.

## Open questions (HALT)

- none
30 changes: 30 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# REGRESSION — dest-symlink-guard-init-remove

The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.

## Base and partition

- **base:** `2b74a74df5f024a638ebc464181ea0bf971b13bd` (`origin/main` at build time; the build is an uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`): `CLAUDE.md`, `README.md`, `src/commands/remove.ts`, `src/lib/install-capabilities.ts`, `tests/install-capabilities.test.ts`, `tests/remove.test.ts`.
- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook scratch) and this
feature's own stage artifacts are not build output.
- **outside gates:** the stdlib `*.test.mjs` / `*.test.cjs` files + whole-repo `validate`; 0 committed eval pairs.
- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` / `lint:md` are absent from both maps.

## Per-gate exit codes

| gate | base | head | flipped? |
| ---------- | ---- | ---- | -------- |
| `tests` | 0 | 0 | no |
| `validate` | 0 | 0 | no |

- `regressions[]`: **empty**
- `pre_existing[]`: **empty**

## Verdict

**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.**
(`regression-report.json` `.verdict` = `no-regressions`.)

Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**` is
owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never the increment.
47 changes: 47 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# REVIEW — dest-symlink-guard-init-remove

Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**.

## Floor-gate findings (blocking)

None.

- **L-floor (P0):** both claims reduce to the existing physical walk `findSymlinkComponent` (lstat per
component) run before the first write/delete; the TOCTOU window is named as an advisory residual in
the code comment, matching `update`.
- **L-eval (P1):** 15 new cases fail on the base source and pass after: 7 symlinked destination roots +
the settings leaf + multi-component naming + pharn-layout root for `init`; 3 symlink positions + the
whole-selection picker refusal for `remove`; a non-symlink control case. The pre-existing
`features/` symlink test was updated from "silently skipped" to "whole install refused".
- **L-trust (P2):** only lstat metadata and a path string (DATA) reach the message; capability names in
that string are validated at ingest (PHARN-01).
- **L-axis (P3):** `install-capabilities.ts` now imports `install-manifest.ts` (no cycle — the manifest
imports only constants/layout/symlink-guard/validate); `remove.ts` imports the shared
`symlink-guard.ts`. No leaf→leaf import.

## Advisory findings

```yaml
- type: FINDING
rule_id: 'P5'
severity: minor
file: 'src/lib/install-capabilities.ts:156'
problem: "The refusal fires after the overwrite-confirm prompt (confirmWriteTargets), so a user may answer 'yes' and then be refused; nothing is written either way."
evidence: 'assertDestinationsInProject(repoDir, projectRoot, selection.selected, paths);'
- type: FINDING
rule_id: 'P7'
severity: minor
file: 'src/lib/install-capabilities.ts'
problem: 'Behavior change: a project whose features/ dir is a symlink used to install everything except features/README.md; it is now refused as a whole. Intentional (consistent posture), but user-visible.'
evidence: 'does NOT write through a symlinked features/ in the PROJECT (no escape)'
- type: FINDING
rule_id: 'P4'
severity: minor
file: 'CHANGELOG.md:8'
problem: "No CHANGELOG `[Unreleased]` entry for the new refusals (not in the plan's `## Files`)."
evidence: '## [Unreleased]'
```

## Verdict

**GREEN** — 0 floor-gate findings, 3 advisory findings (minor). No lesson proposed for canon.
17 changes: 17 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# SHIP — dest-symlink-guard-init-remove

Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` →
`/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2.

| stage | structural verdict (verbatim) |
| -------------------- | ------------------------------------------------------ |
| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit `0` |
| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` |
| `/pharn-dev-verify` | `verify-report.json` `.verdict` = `PASS` |

- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md)
- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if
its CI checks are green.

chain ran; the named floor verdicts are as shown — this is NOT a judgment that the increment is good or
wise; that is the human's call at the post-review gate.
27 changes: 27 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/VERIFY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# VERIFY — dest-symlink-guard-init-remove

## FLOOR layer (owns the verdict)

Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session
proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` /
`update.test.ts` fail for environmental reasons, identically at the baseline).

| gate | exit |
| -------------- | ---- |
| `format:check` | 0 |
| `lint` | 0 |
| `lint:md` | 0 |
| `test` | 0 |
| `typecheck` | 0 |
| `validate` | 0 |

No `structural:*` gate — the increment ships no eval-actual pair.

**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`).

## ADVISORY layer

`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only.

Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense
the suite does not encode.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"base": "2b74a74df5f024a638ebc464181ea0bf971b13bd",
"inside": [
"CLAUDE.md",
"README.md",
"src/commands/remove.ts",
"src/lib/install-capabilities.ts",
"tests/install-capabilities.test.ts",
"tests/remove.test.ts"
],
"outside_gates": {
"tests": {
"base": 0,
"head": 0
},
"validate": {
"base": 0,
"head": 0
}
},
"regressions": [],
"pre_existing": [],
"verdict": "no-regressions"
}
17 changes: 17 additions & 0 deletions .dev/features/dest-symlink-guard-init-remove/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"feature": "dest-symlink-guard-init-remove",
"gates": {
"format:check": 0,
"lint": 0,
"lint:md": 0,
"test": 0,
"typecheck": 0,
"validate": 0
},
"verdict": "PASS",
"failing_gates": [],
"verifiers": {
"registered": 0,
"findings": []
}
}
11 changes: 8 additions & 3 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
{
"scope": [
".dev/features/config-capability-name-validation/SHIP.md"
"src/lib/install-capabilities.ts",
"src/commands/remove.ts",
"tests/install-capabilities.test.ts",
"tests/remove.test.ts",
"README.md",
"CLAUDE.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T07:36:53.788Z"
"set_by": ".dev/features/dest-symlink-guard-init-remove/PLAN.md",
"set_at": "2026-09-24T07:58:14.028Z"
}
Loading
Loading