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

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

## Findings

```yaml
- type: FINDING
rule_id: 'P5'
severity: important
file: '.dev/features/tar-strict-framing/PLAN.md:6'
problem: 'The all-zero tail check must be linear and allocation-free (a byte loop or a single indexOf of a non-zero byte), or a 128 MiB zero tail becomes its own cost.'
evidence: '`requires every remaining byte to be zero`'
- type: FINDING
rule_id: 'P5'
severity: minor
file: '.dev/features/tar-strict-framing/PLAN.md:8'
problem: 'Accept both `ustar\0` (POSIX, what git archive writes) and `ustar ` (old GNU) — the check is the 5-byte `ustar` prefix — so a GNU-written fixture or mirror is not refused over the variant.'
evidence: '`refuses a header without the ustar magic`'
- type: FINDING
rule_id: 'P1'
severity: minor
file: '.dev/features/tar-strict-framing/PLAN.md:24'
problem: 'The oversized-pax eval must fail on the base source by BEHAVIOUR it can observe cheaply (accepted vs refused with the new message), not by timing — a wall-clock assertion is flaky in CI.'
evidence: '`an oversized g payload refused fast (bounded time, before parsing)`'
```

ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — all folded into the build.
56 changes: 56 additions & 0 deletions .dev/features/tar-strict-framing/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# PLAN — tar-strict-framing (PHARN-18: bounded pax parsing and strict tar framing)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: the extractor (1) refuses a pax `g`/`x` header whose payload exceeds a fixed cap (64 KiB)
BEFORE parsing its records; (2) after the first all-zero block requires every remaining byte to be zero
(a zero block mid-archive no longer silently truncates the tree; garbage after the end marker is
refused); (3) refuses an archive that ends without an end-of-archive marker; (4) refuses a header
without the `ustar` magic; (5) refuses empty (`a//b`) and `.` path segments explicitly, instead of
relying on `safeJoin`'s backstop.
- layer(s): the CLI itself (`src/lib/tar-extract.ts`)
- constitution_refs: [P1, P2, P5]

## Discovery — verified this run (P6)

Review repro: a 192,449-byte archive whose global header expands to ~128 MB of tiny records costs
12.99 s CPU and 1,115 MiB RSS in `readPaxKeywords`; with `--max-old-space-size=512` the process aborts
(exit 134) and leaves the clone in the temp dir. Framing: `if (isZeroBlock(header)) break;` stops at the
first zero block (entries after it are silently dropped); a missing end marker, trailing garbage and a
missing `ustar` magic are all accepted; `resolveEntryPath` rejects `..`/absolute/root but not `''`/`.`
segments below the root. Every codeload archive (git archive) has `ustar\0` magic, a pax global header
of one `comment=` record (~52 bytes) and zero-padded end blocks, so none of these rules touches it.

## Files

- `src/lib/tar-extract.ts` — `MAX_PAX_BYTES` gate; end-marker + all-zero tail; missing-marker refusal;
`ustar` magic check; `''`/`.` segment refusal — layer CLI/lib
- `tests/tar-extract.test.ts` — an oversized `g` payload refused fast (bounded time, before parsing);
zero block mid-archive followed by an entry → refused; trailing garbage → refused; no end marker →
refused; missing magic → refused; `a//b` and `a/./b` → refused; the codeload-shaped fixture still
extracts

## Contracts satisfied

- `THREAT-MODEL.md` §2 "post-decompression cap bounds the compression bomb" — now also bounds the cost of
parsing pax records.

## Evals to write (P1)

- listed above; each fails (is accepted, or is slow) on the base source.

## Guarantee audit (P0)

- "a pax payload parse is O(64 KiB)" → floor: size check before `readPaxKeywords`.
- "no entry after the end marker is ever skipped silently" → floor: all-zero tail check.

## Trust audit (P2)

- Only narrows what an untrusted archive may contain.

## Determinism audit (P5)

- Pure byte checks.

## Open questions (HALT)

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

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

## Base and partition

- **base:** `00105215205cbdb994e8bea6d80eca4891d07865` (`origin/main` at build time; 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`.
- **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.
43 changes: 43 additions & 0 deletions .dev/features/tar-strict-framing/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# REVIEW — tar-strict-framing

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

## Floor-gate findings (blocking)

None.

- **L-floor (P0):** a `g`/`x` payload over 64 KiB is refused on its size field before `readPaxKeywords`
runs; after the first zero block every byte must be zero (a linear byte loop, no allocation — grill
#1); a walk that runs out without a zero block is refused; the header must start `ustar` (both POSIX
and old-GNU variants — grill #2); `''`/`.` segments below the root are refused.
- **L-eval (P1):** 7 cases were accepted by the base source; the pax case asserts the refusal message,
not a timing (grill #3). The codeload-shaped fixture still extracts.
- **L-trust (P2):** only narrows what the untrusted archive may contain.
- **L-axis (P3):** contained in `tar-extract.ts`.

## Advisory findings

```yaml
- type: FINDING
rule_id: 'P4'
severity: minor
file: 'THREAT-MODEL.md'
problem: 'THREAT-MODEL.md §2 (the extractor rejection list) should gain these framing rules and the pax payload cap; it is write-protected, so a maintainer adds them.'
evidence: 'tar archive has a pax header of N bytes, over the 65536-byte limit'
- type: FINDING
rule_id: 'P5'
severity: minor
file: 'src/lib/tar-extract.ts'
problem: 'The decompressed archive itself is still held in memory up to the 128 MiB cap; that bound is unchanged (and documented) — only the pax PARSE cost is new here.'
evidence: 'gunzipSync(archive, { maxOutputLength: limits.maxTotalBytes })'
- type: FINDING
rule_id: 'P4'
severity: minor
file: 'CHANGELOG.md:8'
problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)."
evidence: '## [Unreleased]'
```

## Verdict

**GREEN** — 0 floor-gate findings, 3 advisory findings. No lesson proposed for canon.
17 changes: 17 additions & 0 deletions .dev/features/tar-strict-framing/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# SHIP — tar-strict-framing

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/tar-strict-framing/VERIFY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# VERIFY — tar-strict-framing

## 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.
20 changes: 20 additions & 0 deletions .dev/features/tar-strict-framing/regression-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"base": "00105215205cbdb994e8bea6d80eca4891d07865",
"inside": [
"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"
}
17 changes: 17 additions & 0 deletions .dev/features/tar-strict-framing/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"feature": "tar-strict-framing",
"gates": {
"format:check": 0,
"lint": 0,
"lint:md": 0,
"test": 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/terminal-safe-text/SHIP.md"
".dev/features/tar-strict-framing/SHIP.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T10:59:45.280Z"
"set_at": "2026-09-24T11:05:27.882Z"
}
52 changes: 51 additions & 1 deletion src/lib/tar-extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const LEN_SIZE = 12;
const OFF_CHKSUM = 148;
const LEN_CHKSUM = 8;
const OFF_TYPEFLAG = 156;
const OFF_MAGIC = 257;
const OFF_PREFIX = 345;
const LEN_PREFIX = 155;

Expand Down Expand Up @@ -221,6 +222,20 @@ function describeTypeflag(typeflag: string): string {
: `'${typeflag}'`;
}

/** The largest pax (`g`/`x`) payload whose records are parsed at all. */
const MAX_PAX_BYTES = 64 * 1024;

/** Refuse any non-zero byte from `from` to the end (after the end marker). */
function assertZeroTail(tar: Buffer, from: number): void {
for (let i = from; i < tar.length; i += 1) {
if (tar[i] !== 0) {
throw new TarExtractError(
'tar archive has data after its end-of-archive marker.',
);
}
}
}

/** True when the block is 512 NUL bytes — the end-of-archive marker. */
function isZeroBlock(block: Buffer): boolean {
for (const byte of block) if (byte !== 0) return false;
Expand Down Expand Up @@ -292,6 +307,13 @@ function resolveEntryPath(
);
}
const rest = segments.slice(1);
// Below the root, every segment must name something: `a//b` and `a/./b` are
// refused here rather than left to safeJoin's lexical backstop.
if (rest.some((segment) => segment === '' || segment === '.')) {
throw new TarExtractError(
`tar entry has an empty or "." path segment: ${fullPath}`,
);
}
if (rest.length === 0) {
// The root directory entry itself: nothing to write, not a failure.
if (isDirectory) return { rel: null, root };
Expand Down Expand Up @@ -337,12 +359,25 @@ export function extractTar(

while (offset + BLOCK <= tar.length) {
const header = tar.subarray(offset, offset + BLOCK);
if (isZeroBlock(header)) break;
// The end-of-archive marker. Everything after it must be zero padding: a
// zero block followed by more entries used to end the walk there and drop
// the rest of the tree in silence, and trailing garbage was accepted.
if (isZeroBlock(header)) {
assertZeroTail(tar, offset + BLOCK);
return;
}
if (!checksumOk(header)) {
throw new TarExtractError(
'tar header failed its checksum — the archive is corrupt or truncated.',
);
}
// Every header git archive writes is ustar (`ustar\0`); old GNU writes
// `ustar ` — the 5-byte prefix admits both and nothing else.
if (header.toString('latin1', OFF_MAGIC, OFF_MAGIC + 5) !== 'ustar') {
throw new TarExtractError(
'tar header is not a ustar header (missing "ustar" magic).',
);
}

const size = readOctal(header, OFF_SIZE, LEN_SIZE);
const dataStart = offset + BLOCK;
Expand All @@ -355,6 +390,16 @@ export function extractTar(
const next = dataStart + Math.ceil(size / BLOCK) * BLOCK;
const typeflag = String.fromCharCode(header[OFF_TYPEFLAG]!);

// A pax header's records are parsed (below) only when its payload is small:
// git archive writes one `comment=<sha>` record (~52 bytes), while a payload
// inflated to the decompression cap cost seconds of CPU and a gigabyte of
// memory to parse. Refused on the size field alone, before any parse.
if ((typeflag === 'x' || typeflag === 'g') && size > MAX_PAX_BYTES) {
throw new TarExtractError(
`tar archive has a pax header of ${size} bytes, over the ${MAX_PAX_BYTES}-byte limit pharn accepts.`,
);
}

// REJECT — a per-file pax extended header. The decision is the typeflag
// ALONE; the payload is read only afterwards, to name what was seen. (The
// ustar size field above is still read first, so an 'x' whose size field is
Expand Down Expand Up @@ -438,4 +483,9 @@ export function extractTar(

offset = next;
}
// The walk ran out of bytes without meeting a zero block: the archive was cut
// short (every real tar ends with at least one).
throw new TarExtractError(
'tar archive has no end-of-archive marker — truncated.',
);
}
Loading
Loading