PrzemekGalarowicz · Sep 24, 2026 · Sep 24, 2026
@@ -0,0 +1,49 @@
+# GRILL — config-capability-name-validation
+
+Plan: `.dev/features/config-capability-name-validation/PLAN.md` · spec-hash check: `bca940a5…d729d3c4e` matches live `ARCHITECTURE.md` (no drift).
+Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only.
+
+## Findings
+
+### P1 — eval coverage
+
+```yaml
+- type: FINDING
+  rule_id: 'P1'
+  severity: important
+  file: '.dev/features/config-capability-name-validation/PLAN.md:47'
+  problem: 'The ingest check affects every command, but only remove/list are named in the end-to-end evals; update/status/add rely on the shared loader being exercised once.'
+  evidence: '`loadConfigOrExit` / `list --json` route the new error to the named message + exit 1'
+```
+
+Advisory: acceptable because all four go through `loadArchetypeConfigOrExit` → `loadConfigOrExit`; one loader test covers the shared path.
+
+### P4 — docs
+
+```yaml
+- type: FINDING
+  rule_id: 'P4'
+  severity: minor
+  file: '.dev/features/config-capability-name-validation/PLAN.md:34'
+  problem: "SECURITY.md edit is conditional ('if SECURITY.md enumerates them'); it does — line 64 lists records/lock as local-but-user-editable sidecars and omits pharn.config.json."
+  evidence: '`SECURITY.md` — list `pharn.config.json` `capabilities[]` beside records/lock ..., if SECURITY.md enumerates them'
+```
+
+### P7 — scope
+
+```yaml
+- type: FINDING
+  rule_id: 'P7'
+  severity: minor
+  file: '.dev/features/config-capability-name-validation/PLAN.md:31'
+  problem: 'Rejecting non-object entries is a new refusal beyond name/role; justified (consumers dereference .name) but should be stated in the error message so the hand-edit is fixable.'
+  evidence: 'each entry: plain object, `name` via `assertSafeString(…, CAPABILITY_NAME_RE)`'
+```
+
+## Summary
+
+The plan closes the reproduced defect at two independent floors (ingest regex/enum + strict-child
+containment at the delete). Symlinked components are correctly deferred to PHARN-02. Concerns are
+coverage/documentation completeness, not design.
+
+ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — for the human to weigh before /pharn-dev-build.
@@ -0,0 +1,80 @@
+# PLAN — config-capability-name-validation (PHARN-01: `remove` must never delete outside `<subtree>/<name>`)
+
+- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
+- increment: validate every `capabilities[]` entry's `name` (`CAPABILITY_NAME_RE`, no control chars) and
+  `role` (`{griller, lens}`) at config ingest with a new named `CapabilityEntryError`, and make
+  `remove`'s recursive delete accept only a STRICT child of the role subtree (`safeChildJoin`), so a
+  hand-edited `{"name":"../.."}` can no longer make `pharn remove` delete the project root.
+- layer(s): the CLI itself (`src/lib/*`, `src/commands/remove.ts`)
+- constitution_refs: [P0, P1, P2, P3, P5, P7]
+
+## Discovery — verified this run (P6)
+
+HEAD `2fffcb3` on `claude/bold-archimedes-5czyyn`, tree clean. Reproduced in the review
+(`repro/remove-traversal`): config `{"name":"../..","role":"lens"}` + `pharn remove ../..` →
+project root (incl. `.git`) deleted; `../../src` → `src/` deleted; picker path identical.
+
+- `src/lib/pharn-config.ts:22-24` — comment: "`name` and `role` are still passed through unvalidated";
+  `:216` only `validateCapabilitySources` runs.
+- `src/lib/validate.ts:115-124` — `safeJoin` accepts `target === root`, so `${subtree}/../..` = cwd passes.
+- `src/commands/remove.ts:87-96` — `rmSync(safeJoin(cwd, \`${subtree}/${name}\`), {recursive, force})`.
+
+## Files
+
+- `src/lib/validate.ts` — add `safeChildJoin(base, name)`: returns `resolve(base, name)` only when its
+  `dirname` is exactly `resolve(base)` (a strict, single-segment child); throws `ManifestValidationError`
+  otherwise — layer CLI/lib
+- `src/lib/pharn-config.ts` — `CapabilityEntryError` + `validateCapabilityEntries(raw.capabilities)`
+  (each entry: plain object, `name` via `assertSafeString(…, CAPABILITY_NAME_RE)`, `role` via
+  `ROLE_VALUES` membership); joins `isConfigValidationError`; replaces the "unvalidated" comment — layer CLI/lib
+- `src/commands/remove.ts` — `deleteCapabilityDir` resolves `safeChildJoin(safeJoin(cwd, subtree), name)` — layer CLI/command
+- `tests/validate.test.ts` — `safeChildJoin` cases
+- `tests/pharn-config.test.ts` — ingest rejection cases + valid legacy config still loads
+- `tests/remove.test.ts` — end-to-end: traversal names exit 1, tree snapshot unchanged; valid name still removes only its dir
+- `CLAUDE.md` — the `lib/pharn-config.ts` paragraph ("`name`/`role` stay unvalidated") updated to the new truth (P4)
+- `SECURITY.md` — list `pharn.config.json` `capabilities[]` beside records/lock as validated local input, if SECURITY.md enumerates them (P4)
+
+## Contracts satisfied
+
+- CLAUDE.md "`lib/validate.ts` is security-sensitive" — names validated against `CAPABILITY_NAME_RE`, `..` rejected, control chars rejected; `safeJoin` containment preserved.
+- README "Safety model": `remove` "deletes only the selected capability directory" — now floor-backed.
+
+## Evals to write (P1)
+
+- `safeChildJoin(base,'a11y')` → `base/a11y`; `'..'`, `'.'`, `''`, `'a/b'`, `'../x'`, `'/abs'` → throw
+- `readPharnConfig` with name `../..` / `../../src` / `..` / `.` / `""` / `a/b` / `"a\u001b[2J"` / non-string → `CapabilityEntryError` naming `capabilities[i].name`
+- role `"bogus"` / missing → `CapabilityEntryError` naming `capabilities[i].role`
+- non-object entry → `CapabilityEntryError`
+- valid archetype config (with and without `source`) and a legacy module config (no `capabilities`) still load byte-for-byte as before (P7)
+- `loadConfigOrExit` / `list --json` route the new error to the named message + exit 1
+- `runRemove('../..')` on a crafted config → exit 1, project tree snapshot (incl. `.git`, `src/`) unchanged
+- `deleteCapabilityDir` defense-in-depth: even if validation were bypassed, `safeChildJoin` refuses (unit via `safeChildJoin`)
+- `runRemove('a11y')` still deletes only `<lenses>/a11y`, sibling `a11y-extended` intact
+
+## Guarantee audit (P0)
+
+- "a config capability name is a single kebab segment" → floor: enum-regex (`CAPABILITY_NAME_RE` + `CONTROL_CHARS_RE`) at ingest
+- "role is griller|lens" → floor: enum membership (`ROLE_VALUES`)
+- "`remove` deletes only a strict child of the role subtree" → floor: path-containment (`safeChildJoin`: `dirname(target) === base`) layered on `safeJoin`
+- symlinked components (PHARN-02) → NOT claimed here; separate increment
+
+## Trust audit (P2)
+
+- `pharn.config.json` is committed, hand-editable → treated as untrusted local input; taint stops at
+  ingest (named error, exit 1, zero writes); downstream consumers (`remove`, `list`, `update`, `status`,
+  `add`) only ever see validated names, which also closes raw-ESC echo of names in `list`.
+
+## Determinism audit (P5)
+
+- All branches are regex/enum membership or an exact string compare (`dirname === base`); failure is a
+  named hard-fail, never a fallback.
+
+## P7 note
+
+Every `name` this CLI ever wrote came from `parseCapabilityIndex` (already `CAPABILITY_NAME_RE`-validated)
+and every `role` from `assertRole`, so no legitimately written config is newly rejected. Pre-archetype
+configs have no `capabilities` and are untouched.
+
+## Open questions (HALT)
+
+- none
@@ -0,0 +1,39 @@
+# REGRESSION — config-capability-name-validation
+
+The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.
+
+## Base and partition
+
+- **base:** `2fffcb3d5039b570ac8cdf0ac38d46336633fc3b` — passed explicitly (`--base 2fffcb3`, the
+  reviewed baseline; the build is an uncommitted working tree on top of it).
+- **inside** (all declared in `PLAN.md` `## Files`): `CLAUDE.md`, `SECURITY.md`,
+  `src/commands/remove.ts`, `src/lib/pharn-config.ts`, `src/lib/validate.ts`,
+  `tests/pharn-config.test.ts`, `tests/remove.test.ts`, `tests/validate.test.ts`.
+- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []` — no fix #7 finding.
+  `.pharn/writes-scope.json` (hook scratch) and this feature's own `.dev/features/…` stage artifacts are
+  not build output.
+- **outside gates:** 46 stdlib test files (`*.test.mjs` / `*.test.cjs`) + whole-repo `validate`;
+  0 committed eval pairs.
+
+## Style-gate skip (P5/P7)
+
+`lint` / `format:check` / `lint:md` skipped at both sides: `inside` touches no shared style config.
+
+## 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`; helper exit **0**.)
+
+Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising
+`src/**` is inside scope here and is owned by `/pharn-dev-build`'s floor (`npm run check` GREEN, 1277/1277
+as a non-root user on node 22) and `/pharn-dev-verify`. This certifies the comparison, never the increment.
@@ -0,0 +1,49 @@
+# REVIEW — config-capability-name-validation
+
+Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**.
+
+## Floor-gate findings (blocking)
+
+None.
+
+- **L-floor (P0):** both new guarantees reduce to floor primitives — `CAPABILITY_NAME_RE` +
+  `CONTROL_CHARS_RE` + `ROLE_VALUES` membership at ingest (enum-regex), and `safeChildJoin`'s
+  `dirname(target) === resolve(base)` containment at the delete. Symlinked components are explicitly
+  not claimed (PHARN-02).
+- **L-eval (P1):** every new behavior has a vitest test that fails on the base source (38 red before the
+  fix, green after): 21 ingest cases, 9 `safeChildJoin` cases, 10 `remove` end-to-end cases (real loader
+  - bypassed-ingest defense in depth + picker + valid-name/prefix-sibling).
+- **L-trust (P2):** `pharn.config.json` is now treated as untrusted local input at ingest; the error
+  message quotes the offending name via `JSON.stringify` and control characters are reported without
+  echoing them (tested). No instruction-looking content was encountered in the diff.
+- **L-axis (P3):** `validate.ts` gains one lexical primitive (its existing axis); `pharn-config.ts`
+  gains one ingest validator (its axis); `remove.ts` only consumes them. No leaf→leaf import.
+
+## Advisory findings
+
+```yaml
+- type: FINDING
+  rule_id: 'P4'
+  severity: minor
+  file: 'CHANGELOG.md:8'
+  problem: "The user-visible behavior change (a hand-edited invalid capability name/role now exits 1 on every command) has no CHANGELOG `[Unreleased]` entry; CHANGELOG.md was not in the plan's `## Files`."
+  evidence: '## [Unreleased]'
+- type: FINDING
+  rule_id: 'P5'
+  severity: minor
+  file: 'src/commands/remove.ts:94'
+  problem: 'The defense-in-depth refusal throws an uncaught ManifestValidationError (stack trace, no curated message); unreachable while ingest validation holds, so a crash-loud failure is acceptable for a should-never-happen path.'
+  evidence: 'const dir = safeChildJoin('
+- type: FINDING
+  rule_id: 'P1'
+  severity: minor
+  file: 'tests/pharn-config.test.ts'
+  problem: "`list --json`'s own error path is covered only through the shared `isConfigValidationError` union, not by a list-level test with an invalid name."
+  evidence: 'joins isConfigValidationError, so every loader reports it and exits 1'
+```
+
+## Verdict
+
+**GREEN** — 0 floor-gate findings, 3 advisory findings (minor).
+
+No lesson proposed for canon.
@@ -0,0 +1,17 @@
+# SHIP — config-capability-name-validation
+
+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.
@@ -0,0 +1,27 @@
+# VERIFY — config-capability-name-validation
+
+## FLOOR layer (owns the verdict)
+
+Gates run over the whole repo with the feature present, as a non-root user on node 22 with proxy
+variables unset (as root with the session proxy set, 5 pre-existing tests in `init.test.ts` / `update.test.ts` fail for environmental reasons, identically at the baseline).
+
+| gate           | exit          |
+| -------------- | ------------- |
+| `test`         | 0 (1277/1277) |
+| `validate`     | 0             |
+| `lint`         | 0             |
+| `format:check` | 0             |
+| `lint:md`      | 0             |
+| `typecheck`    | 0             |
+
+No `structural:*` gate — the increment ships no eval-actual pair.
+
+**VERDICT: PASS** (`.dev/floor/check-verify.mjs` exit 0, `failing_gates: []`).
+
+## ADVISORY layer
+
+`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — no verifiers registered,
+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.
@@ -0,0 +1,26 @@
+{
+  "base": "2fffcb3d5039b570ac8cdf0ac38d46336633fc3b",
+  "inside": [
+    "CLAUDE.md",
+    "SECURITY.md",
+    "src/commands/remove.ts",
+    "src/lib/pharn-config.ts",
+    "src/lib/validate.ts",
+    "tests/pharn-config.test.ts",
+    "tests/remove.test.ts",
+    "tests/validate.test.ts"
+  ],
+  "outside_gates": {
+    "tests": {
+      "base": 0,
+      "head": 0
+    },
+    "validate": {
+      "base": 0,
+      "head": 0
+    }
+  },
+  "regressions": [],
+  "pre_existing": [],
+  "verdict": "no-regressions"
+}
@@ -0,0 +1,17 @@
+{
+  "feature": "config-capability-name-validation",
+  "gates": {
+    "format:check": 0,
+    "lint": 0,
+    "lint:md": 0,
+    "test": 0,
+    "typecheck": 0,
+    "validate": 0
+  },
+  "verdict": "PASS",
+  "failing_gates": [],
+  "verifiers": {
+    "registered": 0,
+    "findings": []
+  }
+}
@@ -1,7 +1,7 @@
 {
   "scope": [
-    ".dev/features/features-readme-relocation/SHIP.md"
+    ".dev/features/config-capability-name-validation/SHIP.md"
   ],
   "set_by": ".claude/commands/pharn-dev-ship.md",
-  "set_at": "2026-09-10T13:59:50.442Z"
+  "set_at": "2026-09-24T07:36:53.788Z"
 }