diff --git a/CHANGELOG.md b/CHANGELOG.md index 0674799..00e0a47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in ## Unreleased +- ZIP archives declare UTF-8 names in both headers, so ordinary extractors preserve non-ASCII paths declared by the package manifest (#345). + - Machine config saves preserve unpatched raw role and posture values, including typos that diagnostics must still report. An unreadable config without a usable last-good snapshot refuses an unrelated save rather than persisting the restrictive fallback; diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8cd149d..a35be35 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -176,7 +176,7 @@ is an open decision (2026-09-30): | `property-vector-conformance.mjs` | Exports the property suite's canonical-hash and graph findings as a static vector packet, and re-verifies them — in Node and in Python, against the same file. Read-only in all three files. Its value is that it turns property tests, which generate their own inputs and could drift with the generator, into **fixed vectors a second language can check**: canonical hashing invariant under object insertion order and sensitive to authored array order, predecessor closure across generated chains, and descendant invalidation that is transitive, sorted, duplicate-free and permutation-invariant. Two of its five tests guard the fixture rather than the code — that the exported vectors stay synthetic, and that the exported graph and hash values stay bound to the property seeds that produced them. | ADR-0029 | | `r29-workbook-linkage-validator.mjs` | Cross-reference validator for the R29 reviewer-workbook linkage register: 24 task ids, six expected reference kinds (task/gold/source/calibration/warm/lock), and a status reduction that takes the worst of `PASS < INCOMPLETE < FAIL < REOPEN`. **Zero imports** — it is 58 lines of pure functions over `{ register, catalog, pointer }` passed in as arguments, so it touches no filesystem, no network and no clock, and the caller owns every input. That is also why it needed no sealed-record fixture: its tests synthesise a register into a temp directory rather than shipping one. A revoked lock pointer reduces to `REOPEN` rather than `FAIL`, which is the distinction the rank order exists for — a withdrawn approval is not the same as a broken link. | ADR-0029 | | `config.mjs` | A **compatibility shim**, nine lines, re-exporting `machine.mjs`'s config surface so an older import path keeps working. It adds nothing and decides nothing. Listed because it was the one module in `lib/` this map did not mention — found by counting files against rows during the 2026-09-20 reconciliation, which is the only way that kind of omission surfaces. | ADR-0002 | -| `archive.mjs` | **One container format**: `buildZip(entries)` → bytes, `writeZip(file, entries)` → `{ file, bytes, entries }` (written whole or not at all, through `writeBytes`, since 2026-09-28), `crc32`, `entryName`. ZIP with deflate (store when deflating would only grow the payload), written because the kit has no dependencies and cannot shell out to a `zip` binary that may not exist. Knows the container and nothing else — what goes inside a bundle is `lib/audit.mjs`'s. Refuses entry names that are not plain relative paths (an archive must not unpack outside its folder) and an archive with no entries. See ADR-0019. **Changes no name it is handed** (2026-09-28): `entryName` refuses a backslash - it translated one into a separator, which stored `a\b.md` as `a/b.md` while the package manifest still declared the original. | ADR-0019 | +| `archive.mjs` | **One container format**: `buildZip(entries)` → bytes, `writeZip(file, entries)` → `{ file, bytes, entries }` (written whole or not at all, through `writeBytes`, since 2026-09-28), `crc32`, `entryName`. ZIP with deflate (store when deflating would only grow the payload), with UTF-8 name encoding declared by bit 11 in both local and central headers, preserving Unicode manifest paths when ordinary ZIP tools extract them (#345); written because the kit has no dependencies and cannot shell out to a `zip` binary that may not exist. Knows the container and nothing else — what goes inside a bundle is `lib/audit.mjs`'s. Refuses entry names that are not plain relative paths (an archive must not unpack outside its folder) and an archive with no entries. See ADR-0019. **Changes no name it is handed** (2026-09-28): `entryName` refuses a backslash - it translated one into a separator, which stored `a\b.md` as `a/b.md` while the package manifest still declared the original. | ADR-0019 | | `artifact-zip.mjs` | **Reading a ZIP somebody else wrote** — the opposite job from `archive.mjs`, and a separate file on purpose: a writer that also parses hostile input grows a trust boundary in the middle of itself. `openZip(bytes)` → `{ entries, names, problems, has, read }`, plus `checkEntryName`, `readCentralDirectory`, `inspectEntries`, `readEntry` and the exported `ZIP_LIMITS` (entry count, total and per-entry uncompressed bytes, ratio + floor, name length) so tests drive the refusal paths with small fixtures instead of real bombs. **The order of operations is the security property**: every structural refusal — traversal, absolute, drive letter, backslash, NUL, duplicate, case collision, file/directory-prefix collision, symlink/special entry, encryption, unknown method, counts, sizes, ratio — is decided from the **central directory before one byte is inflated**. The directory is attacker-controlled too, so it is never the only defence: `inflateRawSync` runs with `maxOutputLength`, making the ceiling the runtime's to enforce rather than a number compared against a field the attacker wrote. Names are judged as raw text, never `path.normalize`'d first — normalising would let `a/../../b` become `../b` and then be judged, when the honest answer is that `..` has no business in a research artifact. Touches no filesystem: entries come back as buffers. ZIP64 is refused rather than half-read. | ADR-0032 | | `artifact-validator.mjs` | Ledger verification follows actual archive capture entries and parsed evidence rows, so relabelling roles or declaring zero captures cannot disable provenance checking; an empty failed-collection scaffold still passes (#358). **Is this package valid, and does it authorize a build** — two questions, kept apart. `validateArtifact({ file, expectedClientRef })` → `{ status, buildAuthorized, packageId, clientRef, workflowRunId, state, errors, warnings }`, where `status: 'PASS'` means internally consistent and says *nothing* about permission. Offline and read-only by construction: no transport imported, `process.env` never read, nothing from the package executed, no shell, and the only write is a `fs.mkdtemp` copy of `project/` removed in a `finally` — needed because the chain and handoff checks are the kit's existing code (`readCorpus`, `verifyHandoff`) and reuse beats a second implementation of provenance verification nobody tested. Stable error codes (`ZIP-*`, `MANIFEST-*`, `FILE-*`, `LEDGER-*`, `CAPTURE-*`, `AUTHORIZATION-INCONSISTENT`, `SECRET-DETECTED`, `FORMAT-UNSUPPORTED`, `CLIENT-REF-MISMATCH`) and **never a secret value in an error** — the report names the pattern and the line. `authorizationProblems()` is the one implementation of the build rule; the schema encodes it a second time and when the schema catches it first the rule is still reported in its own words. An unsupported format **major** is `INCOMPLETE`, not invalid. A crash is `BLOCKED`, never `PASS`. Its report carries `reviewedBy` (`review.by`, or `undeclared` for a 1.0.0 package) and judges nothing by it (ADR-0074). Report metadata is surfaced only after the manifest passes schema validation; malformed state and identity fields are null, and the CLI escapes control characters in every text diagnostic so hostile strings cannot forge lines. Reads format majors **1 and 2** (2026-09-30, ADR-0107): a 1.x `HUMAN_REVIEW_REQUIRED` is reported as `REVIEW_REQUIRED`, and `eraProblems` refuses a value from the other major by name - a 2.x package with the old state or `review.by: "human"`, or a 1.x one with the new state. A binary body (NUL in its first 8000 bytes) is not matched against the credential patterns, as a body over 512 KiB is not (ADR-0128, 2026-10-02): `docs/` travels whole, and one vendored DLL carrying the PEM marker it parses would have failed a package with no secret in it. Missing, ill-typed, malformed and below-oldest format versions are MANIFEST-SCHEMA/FAIL with regeneration as the remedy; only a numeric format major newer than this reader is FORMAT-UNSUPPORTED/INCOMPLETE (#361). Claimed approvals are re-derived from checked bytes with the portable unpromoted pluralist, 180-day baseline; warnings name that baseline and format-1 legacy compatibility. | ADR-0032, ADR-0160 | | `artifact.mjs` | Raw-cell review reads and explicit ledger reads use projectFile under ADR-0109; Raw reads stay inside the capture folder, and a non-file ledger is refused before reading (#351). **Packaging a project without changing it; shared authorization derivation lives in `artifact-state.mjs`.** `createArtifact` → `{ bytes, name, manifest, entries, derived }`; `writeArtifact` writes then **re-validates with the consumer's own validator** before reporting success; its bytes go through `writeBytes`, so re-creating a package at a name that already holds one cannot trade a good package for a truncated one (2026-09-28). There is no `--build-authorized` and no option that reaches an authorization field: `deriveState(root)` runs the real gate with `env` defaulting to `{}` so a packaging shell's override cannot become a property of a package that outlives it. `mapClassified` and `briefReviewed` have honest machine answers; `findingsReviewState` answers the third **negatively** — a `Finding` byte-identical to what `firstFinding` re-extracts from the capture was demonstrably never rewritten, which detects the unreviewed case without grading the reviewed one (ADR-0013's line, uncrossed). It repeats the collector's own call exactly - the page body after the front-matter, falling back to the page's `title` or `url` from that front-matter - since 2026-09-27: run over the whole file, the extractor read the front-matter as the page, and a URL-only corpus nobody had touched (one JSON capture, run 36287211468) was packaged `findingsReviewed: true`. `packageName` never carries the topic (an artifact listing is world-readable on a public repository) and falls back to `run-`; `checkClientRef` refuses rather than sanitises. Inventory sorted by ZIP path, manifest and digest appended **last**, digest over the exact manifest bytes. README-FIRST in a package that is not yet approved prints the preflight and brief commands and **one `create` line carrying the package's own identity** (`repackageArgs`, 2026-09-27): the manifest is written once, and the package gave its reviewer no way to get one that says `APPROVED_BRIEF`. Commands are spelled by `homeCommand` (ADR-0050). `createArtifact` checks the identity against the manifest schema's own `source` rules before deriving or writing anything (`sourceProblem`, 2026-09-27): `create --commit abc` wrote a package, then reported it did not validate, and left it on disk. **`review.by`** (format 1.1.0, ADR-0074) is the reviewer's declaration read from `BRIEF.md` by `reviewedBy`: `agent` or `undeclared` (`human` was retired in format 2.0.0, ADR-0107). Not an approval condition. README-FIRST no longer says "a human reviewed it"; it names the declared reviewer, or says none was declared. `collectProjectFiles` and the explicit ledger add refuse (`OUTSIDE_PROJECT`) any path whose real location is outside the project: it followed links and packed outside files, a private key included (ADR-0076). The same walk asks the consumer's own `checkEntryName` about every name it is about to store and refuses the run before a byte is written (`UNPACKAGEABLE_NAME`, 2026-09-28): `entryName` translates a backslash into a path separator, so a file named `a\b.md` travelled as `a/b.md` - a directory invented out of a name - while the manifest kept the original, and only the read-back caught the contradiction, after the package had been written. The same refusal covers two files differing only in case (2026-09-28), with the reader's lowercase comparison: the reader's ZIP-CASE-COLLISION fired only after the write. Format **2.0.0** (2026-09-30, ADR-0107): a collected package waiting for review says `REVIEW_REQUIRED`, and README-FIRST gives the review to the agent alone. collectProjectFiles refuses (`UNPACKAGEABLE_FILE`) what packaging cannot read to an end - a FIFO, socket or device, and a folder link back into its own ancestry (judged by real path, so a link to a sibling folder inside the project still packages) - before a byte is read (2026-10-01). **A reviewed brief is complete and current** (2026-10-03, output-reliability audit G1): `deriveState` approves only when every judged section is present and answered (`judgedSection`) and a stamped brief's inputs hash matches the corpus (`draftStamp`, `briefInputsHash`); `briefState` alone read a missing section as answered, so a drafted brief with no review sections derived APPROVED_BRIEF on the repository's own corpus, and a brief whose evidence changed after the draft stayed authorized beside hygiene's brief-stale warning. A brief with no stamp predates ADR-0055 - the root's own and ten decision briefs - and keeps its approval: its currency cannot be checked, `hygiene/brief-unstamped` says so (ADR-0138), a redraft with --force stamps it. **README-FIRST in every state and the collection summary preserve the producer evaluation warnings** through `renderGateWarnings` (ADR-0143). Authorization fields and machine package formats are unchanged. `bin/artifact.mjs` refuses a value flag given no value by name, exit 3, before anything is read or written (2026-10-11, #280): `create` had asked only whether a required flag was present, so a bare `--ref` went into the manifest as "true", a bare `--run-id` as 1 and a bare `--output` wrote a package named `true` that re-validated PASS; a bare `validate --file` tried to open a file called `true`. Caller references use checkClientRef before token lookup, output-folder creation or dispatch (#375, 2026-10-13); CR/LF are refused by the shared checker, and diagnostics are redacted. The artifact CLI awaits exitAfterFlush for its report, help and refusal endings, preserving queued stdout/stderr and the existing exit status on asynchronous POSIX pipes (#359). | ADR-0032, ADR-0031, ADR-0052, ADR-0143 | diff --git a/research-kit/README.md b/research-kit/README.md index b85ab51..c06356d 100644 --- a/research-kit/README.md +++ b/research-kit/README.md @@ -568,7 +568,7 @@ node research-kit/bin/selftest.mjs # all of it node research-kit/bin/selftest.mjs gate hook # just these files ``` -1843 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means +1845 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means the assertions settled (ADR-0021), and each test is raced against a watchdog (`RESEARCH_KIT_TEST_TIMEOUT`, default 60s). diff --git a/research-kit/lib/archive.mjs b/research-kit/lib/archive.mjs index 4244d1c..d418453 100644 --- a/research-kit/lib/archive.mjs +++ b/research-kit/lib/archive.mjs @@ -73,7 +73,7 @@ export function buildZip(entries, { date = new Date(1980, 0, 1, 0, 0, 0) } = {}) const local = Buffer.alloc(30); local.writeUInt32LE(0x04034b50, 0); local.writeUInt16LE(20, 4); - local.writeUInt16LE(0, 6); + local.writeUInt16LE(0x0800, 6); // Names are UTF-8 (APPNOTE 4.4.4, bit 11). local.writeUInt16LE(method, 8); local.writeUInt16LE(time, 10); local.writeUInt16LE(day, 12); @@ -88,7 +88,7 @@ export function buildZip(entries, { date = new Date(1980, 0, 1, 0, 0, 0) } = {}) dir.writeUInt32LE(0x02014b50, 0); dir.writeUInt16LE(20, 4); dir.writeUInt16LE(20, 6); - dir.writeUInt16LE(0, 8); + dir.writeUInt16LE(0x0800, 8); dir.writeUInt16LE(method, 10); dir.writeUInt16LE(time, 12); dir.writeUInt16LE(day, 14); diff --git a/research-kit/test/artifact-producer.test.mjs b/research-kit/test/artifact-producer.test.mjs index 017d5c9..ac26763 100644 --- a/research-kit/test/artifact-producer.test.mjs +++ b/research-kit/test/artifact-producer.test.mjs @@ -6,7 +6,7 @@ // because the way this gets lost is somebody adding a convenient option later. import { execFileSync } from 'node:child_process'; -import { test, describe, assert, fs, path, os, cleanup, tempDir, requireSymlink } from './harness.mjs'; +import { test, describe, assert, fs, path, os, cleanup, tempDir, requireSymlink, requirePython } from './harness.mjs'; import { sha256, canonicalJson, resolve, readText, writeText, today, PATHS, HEADERS } from '../lib/core.mjs'; import { createArtifact, deriveState, collectProjectFiles, packageName, checkClientRef, @@ -25,6 +25,23 @@ describe('artifact-producer'); const scratch = tempDir('rk-producer-'); +test('Python extracts a Unicode document under the path declared by the manifest', () => { + const root = collectedProject(); + const name = 'docs/über-日本語.md'; + writeText(path.join(root, name), 'Unicode document\n'); + const built = build(root); + const declared = `project/${name}`; + assert.ok(built.manifest.files.some((entry) => entry.path === declared)); + const dir = tempDir('rk-unicode-zip-'); + const zipFile = path.join(dir, 'package.zip'); + const extractDir = path.join(dir, 'extracted'); + fs.writeFileSync(zipFile, built.bytes); + const script = 'import json,sys,zipfile\nwith zipfile.ZipFile(sys.argv[1]) as z:\n print(json.dumps(z.namelist()))\n z.extractall(sys.argv[2])\n'; + const names = JSON.parse(execFileSync(requirePython('Unicode ZIP extraction'), ['-I', '-c', script, zipFile, extractDir], { encoding: 'utf8' })); + assert.ok(names.includes(declared), JSON.stringify(names)); + assert.equal(fs.readFileSync(path.join(extractDir, declared), 'utf8'), 'Unicode document\n'); +}); + function build(root, overrides = {}) { return createArtifact({ root, ...IDENTITY, ...overrides }); } diff --git a/research-kit/test/audit.test.mjs b/research-kit/test/audit.test.mjs index 070516f..4b73bb7 100644 --- a/research-kit/test/audit.test.mjs +++ b/research-kit/test/audit.test.mjs @@ -129,7 +129,7 @@ function readZip(bytes) { assert.equal(data.length, uncompressed, `${name}: size disagrees with the directory`); assert.equal(crc32(data), sum, `${name}: CRC disagrees with the directory`); - out.push({ name, text: data.toString('utf8'), method }); + out.push({ name, text: data.toString('utf8'), method, flags: bytes.readUInt16LE(at + 8), localFlags: bytes.readUInt16LE(offset + 6) }); at += 46 + nameLen + bytes.readUInt16LE(at + 30) + bytes.readUInt16LE(at + 32); } return out; @@ -153,6 +153,16 @@ test('two runs produce byte-identical archives, so re-zipping is free', () => { assert.ok(buildZip(entries).equals(buildZip(entries))); }); +test('UTF-8 names declare their encoding in both ZIP headers', () => { + const names = ['ascii.md', 'docs/über-日本語.md']; + const entries = readZip(buildZip(names.map((name) => ({ name, data: 'contents' })))); + assert.deepEqual(entries.map((entry) => entry.name), names); + for (const entry of entries) { + assert.equal(entry.flags & 0x0800, 0x0800, `${entry.name}: central UTF-8 flag`); + assert.equal(entry.localFlags & 0x0800, 0x0800, `${entry.name}: local UTF-8 flag`); + } +}); + // --- the audit --------------------------------------------------------------------- test('an audit renders only when the gate passes', () => {