diff --git a/docs-site/src/content/docs/guides/integrations.md b/docs-site/src/content/docs/guides/integrations.md index 03eaf7fa5b3..5c61ee34936 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/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 4c08a826c59..70852da0e85 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -2,7 +2,7 @@ "version": 1, "root": "tests", "explicit": { - "responses-anthropic-billing-send-budget.test.ts": "responses", "anthropic-billing-ownership.test.ts": "adapters/anthropic", "server-anthropic-billing.test.ts": "server", "server-anthropic-billing-boundaries.test.ts": "server", "router-anthropic-billing-failover.test.ts": "routing", "anthropic-billing-refusal.test.ts": "adapters/anthropic", "messages-request-id-headers.test.ts": "claude-integration", + "responses-anthropic-billing-send-budget.test.ts": "responses", "anthropic-billing-ownership.test.ts": "adapters/anthropic", "server-anthropic-billing.test.ts": "server", "server-anthropic-billing-boundaries.test.ts": "server", "router-anthropic-billing-failover.test.ts": "routing", "anthropic-billing-refusal.test.ts": "adapters/anthropic", "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..90232298b0c 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,10 @@ import type { IntegrationClientId } from "./registry"; * then overwritten. */ 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, @@ -285,7 +290,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 } : {}) }; @@ -300,6 +305,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 IntegrationWriteSizeError( + `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/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 52c1cc57b9d..aece1067fb8 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, @@ -162,6 +162,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 { + 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); + } + return refuse(clientId, "write_failed", args.state, messageOf(error)); + } return refuse(clientId, "write_failed", args.state, messageOf(error), args.snapshotPath); } try { diff --git a/src/lib/bounded-file-read.ts b/src/lib/bounded-file-read.ts new file mode 100644 index 00000000000..42555c57ed2 --- /dev/null +++ b/src/lib/bounded-file-read.ts @@ -0,0 +1,58 @@ +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"); + } + + // Start near the observed size, but keep probing up to limit + 1 if the file grows. + 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.alloc(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; + } + 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 854705b474b..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 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, 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. | @@ -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 (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/cline-writer.test.ts b/tests/clients/cline-writer.test.ts index ff06f29dbc8..cb41449ee1a 100644 --- a/tests/clients/cline-writer.test.ts +++ b/tests/clients/cline-writer.test.ts @@ -354,4 +354,30 @@ 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); + 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-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-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..4a9ed11a1dc 100644 --- a/tests/clients/integrations-writer.test.ts +++ b/tests/clients/integrations-writer.test.ts @@ -1586,3 +1586,49 @@ describe("integration write ownership guard (#4197)", () => { })).not.toThrow(); }); }); + +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", + store: { ...store, discardUncommittedSnapshot: () => { 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(); + /* + * 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); + 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"); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 0015ed71b41..7f1ba5418a4 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -10,6 +10,7 @@ "router-anthropic-billing-failover.test.ts": "routing", "anthropic-billing-refusal.test.ts": "adapters/anthropic", "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..8e9cc60e0d4 --- /dev/null +++ b/tests/lib/bounded-file-read.test.ts @@ -0,0 +1,123 @@ +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"; +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 }); +}); + +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"); + writeFileSync(path, '{"ok":true}'); + 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)); + 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"); + 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", () => { + 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); + }); +});