* test(#2701): failing-first regression for NUL-corrupted plan/state validators * fix(#2701): reject NUL-corrupted plan/state artifacts at the validator entry points * fix(#2701): seed STATE.md in test (writeState); add NUL-path guards to validate/verify for parity (review) * docs(changeset): #2701 validators reject NUL-corrupted artifacts * docs(changeset): backfill #2701 PR number to 2829
This commit is contained in:
Tom Boucher
5
.changeset/humble-badgers-caper.md
Normal file
5
.changeset/humble-badgers-caper.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2829
|
||||
---
|
||||
**Plan, summary, verification, and state validators now reject NUL-corrupted files** — `frontmatter validate`, `verify plan-structure`, and `state validate` now fail loud (valid:false) when a file contains embedded NUL bytes, with an error naming the encoding problem and its downstream consequence. Previously such a file passed as valid:true but was silently skipped by recursive/binary-skipping search tools (rg, grep -I), reading downstream as 'file absent' rather than 'file corrupt.'
|
||||
@@ -12,6 +12,7 @@ import path from 'node:path';
|
||||
import ioMod = require('./io.cjs');
|
||||
const { output, error } = ioMod;
|
||||
import { platformReadSync as safeReadFile, platformWriteSync } from './shell-command-projection.cjs';
|
||||
import { textEncodingError } from './validate.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import unusableInputMod = require('./unusable-input.cjs');
|
||||
const { UNUSABLE_REASON, warnUnusableInput } = unusableInputMod;
|
||||
@@ -713,11 +714,17 @@ function cmdFrontmatterMerge(cwd: string, filePath: string, data: string | undef
|
||||
|
||||
function cmdFrontmatterValidate(cwd: string, filePath: string, schemaName: string | undefined, raw: boolean): void {
|
||||
if (!filePath || !schemaName) { error('file and schema required'); }
|
||||
if (filePath.includes('\0')) { error('file path contains null bytes'); }
|
||||
const schema = FRONTMATTER_SCHEMAS[schemaName as string];
|
||||
if (!schema) { error(`Unknown schema: ${schemaName}. Available: ${Object.keys(FRONTMATTER_SCHEMAS).join(', ')}`); }
|
||||
const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
|
||||
const content = safeReadFile(fullPath);
|
||||
if (!content) { output({ error: 'File not found', path: filePath }, raw, undefined); return; }
|
||||
// #2701: fail loud on NUL/binary corruption before schema checks. A structurally
|
||||
// intact-but-NUL-corrupted file otherwise passes as valid:true and is then silently
|
||||
// skipped by recursive/binary-skipping searchers, reading downstream as "absent."
|
||||
const encErr = textEncodingError(content, filePath);
|
||||
if (encErr) { output({ valid: false, errors: [encErr], schema: schemaName }, raw, 'invalid'); return; }
|
||||
// Pass the resolved path so a truncated file is named in the diagnostic and deduplicated
|
||||
// per file rather than per content digest (#1882, ADR-1411 wiring clause).
|
||||
const fm = extractFrontmatter(content, fullPath);
|
||||
|
||||
@@ -49,6 +49,7 @@ import {
|
||||
import { tokenizeHeadings, collectSection, replaceSection } from './markdown-sectionizer.cjs';
|
||||
import type { HeadingToken } from './markdown-sectionizer.cjs';
|
||||
import { parseMarkdownTable, updateTableCell, deleteTableRow, insertTableRow, splitTableRow, isDelimiterRow } from './markdown-table.cjs';
|
||||
import { textEncodingError } from './validate.cjs';
|
||||
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -2639,6 +2640,14 @@ function cmdStateValidate(cwd: string, raw: boolean): void {
|
||||
}
|
||||
|
||||
const content = fs.readFileSync(statePath, 'utf-8');
|
||||
// #2701: fail loud on NUL/binary corruption before drift checks. A corrupt
|
||||
// STATE.md otherwise validates as clean and is silently skipped by recursive
|
||||
// searchers downstream, reading as "absent" rather than "corrupt."
|
||||
const encErr = textEncodingError(content, 'STATE.md');
|
||||
if (encErr) {
|
||||
output({ valid: false, warnings: [encErr], drift: {} }, raw, undefined);
|
||||
return;
|
||||
}
|
||||
const warnings: string[] = [];
|
||||
const drift: Record<string, unknown> = {};
|
||||
|
||||
|
||||
@@ -152,3 +152,36 @@ export function buildNotStartedPhaseVariants(roadmapContent: string): Set<string
|
||||
}
|
||||
return notStartedPhases;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect binary corruption (embedded NUL bytes) in a text artifact's bytes.
|
||||
*
|
||||
* #2701: the plan/summary/verification/state validators must FAIL LOUD on a
|
||||
* NUL-corrupted file instead of reporting `valid: true`. A NUL byte is the
|
||||
* unambiguous signal — UTF-8 text never contains 0x00 — and a file carrying one
|
||||
* is binary-classified by `file(1)`, then silently OMITTED from recursive /
|
||||
* binary-skipping search results (`rg -l`, `grep -rI`, exit 0), so the corruption
|
||||
* reads downstream as "file absent" rather than "file corrupt." The error message
|
||||
* names that consequence so the next investigator is not misdirected.
|
||||
*
|
||||
* This is a pure, opt-in check called explicitly by each validator at its own
|
||||
* entry point. It is deliberately NOT placed inside the shared `platformReadSync`
|
||||
* read primitive (which dozens of best-effort, tolerant reads flow through and
|
||||
* which must not start hard-failing on encoding). It does NOT strip, sanitize, or
|
||||
* repair the NUL bytes — corruption is a signal of an upstream authoring-tool bug
|
||||
* and must stay visible.
|
||||
*
|
||||
* @param buf the file bytes (Buffer or string; a string is searched char-wise)
|
||||
* @param relPath a path/label for the diagnostic message
|
||||
* @returns an error string when NUL is found, or `null` when the bytes are clean text
|
||||
*/
|
||||
export function textEncodingError(buf: Buffer | string, relPath: string): string | null {
|
||||
const nul = typeof buf === 'string' ? buf.indexOf('\0') : buf.indexOf(0x00);
|
||||
if (nul === -1) return null;
|
||||
return (
|
||||
`${relPath}: file contains NUL bytes (first at offset ${nul}). ` +
|
||||
'Artifact files must be UTF-8 text. A NUL-corrupted file is binary-classified ' +
|
||||
'and silently skipped by recursive / binary-skipping search tools (rg, grep -I), ' +
|
||||
'so downstream verification reports its contents as missing rather than corrupt.'
|
||||
);
|
||||
}
|
||||
|
||||
@@ -11,7 +11,7 @@ import path from 'node:path';
|
||||
import os from 'node:os';
|
||||
import { phaseVariants, buildRoadmapPhaseVariants, buildNotStartedPhaseVariants } from './validate.cjs';
|
||||
import { realClock } from './clock.cjs';
|
||||
import { phaseDirNameRe, PHASE_TOKEN_FROM_DIR_RE, MILESTONE_ARCHIVE_DIR_RE, canonicalPlanStem } from './validate.cjs';
|
||||
import { phaseDirNameRe, PHASE_TOKEN_FROM_DIR_RE, MILESTONE_ARCHIVE_DIR_RE, canonicalPlanStem, textEncodingError } from './validate.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module
|
||||
import planningWorkspace = require('./planning-workspace.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module
|
||||
@@ -811,6 +811,7 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo
|
||||
if (!filePath) {
|
||||
error('file path required');
|
||||
}
|
||||
if (filePath.includes('\0')) { error('file path contains null bytes'); }
|
||||
const fullPath = path.isAbsolute(filePath) ? filePath : path.join(cwd, filePath);
|
||||
const content = safeReadFile(fullPath);
|
||||
if (!content) {
|
||||
@@ -818,6 +819,15 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo
|
||||
return;
|
||||
}
|
||||
|
||||
// #2701: fail loud on NUL/binary corruption before structure checks. A
|
||||
// structurally intact-but-NUL-corrupted plan otherwise passes as valid and is
|
||||
// silently skipped by recursive/binary-skipping searchers downstream.
|
||||
const encErr = textEncodingError(content, filePath);
|
||||
if (encErr) {
|
||||
output({ valid: false, errors: [encErr] }, raw);
|
||||
return;
|
||||
}
|
||||
|
||||
const fm = extractFrontmatter(content, fullPath);
|
||||
const errors: string[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
212
tests/issue-2701-nul-corrupted-validators.test.cjs
Normal file
212
tests/issue-2701-nul-corrupted-validators.test.cjs
Normal file
@@ -0,0 +1,212 @@
|
||||
// Regression tests for #2701 — plan/summary/verification/state validators silently
|
||||
// accept NUL-corrupted files and report valid:true.
|
||||
//
|
||||
// A NUL-corrupted text artifact is binary-classified by file(1) and silently
|
||||
// OMITTED from recursive / binary-skipping search results (rg -l, grep -rI,
|
||||
// exit 0), so the corruption reads downstream as "file absent" rather than
|
||||
// "file corrupt." The validators must fail loud, naming the encoding problem and
|
||||
// its consequence, before any schema/structure check. The fix is at the
|
||||
// validator entry points (a shared textEncodingError helper in validate.cjs),
|
||||
// NOT inside the broadly-shared platformReadSync read primitive.
|
||||
//
|
||||
// NUL bytes are written via Buffer so they survive onto disk (a string write
|
||||
// would not). Cleanup via t.after(() => cleanup(tmpDir)).
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
const { writeState } = require('./fixtures/index.cjs');
|
||||
|
||||
// A structurally-complete PLAN.md that passes both validators when clean.
|
||||
function validPlanBody() {
|
||||
return [
|
||||
'---',
|
||||
'phase: 01-test',
|
||||
'plan: 01',
|
||||
'type: execute',
|
||||
'wave: 1',
|
||||
'depends_on: []',
|
||||
'files_modified: [some/file.ts]',
|
||||
'autonomous: true',
|
||||
'must_haves:',
|
||||
' truths:',
|
||||
' - "something is true"',
|
||||
'---',
|
||||
'',
|
||||
'<tasks>',
|
||||
'',
|
||||
'<task type="auto">',
|
||||
' <name>Task 1: Do something</name>',
|
||||
' <files>some/file.ts</files>',
|
||||
' <action>Do the thing</action>',
|
||||
' <verify><automated>npx vitest run</automated></verify>',
|
||||
' <done>Thing is done</done>',
|
||||
'</task>',
|
||||
'',
|
||||
'</tasks>',
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/** Write `body` to a fresh phase plan path, optionally injecting a NUL at `nulAt`. */
|
||||
function writePlan(tmpDir, name, body, nulAt) {
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-test'), { recursive: true });
|
||||
const p = path.join(tmpDir, '.planning', 'phases', '01-test', name);
|
||||
let buf = Buffer.from(body, 'utf8');
|
||||
if (nulAt !== undefined) {
|
||||
buf = Buffer.concat([buf.subarray(0, nulAt), Buffer.from([0x00]), buf.subarray(nulAt)]);
|
||||
}
|
||||
fs.writeFileSync(p, buf);
|
||||
return p;
|
||||
}
|
||||
|
||||
function parseResult(t, argv, tmpDir) {
|
||||
const r = runGsdTools(argv, tmpDir);
|
||||
assert.ok(r.success, `command failed: ${r.error}`);
|
||||
return JSON.parse(r.output);
|
||||
}
|
||||
|
||||
// ─── frontmatter validate --schema plan|summary|verification ────────────────
|
||||
|
||||
describe('#2701: frontmatter validate rejects NUL-corrupted artifacts', () => {
|
||||
test('PLAN.md with an embedded NUL byte → valid:false, error names encoding + consequence', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const rel = '.planning/phases/01-test/01-01-PLAN.md';
|
||||
writePlan(tmpDir, '01-01-PLAN.md', validPlanBody(), 200);
|
||||
|
||||
const out = parseResult(t, ['frontmatter', 'validate', rel, '--schema', 'plan'], tmpDir);
|
||||
assert.strictEqual(out.valid, false, `expected valid:false; got ${JSON.stringify(out)}`);
|
||||
assert.ok(Array.isArray(out.errors) && out.errors.length > 0, 'must report errors');
|
||||
const msg = out.errors.join(' ');
|
||||
assert.ok(/NUL/i.test(msg), `error must name NUL/encoding: ${msg}`);
|
||||
assert.ok(/skip|search|absent|missing/i.test(msg), `error must name the downstream consequence: ${msg}`);
|
||||
});
|
||||
|
||||
test('SUMMARY.md with an embedded NUL byte → valid:false', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const dir = path.join(tmpDir, '.planning', 'phases', '01-test');
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
const body = ['---', 'phase: 01-test', 'plan: 01', 'status: in_progress', '---', '', '# Summary', 'did the work'].join('\n');
|
||||
const buf = Buffer.concat([Buffer.from(body, 'utf8').subarray(0, 30), Buffer.from([0x00]), Buffer.from(body, 'utf8').subarray(30)]);
|
||||
fs.writeFileSync(path.join(dir, '01-01-SUMMARY.md'), buf);
|
||||
|
||||
const out = parseResult(t, ['frontmatter', 'validate', '.planning/phases/01-test/01-01-SUMMARY.md', '--schema', 'summary'], tmpDir);
|
||||
assert.strictEqual(out.valid, false);
|
||||
assert.ok(out.errors.some((e) => /NUL/i.test(e)));
|
||||
});
|
||||
|
||||
test('VERIFICATION.md with an embedded NUL byte → valid:false', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const dir = path.join(tmpDir, '.planning', 'phases', '01-test');
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
const body = ['---', 'phase: 01-test', 'plan: 01', 'status: passed', '---', '', '# Verification', 'all green'].join('\n');
|
||||
const buf = Buffer.concat([Buffer.from(body, 'utf8').subarray(0, 40), Buffer.from([0x00]), Buffer.from(body, 'utf8').subarray(40)]);
|
||||
fs.writeFileSync(path.join(dir, '01-01-VERIFICATION.md'), buf);
|
||||
|
||||
const out = parseResult(t, ['frontmatter', 'validate', '.planning/phases/01-test/01-01-VERIFICATION.md', '--schema', 'verification'], tmpDir);
|
||||
assert.strictEqual(out.valid, false);
|
||||
assert.ok(out.errors.some((e) => /NUL/i.test(e)));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── verify plan-structure ──────────────────────────────────────────────────
|
||||
|
||||
describe('#2701: verify plan-structure rejects NUL-corrupted PLAN.md', () => {
|
||||
test('PLAN.md with an embedded NUL byte → valid:false, error names encoding', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const rel = '.planning/phases/01-test/01-01-PLAN.md';
|
||||
writePlan(tmpDir, '01-01-PLAN.md', validPlanBody(), 200);
|
||||
|
||||
const out = parseResult(t, ['verify', 'plan-structure', rel], tmpDir);
|
||||
assert.strictEqual(out.valid, false, `expected valid:false; got ${JSON.stringify(out)}`);
|
||||
assert.ok(out.errors.some((e) => /NUL/i.test(e)), `error must name NUL: ${JSON.stringify(out.errors)}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── state validate ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('#2701: state validate rejects NUL-corrupted STATE.md', () => {
|
||||
test('STATE.md with an embedded NUL byte → valid:false', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
// createTempProject() does NOT seed STATE.md; use writeState to create one,
|
||||
// then corrupt it in place with a NUL byte (Buffer write so it survives).
|
||||
const seed = [
|
||||
'# Project',
|
||||
'',
|
||||
'## Status',
|
||||
'executing',
|
||||
'## Current Phase',
|
||||
'01 of 01',
|
||||
'## Total Plans in Phase',
|
||||
'1',
|
||||
].join('\n');
|
||||
const statePath = writeState(tmpDir, seed);
|
||||
const body = Buffer.from(seed, 'utf8');
|
||||
const buf = Buffer.concat([body.subarray(0, 50), Buffer.from([0x00]), body.subarray(50)]);
|
||||
fs.writeFileSync(statePath, buf);
|
||||
|
||||
const out = parseResult(t, ['state', 'validate'], tmpDir);
|
||||
assert.strictEqual(out.valid, false, `expected valid:false; got ${JSON.stringify(out)}`);
|
||||
assert.ok(out.warnings.some((w) => /NUL/i.test(w)), `warning must name NUL: ${JSON.stringify(out.warnings)}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── negative space: clean files still pass; non-ASCII UTF-8 not over-rejected ─
|
||||
|
||||
describe('#2701: clean and valid-UTF-8 files are not over-rejected', () => {
|
||||
test('clean PLAN.md (no NUL) → frontmatter validate valid:true', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const rel = '.planning/phases/01-test/01-01-PLAN.md';
|
||||
writePlan(tmpDir, '01-01-PLAN.md', validPlanBody());
|
||||
|
||||
const out = parseResult(t, ['frontmatter', 'validate', rel, '--schema', 'plan'], tmpDir);
|
||||
assert.strictEqual(out.valid, true, `clean plan must pass; got ${JSON.stringify(out)}`);
|
||||
});
|
||||
|
||||
test('clean PLAN.md (no NUL) → verify plan-structure valid:true', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const rel = '.planning/phases/01-test/01-01-PLAN.md';
|
||||
writePlan(tmpDir, '01-01-PLAN.md', validPlanBody());
|
||||
|
||||
const out = parseResult(t, ['verify', 'plan-structure', rel], tmpDir);
|
||||
assert.strictEqual(out.valid, true, `clean plan must pass; got ${JSON.stringify(out)}`);
|
||||
});
|
||||
|
||||
test('non-ASCII UTF-8 (é, emoji) without NUL is NOT rejected', (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const rel = '.planning/phases/01-test/01-01-PLAN.md';
|
||||
// High bytes are valid UTF-8; only a NUL (0x00) is the corruption signal.
|
||||
const body = validPlanBody().replace('Do the thing', 'Do the thing — café ☕ naïve');
|
||||
writePlan(tmpDir, '01-01-PLAN.md', body);
|
||||
|
||||
const out = parseResult(t, ['frontmatter', 'validate', rel, '--schema', 'plan'], tmpDir);
|
||||
assert.strictEqual(out.valid, true, `valid UTF-8 high bytes must not be rejected; got ${JSON.stringify(out)}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── boundary: NUL at offset 0 and mid-file both rejected ───────────────────
|
||||
|
||||
describe('#2701: NUL position does not matter (start and middle both rejected)', () => {
|
||||
for (const nulAt of [0, 5, 250]) {
|
||||
test(`NUL at offset ${nulAt} → frontmatter validate valid:false`, (t) => {
|
||||
const tmpDir = createTempProject();
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const rel = '.planning/phases/01-test/01-01-PLAN.md';
|
||||
writePlan(tmpDir, '01-01-PLAN.md', validPlanBody(), nulAt);
|
||||
|
||||
const out = parseResult(t, ['frontmatter', 'validate', rel, '--schema', 'plan'], tmpDir);
|
||||
assert.strictEqual(out.valid, false, `NUL at offset ${nulAt} must be rejected; got ${JSON.stringify(out)}`);
|
||||
});
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user