From 41bac07dec80d5022a86750fbe221c7e91f4f950 Mon Sep 17 00:00:00 2001 From: Jerry Gamblin Date: Tue, 21 Jul 2026 15:17:00 -0500 Subject: [PATCH 1/3] Harden WarningRegistry against a malformed registry file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Diagnostics result constructs a WarningRegistry, and WarningRegistry.read threw on unparseable or wrong-shaped registry JSON. Because validation builds a Diagnostics for every result, a corrupt warnings.json turned every validation call — including otherwise-successful ones — into a thrown exception. - read() now fails soft: on a JSON parse error or invalid shape it warns on stderr and returns no warnings, so validation is never taken down by a bad registry file. - addWarning() refuses to run when the existing file could not be read, so a corrupt registry is never silently overwritten and its contents destroyed. - Add smoke-test coverage for malformed JSON, wrong-shape content, and the no-clobber guard (with console.warn captured). Co-Authored-By: Claude Opus 4.8 --- scripts/test-local.mjs | 74 +++++++++++++++++++++++++++++++++++++++- src/registry/warnings.ts | 44 +++++++++++++++++++----- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/scripts/test-local.mjs b/scripts/test-local.mjs index 8e058a1..2334f75 100644 --- a/scripts/test-local.mjs +++ b/scripts/test-local.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -331,9 +331,81 @@ async function runWarningTests() { } } +// Runs `callback` with console.warn captured (and silenced), returning the +// collected warning messages so tests can assert a warning was surfaced +// without polluting test output. +async function withCapturedWarnings(callback) { + const messages = []; + const originalWarn = console.warn; + console.warn = (...args) => messages.push(args.join(" ")); + + try { + await callback(); + } finally { + console.warn = originalWarn; + } + + return messages; +} + +async function runWarningRegistryHardeningTests() { + const validWarning = { + messageId: "CVE_SCHEMA_DEPRECATION", + notificationMessage: "This field will be removed in a future schema version.", + notificationDetails: "Use the replacement field before the warning end date.", + schemaPath: "#/definitions/example/properties/deprecatedField", + dateAdded: "2026-07-01T00:00:00.000Z", + dateUpdated: "2026-07-02T00:00:00.000Z", + dateStart: "2026-08-01T00:00:00.000Z", + dateEnd: "2027-08-01T00:00:00.000Z", + priority: 1, + }; + + const temporaryDirectory = await mkdtemp(join(tmpdir(), "cve-warning-registry-hardening-")); + const malformedJsonPath = join(temporaryDirectory, "malformed.json"); + const wrongShapePath = join(temporaryDirectory, "wrong-shape.json"); + + try { + // A registry file containing invalid JSON must not throw on construction, + // and must surface the problem via console.warn while yielding no warnings. + writeFileSync(malformedJsonPath, "{ this is not valid json", "utf8"); + const malformedWarnings = await withCapturedWarnings(() => { + const registry = new WarningRegistry(malformedJsonPath); + assert.deepEqual(registry.warnings, []); + + // Diagnostics construction reads the registry; it must not throw either. + const diagnostics = Diagnostics.success(new WarningRegistry(malformedJsonPath)); + assert.deepEqual(diagnostics.warnings, []); + }); + assert.ok( + malformedWarnings.some((message) => message.includes(malformedJsonPath)), + "expected a console.warn mentioning the malformed registry path", + ); + + // Valid JSON of the wrong shape is treated the same way: fail soft, no throw. + writeFileSync(wrongShapePath, JSON.stringify([{ notAWarning: true }]), "utf8"); + await withCapturedWarnings(() => { + const registry = new WarningRegistry(wrongShapePath); + assert.deepEqual(registry.warnings, []); + }); + + // addWarning must refuse to overwrite an unreadable registry so its contents + // are never silently destroyed; the file stays byte-for-byte unchanged. + const before = readFileSync(malformedJsonPath, "utf8"); + await withCapturedWarnings(() => { + const registry = new WarningRegistry(malformedJsonPath); + assert.throws(() => registry.addWarning(validWarning), /refusing to modify/); + }); + assert.equal(readFileSync(malformedJsonPath, "utf8"), before); + } finally { + await rm(temporaryDirectory, { recursive: true, force: true }); + } +} + runSmokeTests(); runSchemaValidationIntegrationTests(); runBusinessRuleIntegrationTests(); await runWarningTests(); +await runWarningRegistryHardeningTests(); console.log("Local tests passed."); diff --git a/src/registry/warnings.ts b/src/registry/warnings.ts index 00dee83..5a2e655 100644 --- a/src/registry/warnings.ts +++ b/src/registry/warnings.ts @@ -78,6 +78,7 @@ function defaultWarningRegistryPath(): string { export class WarningRegistry { readonly #registryPath: string; readonly #warnings: DiagnosticWarning[]; + readonly #loadError: Error | null; /** * Creates a warning registry. @@ -88,7 +89,10 @@ export class WarningRegistry { this.#registryPath = registryPath instanceof URL ? fileURLToPath(registryPath) : registryPath; - this.#warnings = this.read(); + + const { warnings, error } = this.read(); + this.#warnings = warnings; + this.#loadError = error; } /** All warnings currently registered. */ @@ -141,22 +145,44 @@ export class WarningRegistry { throw new TypeError('Invalid diagnostic warning'); } + if (this.#loadError) { + // The existing file could not be parsed. Refuse to append, otherwise + // save() would overwrite the unreadable file and destroy its contents. + throw new Error( + `WarningRegistry: refusing to modify ${this.#registryPath} because it could not be read (${this.#loadError.message}).`, + ); + } + this.#warnings.push({ ...warning }); this.save(); } - private read(): DiagnosticWarning[] { + private read(): { warnings: DiagnosticWarning[]; error: Error | null } { if (!existsSync(this.#registryPath)) { - return []; + return { warnings: [], error: null }; } - const data: unknown = JSON.parse(readFileSync(this.#registryPath, 'utf8')); - - if (!Array.isArray(data) || !data.every(isDiagnosticWarning)) { - throw new TypeError(`Invalid warning registry: ${this.#registryPath}`); + try { + const data: unknown = JSON.parse(readFileSync(this.#registryPath, 'utf8')); + + if (!Array.isArray(data) || !data.every(isDiagnosticWarning)) { + throw new TypeError(`Invalid warning registry: ${this.#registryPath}`); + } + + return { warnings: data.map((warning) => ({ ...warning })), error: null }; + } catch (error) { + // Fail soft: validation constructs a WarningRegistry for every Diagnostics + // result, so a malformed registry file must not throw and take down + // otherwise-successful validation. Surface the problem on stderr and + // continue with no warnings; addWarning still refuses to overwrite the + // unreadable file. + const readError = error instanceof Error ? error : new Error(String(error)); + console.warn( + `WarningRegistry: could not read ${this.#registryPath} (${readError.message}); continuing with no warnings.`, + ); + + return { warnings: [], error: readError }; } - - return data.map((warning) => ({ ...warning })); } private save(): void { From 82714ffcd02d3b05abef95fa414f8c4295f06928 Mon Sep 17 00:00:00 2001 From: Jerry Gamblin Date: Tue, 21 Jul 2026 15:34:26 -0500 Subject: [PATCH 2/3] Address review findings on registry hardening - Warn at most once per registry path (module-level set) so a corrupt default registry does not emit one identical stderr line per validated record during batch validation. - Tests: assert the wrong-shape case also surfaces a console.warn, and exercise the no-clobber addWarning guard against both malformed-JSON and wrong-shape files (with a byte-for-byte before/after check). Co-Authored-By: Claude Opus 4.8 --- scripts/test-local.mjs | 26 +++++++++++++++++--------- src/registry/warnings.ts | 17 ++++++++++++++--- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/scripts/test-local.mjs b/scripts/test-local.mjs index 2334f75..065a1d8 100644 --- a/scripts/test-local.mjs +++ b/scripts/test-local.mjs @@ -382,21 +382,29 @@ async function runWarningRegistryHardeningTests() { "expected a console.warn mentioning the malformed registry path", ); - // Valid JSON of the wrong shape is treated the same way: fail soft, no throw. + // Valid JSON of the wrong shape is treated the same way: fail soft, no throw, + // and the problem is surfaced on stderr. writeFileSync(wrongShapePath, JSON.stringify([{ notAWarning: true }]), "utf8"); - await withCapturedWarnings(() => { + const wrongShapeWarnings = await withCapturedWarnings(() => { const registry = new WarningRegistry(wrongShapePath); assert.deepEqual(registry.warnings, []); }); + assert.ok( + wrongShapeWarnings.some((message) => message.includes(wrongShapePath)), + "expected a console.warn mentioning the wrong-shape registry path", + ); // addWarning must refuse to overwrite an unreadable registry so its contents - // are never silently destroyed; the file stays byte-for-byte unchanged. - const before = readFileSync(malformedJsonPath, "utf8"); - await withCapturedWarnings(() => { - const registry = new WarningRegistry(malformedJsonPath); - assert.throws(() => registry.addWarning(validWarning), /refusing to modify/); - }); - assert.equal(readFileSync(malformedJsonPath, "utf8"), before); + // are never silently destroyed; the file stays byte-for-byte unchanged. This + // holds for both malformed JSON and valid-JSON-of-the-wrong-shape. + for (const corruptPath of [malformedJsonPath, wrongShapePath]) { + const before = readFileSync(corruptPath, "utf8"); + await withCapturedWarnings(() => { + const registry = new WarningRegistry(corruptPath); + assert.throws(() => registry.addWarning(validWarning), /refusing to modify/); + }); + assert.equal(readFileSync(corruptPath, "utf8"), before); + } } finally { await rm(temporaryDirectory, { recursive: true, force: true }); } diff --git a/src/registry/warnings.ts b/src/registry/warnings.ts index 5a2e655..c07475b 100644 --- a/src/registry/warnings.ts +++ b/src/registry/warnings.ts @@ -74,6 +74,13 @@ function defaultWarningRegistryPath(): string { ); } +/** + * Registry paths already reported as unreadable. Validation constructs a + * WarningRegistry per Diagnostics result, so this keeps a corrupt registry from + * emitting one identical stderr line per validated record. + */ +const warnedRegistryPaths = new Set(); + /** File-backed registry for non-fatal warning messages. */ export class WarningRegistry { readonly #registryPath: string; @@ -177,9 +184,13 @@ export class WarningRegistry { // continue with no warnings; addWarning still refuses to overwrite the // unreadable file. const readError = error instanceof Error ? error : new Error(String(error)); - console.warn( - `WarningRegistry: could not read ${this.#registryPath} (${readError.message}); continuing with no warnings.`, - ); + + if (!warnedRegistryPaths.has(this.#registryPath)) { + warnedRegistryPaths.add(this.#registryPath); + console.warn( + `WarningRegistry: could not read ${this.#registryPath} (${readError.message}); continuing with no warnings.`, + ); + } return { warnings: [], error: readError }; } From 2a3e6552f5dec75c1c9a9f56f6020e2238eb4ee7 Mon Sep 17 00:00:00 2001 From: Jerry Gamblin Date: Tue, 21 Jul 2026 15:40:45 -0500 Subject: [PATCH 3/3] Report each registry corruption episode, not once per process The warn-once dedupe set was never cleared, so a registry that was corrupt, repaired, then re-corrupted within one process would silently swallow the second corruption. Clear the dedupe entry on a successful (or missing-file) read, so each distinct corruption episode is reported exactly once. Add a corrupt -> repair -> re-corrupt test proving the recurrence warns again. Co-Authored-By: Claude Opus 4.8 --- scripts/test-local.mjs | 35 +++++++++++++++++++++++++++++++++++ src/registry/warnings.ts | 17 +++++++++++++---- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/scripts/test-local.mjs b/scripts/test-local.mjs index 065a1d8..7ee44eb 100644 --- a/scripts/test-local.mjs +++ b/scripts/test-local.mjs @@ -405,6 +405,41 @@ async function runWarningRegistryHardeningTests() { }); assert.equal(readFileSync(corruptPath, "utf8"), before); } + + // A corruption episode is reported once (deduped), but after a successful + // read the dedupe entry is cleared, so a later re-corruption warns again and + // is not silently swallowed. + const recurringPath = join(temporaryDirectory, "recurring.json"); + + writeFileSync(recurringPath, "broken", "utf8"); + const firstCorruption = await withCapturedWarnings(() => { + new WarningRegistry(recurringPath); + // A second construction during the same episode must not warn again. + new WarningRegistry(recurringPath); + }); + assert.equal( + firstCorruption.filter((message) => message.includes(recurringPath)).length, + 1, + "a corruption episode should warn exactly once", + ); + + // Repair with a valid (empty) registry: reads cleanly, no warning. + writeFileSync(recurringPath, "[]", "utf8"); + const afterRepair = await withCapturedWarnings(() => { + const registry = new WarningRegistry(recurringPath); + assert.deepEqual(registry.warnings, []); + }); + assert.equal(afterRepair.length, 0, "a valid registry must not warn"); + + // Re-corrupt: the new episode must be reported again. + writeFileSync(recurringPath, "broken again", "utf8"); + const secondCorruption = await withCapturedWarnings(() => { + new WarningRegistry(recurringPath); + }); + assert.ok( + secondCorruption.some((message) => message.includes(recurringPath)), + "a new corruption episode after repair must warn again", + ); } finally { await rm(temporaryDirectory, { recursive: true, force: true }); } diff --git a/src/registry/warnings.ts b/src/registry/warnings.ts index c07475b..15c23c2 100644 --- a/src/registry/warnings.ts +++ b/src/registry/warnings.ts @@ -75,9 +75,11 @@ function defaultWarningRegistryPath(): string { } /** - * Registry paths already reported as unreadable. Validation constructs a - * WarningRegistry per Diagnostics result, so this keeps a corrupt registry from - * emitting one identical stderr line per validated record. + * Registry paths already reported as unreadable during the current corruption + * episode. Validation constructs a WarningRegistry per Diagnostics result, so + * this keeps a corrupt registry from emitting one identical stderr line per + * validated record. Entries are cleared on the next successful read, so a later + * re-corruption of the same path is reported again. */ const warnedRegistryPaths = new Set(); @@ -166,6 +168,9 @@ export class WarningRegistry { private read(): { warnings: DiagnosticWarning[]; error: Error | null } { if (!existsSync(this.#registryPath)) { + // A missing file is a valid empty state, not corruption; clear any prior + // dedupe entry so a later re-corruption of this path is reported again. + warnedRegistryPaths.delete(this.#registryPath); return { warnings: [], error: null }; } @@ -176,13 +181,17 @@ export class WarningRegistry { throw new TypeError(`Invalid warning registry: ${this.#registryPath}`); } + // A successful read ends any prior corruption episode for this path. + warnedRegistryPaths.delete(this.#registryPath); return { warnings: data.map((warning) => ({ ...warning })), error: null }; } catch (error) { // Fail soft: validation constructs a WarningRegistry for every Diagnostics // result, so a malformed registry file must not throw and take down // otherwise-successful validation. Surface the problem on stderr and // continue with no warnings; addWarning still refuses to overwrite the - // unreadable file. + // unreadable file. Warn once per corruption episode per path (the entry is + // cleared on the next successful read) to avoid flooding stderr while a + // corrupt registry persists. const readError = error instanceof Error ? error : new Error(String(error)); if (!warnedRegistryPaths.has(this.#registryPath)) {