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/terminal-safe-text/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# GRILL — terminal-safe-text

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

## Findings

```yaml
- type: FINDING
rule_id: 'P2'
severity: important
file: '.dev/features/terminal-safe-text/PLAN.md:8'
problem: 'Keeping `\n` in the sink still lets a hostile name start a new line that LOOKS like a separate pharn message; acceptable (no terminal control), but the extractor refusal must itself render the name with newlines stripped so the refusal line is one line.'
evidence: '`sanitizes its whole message (keeping \n/\t)`'
- type: FINDING
rule_id: 'P5'
severity: minor
file: '.dev/features/terminal-safe-text/PLAN.md:12'
problem: 'Check the FULL reconstructed path (prefix + name) before any other rule, so a control byte in the ustar `prefix` field is refused too, and before the typeflag check so an unsupported-type entry is refused by path first when both apply — or render both safely; either way no raw byte.'
evidence: '`REFUSES a tar entry whose path contains …`'
- type: FINDING
rule_id: 'P1'
severity: minor
file: '.dev/features/terminal-safe-text/PLAN.md:31'
problem: 'The sink test must spy on the real stream write (stderr), not on the argument to logError, or it passes by construction.'
evidence: '`an ESC sequence in a fatal message never reaches stderr raw`'
```

ADVISORY VERDICT: 3 concerns raised (0 blocking-severity, 3 advisory) — all folded into the build.
60 changes: 60 additions & 0 deletions .dev/features/terminal-safe-text/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# PLAN — terminal-safe-text (PHARN-17: upstream-derived text never reaches the terminal raw)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: three layers. (1) The extractor REFUSES a tar entry whose path contains a C0/C1 control
character or a Unicode format character (`\p{Cf}`: U+202E, U+200B, …) — upstream has none (2161 paths
checked in the review), so nothing with such a name is ever installed or later printed by
`status`/`update`. (2) One shared sanitizer, `lib/terminal-safe.ts`, strips C0/C1 + `\p{Cf}` and caps
length; `unknown-capabilities.ts` uses it (gaining `\p{Cf}`), and every extractor message that
interpolates an entry name or typeflag renders it through it. (3) The fatal sink `logError`
(`report-error.ts`) sanitizes its whole message (keeping `\n`/`\t`), so no fatal path — present or
future — can print a raw escape sequence.
- layer(s): the CLI itself (`src/lib/*`)
- constitution_refs: [P1, P2, P3, P5]

## Discovery — verified this run (P6)

Review repro: `unsupported type '2': pharn-oss-…/n^[[2K^M^[[32mOK` printed by `reportFatal` in
`init`/`status` (a faked "OK", line erase, OSC 52 clipboard write possible). `unknown-capabilities.ts`
strips `[\x00-\x1f\x7f-\x9f]` but not U+202E/U+200B. `resolveEntryPath` (`tar-extract.ts`) checks `..`,
absolute, root only. `logError` passes messages verbatim; no caller passes colored text (grep: 31 call
sites, none with `pc.`/ANSI).

## Files

- `src/lib/terminal-safe.ts` — NEW: `terminalSafe(value, opts)` (strip C0/C1 + `\p{Cf}`, optional keep
`\n`/`\t`, optional length cap) + `hasUnsafeChars(value)` — layer CLI/lib
- `src/lib/unknown-capabilities.ts` — use the shared sanitizer — layer CLI/lib
- `src/lib/report-error.ts` — `logError` sanitizes (keeps newlines/tabs) — layer CLI/lib
- `src/lib/tar-extract.ts` — reject unsafe entry paths; render interpolated names/typeflags safely — layer CLI/lib
- `tests/terminal-safe.test.ts` — NEW: ESC/CSI, OSC, C1, U+202E, U+200B stripped; newline policy; cap
- `tests/unknown-capabilities.test.ts` — a U+202E name is neutralised
- `tests/report-error.test.ts` — an ESC sequence in a fatal message never reaches stderr raw
- `tests/tar-extract.test.ts` — entry paths with ESC / U+202E refused; the unsupported-type message
carries no raw control byte

## Contracts satisfied

- `unknown-capabilities.ts` header: "control characters are stripped … BEFORE it reaches the terminal" —
now including format characters, and extended to the fatal path.

## Evals to write (P1)

- listed above; the extractor, sink and U+202E cases fail on the base source.

## Guarantee audit (P0)

- "no upstream-derived byte in {C0, C1, Cf} reaches stderr through the fatal sink" → floor: regex strip
at `logError` + refusal at extraction.

## Trust audit (P2)

- Narrows accepted archive names; sanitizes display only (never used for paths).

## Determinism audit (P5)

- Pure string functions.

## Open questions (HALT)

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

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

## Base and partition

- **base:** `1c4b10737729faddaf7deb5ef4d1c06471d3d94d` (`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/report-error.ts`, `src/lib/tar-extract.ts`, `src/lib/terminal-safe.ts`, `src/lib/unknown-capabilities.ts`, `tests/report-error.test.ts`, `tests/tar-extract.test.ts`, `tests/terminal-safe.test.ts`, `tests/unknown-capabilities.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.
49 changes: 49 additions & 0 deletions .dev/features/terminal-safe-text/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# REVIEW — terminal-safe-text

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

## Floor-gate findings (blocking)

None.

- **L-floor (P0):** (1) the extractor refuses any entry whose full path (prefix + name, UTF-8 decoded)
holds a C0/C1/`\p{Cf}` character — judged before every other entry rule (grill #2), and the refusal
renders the name sanitized on one line (grill #1); (2) `unknown-capabilities.ts` now uses the shared
sanitizer, gaining `\p{Cf}`; (3) `logError` strips the same set (keeping `\n`/`\t`) for every fatal.
- **L-eval (P1):** 9 cases fail on the base source (extractor ×6, sink ×2, U+202E in the unknown
list); `terminal-safe.test.ts` pins the sanitizer. An ordinary non-ASCII name still extracts.
- **L-trust (P2):** narrows accepted archive names; sanitizing is display-only.
- **L-axis (P3):** one sanitizer module; the extractor owns the refusal, the sink owns display.

## Advisory findings

```yaml
- type: FINDING
rule_id: 'P1'
severity: minor
file: 'tests/report-error.test.ts'
problem: 'The sink test asserts the argument handed to the mocked clack `log.error` (the last hop inside pharn), not bytes on the real stderr stream (grill #3); clack adds only its own prefix.'
evidence: "const sent = vi.mocked(log.error).mock.calls[0]![0];"
- type: FINDING
rule_id: 'P4'
severity: minor
file: 'THREAT-MODEL.md'
problem: 'THREAT-MODEL.md §2 lists what the extractor rejects; the new control/format-character refusal should be added by a maintainer (write-protected, human-only).'
evidence: 'see `THREAT-MODEL.md` §2 for what the extractor rejects'
- type: FINDING
rule_id: 'P5'
severity: minor
file: 'src/lib/tar-extract.ts'
problem: 'Non-fatal warnings (log.warn outside unknown-capabilities) are not routed through the sanitizer; none of them interpolates archive-derived text today.'
evidence: 'logError sanitizes (the fatal sink only)'
- 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, 4 advisory findings. No lesson proposed for canon.
17 changes: 17 additions & 0 deletions .dev/features/terminal-safe-text/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# SHIP — terminal-safe-text

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

## 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.
26 changes: 26 additions & 0 deletions .dev/features/terminal-safe-text/regression-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"base": "1c4b10737729faddaf7deb5ef4d1c06471d3d94d",
"inside": [
"src/lib/report-error.ts",
"src/lib/tar-extract.ts",
"src/lib/terminal-safe.ts",
"src/lib/unknown-capabilities.ts",
"tests/report-error.test.ts",
"tests/tar-extract.test.ts",
"tests/terminal-safe.test.ts",
"tests/unknown-capabilities.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/terminal-safe-text/verify-report.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"feature": "terminal-safe-text",
"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/init-dest-type-preflight/SHIP.md"
".dev/features/terminal-safe-text/SHIP.md"
],
"set_by": ".claude/commands/pharn-dev-ship.md",
"set_at": "2026-09-24T10:53:06.579Z"
"set_at": "2026-09-24T10:59:45.280Z"
}
12 changes: 9 additions & 3 deletions src/lib/report-error.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { log } from '@clack/prompts';
import { terminalSafe } from './terminal-safe.js';

// The ONE place the CLI turns a failure into user-visible output. Two axes, one
// file (P3): WHICH STREAM the line goes to, and WHETHER the failure came from an
Expand Down Expand Up @@ -48,11 +49,16 @@ export function errorMessage(err: unknown): string {
* exited 1 with 0 bytes on stderr, leaving the operator grepping an empty file.
* Exit codes were always correct — this is the stream contract, not correctness.
*
* The message is passed through verbatim. Callers own their wording; this owns
* the stream.
* Callers own their wording; this owns the stream — and what the terminal is
* allowed to INTERPRET. A fatal message can carry text from the fetched archive
* (an extractor refusal naming an entry), so control and Unicode format
* characters are stripped here, at the one sink every fatal path shares; `\n`
* and `\t` are kept for multi-line messages. No caller passes styled text.
*/
export function logError(message: string): void {
log.error(message, { output: process.stderr });
log.error(terminalSafe(message, { keepNewlines: true }), {
output: process.stderr,
});
}

/**
Expand Down
41 changes: 37 additions & 4 deletions src/lib/tar-extract.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { gunzipSync } from 'node:zlib';
import { mkdirSync, writeFileSync } from 'node:fs';
import { dirname } from 'node:path';
import { hasUnsafeChars, terminalSafe } from './terminal-safe.js';
import { safeJoin } from './validate.js';

/**
Expand Down Expand Up @@ -192,6 +193,34 @@ function describeKeywords(keywords: string[] | null): string {
return `records ${shown.join(', ')}${more > 0 ? ` (+${more} more)` : ''}`;
}

/**
* Refuse an entry path holding a control character or a Unicode format
* character (U+202E, U+200B, …). Such a name would be printed later — by an
* error here, or by `status`/`update` listing installed files — and a terminal
* interprets those characters. Names are read as latin1 (one char per byte), so
* the check runs on the UTF-8 decoding of the same bytes: that is where a
* multi-byte U+202E becomes visible, while an ordinary non-ASCII name (`é`,
* `—`) decodes to printable text and passes. pharn-oss ships no such name.
*/
function assertDisplayablePath(fullPath: string): void {
if (hasUnsafeChars(Buffer.from(fullPath, 'latin1').toString('utf8'))) {
throw new TarExtractError(
`tar entry path contains a control or format character: ${JSON.stringify(
terminalSafe(Buffer.from(fullPath, 'latin1').toString('utf8'), {
max: 200,
}),
)}`,
);
}
}

/** A typeflag for a message: the character if printable, else its code. */
function describeTypeflag(typeflag: string): string {
return hasUnsafeChars(typeflag)
? `0x${typeflag.charCodeAt(0).toString(16).padStart(2, '0')}`
: `'${typeflag}'`;
}

/** 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 @@ -364,17 +393,21 @@ export function extractTar(
continue;
}

// The FULL path (prefix + name) is judged before any other entry rule, so
// no later message can interpolate a control or format character from it.
const name = readString(header, OFF_NAME, LEN_NAME);
const prefix = readString(header, OFF_PREFIX, LEN_PREFIX);
const fullPath = prefix === '' ? name : `${prefix}/${name}`;
assertDisplayablePath(fullPath);

const isFile = typeflag === '0' || typeflag === '\0';
const isDirectory = typeflag === '5';
if (!isFile && !isDirectory) {
throw new TarExtractError(
`tar entry has an unsupported type '${typeflag}': ${readString(header, OFF_NAME, LEN_NAME)}`,
`tar entry has an unsupported type ${describeTypeflag(typeflag)}: ${fullPath}`,
);
}

const name = readString(header, OFF_NAME, LEN_NAME);
const prefix = readString(header, OFF_PREFIX, LEN_PREFIX);
const fullPath = prefix === '' ? name : `${prefix}/${name}`;
const { rel, root } = resolveEntryPath(fullPath, isDirectory, expectedRoot);
expectedRoot = root;

Expand Down
Loading
Loading