From 29ee1811e4a8fd41b27f8cebac236499fb6fb2ee Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sat, 11 Jul 2026 10:19:12 +0800 Subject: [PATCH 1/2] fix(acp): reject fractional readTextFile limits Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- packages/acp-bridge/src/bridgeClient.test.ts | 20 +++++++++++++++++ packages/acp-bridge/src/bridgeClient.ts | 10 +++++++++ .../serve/bridge-file-system-adapter.test.ts | 22 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/packages/acp-bridge/src/bridgeClient.test.ts b/packages/acp-bridge/src/bridgeClient.test.ts index 2e35eda08c1..bdc2ba7c7d1 100644 --- a/packages/acp-bridge/src/bridgeClient.test.ts +++ b/packages/acp-bridge/src/bridgeClient.test.ts @@ -401,6 +401,26 @@ describe('BridgeClient — BridgeFileSystem injection seam (F1 step 5)', () => { expect(response.content).toBe('on-disk-content'); }); + + it('readTextFile rejects fractional positive limits before slicing inline content', async () => { + const client = makeClient(/* no fileSystem */); + const target = path.join(tmpDir, 'lines.txt'); + await fsp.writeFile(target, 'a\nb\nc\n', 'utf8'); + + const err = await client + .readTextFile({ + path: target, + sessionId: 'sess:test', + line: 1, + limit: 1.5, + }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(RequestError); + expect((err as Error).message).toContain( + '`limit` must be a positive integer.', + ); + }); }); }); diff --git a/packages/acp-bridge/src/bridgeClient.ts b/packages/acp-bridge/src/bridgeClient.ts index 069c22b539a..2fd701b2c9d 100644 --- a/packages/acp-bridge/src/bridgeClient.ts +++ b/packages/acp-bridge/src/bridgeClient.ts @@ -1879,6 +1879,16 @@ export class BridgeClient implements Client { if (typeof params.limit === 'number' && params.limit <= 0) { return { content: '' }; } + if ( + typeof params.limit === 'number' && + params.limit > 0 && + !Number.isSafeInteger(params.limit) + ) { + throw RequestError.invalidParams( + undefined, + '`limit` must be a positive integer.', + ); + } // BSA0E: cap the file size we'll buffer into RSS at 100 MiB so a // request like `{ line: 1, limit: 10 }` against a 500 MB log // doesn't cost the daemon 500 MB of memory just to return 10 diff --git a/packages/cli/src/serve/bridge-file-system-adapter.test.ts b/packages/cli/src/serve/bridge-file-system-adapter.test.ts index 7be95010ad7..292f1654567 100644 --- a/packages/cli/src/serve/bridge-file-system-adapter.test.ts +++ b/packages/cli/src/serve/bridge-file-system-adapter.test.ts @@ -365,6 +365,28 @@ describe('createBridgeFileSystemAdapter', () => { }); expect(response.content).toBe('x\ny\n'); }); + + it('propagates parse_error for fractional positive limits', async () => { + const target = path.join(tmpDir, 'fractional-limit.txt'); + await fsp.writeFile(target, 'x\ny\nz\n', 'utf8'); + const adapter = createBridgeFileSystemAdapter( + buildFactory({ trusted: true }), + ); + + const err = await adapter + .readText({ + path: target, + sessionId: 'sess:test', + line: 1, + limit: 1.5, + }) + .catch((e: unknown) => e); + + expect((err as { kind?: string }).kind).toBe('parse_error'); + expect((err as Error).message).toContain( + 'limit must be a positive integer', + ); + }); }); describe('boundary enforcement', () => { From 46c170b5aa34d41132c8c9e30d2c2bd5136089de Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sat, 11 Jul 2026 16:46:01 +0800 Subject: [PATCH 2/2] fix(acp): align readTextFile window validation Reject positive fractional line and limit values in the BridgeClient readTextFile inline fallback before they reach manual line slicing. Include the received value in the invalid params message, and keep the serve adapter regression test labeled as WorkspaceFileSystem path coverage. Also map FsError(parse_error) through the ACP wrapper as JSON-RPC Invalid Params so injected and inline read-window validation classify malformed params consistently while preserving errorKind/status/hint. Co-authored-by: qwen-code-ci-bot <253268222+qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen --- packages/acp-bridge/src/bridgeClient.test.ts | 44 ++++++++++++++++++- packages/acp-bridge/src/bridgeClient.ts | 15 ++++++- .../serve/bridge-file-system-adapter.test.ts | 2 +- 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/packages/acp-bridge/src/bridgeClient.test.ts b/packages/acp-bridge/src/bridgeClient.test.ts index bdc2ba7c7d1..c7cd08801b1 100644 --- a/packages/acp-bridge/src/bridgeClient.test.ts +++ b/packages/acp-bridge/src/bridgeClient.test.ts @@ -268,6 +268,28 @@ describe('BridgeClient — BridgeFileSystem injection seam (F1 step 5)', () => { expect((err.data as { hint?: unknown }).hint).toBeUndefined(); }); + it('readTextFile maps parse_error FsError to invalid params', async () => { + const readText = vi.fn(async (): Promise => { + throw makeFsError( + 'parse_error', + 'limit must be a positive integer, got 1.5', + { status: 400 }, + ); + }); + const client = makeClient({ writeText: vi.fn(), readText }); + + const err = (await client + .readTextFile({ path: '/x', sessionId: 'sess:test', limit: 1.5 }) + .catch((e) => e)) as Error & { code?: number; data?: unknown }; + + expect(err.name).toBe('RequestError'); + expect(err.code).toBe(-32602); + expect(err.data).toMatchObject({ + errorKind: 'parse_error', + status: 400, + }); + }); + it('passes non-FsError errors through unchanged (no RequestError wrap)', async () => { // Plain Error → bridgeClient must NOT wrap it. Only structured // FsError gets the reshape. ACP's default serialization is @@ -418,7 +440,27 @@ describe('BridgeClient — BridgeFileSystem injection seam (F1 step 5)', () => { expect(err).toBeInstanceOf(RequestError); expect((err as Error).message).toContain( - '`limit` must be a positive integer.', + '`limit` must be a positive integer, got 1.5', + ); + }); + + it('readTextFile rejects fractional positive lines before slicing inline content', async () => { + const client = makeClient(/* no fileSystem */); + const target = path.join(tmpDir, 'lines.txt'); + await fsp.writeFile(target, 'a\nb\nc\n', 'utf8'); + + const err = await client + .readTextFile({ + path: target, + sessionId: 'sess:test', + line: 1.5, + limit: 1, + }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(RequestError); + expect((err as Error).message).toContain( + '`line` must be a positive integer, got 1.5', ); }); }); diff --git a/packages/acp-bridge/src/bridgeClient.ts b/packages/acp-bridge/src/bridgeClient.ts index 2fd701b2c9d..c7dd09e6e41 100644 --- a/packages/acp-bridge/src/bridgeClient.ts +++ b/packages/acp-bridge/src/bridgeClient.ts @@ -320,7 +320,8 @@ function isTrustedArtifactToolUpdate( */ function preserveFsErrorOverAcp(err: unknown): never { if (isFsErrorShape(err)) { - throw new RequestError(-32603, err.message, { + const code = err.kind === 'parse_error' ? -32602 : -32603; + throw new RequestError(code, err.message, { errorKind: err.kind, ...(err.hint !== undefined ? { hint: err.hint } : {}), ...(err.status !== undefined ? { status: err.status } : {}), @@ -1886,7 +1887,17 @@ export class BridgeClient implements Client { ) { throw RequestError.invalidParams( undefined, - '`limit` must be a positive integer.', + `\`limit\` must be a positive integer, got ${params.limit}`, + ); + } + if ( + typeof params.line === 'number' && + params.line > 0 && + !Number.isSafeInteger(params.line) + ) { + throw RequestError.invalidParams( + undefined, + `\`line\` must be a positive integer, got ${params.line}`, ); } // BSA0E: cap the file size we'll buffer into RSS at 100 MiB so a diff --git a/packages/cli/src/serve/bridge-file-system-adapter.test.ts b/packages/cli/src/serve/bridge-file-system-adapter.test.ts index 292f1654567..42abe9b568c 100644 --- a/packages/cli/src/serve/bridge-file-system-adapter.test.ts +++ b/packages/cli/src/serve/bridge-file-system-adapter.test.ts @@ -366,7 +366,7 @@ describe('createBridgeFileSystemAdapter', () => { expect(response.content).toBe('x\ny\n'); }); - it('propagates parse_error for fractional positive limits', async () => { + it('readText surfaces workspace-file-system parse_error for fractional positive limits', async () => { const target = path.join(tmpDir, 'fractional-limit.txt'); await fsp.writeFile(target, 'x\ny\nz\n', 'utf8'); const adapter = createBridgeFileSystemAdapter(