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
38 changes: 38 additions & 0 deletions .dev/features/tar-entry-names/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# GRILL — tar-entry-names

Plan: `.dev/features/tar-entry-names/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/tar-entry-names/PLAN.md:37'
problem: 'A `TextDecoder` built with only `{ fatal: true }` silently STRIPS a leading byte-order mark (measured: EF BB BF 61 decodes to "a"). A name whose field starts with U+FEFF, itself a Cf format character, would lose it before the check and be written under a different name than the archive holds: neither refused nor faithful. Build the decoder with `ignoreBOM: true` so the BOM stays in the string and the Cf check refuses it.'
evidence: '`readPathField` (strict `TextDecoder(''utf-8'', { fatal: true })`'
- type: FINDING
rule_id: 'P1'
severity: minor
file: '.dev/features/tar-entry-names/PLAN.md:41'
problem: 'The byte-exact claim should be asserted on BYTES: read the extracted directory with `readdirSync(dir, { encoding: ''buffer'' })` and compare against the archive bytes (e2 80 94 for the em dash), not the decoded string, which would pass even if both sides were decoded the same wrong way. Add a BOM-prefixed name to the refusal cases.'
evidence: 'valid UTF-8 `a—b.md` → written as exactly those bytes (FAILS on base)'
- type: FINDING
rule_id: 'P0'
severity: minor
file: '.dev/features/tar-entry-names/PLAN.md:62'
problem: '"on-disk name is its archive bytes decoded as UTF-8" holds for the name pharn passes to the filesystem; a filesystem that normalizes Unicode (HFS+ stores NFD) may store different bytes. Scope the claim to the name written, or label the on-disk half advisory for such filesystems.'
evidence: '"an extracted entry''s on-disk name is its archive bytes decoded as UTF-8'
```

## Summary

The plan correctly moves the check onto the string that is written, which is the actual defect.
The one soundness gap is the decoder's default BOM handling: without `ignoreBOM: true` a leading U+FEFF
is removed before the check ever sees it, which recreates, for one character, exactly the check-vs-write
mismatch this increment exists to close. The pax budget and the escaped numeric message are sound as
planned (overlong forms and encoded surrogates are already refused by a fatal decoder — measured).

ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory — 1 important) — for the human to
weigh before /pharn-dev-build; all three fold into the build without changing the plan's Files.
80 changes: 80 additions & 0 deletions .dev/features/tar-entry-names/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# PLAN — tar-entry-names (judge the name that is written; bound pax work per archive, not per header)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: the extractor (1) decodes each entry's `prefix`/`name` fields ONCE, as strict UTF-8,
refuses a field that is not valid UTF-8, runs the control/format-character check on that decoded
string, and uses the SAME string for the path rules and the write; (2) bounds pax work per
ARCHIVE — a cumulative 64 KiB payload budget across every `g` header, and every `g` header counts
toward `maxEntries`; (3) renders a malformed numeric field without its raw bytes, so no newline or
control byte from a header reaches the fatal-error sink.
- layer(s): the CLI itself (`src/lib/tar-extract.ts`)
- constitution_refs: [P0, P1, P2, P5]

## Discovery — verified this run (P6)

- `readString` (`tar-extract.ts:100-104`) decodes header fields as latin1; the entry path is built
from it (`:442-445`) and written as that latin1 string (`:477-480`), which fs encodes as UTF-8.
`assertDisplayablePath` (`:206-217`, PHARN-17) checks a DIFFERENT string — the UTF-8 decoding of
the latin1 bytes. Reproduced by review (end-to-end with `extractTar`, then real `runStatus` /
`runUpdate` on a git-archive of pharn-oss): a raw `0x9B` byte decodes to U+FFFD for the check
(passes) and lands on disk as U+009B (bytes `c2 9b`, the C1 CSI); `status`'s MISSING list and
`update`'s SKIPPED list then print it raw. A legitimate UTF-8 name (`a—b.md`) lands as mojibake
(`a` U+00E2 U+0080 U+0094 `b`). The latin1 decoding predates the range (#153); PHARN-17's check
is the part that disagrees with the write.
- Every codeload entry in the live archive is ASCII (review: pharn-oss@b31e540 via `git archive`,
2623 entries), so strict UTF-8 changes nothing for the real archive.
- pax (PHARN-18): `MAX_PAX_BYTES` (64 KiB) is checked per header (`:397`), `g` headers are unlimited
in number and skip the `entries`/`totalBytes` accounting (`:421-438` vs `:459-471`). Measured by
review on node 22: 2030 `g` headers × 64 KiB (a 288 KiB .tgz) → accepted after 8.9 s CPU / 375 MB
RSS, vs 11.5 s / 906 MB for the single-header archive PHARN-18 fixed. Real archives carry exactly
one `g` (`comment=<sha>`, ~52 bytes). `MAX_ENTRIES` is 20 000 (`repo.ts:21`).
- `readOctal` (`:112-126`) interpolates the raw field text; `logError` keeps `\n`, so a size field
`1\n Done` prints an attacker-chosen second line on the fatal path (reproduced by review). The
checksum field reaches the same `readOctal`.

## Files

- `src/lib/tar-extract.ts` — `readPathField` (strict `TextDecoder('utf-8', { fatal: true })`, refusal
message renders the bytes as printable ASCII + `\xNN` escapes, capped); `assertDisplayablePath` runs
on the decoded path with no re-decoding; the decoded path feeds `resolveEntryPath` and the write;
a per-archive `paxBytes` budget checked BEFORE `readPaxKeywords`; `g` headers increment `entries`;
`readOctal`'s message shows the field via the same escaped rendering — layer CLI/lib
- `tests/tar-extract.test.ts` — raw `0x9B` in `name` → refused, nothing written (FAILS on base);
invalid UTF-8 in `prefix` → refused; valid UTF-8 `a—b.md` → written as exactly those bytes (FAILS
on base); valid-UTF-8 U+009B and U+202E still refused; the refusal message holds no C0/C1 byte;
two `g` headers each under 64 KiB but over it together → refused (FAILS on base); `maxEntries`+1
empty `g` headers → refused (FAILS on base); a numeric field containing `\n` → message has no
newline (FAILS on base); the codeload-shaped fixture still extracts
- `CHANGELOG.md` — `[Unreleased]` → `### Security` entry

## Contracts satisfied

- `THREAT-MODEL.md` §2 surface 7 / §3 "malformed / hostile archive entry" and PHARN-17's
"no such name is installed or printed" — now true of the bytes written (cited, P4).

## Evals to write (P1)

- listed under Files.

## Guarantee audit (P0)

- "an extracted entry's on-disk name is its archive bytes decoded as UTF-8 and contains no C0/C1/Cf
character" → floor: fatal UTF-8 decode + the `hasUnsafeChars` regex over the exact string written.
- "pax parsing costs O(64 KiB) per archive" → floor: cumulative size compare before any parse.
- "every header counts toward `maxEntries`" → floor: counter + compare.
- "no header byte reaches a message raw" → floor: the escaped rendering is total over bytes
(printable ASCII 0x20-0x7E kept, everything else `\xNN`).

## Trust audit (P2)

- Only narrows what an untrusted archive may contain; untrusted bytes reach messages only in the
escaped rendering.

## Determinism audit (P5)

- Byte/regex checks and integer compares; no fallback decoding (invalid UTF-8 is a refusal, not a
replacement character).

## Open questions (HALT)

- none
36 changes: 36 additions & 0 deletions .dev/features/tar-entry-names/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# REGRESSION — tar-entry-names

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

## Base and partition

- **base:** `431f02b59c39dbbcda028b4291fbc92d877162f6` (`HEAD` — `origin/main` after #217; the build is an
uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`): `src/lib/tar-extract.ts`, `tests/tar-extract.test.ts`,
`CHANGELOG.md`.
- **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 46 stdlib `*.test.mjs` / `*.test.cjs` files `scope` returned (754 tests) +
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.
- **environment:** both sides ran with no proxy variables and as root **without** `CAP_DAC_OVERRIDE` /
`CAP_DAC_READ_SEARCH` / `CAP_FOWNER` (`setpriv`), the CI-equivalent of this root sandbox.

## 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.
50 changes: 50 additions & 0 deletions .dev/features/tar-entry-names/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# REVIEW — tar-entry-names

Increment: `src/lib/tar-extract.ts` (`readPathField` — strict UTF-8 with `ignoreBOM`; the check, the path
rules and the write share the decoded string; a per-archive pax budget; global headers counted toward the
entry cap; `describeBytes` for header bytes in messages), `tests/tar-extract.test.ts` (nine cases),
`CHANGELOG.md` (`### Security`). Treated as `trust: untrusted`; nothing in it read as an instruction.

## Floor first (P0)

`node .dev/floor/validate.mjs .` → `FLOOR: GREEN` (exit 0). `/pharn-dev-build`'s `npm run check` exit 0
(1507 tests), `/pharn-dev-regress` `no-regressions`, `/pharn-dev-verify` `PASS`.

## Floor-gate findings (blocking)

None.

- L-floor (P0): "the name passed to the filesystem holds no C0/C1/Cf character and is the archive's bytes
decoded as UTF-8" → a fatal decode plus the `hasUnsafeChars` regex over the exact string written; the
on-disk half is scoped to the name written (a Unicode-normalizing filesystem is named in VERIFY.md).
"Pax parsing costs O(64 KiB) per archive" → a cumulative compare before any parse. "Every global header
counts toward the entry cap" → counter + compare. "No header byte reaches a message raw" →
`describeBytes` is total over bytes; every other message interpolates only a path that already passed
the check.
- L-eval (P1): eight of the nine new cases fail on the base `tar-extract.ts` (checked by stashing it);
the ninth (a leading byte-order mark) passes on both by design — it pins the decoder option the grill
found. The byte-exact case asserts on the BYTES `readdirSync` returns. A git-archive of pharn-oss
extracts to the same 2208 files / 414 directories as before.
- L-trust (P2): the increment only narrows what an untrusted archive may contain.
- L-axis (P3): everything stays in the fetch boundary's own module.

## Advisory findings (warn — severity is this reviewer's judgment, fix #3)

```yaml
- type: FINDING
rule_id: 'P3'
severity: minor
file: 'src/lib/tar-extract.ts:106'
problem: '`describeBytes` is a second display renderer beside terminal-safe.ts, which PHARN-17 called the one display sanitizer. The input differs (raw bytes that may not decode, vs a string), which justifies a byte-level form; if a second caller needs it, it belongs in terminal-safe.ts.'
evidence: 'function describeBytes(bytes: Buffer, max = 200): string {'
- type: FINDING
rule_id: 'P7'
severity: minor
file: 'src/lib/tar-extract.ts:137'
problem: 'One non-UTF-8 name refuses the whole archive — the extractor''s reject-don''t-skip posture, but upstream main is a live input to every deployed CLI (LIMITS.md §3e), so a single such file upstream would fail every install until reverted. pharn-oss ships only ASCII names today; named, not changed.'
evidence: 'tar entry path is not valid UTF-8'
```

## Verdict

**GREEN — 0 floor-gate findings, 2 advisory (minor).** The standing decision is the human's (GATE 2).
18 changes: 18 additions & 0 deletions .dev/features/tar-entry-names/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# SHIP — tar-entry-names

Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: all four plans of this batch accepted — "deliver
all of them one by one using pharn-dev-ship") → `/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: after each increment, open a
pull request and merge it once its checks are green, then start the next plan.

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.
33 changes: 33 additions & 0 deletions .dev/features/tar-entry-names/VERIFY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# VERIFY — tar-entry-names

## FLOOR layer (owns the verdict)

Gates run over the whole repo with the feature present, on node 22 with the session proxy variables unset
and as root **without** `CAP_DAC_OVERRIDE` / `CAP_DAC_READ_SEARCH` / `CAP_FOWNER` (`setpriv`) — the
CI-equivalent of this root sandbox.

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

`test` is vitest (1507 tests), which collects this increment's own tests (`tests/tar-extract.test.ts`).
`test:floor` is floor.yml's `node --test` run (754 tests). No `structural:*` gate — the increment ships no
eval-actual pair. Outside the verdict, a git-archive of pharn-oss@b31e540 (the review's real-archive
fixture) was extracted with the new code: 2208 files and 414 directories, the same as before.

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

## ADVISORY layer

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

Residual (P0/P7): verified = the named gates passed; this is NOT a guarantee of correctness beyond what those
gates check — verifier concerns are advisory help, not assurance. The byte-exact name holds for the name
pharn passes to the filesystem; a filesystem that normalizes Unicode (HFS+) may store other bytes.
21 changes: 21 additions & 0 deletions .dev/features/tar-entry-names/regression-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"base": "431f02b59c39dbbcda028b4291fbc92d877162f6",
"inside": [
"CHANGELOG.md",
"src/lib/tar-extract.ts",
"tests/tar-extract.test.ts"
],
"outside_gates": {
"tests": {
"base": 0,
"head": 0
},
"validate": {
"base": 0,
"head": 0
}
},
"regressions": [],
"pre_existing": [],
"verdict": "no-regressions"
}
18 changes: 18 additions & 0 deletions .dev/features/tar-entry-names/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"feature": "tar-entry-names",
"gates": {
"format:check": 0,
"lint": 0,
"lint:md": 0,
"test": 0,
"test:floor": 0,
"typecheck": 0,
"validate": 0
},
"verdict": "PASS",
"failing_gates": [],
"verifiers": {
"registered": 0,
"findings": []
}
}
4 changes: 2 additions & 2 deletions .pharn/writes-scope.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"scope": [
".dev/features/update-frozen-recheck/SHIP.md"
".dev/features/tar-entry-names/SHIP.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T19:24:04.227Z"
"set_at": "2026-09-24T19:32:56.865Z"
}
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **A re-run `pharn init` dropped the `pharn.config.json` keys you added by hand.** Upstream PHARN reads top-level keys that users add themselves: `testResults` (without it `/pharn-loop` stops with `blocked: no-test-runner`) and `ship.requireAttestation`. `add`, `update` and `remove` kept them, but `init` rebuilt the config from its own fields alone. It now copies every top-level key pharn does not own across from the config it replaces, unchanged. It does so from any file that parses as a JSON object, including one the other commands refuse. Keys pharn owns are still written fresh.
- **`pharn update` could stop re-checking a capability whose files were still out of date.** When pharn cannot read a capability upstream, `update` keeps it, skips its files, and moves the skills version on. Once pharn could read it again, the next run removed it from `frozenCapabilities` even if one of its files had to be skipped, for example because you edited it. Every later run then said "Already up to date", and that file stayed at the old version even after you resolved your edit. The capability now stays listed until none of its files are skipped, so each run checks it again. `update` also no longer writes a `pendingSkillsVersion` equal to the recorded `skillsVersion`.

### Security

- **The extractor's name check now judges the name it writes.** It checked one decoding of a tar entry's name and wrote another. A raw C1 byte (such as `0x9B`, the terminal CSI) passed the control-character check and landed on disk as that control character, where `pharn status` and `pharn update` then printed it raw. Every non-ASCII name was also written garbled. Entry names are now decoded once, as strict UTF-8, and the check, the path rules and the write all use that one string. A name that is not valid UTF-8, or that starts with a byte-order mark, is refused.
- **Pax parsing is bounded per archive.** The 64 KiB cap applied to each global header, not to their number, so many headers each under the cap still cost seconds of CPU. The cap now covers all global headers in the archive together, and every header counts toward the entry limit.
- **A malformed numeric header field no longer reaches the error output raw.** It is shown with non-printable bytes escaped, so an archive cannot add a line of its own to the fatal message.

## [0.5.0] - 2026-09-10

### Changed — BREAKING
Expand Down
Loading
Loading