Skip to content

fix(cli): validate ACP file read windows #5482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 1 commit into from
Jun 21, 2026
Merged

fix(cli): validate ACP file read windows #5482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 104 additions & 21 deletions packages/cli/src/serve/acpHttp/dispatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import {
} from '@qwen-code/acp-bridge/bridgeErrors';
import { writeStderrLine } from '../../utils/stdioHelpers.js';
import { MAX_WORKSPACE_PATH_LENGTH } from '../fs/paths.js';
import type { WorkspaceFileSystemFactory } from '../fs/index.js';
import {
MAX_READ_BYTES,
type WorkspaceFileSystemFactory,
} from '../fs/index.js';
import type { DeviceFlowRegistry } from '../auth/deviceFlow.js';
import { collectWorkspaceMemoryStatus } from '../workspaceMemory.js';
import {
Expand Down Expand Up @@ -155,6 +158,7 @@ const CONN_ROUTED_METHODS = new Set<string>([
const MAX_NAME_LENGTH = 256;
const DEFAULT_FILE_GLOB_MAX_RESULTS = 5000;
const MAX_FILE_GLOB_MAX_RESULTS = 50_000;
const MAX_FILE_LINE_LIMIT = 2000;

@wenshao wenshao Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] MAX_FILE_LINE_LIMIT = 2000 is a duplicate of the exported constant at packages/cli/src/serve/routes/workspaceFileRead.ts:37. If the REST route's value changes (e.g., raised to 3000), the ACP boundary silently retains 2000 — divergent enforcement on the same logical constraint.

Suggested change
const MAX_FILE_LINE_LIMIT = 2000;
import { MAX_FILE_LINE_LIMIT } from '../routes/workspaceFileRead.js';

(Remove the local const MAX_FILE_LINE_LIMIT = 2000; declaration.)

— DeepSeek/deepseek-v4-pro via Qwen Code /review


class AcpParamError extends Error {}

Expand All @@ -175,6 +179,23 @@ function parseOptionalPositiveInteger(
return value;
}

function parseOptionalSafeIntegerInRange(
value: unknown,
min: number,
max: number,
): number | null | undefined {
if (value === undefined) return undefined;
if (
typeof value !== 'number' ||
!Number.isSafeInteger(value) ||
value < min ||
value > max
) {
return null;
}
return value;
}

/**
* Validate an optional `cwd` param the same way the REST `POST /session`
* route does: when present it must be a string, ≤ PATH_MAX, and absolute.
Expand Down Expand Up @@ -1500,17 +1521,56 @@ export class AcpDispatcher {
originatorClientId: conn.clientId,
route: `ACP ${method}`,
});
const maxBytes = parseOptionalSafeIntegerInRange(
params['maxBytes'],
1,
MAX_READ_BYTES,
);
if (maxBytes === null) {
if (id !== undefined)
conn.sendConn(
error(
id,

@wenshao wenshao Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The new parameter validation rejects invalid params with INVALID_PARAMS errors, but produces zero server-side log output. If a client sends malformed params (e.g., maxBytes: 0), the file read fails silently from an operations perspective — there is no way to distinguish "request never arrived" from "request arrived but had invalid params" in server logs.

Consider adding this.logger?.warn(...) before each conn.sendConn(error(...)) call in the new validation blocks, logging the rejected parameter name, the invalid value, and the client ID.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

RPC.INVALID_PARAMS,
`\`maxBytes\` must be a positive integer in [1, ${MAX_READ_BYTES}]`,
),
);
return;
}
const line = parseOptionalSafeIntegerInRange(
params['line'],
1,
Number.MAX_SAFE_INTEGER,
);
if (line === null) {
if (id !== undefined)
conn.sendConn(
error(
id,
RPC.INVALID_PARAMS,
'`line` must be a positive integer',
),
);
return;
}
const limit = parseOptionalSafeIntegerInRange(
params['limit'],
1,
MAX_FILE_LINE_LIMIT,
);
if (limit === null) {
if (id !== undefined)
conn.sendConn(
error(
id,
RPC.INVALID_PARAMS,
`\`limit\` must be a positive integer in [1, ${MAX_FILE_LINE_LIMIT}]`,
),
);
return;
}
const resolved = await fs.resolve(p, 'read');
const out = await fs.readText(resolved, {
maxBytes:
typeof params['maxBytes'] === 'number'
? params['maxBytes']
: undefined,
line:
typeof params['line'] === 'number' ? params['line'] : undefined,
limit:
typeof params['limit'] === 'number' ? params['limit'] : undefined,
});
const out = await fs.readText(resolved, { maxBytes, line, limit });
this.replyConn(conn, id, {
path: p,
content: out.content,
Expand All @@ -1537,17 +1597,40 @@ export class AcpDispatcher {
originatorClientId: conn.clientId,
route: `ACP ${method}`,
});
const offset = parseOptionalSafeIntegerInRange(
params['offset'],
0,
Number.MAX_SAFE_INTEGER,
);
if (offset === null) {
if (id !== undefined)
conn.sendConn(
error(
id,
RPC.INVALID_PARAMS,
'`offset` must be a non-negative safe integer',
),
);
return;
}
const maxBytes = parseOptionalSafeIntegerInRange(
params['maxBytes'],
1,
MAX_READ_BYTES,
);
if (maxBytes === null) {
if (id !== undefined)
conn.sendConn(
error(
id,
RPC.INVALID_PARAMS,
`\`maxBytes\` must be a positive integer in [1, ${MAX_READ_BYTES}]`,
),
);
return;
}
const resolved = await fs.resolve(p, 'read');
const buf = await fs.readBytesWindow(resolved, {
offset:
typeof params['offset'] === 'number'
? params['offset']
: undefined,
maxBytes:
typeof params['maxBytes'] === 'number'
? params['maxBytes']
: undefined,
});
const buf = await fs.readBytesWindow(resolved, { offset, maxBytes });
this.replyConn(conn, id, { path: p, ...buf } as unknown);
return;
}
Expand Down
Loading
Loading