From 30d525e32983d2f1717b6dbf07ccba28188f9a16 Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Mon, 12 Oct 2026 07:21:08 +0000 Subject: [PATCH 1/7] fix(config): bound regular-file reads before parsing Preserve bounded runtime ownership and failure behavior, with focused regression coverage and the owning architecture contract. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- scripts/test-layout/layout.json | 2 +- scripts/test-layout/seeds.json | 4 +- src/integrations/config-io.ts | 6 ++- src/lib/bounded-file-read.ts | 51 +++++++++++++++++++++++ structure/clients/integrations.md | 2 +- tests/fixtures/test-layout-expected.json | 2 +- tests/lib/bounded-file-read.test.ts | 53 ++++++++++++++++++++++++ 7 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 src/lib/bounded-file-read.ts create mode 100644 tests/lib/bounded-file-read.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 3d677b90ea1..1e6699e7d43 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,7 +2,7 @@ "version": 1, "root": "tests", "explicit": { - "messages-request-id-headers.test.ts": "claude-integration", + "messages-request-id-headers.test.ts": "claude-integration", "bounded-file-read.test.ts": "lib", "messages-request-id-endpoint.test.ts": "claude-integration", "anthropic-revoked-token.test.ts": "adapters/anthropic", "messages-revoked-token.test.ts": "claude-integration", diff --git a/scripts/test-layout/seeds.json b/scripts/test-layout/seeds.json index 45792a9c4bc..0db0d3d33cd 100644 --- a/scripts/test-layout/seeds.json +++ b/scripts/test-layout/seeds.json @@ -40,7 +40,7 @@ "^context-history\\.test\\.ts$", "^aside-profiles-routes", "^update-async-routes\\.test\\.ts$", - "^(?:account|alias|bounded|cancel|config\\.test\\.ts|consume|data|debug|error|errors|fetch|health|input|loopback|management|memory|outbound|owned|passive|port|ports\\.test\\.ts|proxy|relay|response|retry|server|session|sidebar|stream|v2)-", + "^(?!bounded-file-read\\.test\\.ts$)(?:account|alias|bounded|cancel|config\\.test\\.ts|consume|data|debug|error|errors|fetch|health|input|loopback|management|memory|outbound|owned|passive|port|ports\\.test\\.ts|proxy|relay|response|retry|server|session|sidebar|stream|v2)-", "^subagent-model-force-api\\.test\\.ts$" ] }, @@ -106,7 +106,7 @@ }, "lib": { "match": [ - "^(?:acl|clearable|credential|debug\\.test\\.ts|optional|redact\\.test\\.ts|remove|self|stall|strict|transient)-" + "^(?:acl|bounded|clearable|credential|debug\\.test\\.ts|optional|redact\\.test\\.ts|remove|self|stall|strict|transient)-" ] }, "clients": { diff --git a/src/integrations/config-io.ts b/src/integrations/config-io.ts index 45f5458cf25..e57ab1442c3 100644 --- a/src/integrations/config-io.ts +++ b/src/integrations/config-io.ts @@ -7,8 +7,9 @@ * * Design of record: devlog/_fin/260802_client_toggle_api/021 §5-6. */ -import { lstatSync, mkdirSync, readFileSync, rmSync, statSync } from "node:fs"; +import { lstatSync, mkdirSync, rmSync, statSync } from "node:fs"; import type { ConfigFormat } from "../clients/config-export"; +import { readBoundedRegularFileSync } from "../lib/bounded-file-read"; import { canonicalizeJsonc } from "../lib/jsonc"; import { MAX_JSON_NESTING } from "./serialize"; import { atomicWriteFileNoFollow, isMissingPathError } from "../config/atomic-write"; @@ -24,6 +25,7 @@ import type { IntegrationClientId } from "./registry"; * then overwritten. */ export const PARSE_FAILED = Symbol("parse-failed"); +const MAX_INTEGRATION_CONFIG_FILE_BYTES = 4 * 1024 * 1024; /** * What `JSON.parse` has already discarded by the time we hold the parsed value, @@ -285,7 +287,7 @@ export function fileIO(): Omit { try { - return { kind: "text", text: readFileSync(path, "utf8") }; + return { kind: "text", text: readBoundedRegularFileSync(path, MAX_INTEGRATION_CONFIG_FILE_BYTES).toString("utf8") }; } catch (error) { const code = (error as NodeJS.ErrnoException).code; return code === "ENOENT" ? { kind: "missing" } : { kind: "failed", ...(code ? { code } : {}) }; diff --git a/src/lib/bounded-file-read.ts b/src/lib/bounded-file-read.ts new file mode 100644 index 00000000000..06fb36ef1bd --- /dev/null +++ b/src/lib/bounded-file-read.ts @@ -0,0 +1,51 @@ +import { closeSync, constants, fstatSync, lstatSync, openSync, readSync } from "node:fs"; + +const MAX_BOUNDED_READ_BYTES = 64 * 1024 * 1024; +const READ_CHUNK_BYTES = 64 * 1024; + +function sameIdentity( + left: { dev: bigint; ino: bigint }, + right: { dev: bigint; ino: bigint }, +): boolean { + return left.dev === right.dev && left.ino === right.ino; +} + +export function readBoundedRegularFileSync(path: string, limit: number): Buffer { + if (!Number.isSafeInteger(limit) || limit < 1 || limit > MAX_BOUNDED_READ_BYTES) { + throw new RangeError("invalid bounded file read limit"); + } + + const windows = process.platform === "win32"; + const flags = constants.O_RDONLY | (windows ? 0 : constants.O_NONBLOCK | constants.O_NOFOLLOW); + const entryBefore = windows ? lstatSync(path, { bigint: true }) : undefined; + if (entryBefore && (entryBefore.isSymbolicLink() || !entryBefore.isFile())) { + throw new Error("not a regular file"); + } + const fd = openSync(path, flags); + try { + const opened = fstatSync(fd, { bigint: true }); + if (!opened.isFile() || opened.size > BigInt(limit)) throw new Error("file is not bounded regular data"); + if (entryBefore && (!sameIdentity(entryBefore, opened) || opened.ino === 0n)) { + throw new Error("file entry changed while opening"); + } + + const bytes = Buffer.alloc(limit + 1); + let length = 0; + while (length < bytes.length) { + const count = readSync(fd, bytes, length, Math.min(READ_CHUNK_BYTES, bytes.length - length), null); + if (count === 0) break; + length += count; + } + if (length > limit) throw new Error("file exceeds byte limit"); + + if (entryBefore) { + const entryAfter = lstatSync(path, { bigint: true }); + if (entryAfter.isSymbolicLink() || !entryAfter.isFile() || !sameIdentity(opened, entryAfter)) { + throw new Error("file entry changed while reading"); + } + } + return bytes.subarray(0, length); + } finally { + closeSync(fd); + } +} diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 681098bd89c..1f59eee6fb2 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -31,7 +31,7 @@ parsing and ownership rules below. | `src/clients/config-export.ts` | Pure per-client config builders and the exact managed fragments each client receives. It never writes files. | | `src/integrations/registry.ts` | Canonical config/detection paths, current-provider-store declarations, source-preserving YAML declarations, writer-lock behavior, and client IDs. | | `src/integrations/target.ts` | Which file one operation reads, writes and records, and whether a write there reaches the client. | -| `src/integrations/config-io.ts` | Bounded file loading and parsing. Values that cannot round-trip through the target serializer are rejected before mutation. | +| `src/integrations/config-io.ts` | Bounded regular-file loading and parsing; production reads reject symlinks and stop after 4 MiB. Values that cannot round-trip through the target serializer are rejected before mutation. | | `src/integrations/kilo-candidates.ts` | Inspects all Kilo global config candidates for unsafe files and a competing `provider.opencodex` block before status or any operation that adds or replaces a block. | | `src/integrations/state.ts` | The single `absent` / `current` / `stale` / `conflict` / `unsafe` classifier used by status and every writer operation. | | `src/integrations/ownership.ts` | Durable ownership records: file, generated contribution, protected contribution, exact fragment paths, and operation identity. | diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 207e9e64782..481d8957b50 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,5 +1,5 @@ { - "messages-request-id-headers.test.ts": "claude-integration", + "messages-request-id-headers.test.ts": "claude-integration", "bounded-file-read.test.ts": "lib", "messages-request-id-endpoint.test.ts": "claude-integration", "anthropic-revoked-token.test.ts": "adapters/anthropic", "messages-revoked-token.test.ts": "claude-integration", diff --git a/tests/lib/bounded-file-read.test.ts b/tests/lib/bounded-file-read.test.ts new file mode 100644 index 00000000000..cc05e2f9f3d --- /dev/null +++ b/tests/lib/bounded-file-read.test.ts @@ -0,0 +1,53 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { repoPath } from "../helpers/repo-root"; +import { readBoundedRegularFileSync } from "../../src/lib/bounded-file-read"; + +const repoRoot = repoPath(); +let root = ""; + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "ocx-bounded-read-")); +}); + +afterEach(() => { + rmSync(root, { recursive: true, force: true }); +}); + +describe("readBoundedRegularFileSync", () => { + test("reads a bounded regular file", () => { + const path = join(root, "config.json"); + writeFileSync(path, '{"ok":true}'); + expect(readBoundedRegularFileSync(path, 32).toString("utf8")).toBe(readFileSync(path, "utf8")); + }); + + test("rejects oversized files and final symlinks", () => { + const path = join(root, "large.json"); + writeFileSync(path, "x".repeat(33)); + expect(() => readBoundedRegularFileSync(path, 32)).toThrow(); + + const link = join(root, "linked.json"); + symlinkSync(path, link, "file"); + expect(() => readBoundedRegularFileSync(link, 64)).toThrow(); + }); + + test.skipIf(process.platform === "win32")("rejects a FIFO without blocking", () => { + const fifo = join(root, "input.pipe"); + const created = spawnSync("mkfifo", [fifo], { timeout: 2_000 }); + expect(created.status).toBe(0); + const result = spawnSync(process.execPath, ["--eval", ` + const { readBoundedRegularFileSync } = require("./src/lib/bounded-file-read.ts"); + try { readBoundedRegularFileSync(process.env.BOUNDED_READ_FIFO, 1024); process.exitCode = 2; } + catch { process.exitCode = 0; } + `], { + cwd: repoRoot, + env: { ...process.env, BOUNDED_READ_FIFO: fifo }, + timeout: 2_000, + }); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + }); +}); From 5a9c154a8a4e9e17bc90becd3b3720c2b0ea23f6 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:33:44 +0000 Subject: [PATCH 2/7] fix(config): bound integration writes at the read limit The bounded reader rejects files past 4 MiB, but the same seam's writeText could still produce one: a serialized document can grow past the cap, and a Cline pending marker embeds the before-and-after pair. Either write strands the integration against a file its own reader then refuses.\n\nEnforce the same bound on the write half of the seam so an oversized mutation refuses instead of landing, and pin the boundary in the state, writer and Cline suites plus the structure doc. Co-Authored-By: Epinephrine --- src/integrations/config-io.ts | 13 +++++++++++++ structure/clients/integrations.md | 2 +- tests/clients/cline-writer.test.ts | 19 +++++++++++++++++++ tests/clients/integrations-state.test.ts | 17 +++++++++++++++++ tests/clients/integrations-writer.test.ts | 19 +++++++++++++++++++ 5 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/integrations/config-io.ts b/src/integrations/config-io.ts index e57ab1442c3..8ed19430f85 100644 --- a/src/integrations/config-io.ts +++ b/src/integrations/config-io.ts @@ -302,6 +302,19 @@ export function fileIO(): Omit { + /* + * One bound on both halves of the seam: a file past this size is one + * readText refuses, so writing it would strand the integration against + * its own configuration — or, for Cline's pending marker, leave a + * transaction recovery could never read back. + */ + const bytes = Buffer.byteLength(text, "utf8"); + if (bytes > MAX_INTEGRATION_CONFIG_FILE_BYTES) { + throw new Error( + `refusing oversized integration write to ${path}: ` + + `${bytes} bytes is past the ${MAX_INTEGRATION_CONFIG_FILE_BYTES}-byte read bound`, + ); + } const kind = lstatKind(path); if (kind !== "file" && kind !== "missing") { throw new Error(`refusing unsafe integration write target: ${path}`); diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 1f59eee6fb2..6cce0c541af 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -31,7 +31,7 @@ parsing and ownership rules below. | `src/clients/config-export.ts` | Pure per-client config builders and the exact managed fragments each client receives. It never writes files. | | `src/integrations/registry.ts` | Canonical config/detection paths, current-provider-store declarations, source-preserving YAML declarations, writer-lock behavior, and client IDs. | | `src/integrations/target.ts` | Which file one operation reads, writes and records, and whether a write there reaches the client. | -| `src/integrations/config-io.ts` | Bounded regular-file loading and parsing; production reads reject symlinks and stop after 4 MiB. Values that cannot round-trip through the target serializer are rejected before mutation. | +| `src/integrations/config-io.ts` | Bounded regular-file loading and parsing; production reads reject symlinks and stop after 4 MiB, and writes past the same bound are refused so a mutation cannot strand a file it cannot read back. Values that cannot round-trip through the target serializer are rejected before mutation. | | `src/integrations/kilo-candidates.ts` | Inspects all Kilo global config candidates for unsafe files and a competing `provider.opencodex` block before status or any operation that adds or replaces a block. | | `src/integrations/state.ts` | The single `absent` / `current` / `stale` / `conflict` / `unsafe` classifier used by status and every writer operation. | | `src/integrations/ownership.ts` | Durable ownership records: file, generated contribution, protected contribution, exact fragment paths, and operation identity. | diff --git a/tests/clients/cline-writer.test.ts b/tests/clients/cline-writer.test.ts index ff06f29dbc8..844b20cbb4d 100644 --- a/tests/clients/cline-writer.test.ts +++ b/tests/clients/cline-writer.test.ts @@ -354,4 +354,23 @@ describe("Cline journaled pair", () => { expect(decodeClinePair(result.text)).toEqual({ settings: originalSettings, catalog: originalCatalog }); expect(adapter.statKind(dirname(settings))).toBe("dir"); }); + + test("a transaction journal past the read bound refuses before either file moves", () => { + /* + * The pending marker embeds the before-and-after pair, so a pair near the + * bound serializes past it. beginTransaction must refuse: writing the + * marker would leave a pending file recovery itself could never read + * back, stranding every later Cline operation behind an unreadable + * transaction. + */ + const pad = "x".repeat(3 * 1024 * 1024); + const seeded = `{"pad":"${pad}","version":1,"providers":{"mine":{"settings":{"provider":"mine"},"updatedAt":"2026-01-01T00:00:00.000Z","tokenSource":"manual"}}}\n`; + writeFileSync(settings, seeded); + const result = applyIntegration(input); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("write_failed"); + expect(readFileSync(settings, "utf8")).toBe(seeded); + expect(existsSync(catalog)).toBe(false); + expect(existsSync(clinePendingPath(input.store!, settings))).toBe(false); + }); }); diff --git a/tests/clients/integrations-state.test.ts b/tests/clients/integrations-state.test.ts index d04310324ce..df06308f431 100644 --- a/tests/clients/integrations-state.test.ts +++ b/tests/clients/integrations-state.test.ts @@ -805,3 +805,20 @@ describe("the loopback-only set is one fact, read through one seam", () => { } }); }); + +describe("the managed file size bound", () => { + test("a config past the read bound classifies unsafe, never absent", () => { + mkdirSync(join(home, ".pi", "agent"), { recursive: true }); + const path = join(home, ".pi", "agent", "models.json"); + writeFileSync(path, `{"pad":"${"x".repeat(4 * 1024 * 1024)}"}\n`); + const status = readIntegrationState(input()); + /* + * Too large to read is a read failure like any other: the file reports + * unsafe rather than absent, so nothing mistakes it for an empty target + * a write could replace. + */ + expect(status.state).toBe("unsafe"); + expect(status.reason).toBe("unparseable"); + expect(loadTarget(fileIO(), path)).toEqual({ ok: false, why: "read-failed" }); + }); +}); diff --git a/tests/clients/integrations-writer.test.ts b/tests/clients/integrations-writer.test.ts index 378ea85a997..871c8768310 100644 --- a/tests/clients/integrations-writer.test.ts +++ b/tests/clients/integrations-writer.test.ts @@ -1586,3 +1586,22 @@ describe("integration write ownership guard (#4197)", () => { })).not.toThrow(); }); }); + +describe("the managed file size bound", () => { + test("a serialized write past the read bound refuses and leaves the file alone", () => { + const configPath = installPi(); + /* + * Just under the bound, so status and the compare-before-commit recheck + * still read it, while the merged document lands past it. The write must + * refuse: landing it would leave a config the same seam then refuses to + * read, stranding the integration against its own file. + */ + const seed = `{"pad":"${"x".repeat(4 * 1024 * 1024 - 256)}"}\n`; + writeFileSync(configPath, seed); + const result = applyIntegration(input({ clientId: "pi" })); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("write_failed"); + expect(readFileSync(configPath, "utf8")).toBe(seed); + expect(readIntegrationState(input({ clientId: "pi" })).state).toBe("absent"); + }); +}); From b3f5933c723301df94044ec1579a04cf0791a3b3 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:52:28 +0000 Subject: [PATCH 3/7] Clean unused snapshots after size refusals Co-Authored-By: Epinephrine --- .../src/content/docs/guides/integrations.md | 11 +++++++ src/integrations/config-io.ts | 5 ++- src/integrations/writer.ts | 11 ++++++- structure/clients/integrations.md | 2 +- tests/clients/cline-writer.test.ts | 13 ++++++-- tests/clients/integrations-writer.test.ts | 33 +++++++++++++++++-- 6 files changed, 66 insertions(+), 9 deletions(-) diff --git a/docs-site/src/content/docs/guides/integrations.md b/docs-site/src/content/docs/guides/integrations.md index fab56771475..81bab4a9f5c 100644 --- a/docs-site/src/content/docs/guides/integrations.md +++ b/docs-site/src/content/docs/guides/integrations.md @@ -287,6 +287,17 @@ profiles** remains a separate bulk action and is not bound to one combined previ ## What to expect, honestly +**Managed configuration files are limited to 4 MiB (4,194,304 UTF-8 bytes).** +The shared integration reader refuses larger files as unsafe to read, and the writer +refuses a merged configuration that exceeds the same limit with `write_failed`, leaving +the original configuration unchanged. Cline's recovery marker also has this limit; +it includes both files' before-and-after contents plus metadata, so Cline can hit the +limit even when each individual file is smaller than 4 MiB. + +Back up the client configuration yourself, then remove unneeded entries or reduce the +exported model list before retrying. **Replace** does not bypass the limit. If the +configuration must stay larger, manage it manually instead of using the Connect switch. + **Formatting is generally not preserved.** Applying parses a config and writes it back out, so JSON, JSON5 and TOML may be reformatted and comments in JSON5 or TOML are lost. OMP, DSH and Hermes are the exceptions: OMP and Hermes patch only `providers.opencodex`. diff --git a/src/integrations/config-io.ts b/src/integrations/config-io.ts index 8ed19430f85..90232298b0c 100644 --- a/src/integrations/config-io.ts +++ b/src/integrations/config-io.ts @@ -27,6 +27,9 @@ import type { IntegrationClientId } from "./registry"; export const PARSE_FAILED = Symbol("parse-failed"); const MAX_INTEGRATION_CONFIG_FILE_BYTES = 4 * 1024 * 1024; +/** Raised before any target mutation, including pending-marker creation. */ +export class IntegrationWriteSizeError extends Error {} + /** * What `JSON.parse` has already discarded by the time we hold the parsed value, * and a rewrite would therefore silently change. Both classes are invisible in @@ -310,7 +313,7 @@ export function fileIO(): Omit MAX_INTEGRATION_CONFIG_FILE_BYTES) { - throw new Error( + throw new IntegrationWriteSizeError( `refusing oversized integration write to ${path}: ` + `${bytes} bytes is past the ${MAX_INTEGRATION_CONFIG_FILE_BYTES}-byte read bound`, ); diff --git a/src/integrations/writer.ts b/src/integrations/writer.ts index f7d71c7a271..8d8b296780b 100644 --- a/src/integrations/writer.ts +++ b/src/integrations/writer.ts @@ -18,7 +18,7 @@ import { shouldInjectApiAuthHeader } from "../codex/inject"; import { detachedConfigSnapshot } from "../config/admitted-identity"; import { copyPlainData } from "../lib/plain-data"; import type { OcxConfig } from "../types"; -import { PARSE_FAILED, defaultIntegrationIO, loadTarget, parseConfig, type IntegrationIO } from "./config-io"; +import { IntegrationWriteSizeError, PARSE_FAILED, defaultIntegrationIO, loadTarget, parseConfig, type IntegrationIO } from "./config-io"; import { inspectKiloCandidates } from "./kilo-candidates"; import { fingerprint, @@ -161,6 +161,15 @@ function commit(args: CommitArgs): WriteOutcome { } } catch (error) { if (transactionStarted) return compensate(args, error, "could not write both Cline files"); + if (error instanceof IntegrationWriteSizeError && args.snapshotPath) { + try { + io.removeFile(args.snapshotPath); + } catch (cleanupError) { + return refuse(clientId, "write_failed", args.state, + `${messageOf(error)}; could not remove the unused backup: ${messageOf(cleanupError)}`, args.snapshotPath); + } + return refuse(clientId, "write_failed", args.state, messageOf(error)); + } return refuse(clientId, "write_failed", args.state, messageOf(error), args.snapshotPath); } try { diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 6cce0c541af..eba2216d6ff 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -131,7 +131,7 @@ client registry + export context -> build managed contribution -> load and parse current config -> classify exact recorded fragments - -> snapshot prior bytes + -> snapshot prior bytes (discard the unused snapshot on a size refusal before mutation; report cleanup failures with its recovery path) -> merge or remove only recorded paths -> compare current bytes again -> atomic write diff --git a/tests/clients/cline-writer.test.ts b/tests/clients/cline-writer.test.ts index 844b20cbb4d..cb41449ee1a 100644 --- a/tests/clients/cline-writer.test.ts +++ b/tests/clients/cline-writer.test.ts @@ -366,9 +366,16 @@ describe("Cline journaled pair", () => { const pad = "x".repeat(3 * 1024 * 1024); const seeded = `{"pad":"${pad}","version":1,"providers":{"mine":{"settings":{"provider":"mine"},"updatedAt":"2026-01-01T00:00:00.000Z","tokenSource":"manual"}}}\n`; writeFileSync(settings, seeded); - const result = applyIntegration(input); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.reason).toBe("write_failed"); + for (let attempt = 0; attempt < 2; attempt++) { + const result = applyIntegration(input); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("write_failed"); + expect(result.snapshotPath).toBeUndefined(); + } + expect(input.store!.countSnapshots("cline")).toBe(0); + expect(input.store!.listOperations("cline")).toEqual([]); + } expect(readFileSync(settings, "utf8")).toBe(seeded); expect(existsSync(catalog)).toBe(false); expect(existsSync(clinePendingPath(input.store!, settings))).toBe(false); diff --git a/tests/clients/integrations-writer.test.ts b/tests/clients/integrations-writer.test.ts index 871c8768310..eb25a2d265c 100644 --- a/tests/clients/integrations-writer.test.ts +++ b/tests/clients/integrations-writer.test.ts @@ -1588,6 +1588,26 @@ describe("integration write ownership guard (#4197)", () => { }); describe("the managed file size bound", () => { + test("reports the retained backup if an oversized write cannot clean it up", () => { + const configPath = join(home, ".pi", "agent", "models.json"); + mkdirSync(dirname(configPath), { recursive: true }); + const seed = `{"pad":"${"x".repeat(4 * 1024 * 1024 - 256)}"}\n`; + writeFileSync(configPath, seed); + const result = applyIntegration(input({ + clientId: "pi", + io: { ...store.io(), removeFile: () => { throw new Error("cleanup denied"); } }, + })); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("write_failed"); + expect(result.message).toContain("could not remove the unused backup: cleanup denied"); + expect(readFileSync(result.snapshotPath!, "utf8")).toBe(seed); + } + expect(readFileSync(configPath, "utf8")).toBe(seed); + expect(store.countSnapshots("pi")).toBe(1); + expect(store.listOperations("pi")).toEqual([]); + }); + test("a serialized write past the read bound refuses and leaves the file alone", () => { const configPath = installPi(); /* @@ -1598,9 +1618,16 @@ describe("the managed file size bound", () => { */ const seed = `{"pad":"${"x".repeat(4 * 1024 * 1024 - 256)}"}\n`; writeFileSync(configPath, seed); - const result = applyIntegration(input({ clientId: "pi" })); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.reason).toBe("write_failed"); + for (let attempt = 0; attempt < 2; attempt++) { + const result = applyIntegration(input({ clientId: "pi" })); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("write_failed"); + expect(result.snapshotPath).toBeUndefined(); + } + expect(store.countSnapshots("pi")).toBe(0); + expect(store.listOperations("pi")).toEqual([]); + } expect(readFileSync(configPath, "utf8")).toBe(seed); expect(readIntegrationState(input({ clientId: "pi" })).state).toBe("absent"); }); From 72bc3947a9c7dfaaffd4a8d3dccb2768c034354c Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Mon, 12 Oct 2026 10:13:08 +0000 Subject: [PATCH 4/7] fix(integrations): discard unused snapshots through their store --- src/integrations/journal.ts | 16 +++++++++- src/integrations/store.ts | 3 ++ src/integrations/writer.ts | 2 +- structure/clients/integrations.md | 2 +- tests/clients/aside-profiles.test.ts | 14 +++++++++ tests/clients/integrations-journal.test.ts | 36 ++++++++++++++++++++++ tests/clients/integrations-writer.test.ts | 2 +- 7 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/integrations/journal.ts b/src/integrations/journal.ts index 9c4444d5e85..945078641e9 100644 --- a/src/integrations/journal.ts +++ b/src/integrations/journal.ts @@ -16,7 +16,7 @@ import { randomUUID } from "node:crypto"; import { appendFileSync, existsSync, readFileSync, readdirSync, rmSync } from "node:fs"; import { join, resolve, sep } from "node:path"; import { atomicWriteFile } from "../config"; -import { ensureDir, fingerprint, integrationsDir, isOwnershipRecord, type OwnershipRecord } from "./ownership"; +import { ensureDir, fingerprint, integrationsDir, isOwnershipRecord, readRecordsStrict, type OwnershipRecord } from "./ownership"; import { isIntegrationClientId, type IntegrationClientId } from "./registry"; /** @@ -171,6 +171,20 @@ export function captureSnapshot( return { kind: "stored", relPath: join("snapshots", clientId, opId) }; } +/** Remove only an unused pre-write snapshot; uncertain or committed evidence survives. */ +export function discardUncommittedSnapshot( + clientId: IntegrationClientId, + opId: string, + dir: string = integrationsDir(), +): void { + const target = containedPath(dir, "snapshots", assertSafeComponent(clientId, "clientId"), assertSafeComponent(opId, "opId")); + if (findCommittedOperation(opId, dir) + || Object.values(readRecordsStrict(dir)).some(record => record?.opId === opId)) { + throw new Error("integration snapshot still belongs to a recorded operation"); + } + rmSync(target, { force: true }); +} + /** Commit the row. Pruning is post-commit and can never fail the append. */ export function appendOperation(entry: JournalEntry, dir: string = integrationsDir()): void { ensureDir(journalPath(dir)); diff --git a/src/integrations/store.ts b/src/integrations/store.ts index 4e04fd3021f..701950c313d 100644 --- a/src/integrations/store.ts +++ b/src/integrations/store.ts @@ -16,6 +16,7 @@ import { captureSnapshot, clearPruneFailure, countSnapshots, + discardUncommittedSnapshot, findOperation, findCommittedOperation, listOperations, @@ -53,6 +54,7 @@ export interface IntegrationStateStore { findOperation(opId: string): JournalEntry | null; findCommittedOperation(opId: string): JournalEntry | null; captureSnapshot(clientId: IntegrationClientId, opId: string, text: string | null): SnapshotRef; + discardUncommittedSnapshot(clientId: IntegrationClientId, opId: string): void; readSnapshot(entry: JournalEntry): | { kind: "none" } | { kind: "stored"; text: string; path: string } @@ -92,6 +94,7 @@ export function createIntegrationStateStore(root: string = integrationsDir()): I findOperation: opId => findOperation(opId, dir), findCommittedOperation: opId => findCommittedOperation(opId, dir), captureSnapshot: (clientId, opId, text) => captureSnapshot(clientId, opId, text, dir), + discardUncommittedSnapshot: (clientId, opId) => discardUncommittedSnapshot(clientId, opId, dir), readSnapshot: entry => readSnapshot(entry, dir), countSnapshots: clientId => countSnapshots(clientId, dir), pruneSnapshots: clientId => pruneSnapshots(clientId, dir), diff --git a/src/integrations/writer.ts b/src/integrations/writer.ts index f722f1e246f..aece1067fb8 100644 --- a/src/integrations/writer.ts +++ b/src/integrations/writer.ts @@ -164,7 +164,7 @@ function commit(args: CommitArgs): WriteOutcome { if (transactionStarted) return compensate(args, error, "could not write both Cline files"); if (error instanceof IntegrationWriteSizeError && args.snapshotPath) { try { - io.removeFile(args.snapshotPath); + args.store.discardUncommittedSnapshot(clientId, args.entry.opId); } catch (cleanupError) { return refuse(clientId, "write_failed", args.state, `${messageOf(error)}; could not remove the unused backup: ${messageOf(cleanupError)}`, args.snapshotPath); diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 662e29d44e7..b63dbf71076 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -131,7 +131,7 @@ client registry + export context -> build managed contribution -> load and parse current config -> classify exact recorded fragments - -> snapshot prior bytes (discard the unused snapshot on a size refusal before mutation; report cleanup failures with its recovery path) + -> snapshot prior bytes (on a size refusal before mutation, the store discards its unused snapshot by validated client/operation IDs; committed, owned, or uncertain evidence survives; report cleanup failures with its recovery path) -> merge or remove only recorded paths -> compare current bytes again -> atomic write diff --git a/tests/clients/aside-profiles.test.ts b/tests/clients/aside-profiles.test.ts index b5b3cad956f..533149bb17f 100644 --- a/tests/clients/aside-profiles.test.ts +++ b/tests/clients/aside-profiles.test.ts @@ -64,6 +64,20 @@ describe("Aside profile desired state, ownership and history", () => { }); afterEach(() => removeTreeWithRetry(root)); + test("repeated oversized profile writes discard only their unused store snapshots", async () => { + const seed = `{"pad":"${"x".repeat(4 * 1024 * 1024 - 256)}"}\n`; + writeFileSync(path(0), seed); + config.asideProfileSync = { allProfiles: false, legacyProfileId: 0 }; + for (let attempt = 0; attempt < 2; attempt++) { + const result = await mutateAsideProfiles(input(), { profileId: 0, enabled: true }); + expect(result.ok).toBe(false); + expect(result.result).toMatchObject({ ok: false, reason: "write_failed" }); + expect(readFileSync(path(0), "utf8")).toBe(seed); + expect(store.listOperations("aside")).toEqual([]); + } + expect(store.countSnapshots("aside")).toBe(0); + }); + test.each([false, true])("implicit sync stays quiet for unconfigured or disabled Aside (legacy=%s)", async legacy => { if (legacy) { seedLegacy(); diff --git a/tests/clients/integrations-journal.test.ts b/tests/clients/integrations-journal.test.ts index 07010fc0318..dd3c97049e2 100644 --- a/tests/clients/integrations-journal.test.ts +++ b/tests/clients/integrations-journal.test.ts @@ -73,6 +73,42 @@ describe("append and read back", () => { }); describe("snapshots", () => { + test("unused snapshot cleanup is bound to one store and client", () => { + const other = createIntegrationStateStore(join(root, "other")); + store.captureSnapshot("pi", "unused", "pi bytes"); + store.captureSnapshot("kimi", "unused", "kimi bytes"); + other.captureSnapshot("pi", "unused", "other bytes"); + store.discardUncommittedSnapshot("pi", "unused"); + expect(store.countSnapshots("pi")).toBe(0); + expect(readFileSync(join(root, "snapshots", "kimi", "unused"), "utf8")).toBe("kimi bytes"); + expect(readFileSync(join(other.root, "snapshots", "pi", "unused"), "utf8")).toBe("other bytes"); + expect(() => store.discardUncommittedSnapshot("pi", "../../escaped")).toThrow(/unsafe opId/); + }); + + test.each([false, true])("unused cleanup preserves committed evidence, including retired rows (retired=%s)", retired => { + const snapshot = store.captureSnapshot("pi", "committed", "recovery bytes"); + store.appendJournal(entry({ opId: "committed", snapshot, resultFingerprint: "a".repeat(16) })); + if (retired) store.retireOperation({ tombstone: "committed", at: new Date().toISOString(), by: "test" }); + expect(() => store.discardUncommittedSnapshot("pi", "committed")).toThrow(/recorded operation/); + expect(readFileSync(join(root, "snapshots", "pi", "committed"), "utf8")).toBe("recovery bytes"); + }); + + test.each(["records.json", "journal.jsonl"])("unused cleanup retains evidence when %s cannot establish non-commit", filename => { + store.captureSnapshot("pi", "uncertain", "recovery bytes"); + writeFileSync(join(root, filename), "{broken"); + expect(() => store.discardUncommittedSnapshot("pi", "uncertain")).toThrow(); + expect(readFileSync(join(root, "snapshots", "pi", "uncertain"), "utf8")).toBe("recovery bytes"); + }); + + test("unused cleanup preserves an owned snapshot even before its journal append", () => { + store.captureSnapshot("pi", "owned", "recovery bytes"); + store.putRecord({ clientId: "pi", configPath: join(root, "client.json"), + fileFingerprint: "f".repeat(16), blockFingerprint: "b".repeat(16), + fragmentPaths: [["providers", "opencodex"]], appliedAt: new Date().toISOString(), opId: "owned" }); + expect(() => store.discardUncommittedSnapshot("pi", "owned")).toThrow(/recorded operation/); + expect(readFileSync(join(root, "snapshots", "pi", "owned"), "utf8")).toBe("recovery bytes"); + }); + test("a missing file records `none`, which is not a failure", () => { const ref = store.captureSnapshot("pi", "op-1", null); expect(ref).toEqual({ kind: "none" }); diff --git a/tests/clients/integrations-writer.test.ts b/tests/clients/integrations-writer.test.ts index eb25a2d265c..4a9ed11a1dc 100644 --- a/tests/clients/integrations-writer.test.ts +++ b/tests/clients/integrations-writer.test.ts @@ -1595,7 +1595,7 @@ describe("the managed file size bound", () => { writeFileSync(configPath, seed); const result = applyIntegration(input({ clientId: "pi", - io: { ...store.io(), removeFile: () => { throw new Error("cleanup denied"); } }, + store: { ...store, discardUncommittedSnapshot: () => { throw new Error("cleanup denied"); } }, })); expect(result.ok).toBe(false); if (!result.ok) { From c0e9e45e542b9e292160568648971362935c53b8 Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:10:32 +0000 Subject: [PATCH 5/7] perf(config): grow bounded read buffers from the observed size Avoid allocating the complete read limit for small configurations. Grow only when the buffer fills and still read up to limit plus one to detect concurrent growth past the cap. Copy and return only initialized bytes; retain descriptor and path identity checks. Cover small allocations, empty and exact-limit files, growth after fstat, overflow after fstat, and concurrent shrink alongside integration regressions. --- src/lib/bounded-file-read.ts | 11 ++++- structure/clients/integrations.md | 2 +- tests/lib/bounded-file-read.test.ts | 65 ++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/lib/bounded-file-read.ts b/src/lib/bounded-file-read.ts index 06fb36ef1bd..4eb866602e7 100644 --- a/src/lib/bounded-file-read.ts +++ b/src/lib/bounded-file-read.ts @@ -29,9 +29,16 @@ export function readBoundedRegularFileSync(path: string, limit: number): Buffer throw new Error("file entry changed while opening"); } - const bytes = Buffer.alloc(limit + 1); + // Start near the observed size, but keep probing up to limit + 1 if the file grows. + let bytes = Buffer.allocUnsafe(Number(opened.size) + 1); let length = 0; - while (length < bytes.length) { + while (length < limit + 1) { + if (length === bytes.length) { + const capacity = Math.min(limit + 1, Math.max(bytes.length * 2, bytes.length + READ_CHUNK_BYTES)); + const next = Buffer.allocUnsafe(capacity); + bytes.copy(next, 0, 0, length); + bytes = next; + } const count = readSync(fd, bytes, length, Math.min(READ_CHUNK_BYTES, bytes.length - length), null); if (count === 0) break; length += count; diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index e30a0c0aee2..cb90699829d 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -31,7 +31,7 @@ parsing and ownership rules below. | `src/clients/config-export.ts` | Pure per-client config builders and the exact managed fragments each client receives. It never writes files. | | `src/integrations/registry.ts` | Canonical config/detection paths, current-provider-store declarations, source-preserving YAML declarations, writer-lock behavior, and client IDs. | | `src/integrations/target.ts` | Which file one operation reads, writes and records, and whether a write there reaches the client. | -| `src/integrations/config-io.ts` | Bounded regular-file loading and parsing; production reads reject symlinks and stop after 4 MiB, and writes past the same bound are refused so a mutation cannot strand a file it cannot read back. Values that cannot round-trip through the target serializer are rejected before mutation. | +| `src/integrations/config-io.ts` | Bounded regular-file loading and parsing; production reads reject symlinks and stop after 4 MiB, and writes past the same bound are refused so a mutation cannot strand a file it cannot read back. `src/lib/bounded-file-read.ts` starts its buffer from the opened size and grows only as needed, retaining the limit-plus-one overflow probe when a file grows concurrently and returning only bytes actually read. Values that cannot round-trip through the target serializer are rejected before mutation. | | `src/integrations/kilo-candidates.ts` | Inspects all Kilo global config candidates for unsafe files and a competing `provider.opencodex` block before status or any operation that adds or replaces a block. | | `src/integrations/state.ts` | The single `absent` / `current` / `stale` / `conflict` / `unsafe` classifier used by status and every writer operation. | | `src/integrations/ownership.ts` | Durable ownership records: file, generated contribution, protected contribution, exact fragment paths, and operation identity. | diff --git a/tests/lib/bounded-file-read.test.ts b/tests/lib/bounded-file-read.test.ts index cc05e2f9f3d..99453120b94 100644 --- a/tests/lib/bounded-file-read.test.ts +++ b/tests/lib/bounded-file-read.test.ts @@ -1,4 +1,5 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; @@ -17,6 +18,18 @@ afterEach(() => { rmSync(root, { recursive: true, force: true }); }); +function afterOpenedStat(mutate: () => void, read: () => Buffer): Buffer { + const original = fs.fstatSync; + let changed = false; + const stat = spyOn(fs, "fstatSync").mockImplementation(((fd: number, options?: fs.StatOptions) => { + const opened = original(fd, options); + if (!changed) { changed = true; mutate(); } + return opened; + }) as typeof fs.fstatSync); + try { return read(); } + finally { stat.mockRestore(); expect(changed).toBe(true); } +} + describe("readBoundedRegularFileSync", () => { test("reads a bounded regular file", () => { const path = join(root, "config.json"); @@ -24,6 +37,56 @@ describe("readBoundedRegularFileSync", () => { expect(readBoundedRegularFileSync(path, 32).toString("utf8")).toBe(readFileSync(path, "utf8")); }); + test("small files do not allocate the configured limit", () => { + const path = join(root, "tiny.json"); + writeFileSync(path, '{"ok":true}'); + const initialized = spyOn(Buffer, "alloc"); + const uninitialized = spyOn(Buffer, "allocUnsafe"); + try { + const result = readBoundedRegularFileSync(path, 4 * 1024 * 1024); + const sizes = [...initialized.mock.calls, ...uninitialized.mock.calls].map(([size]) => size); + expect(result.toString("utf8")).toBe('{"ok":true}'); + expect(sizes.length).toBeGreaterThan(0); + expect(Math.max(...sizes)).toBeLessThanOrEqual(64 * 1024); + } finally { initialized.mockRestore(); uninitialized.mockRestore(); } + }); + + test("reads an empty file without returning unwritten buffer bytes", () => { + const path = join(root, "empty.json"); + writeFileSync(path, ""); + expect(readBoundedRegularFileSync(path, 32).length).toBe(0); + }); + + test("grows the buffer for data appended after the descriptor size check", () => { + const path = join(root, "growing.json"); + writeFileSync(path, "tiny"); + const grown = Buffer.alloc(96 * 1024, 0x61); + const result = afterOpenedStat(() => writeFileSync(path, grown), () => readBoundedRegularFileSync(path, 128 * 1024)); + expect(result.equals(grown)).toBe(true); + }); + + test("keeps the overflow probe when a file grows past the limit after fstat", () => { + const path = join(root, "overflow.json"); + writeFileSync(path, "tiny"); + expect(() => afterOpenedStat(() => writeFileSync(path, "x".repeat(33)), + () => readBoundedRegularFileSync(path, 32))).toThrow("file exceeds byte limit"); + }); + + test("returns only bytes actually read if the file shrinks after fstat", () => { + const path = join(root, "shrinking.json"); + writeFileSync(path, "a longer initial document"); + const result = afterOpenedStat(() => writeFileSync(path, "ok"), () => readBoundedRegularFileSync(path, 64)); + expect(result.toString("utf8")).toBe("ok"); + expect(result.length).toBe(2); + }); + + test("accepts a file exactly at the byte limit", () => { + const path = join(root, "boundary.json"); + const content = Buffer.alloc(64 * 1024 + 3, 0x62); + writeFileSync(path, content); + expect(readBoundedRegularFileSync(path, content.length).equals(content)).toBe(true); + }); + test("rejects oversized files and final symlinks", () => { const path = join(root, "large.json"); writeFileSync(path, "x".repeat(33)); From 1f01b570cda64c51f0e0312f0f030d2a8959ec38 Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:17:33 +0000 Subject: [PATCH 6/7] perf(config): preserve initialized bounded read buffers Retain the original zero-initialization of backing buffers while sizing small reads from the observed file size. Growth remains bounded by limit plus one and exposes only the bytes read. --- src/lib/bounded-file-read.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/bounded-file-read.ts b/src/lib/bounded-file-read.ts index 4eb866602e7..42555c57ed2 100644 --- a/src/lib/bounded-file-read.ts +++ b/src/lib/bounded-file-read.ts @@ -30,12 +30,12 @@ export function readBoundedRegularFileSync(path: string, limit: number): Buffer } // Start near the observed size, but keep probing up to limit + 1 if the file grows. - let bytes = Buffer.allocUnsafe(Number(opened.size) + 1); + let bytes = Buffer.alloc(Number(opened.size) + 1); let length = 0; while (length < limit + 1) { if (length === bytes.length) { const capacity = Math.min(limit + 1, Math.max(bytes.length * 2, bytes.length + READ_CHUNK_BYTES)); - const next = Buffer.allocUnsafe(capacity); + const next = Buffer.alloc(capacity); bytes.copy(next, 0, 0, length); bytes = next; } From 49802e7cffb92e1a735b81690d83f84d48b196c4 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:47:15 +0900 Subject: [PATCH 7/7] test(lib): keep the oversize assertion when Windows denies symlink creation symlinkSync throws without developer mode or privileges, which prevented the bounded-reader oversize check from running. The link assertion now runs only when creation succeeds. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- tests/lib/bounded-file-read.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/lib/bounded-file-read.test.ts b/tests/lib/bounded-file-read.test.ts index 99453120b94..8e9cc60e0d4 100644 --- a/tests/lib/bounded-file-read.test.ts +++ b/tests/lib/bounded-file-read.test.ts @@ -92,9 +92,16 @@ describe("readBoundedRegularFileSync", () => { writeFileSync(path, "x".repeat(33)); expect(() => readBoundedRegularFileSync(path, 32)).toThrow(); + // Windows can deny symlink creation without developer mode or privileges; the oversize + // assertion above already ran, so skip only the link-specific assertion in that case. const link = join(root, "linked.json"); - symlinkSync(path, link, "file"); - expect(() => readBoundedRegularFileSync(link, 64)).toThrow(); + let linked = true; + try { + symlinkSync(path, link, "file"); + } catch { + linked = false; + } + if (linked) expect(() => readBoundedRegularFileSync(link, 64)).toThrow(); }); test.skipIf(process.platform === "win32")("rejects a FIFO without blocking", () => {