From 944d7260517ce650722e88e85880f91838e6c40b Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 18 May 2026 13:43:00 +0800 Subject: [PATCH 1/6] refactor(serve/fs): glob audit hashes workspace + emits pattern Closes PR #4250 follow-up #4. Hashing the per-call cwd for glob audit produced a different pathHash for every subdirectory glob without giving operators any actionable difference (raw paths are privacy-gated). Replace the hash basis with the bound workspace itself and surface the literal pattern on a new schema field, so every glob row carries a stable workspace marker and a per-call pattern. The pattern field also fires on parse_error denials (path-escape patterns, non-relative patterns) so audit consumers debugging a production glob rejection can see the exact rejected pattern without needing QWEN_AUDIT_RAW_PATHS=1. --- packages/cli/src/serve/fs/audit.test.ts | 52 +++++++++++++++++++ packages/cli/src/serve/fs/audit.ts | 21 ++++++++ .../src/serve/fs/workspaceFileSystem.test.ts | 36 ++++++++++++- .../cli/src/serve/fs/workspaceFileSystem.ts | 21 +++++++- 4 files changed, 128 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/serve/fs/audit.test.ts b/packages/cli/src/serve/fs/audit.test.ts index 31ae4f4e114..b1c3f75d254 100644 --- a/packages/cli/src/serve/fs/audit.test.ts +++ b/packages/cli/src/serve/fs/audit.test.ts @@ -149,6 +149,58 @@ describe('createAuditPublisher', () => { expect(events[0].data).not.toHaveProperty('hint'); }); + it('attaches pattern field for fs.access on glob intent', () => { + const { events, publisher, workspace } = setup(); + publisher.recordAccess( + { route: 'GET /glob' }, + { + intent: 'glob', + absolute: workspace, + durationMs: 7, + sizeBytes: 12, + pattern: '**/*.ts', + }, + ); + expect(events[0].data).toMatchObject({ + kind: FS_ACCESS_EVENT_TYPE, + intent: 'glob', + pattern: '**/*.ts', + pathHash: expectedHash(workspace), + }); + }); + + it('attaches pattern field for fs.denied on glob intent', () => { + const { events, publisher } = setup(); + publisher.recordDenied( + { route: 'GET /glob' }, + { + intent: 'glob', + input: '../../**', + errorKind: 'parse_error', + pattern: '../../**', + }, + ); + expect(events[0].data).toMatchObject({ + kind: FS_DENIED_EVENT_TYPE, + intent: 'glob', + errorKind: 'parse_error', + pattern: '../../**', + }); + }); + + it('omits pattern when not provided', () => { + const { events, publisher, workspace } = setup(); + publisher.recordAccess( + { route: 'GET /file' }, + { + intent: 'read', + absolute: path.join(workspace, 'a.ts') as ResolvedPath, + durationMs: 0, + }, + ); + expect(events[0].data).not.toHaveProperty('pattern'); + }); + it('respects QWEN_AUDIT_RAW_PATHS=1 via env when includeRawPaths is unset', () => { const original = process.env['QWEN_AUDIT_RAW_PATHS']; process.env['QWEN_AUDIT_RAW_PATHS'] = '1'; diff --git a/packages/cli/src/serve/fs/audit.ts b/packages/cli/src/serve/fs/audit.ts index b80a3cdcfca..a9ad3e7d2a2 100644 --- a/packages/cli/src/serve/fs/audit.ts +++ b/packages/cli/src/serve/fs/audit.ts @@ -77,6 +77,16 @@ export interface FsAccessAuditPayload { truncated?: boolean; matchedIgnore?: 'file' | 'directory'; durationMs: number; + /** + * Literal glob pattern. Populated only for `intent === 'glob'`, + * where `pathHash` would otherwise hash the bound workspace and + * provide no per-call information. The pattern is recorded + * verbatim (not hashed) because it does not carry path content + * — the per-hit canonical paths are NOT logged here. Audit + * consumers correlate the workspace via `pathHash` and the + * specific call via `pattern`. + */ + pattern?: string; } export interface FsDeniedAuditPayload { @@ -99,6 +109,8 @@ export interface FsDeniedAuditPayload { * error into an `FsError` whose message we can quote. */ message?: string; + /** See `FsAccessAuditPayload.pattern` — same semantics. */ + pattern?: string; } /** @@ -129,6 +141,13 @@ export interface AuditPublisher { ): void; } +/** + * Per-payload `pattern` is part of the public schema but never + * usefully sourced from anywhere other than the orchestrator's + * own glob path — the request `Omit` keeps it surfaced so the + * orchestrator (and tests) can pass it without TS errors. + */ + /** * SHA-256 over the canonical absolute path, truncated to 16 hex * chars. The truncation matches claude-code's privacy model: long @@ -227,6 +246,7 @@ export function createAuditPublisher( if (record.sizeBytes !== undefined) payload.sizeBytes = record.sizeBytes; if (record.truncated) payload.truncated = true; if (record.matchedIgnore) payload.matchedIgnore = record.matchedIgnore; + if (record.pattern !== undefined) payload.pattern = record.pattern; if (includeRawPaths) { payload.relPath = relForAudit(absolute, boundWorkspace); } @@ -262,6 +282,7 @@ export function createAuditPublisher( if (record.message && includeRawPaths) { payload.message = record.message; } + if (record.pattern !== undefined) payload.pattern = record.pattern; if (includeRawPaths) { payload.relPath = relForAudit(record.input, boundWorkspace); } diff --git a/packages/cli/src/serve/fs/workspaceFileSystem.test.ts b/packages/cli/src/serve/fs/workspaceFileSystem.test.ts index 5745ea6d6f3..7bb938eb9b9 100644 --- a/packages/cli/src/serve/fs/workspaceFileSystem.test.ts +++ b/packages/cli/src/serve/fs/workspaceFileSystem.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { promises as fsp } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { randomBytes } from 'node:crypto'; +import { createHash, randomBytes } from 'node:crypto'; import { Ignore } from '@qwen-code/qwen-code-core'; import { FS_ACCESS_EVENT_TYPE, @@ -786,6 +786,40 @@ describe('WorkspaceFileSystem - glob escape audit', () => { expect((denied!.data as { hint?: string }).hint).toMatch( /\d+ hit\(s\) that resolved outside workspace/, ); + expect((denied!.data as { pattern?: string }).pattern).toBe('*.ts'); + }); + + it('records fs.access with workspace-hashed pathHash and pattern field on glob success', async () => { + await fsp.writeFile(path.join(h.workspace, 'one.ts'), 'a'); + await h.fs.glob('*.ts'); + const access = h.events.find( + (e) => + e.type === FS_ACCESS_EVENT_TYPE && + (e.data as { intent: string }).intent === 'glob', + ); + expect(access).toBeDefined(); + const data = access!.data as { pathHash: string; pattern?: string }; + expect(data.pattern).toBe('*.ts'); + // Hash equals sha256(boundWorkspace) sliced to 16 hex chars — + // every glob audit row in this workspace shares the same + // pathHash, and `pattern` is the per-call signal. + const expectedHash = createHash('sha256') + .update(h.workspace) + .digest('hex') + .slice(0, 16); + expect(data.pathHash).toBe(expectedHash); + }); + + it('emits fs.denied with pattern when glob pattern is rejected as parse_error', async () => { + await expect(h.fs.glob('../../**')).rejects.toThrow(/'..' segments/); + const denied = h.events.find( + (e) => + e.type === FS_DENIED_EVENT_TYPE && + (e.data as { intent: string }).intent === 'glob' && + (e.data as { errorKind: string }).errorKind === 'parse_error', + ); + expect(denied).toBeDefined(); + expect((denied!.data as { pattern?: string }).pattern).toBe('../../**'); }); }); diff --git a/packages/cli/src/serve/fs/workspaceFileSystem.ts b/packages/cli/src/serve/fs/workspaceFileSystem.ts index 9db91238fdb..5bb9ff2441c 100644 --- a/packages/cli/src/serve/fs/workspaceFileSystem.ts +++ b/packages/cli/src/serve/fs/workspaceFileSystem.ts @@ -681,6 +681,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { input: pattern, errorKind: 'symlink_escape', hint: `glob filtered ${escapedCount} hit(s) that resolved outside workspace`, + pattern, }); } if (permissionErrorCount > 0) { @@ -689,6 +690,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { input: pattern, errorKind: 'permission_denied', hint: `glob skipped ${permissionErrorCount} hit(s) due to EACCES/EPERM`, + pattern, }); } if (transientErrorCount > 0) { @@ -704,13 +706,23 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { // mappings). errorKind: 'io_error', hint: `glob skipped ${transientErrorCount} hit(s) due to transient I/O errors (EIO/EBUSY/ENAMETOOLONG/EMFILE)`, + pattern, }); } + // `absolute: boundWorkspace` (rather than `cwd`) ties every + // glob audit row's `pathHash` to the workspace itself. + // Hashing `cwd` made each per-subdirectory glob produce a + // distinct hash with no operator-actionable difference (the + // raw path is privacy-gated). The literal `pattern` field is + // the per-call signal; `pathHash` is the workspace marker + // operators correlate across audit rows. Follow-up #4 from + // PR #4250. this.deps.audit.recordAccess(this.deps.ctx, { intent: 'glob', - absolute: cwd as ResolvedPath, + absolute: this.deps.boundWorkspace, durationMs: performance.now() - start, sizeBytes: out.length, + pattern, }); return out; } catch (err) { @@ -920,6 +932,13 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { // actual cause (errno text, byte counts, glob pattern, // etc.) rather than just `errorKind` + `hint`. message: fs.message, + // For glob denials (parse_error pattern rejection, + // catastrophic walk failures) the input IS the pattern + // already; surfacing it on the dedicated `pattern` field + // keeps the schema parallel with successful `recordAccess` + // glob rows so consumers can `data.pattern` without + // branching on intent. + pattern: intent === 'glob' ? input : undefined, }); return fs; } From c1c7e13f95b11e77425a78565a82531e8051e510 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 18 May 2026 13:51:06 +0800 Subject: [PATCH 2/6] feat(serve): safe workspace file read routes (#4175 PR 19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add four read-only HTTP routes that consume PR 18's per-request WorkspaceFileSystem boundary: - GET /file?path=... text content + meta (encoding/BOM/lineEnding) - GET /list?path=... directory entries (name/kind/ignored) - GET /glob?pattern=... workspace-relative match paths - GET /stat?path=... file/directory metadata The routes share one error envelope (sendFsError) that maps FsError.kind through the boundary's existing DEFAULT_STATUS_BY_KIND table to a typed JSON response. All four 200 responses set Cache-Control: no-store and X-Content-Type-Options: nosniff so a browser-adjacent client cannot cache or sniff source content. Routes are advertised under a single workspace_file_read capability tag — the four endpoints share the same backing boundary and the same failure shape, so per-route tags would force four simultaneous registry entries with no operator-meaningful difference between them. Mutating routes will ship in PR 20 under their own workspace_file_write tag. Trust gate is unchanged: read intents pass on untrusted workspaces per PR 18's policy.ts. Auth follows the global bearer flow only; read routes never run mutate(), since none of them mutate state. --- packages/cli/src/serve/capabilities.ts | 12 + .../serve/routes/workspaceFileRead.test.ts | 333 ++++++++++++++ .../cli/src/serve/routes/workspaceFileRead.ts | 428 ++++++++++++++++++ packages/cli/src/serve/server.test.ts | 3 + packages/cli/src/serve/server.ts | 17 + 5 files changed, 793 insertions(+) create mode 100644 packages/cli/src/serve/routes/workspaceFileRead.test.ts create mode 100644 packages/cli/src/serve/routes/workspaceFileRead.ts diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index 5948a71a186..1a23333f1cf 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -85,6 +85,18 @@ export const SERVE_CAPABILITY_REGISTRY = { // `require_auth` is the only conditional tag, kept last for // visibility in `Object.keys(SERVE_CAPABILITY_REGISTRY)`. mcp_guardrails: { since: 'v1', modes: ['warn', 'enforce'] }, + // Issue #4175 PR 19. Daemon supports the read-only workspace file + // surface: `GET /file`, `GET /list`, `GET /glob`, `GET /stat`. The + // four routes are gated as a single feature because they share the + // same backing `WorkspaceFileSystem` boundary (PR 18) and the same + // failure shape — clients that pre-flight one of them get the + // others for free, and a future deprecation would have to coordinate + // across all four anyway. Per-route tags would force four + // simultaneous registry entries with no operator-meaningful + // difference between them. Mutating routes (`POST /file/write`, + // `POST /file/edit`) ship under a separate `workspace_file_write` + // tag in PR 20. + workspace_file_read: { since: 'v1' }, // Issue #4175 PR 15. Daemon was booted with `--require-auth` (or // `requireAuth: true`), so even loopback callers must carry a bearer // token. Advertised CONDITIONALLY — only when the flag is on — so diff --git a/packages/cli/src/serve/routes/workspaceFileRead.test.ts b/packages/cli/src/serve/routes/workspaceFileRead.test.ts new file mode 100644 index 00000000000..4b86cda6aee --- /dev/null +++ b/packages/cli/src/serve/routes/workspaceFileRead.test.ts @@ -0,0 +1,333 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fsp } from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { randomBytes } from 'node:crypto'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import { Ignore } from '@qwen-code/qwen-code-core'; +import { createServeApp } from '../server.js'; +import { + canonicalizeWorkspace, + createWorkspaceFileSystemFactory, +} from '../fs/index.js'; +import type { BridgeEvent } from '../eventBus.js'; +import type { ServeOptions } from '../types.js'; + +const baseOpts: ServeOptions = { + hostname: '127.0.0.1', + port: 4180, + mode: 'http-bridge', +}; + +interface Harness { + workspace: string; + scratch: string; + events: BridgeEvent[]; + app: ReturnType; +} + +async function makeHarness(opts?: { + trusted?: boolean; + ignore?: Ignore; +}): Promise { + const scratch = await fsp.mkdtemp( + path.join(os.tmpdir(), `qwen-routes-${randomBytes(4).toString('hex')}-`), + ); + const wsDir = path.join(scratch, 'ws'); + await fsp.mkdir(wsDir); + const workspace = canonicalizeWorkspace(wsDir); + const events: BridgeEvent[] = []; + const fsFactory = createWorkspaceFileSystemFactory({ + boundWorkspace: workspace, + trusted: opts?.trusted ?? true, + emit: (e) => events.push(e), + ignore: opts?.ignore, + }); + const app = createServeApp({ ...baseOpts, workspace }, undefined, { + fsFactory, + }); + return { workspace, scratch, events, app }; +} + +async function teardown(h: Harness): Promise { + await fsp.rm(h.scratch, { recursive: true, force: true }); +} + +function loopbackHost(): string { + return `127.0.0.1:${baseOpts.port}`; +} + +describe('GET /file', () => { + let h: Harness; + beforeEach(async () => { + h = await makeHarness(); + }); + afterEach(async () => teardown(h)); + + it('returns content + meta for a UTF-8 text file', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), 'hello\nworld\n'); + const res = await request(h.app) + .get('/file?path=a.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + kind: 'file', + path: 'a.txt', + content: 'hello\nworld\n', + lineEnding: 'lf', + truncated: false, + matchedIgnore: null, + }); + expect(res.body.sizeBytes).toBe(12); + }); + + it('attaches Cache-Control: no-store and X-Content-Type-Options: nosniff', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), 'x'); + const res = await request(h.app) + .get('/file?path=a.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.headers['cache-control']).toBe('no-store'); + expect(res.headers['x-content-type-options']).toBe('nosniff'); + }); + + it('returns 400 path_outside_workspace for ".." escapes', async () => { + const res = await request(h.app) + .get('/file?path=../escape') + .set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ + errorKind: 'path_outside_workspace', + status: 400, + }); + }); + + it('returns 404 path_not_found for missing files', async () => { + const res = await request(h.app) + .get('/file?path=missing.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(404); + expect(res.body.errorKind).toBe('path_not_found'); + }); + + it('returns 422 binary_file when target contains NULs', async () => { + const buf = Buffer.alloc(64); + buf[5] = 0; + await fsp.writeFile(path.join(h.workspace, 'bin.dat'), buf); + const res = await request(h.app) + .get('/file?path=bin.dat') + .set('Host', loopbackHost()); + expect(res.status).toBe(422); + expect(res.body.errorKind).toBe('binary_file'); + }); + + it('truncates content above maxBytes and reports truncated=true', async () => { + await fsp.writeFile(path.join(h.workspace, 'big.txt'), 'a'.repeat(2048)); + const res = await request(h.app) + .get('/file?path=big.txt&maxBytes=512') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.truncated).toBe(true); + expect(res.body.content.length).toBeLessThanOrEqual(512); + }); + + it('rejects malformed maxBytes with parse_error 400', async () => { + const res = await request(h.app) + .get('/file?path=a.txt&maxBytes=-1') + .set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('requires the path query param', async () => { + const res = await request(h.app).get('/file').set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('emits one fs.access audit event per successful read', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), 'x'); + await request(h.app).get('/file?path=a.txt').set('Host', loopbackHost()); + const accesses = h.events.filter((e) => e.type === 'fs.access'); + expect(accesses).toHaveLength(1); + expect(accesses[0].data).toMatchObject({ + intent: 'read', + route: 'GET /file', + }); + }); +}); + +describe('GET /stat', () => { + let h: Harness; + beforeEach(async () => { + h = await makeHarness(); + }); + afterEach(async () => teardown(h)); + + it('returns metadata for a regular file', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), 'hi'); + const res = await request(h.app) + .get('/stat?path=a.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + kind: 'stat', + path: 'a.txt', + type: 'file', + sizeBytes: 2, + }); + expect(typeof res.body.modifiedMs).toBe('number'); + }); + + it('returns metadata for a directory', async () => { + await fsp.mkdir(path.join(h.workspace, 'sub')); + const res = await request(h.app) + .get('/stat?path=sub') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.type).toBe('directory'); + }); + + it('returns 400 symlink_escape when target points outside the workspace', async () => { + const outside = path.join(h.scratch, 'evil.txt'); + await fsp.writeFile(outside, 'x'); + await fsp.symlink(outside, path.join(h.workspace, 'leak.txt'), 'file'); + const res = await request(h.app) + .get('/stat?path=leak.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('symlink_escape'); + }); +}); + +describe('GET /list', () => { + let h: Harness; + beforeEach(async () => { + h = await makeHarness(); + }); + afterEach(async () => teardown(h)); + + it('lists entries with name + kind + ignored, no path field', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), ''); + await fsp.mkdir(path.join(h.workspace, 'sub')); + const res = await request(h.app) + .get('/list?path=.') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.kind).toBe('list'); + expect(res.body.path).toBe('.'); + expect(res.body.truncated).toBe(false); + const names = (res.body.entries as Array<{ name: string }>) + .map((e) => e.name) + .sort(); + expect(names).toEqual(['a.txt', 'sub']); + for (const e of res.body.entries) { + expect(e).toHaveProperty('name'); + expect(e).toHaveProperty('kind'); + expect(e).toHaveProperty('ignored'); + expect(e).not.toHaveProperty('path'); + } + }); + + it('drops ignored entries by default and includes them with includeIgnored=1', async () => { + // Build the Ignore instance manually because `loadIgnoreRules` + // is invoked at factory construction time — writing a .gitignore + // file post-construction would be ignored. The test harness + // resets between specs, so we get a fresh factory here. + await teardown(h); + h = await makeHarness({ ignore: new Ignore().add(['secret.txt']) }); + await fsp.writeFile(path.join(h.workspace, 'public.txt'), 'p'); + await fsp.writeFile(path.join(h.workspace, 'secret.txt'), 's'); + + const filtered = await request(h.app) + .get('/list?path=.') + .set('Host', loopbackHost()); + const filteredNames = ( + filtered.body.entries as Array<{ name: string }> + ).map((e) => e.name); + expect(filteredNames).not.toContain('secret.txt'); + expect(filteredNames).toContain('public.txt'); + + const all = await request(h.app) + .get('/list?path=.&includeIgnored=1') + .set('Host', loopbackHost()); + const allNames = (all.body.entries as Array<{ name: string }>) + .map((e) => e.name) + .sort(); + expect(allNames).toContain('secret.txt'); + expect(allNames).toContain('public.txt'); + }); + + it('returns 400 parse_error when listing a regular file', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), ''); + const res = await request(h.app) + .get('/list?path=a.txt') + .set('Host', loopbackHost()); + // ENOTDIR maps to parse_error per fs/errors.ts + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); +}); + +describe('GET /glob', () => { + let h: Harness; + beforeEach(async () => { + h = await makeHarness(); + }); + afterEach(async () => teardown(h)); + + it('returns workspace-relative match paths', async () => { + await fsp.writeFile(path.join(h.workspace, 'one.ts'), ''); + await fsp.writeFile(path.join(h.workspace, 'two.ts'), ''); + await fsp.writeFile(path.join(h.workspace, 'README.md'), ''); + const res = await request(h.app) + .get('/glob?pattern=*.ts') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.kind).toBe('glob'); + expect(res.body.pattern).toBe('*.ts'); + expect(res.body.matches.sort()).toEqual(['one.ts', 'two.ts']); + expect(res.body.count).toBe(2); + for (const m of res.body.matches) { + expect(path.isAbsolute(m)).toBe(false); + expect(m.startsWith('..')).toBe(false); + } + }); + + it('returns 400 parse_error for ".." escape patterns', async () => { + const res = await request(h.app) + .get('/glob?pattern=../**') + .set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + + it('returns 400 parse_error when maxResults is malformed', async () => { + const res = await request(h.app) + .get('/glob?pattern=*&maxResults=zero') + .set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); +}); + +describe('capability advertisement', () => { + it('advertises workspace_file_read on /capabilities', async () => { + const h = await makeHarness(); + try { + const res = await request(h.app) + .get('/capabilities') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.features).toContain('workspace_file_read'); + } finally { + await teardown(h); + } + }); +}); diff --git a/packages/cli/src/serve/routes/workspaceFileRead.ts b/packages/cli/src/serve/routes/workspaceFileRead.ts new file mode 100644 index 00000000000..91307cad347 --- /dev/null +++ b/packages/cli/src/serve/routes/workspaceFileRead.ts @@ -0,0 +1,428 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as path from 'node:path'; +import type { Application, Request, Response } from 'express'; +import { writeStderrLine } from '../../utils/stdioHelpers.js'; +import { + isFsError, + type FsError, + type WorkspaceFileSystemFactory, +} from '../fs/index.js'; + +/** + * Hard cap on entries returned from `GET /list`. The directory walk + * runs in a single tick (`fsp.readdir`), so an unbounded listing of + * a `node_modules`-style directory pins the event loop and the + * resulting JSON payload (which the JSON.stringify pass already + * holds in memory) is too large to be useful. 2000 is generous for + * legitimate listings while staying well under the 10MB request + * limit when each entry serializes to ~80 bytes. + * + * Ties into the response's `truncated: true` flag so SDK consumers + * can ask the daemon to paginate (PR 19 emits the flag; pagination + * itself is a future PR — this PR's job is to advertise the + * truncation so the SDK doesn't quietly assume the full set). + */ +export const MAX_LIST_ENTRIES = 2000; + +/** + * Default cap when the caller omits `?maxResults` on `GET /glob`. + * Mirrors the orchestrator's default behavior (no cap) clipped to a + * concrete number so route consumers see consistent ceilings without + * needing to know the orchestrator's defaults. + */ +export const DEFAULT_GLOB_MAX_RESULTS = 5000; + +/** + * Hard upper bound for caller-supplied `?maxResults` on `GET /glob`. + * Anything above this rejects with `parse_error` rather than + * silently capping; a caller asking for 1M results almost + * certainly meant to stream. + */ +export const MAX_GLOB_MAX_RESULTS = 50_000; + +/** + * Privacy + correctness headers shared by every read route. The + * `no-store` directive blocks intermediaries (browser caches, + * forwarding proxies in development) from snapshotting workspace + * file contents — even on a localhost daemon, a misconfigured CDN + * or a developer browser extension that mirrors XHR responses to + * disk would otherwise persist source contents past the request + * lifetime. `nosniff` blocks MIME-sniffing fallbacks that would let + * a UTF-8 source file render as HTML in a browser that loaded it + * directly. Both are harmless on the SDK / curl path and + * mandatory for any browser-adjacent client. + */ +function applyReadHeaders(res: Response): void { + res.set('Cache-Control', 'no-store'); + res.set('X-Content-Type-Options', 'nosniff'); +} + +/** + * Common error envelope. Mirrors `sendBridgeError` in + * `serve/server.ts` so SDK consumers see one shape across daemon + * routes. `FsError` carries its own `status` from + * `DEFAULT_STATUS_BY_KIND` (`fs/errors.ts`), so the route doesn't + * re-derive it — that keeps the kind→status mapping authoritative + * in a single place. Non-`FsError` paths log to stderr and 500; + * the route's own try-catch should already have wrapped expected + * boundary errors via `wrapAsFsError`. + */ +export function sendFsError(res: Response, err: unknown, route: string): void { + applyReadHeaders(res); + if (isFsError(err)) { + const fs: FsError = err; + res.status(fs.status).json({ + errorKind: fs.kind, + error: fs.message, + hint: fs.hint, + status: fs.status, + }); + return; + } + writeStderrLine( + `qwen serve: ${route} unexpected error: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + errorKind: 'internal_error', + error: err instanceof Error ? err.message : String(err), + status: 500, + }); +} + +/** + * Parse a positive-integer query value within `[min, max]`. Returns: + * - `undefined` when the param is absent (caller defaults). + * - `null` when the param is malformed/out-of-range — the route + * short-circuits with a 400 + `parse_error` envelope. + * - the parsed integer otherwise. + * + * Strict on `^\d+$` so `''`, `'abc'`, `'1.5'`, `'-3'` all reject — + * the daemon's other range parsers (`parseMaxQueuedQuery`) use the + * same regex shape, keeping query-validation behavior consistent. + */ +function parseIntInRange( + raw: unknown, + min: number, + max: number, +): number | null | undefined { + if (raw === undefined) return undefined; + if (typeof raw !== 'string' || !/^\d+$/.test(raw)) return null; + const n = Number.parseInt(raw, 10); + if (!Number.isFinite(n) || n < min || n > max) return null; + return n; +} + +/** Treat `'1'` and `'true'` as true; everything else (including absence) as false. */ +function parseBoolFlag(raw: unknown): boolean { + return raw === '1' || raw === 'true'; +} + +/** + * Extract a required string query param. Returns the value or sends + * a 400 envelope and returns `null`. Empty strings count as absent + * — the daemon doesn't accept `?path=` as "the workspace root"; + * callers asking for the root pass `?path=.` explicitly. + */ +function requireStringQuery( + res: Response, + raw: unknown, + name: string, + route: string, +): string | null { + if (typeof raw !== 'string' || raw.length === 0) { + applyReadHeaders(res); + res.status(400).json({ + errorKind: 'parse_error', + error: `${name} query parameter is required`, + status: 400, + }); + writeStderrLine( + `qwen serve: ${route} rejected request missing required ?${name}`, + ); + return null; + } + return raw; +} + +/** + * Pull `WorkspaceFileSystemFactory` off `app.locals` (set by + * `createServeApp`). Returns `null` and sends a 500 envelope when + * the factory is missing — that means `createServeApp` was bypassed + * by a custom embed without injecting `deps.fsFactory`, which is a + * deployment misconfiguration the route can't recover from. + */ +function getFsFactory( + req: Request, + res: Response, +): WorkspaceFileSystemFactory | null { + const factory = (req.app.locals as { fsFactory?: WorkspaceFileSystemFactory }) + .fsFactory; + if (!factory) { + applyReadHeaders(res); + res.status(500).json({ + errorKind: 'internal_error', + error: 'workspace filesystem factory is not configured', + status: 500, + }); + return null; + } + return factory; +} + +interface RegisterDeps { + /** + * Pulls the daemon-stamped client identity off the request. Re-used + * from `serve/server.ts` so the X-Qwen-Client-Id validation lives + * in one place; PR 19 routes thread the trusted id into the audit + * context. Returning `null` means the helper already sent a 400 + * — the route must short-circuit. + */ + parseClientId: (req: Request, res: Response) => string | undefined | null; +} + +async function handleGetFile( + req: Request, + res: Response, + _deps: RegisterDeps, +): Promise { + const ROUTE = 'GET /file'; + const factory = getFsFactory(req, res); + if (!factory) return; + const clientId = _deps.parseClientId(req, res); + if (clientId === null) return; + const queryPath = requireStringQuery(res, req.query['path'], 'path', ROUTE); + if (queryPath === null) return; + const maxBytes = parseIntInRange(req.query['maxBytes'], 1, 256 * 1024); + if (maxBytes === null) { + applyReadHeaders(res); + res.status(400).json({ + errorKind: 'parse_error', + error: '`maxBytes` must be a positive integer in [1, 262144]', + status: 400, + }); + return; + } + const line = parseIntInRange(req.query['line'], 1, Number.MAX_SAFE_INTEGER); + if (line === null) { + applyReadHeaders(res); + res.status(400).json({ + errorKind: 'parse_error', + error: '`line` must be a positive integer', + status: 400, + }); + return; + } + const limit = parseIntInRange(req.query['limit'], 1, MAX_LIST_ENTRIES); + if (limit === null) { + applyReadHeaders(res); + res.status(400).json({ + errorKind: 'parse_error', + error: '`limit` must be a positive integer in [1, 2000]', + status: 400, + }); + return; + } + const fs = factory.forRequest({ + originatorClientId: clientId ?? undefined, + route: ROUTE, + }); + try { + const resolved = await fs.resolve(queryPath, 'read'); + const out = await fs.readText(resolved, { maxBytes, line, limit }); + applyReadHeaders(res); + res.status(200).json({ + kind: 'file', + path: workspaceRelative(req, resolved), + content: out.content, + encoding: out.meta.encoding ?? 'utf-8', + bom: out.meta.bom === true, + lineEnding: out.meta.lineEnding, + sizeBytes: Buffer.byteLength(out.content, 'utf-8'), + truncated: out.meta.truncated === true, + matchedIgnore: out.meta.matchedIgnore ?? null, + originalLineCount: out.meta.originalLineCount, + }); + } catch (err) { + sendFsError(res, err, ROUTE); + } +} + +async function handleGetStat( + req: Request, + res: Response, + _deps: RegisterDeps, +): Promise { + const ROUTE = 'GET /stat'; + const factory = getFsFactory(req, res); + if (!factory) return; + const clientId = _deps.parseClientId(req, res); + if (clientId === null) return; + const queryPath = requireStringQuery(res, req.query['path'], 'path', ROUTE); + if (queryPath === null) return; + const fs = factory.forRequest({ + originatorClientId: clientId ?? undefined, + route: ROUTE, + }); + try { + const resolved = await fs.resolve(queryPath, 'stat'); + const st = await fs.stat(resolved); + applyReadHeaders(res); + res.status(200).json({ + kind: 'stat', + path: workspaceRelative(req, resolved), + type: st.kind, + sizeBytes: st.sizeBytes, + modifiedMs: st.modifiedMs, + }); + } catch (err) { + sendFsError(res, err, ROUTE); + } +} + +async function handleGetList( + req: Request, + res: Response, + _deps: RegisterDeps, +): Promise { + const ROUTE = 'GET /list'; + const factory = getFsFactory(req, res); + if (!factory) return; + const clientId = _deps.parseClientId(req, res); + if (clientId === null) return; + const queryPath = requireStringQuery(res, req.query['path'], 'path', ROUTE); + if (queryPath === null) return; + const includeIgnored = parseBoolFlag(req.query['includeIgnored']); + const fs = factory.forRequest({ + originatorClientId: clientId ?? undefined, + route: ROUTE, + }); + try { + const resolved = await fs.resolve(queryPath, 'list'); + const entries = await fs.list(resolved, { includeIgnored }); + let truncated = false; + let returned = entries; + if (returned.length > MAX_LIST_ENTRIES) { + // Slice on the route side rather than passing a cap into + // `list()` so the boundary's audit row reports the FULL + // directory size (`sizeBytes` = entries.length) — operators + // see the directory's true cardinality, not just what the + // route handed back. PR 18's `list()` doesn't accept a cap + // anyway; pagination support lives inside the route. + returned = returned.slice(0, MAX_LIST_ENTRIES); + truncated = true; + } + applyReadHeaders(res); + res.status(200).json({ + kind: 'list', + path: workspaceRelative(req, resolved), + entries: returned, + truncated, + matchedIgnore: null, + }); + } catch (err) { + sendFsError(res, err, ROUTE); + } +} + +async function handleGetGlob( + req: Request, + res: Response, + _deps: RegisterDeps, +): Promise { + const ROUTE = 'GET /glob'; + const factory = getFsFactory(req, res); + if (!factory) return; + const clientId = _deps.parseClientId(req, res); + if (clientId === null) return; + const pattern = requireStringQuery( + res, + req.query['pattern'], + 'pattern', + ROUTE, + ); + if (pattern === null) return; + const includeIgnored = parseBoolFlag(req.query['includeIgnored']); + const maxResults = parseIntInRange( + req.query['maxResults'], + 1, + MAX_GLOB_MAX_RESULTS, + ); + if (maxResults === null) { + applyReadHeaders(res); + res.status(400).json({ + errorKind: 'parse_error', + error: `\`maxResults\` must be a positive integer in [1, ${MAX_GLOB_MAX_RESULTS}]`, + status: 400, + }); + return; + } + const cwdRaw = req.query['cwd']; + const cwdString = + typeof cwdRaw === 'string' && cwdRaw.length > 0 ? cwdRaw : undefined; + const fs = factory.forRequest({ + originatorClientId: clientId ?? undefined, + route: ROUTE, + }); + const start = performance.now(); + try { + const cwdResolved = cwdString + ? await fs.resolve(cwdString, 'glob') + : undefined; + const matches = await fs.glob(pattern, { + cwd: cwdResolved, + includeIgnored, + maxResults: maxResults ?? DEFAULT_GLOB_MAX_RESULTS, + }); + const boundWorkspace = (req.app.locals as { boundWorkspace?: string }) + .boundWorkspace; + const relMatches = matches.map((m) => + boundWorkspace + ? path.relative(boundWorkspace, m as string) + : (m as string), + ); + applyReadHeaders(res); + res.status(200).json({ + kind: 'glob', + pattern, + cwd: cwdString ?? '', + matches: relMatches, + count: relMatches.length, + truncated: relMatches.length === (maxResults ?? DEFAULT_GLOB_MAX_RESULTS), + durationMs: Math.round(performance.now() - start), + }); + } catch (err) { + sendFsError(res, err, ROUTE); + } +} + +/** + * Compute the workspace-relative form of a `ResolvedPath` for the + * response payload. Falls back to the absolute path when + * `boundWorkspace` is not on `app.locals` (older test embeds that + * skipped the fixture); production always sets it via + * `createServeApp`. + */ +function workspaceRelative(req: Request, resolved: string): string { + const boundWorkspace = (req.app.locals as { boundWorkspace?: string }) + .boundWorkspace; + if (!boundWorkspace) return resolved; + const rel = path.relative(boundWorkspace, resolved); + return rel === '' ? '.' : rel; +} + +export function registerWorkspaceFileReadRoutes( + app: Application, + deps: RegisterDeps, +): void { + app.get('/file', (req, res) => handleGetFile(req, res, deps)); + app.get('/stat', (req, res) => handleGetStat(req, res, deps)); + app.get('/list', (req, res) => handleGetList(req, res, deps)); + app.get('/glob', (req, res) => handleGetGlob(req, res, deps)); +} diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 4f82c4ea69f..d5d0bf3d220 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -107,6 +107,9 @@ const EXPECTED_STAGE1_FEATURES = [ // `budgets[]` on `/workspace/mcp`, `disabledReason: 'budget'` on // refused per-server cells). 'mcp_guardrails', + // Issue #4175 PR 19. Always-on. Daemon exposes the read-only file + // surface: `GET /file`, `GET /list`, `GET /glob`, `GET /stat`. + 'workspace_file_read', ] as const; // Issue #4175 PR 15. `require_auth` is registered but conditionally diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 75006de252e..c49953144d6 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -43,6 +43,7 @@ import { createWorkspaceFileSystemFactory, type WorkspaceFileSystemFactory, } from './fs/index.js'; +import { registerWorkspaceFileReadRoutes } from './routes/workspaceFileRead.js'; /** * Build a no-op fs-audit emitter that logs a warning every @@ -237,6 +238,11 @@ export function createServeApp( // a generic record; we cast to keep a precise property name. (app.locals as { fsFactory?: WorkspaceFileSystemFactory }).fsFactory = fsFactory; + // Surface the bound workspace on `app.locals` so the PR 19 read + // routes can compute workspace-relative response paths without + // re-resolving. Same canonical form `/capabilities` advertises + // and the bridge enforces — keeping every layer in agreement. + (app.locals as { boundWorkspace?: string }).boundWorkspace = boundWorkspace; // Order matters: rejection guards (CORS / Host allowlist / bearer auth) // run BEFORE the JSON body parser. Otherwise an unauthenticated POST @@ -401,6 +407,17 @@ export function createServeApp( } }); + // Issue #4175 PR 19 — read-only workspace file routes + // (`GET /file|/list|/glob|/stat`). Registered after the workspace + // diagnostics routes so the file surface sits next to its sibling + // workspace-scoped reads and shares the same auth posture (no + // `mutate()` gate; only the global `bearerAuth` middleware + // applies). Mutation file routes (`POST /file/write|/edit`) come + // in PR 20. + registerWorkspaceFileReadRoutes(app, { + parseClientId: parseClientIdHeader, + }); + app.post('/session', mutate(), async (req, res) => { const body = safeBody(req); // #3803 §02: 1 daemon = 1 workspace. Three input shapes: From f0709ae1125f7274b309d339bb779be3bccda712 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 18 May 2026 13:57:09 +0800 Subject: [PATCH 3/6] feat(serve): runQwenServe injects fsFactory + emit pipeline Closes PR #4250 follow-up #2. runQwenServe now constructs a WorkspaceFileSystem factory from the bound workspace, threads its emit hook through to the read routes, and exposes the trust snapshot via deps.trustedWorkspace. Test additions pin the wiring contract: - audit events emitted on success / denial flow back through the test-supplied fsAuditEmit hook - deps.fsFactory override is honored (built-in default does not silently shadow injection) - trust snapshot defaults to true (operator-chosen workspace) - trust=false routes through to the boundary and trips untrusted_workspace on write intents Default emit stays a stderr warning so a wiring regression that drops events remains visible. PR 21's SSE fan-out will replace the default with a workspace-scoped event channel. --- packages/cli/src/serve/runQwenServe.ts | 67 +++++++++ packages/cli/src/serve/server.test.ts | 190 ++++++++++++++++++++++++- 2 files changed, 256 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/serve/runQwenServe.ts b/packages/cli/src/serve/runQwenServe.ts index 8015158b535..05096ffb021 100644 --- a/packages/cli/src/serve/runQwenServe.ts +++ b/packages/cli/src/serve/runQwenServe.ts @@ -8,6 +8,7 @@ import * as fs from 'node:fs'; import { type Server } from 'node:http'; import * as path from 'node:path'; import { writeStderrLine, writeStdoutLine } from '../utils/stdioHelpers.js'; +import type { BridgeEvent } from './eventBus.js'; import { canonicalizeWorkspace, createHttpAcpBridge, @@ -16,6 +17,10 @@ import { import { isLoopbackBind } from './loopbackBinds.js'; import { createServeApp } from './server.js'; import type { ServeOptions } from './types.js'; +import { + createWorkspaceFileSystemFactory, + type WorkspaceFileSystemFactory, +} from './fs/index.js'; const QWEN_SERVER_TOKEN_ENV = 'QWEN_SERVER_TOKEN'; const SHUTDOWN_FORCE_CLOSE_MS = 5_000; @@ -52,6 +57,38 @@ export interface RunHandle { export interface RunQwenServeDeps { /** Bridge instance; tests inject a fake. Defaults to a fresh real one. */ bridge?: HttpAcpBridge; + /** + * Workspace filesystem factory (#4175 PR 19). When omitted, + * `runQwenServe` constructs one using `boundWorkspace`, + * `trustedWorkspace`, and a default warning-emit hook. Tests + * inject a real factory + custom emit to capture audit events, + * or override `trustedWorkspace` to flip the trust snapshot + * without re-routing through the OS-level trustedFolders config + * file. + */ + fsFactory?: WorkspaceFileSystemFactory; + /** + * Trust snapshot for the bound workspace at boot. Drives the + * `WorkspaceFileSystem`'s `assertTrustedForIntent` gate — read + * intents always pass; mutating intents (`write`, `edit`) throw + * `untrusted_workspace` when this is false. Defaults to true: + * the daemon binds at boot to a workspace the operator + * explicitly chose, and the trust dialog flow that ungates write + * permissions in the interactive CLI is not yet replicated for + * the daemon. Tests pin this to false to assert the gate is + * actually wired through `runQwenServe → createServeApp → + * fsFactory`. + */ + trustedWorkspace?: boolean; + /** + * Audit-emit hook for `fs.access` / `fs.denied`. Defaults to a + * stderr warning every 100 events so a regression that drops + * audit emission stays visible in the operator log. PR 21's SSE + * fan-out will replace the default with a workspace-scoped event + * channel; for now tests inject a recording array to assert the + * audit pipeline. + */ + fsAuditEmit?: (event: BridgeEvent) => void; } /** @@ -249,9 +286,39 @@ export async function runQwenServe( // syscall — idempotent but unnecessary I/O at boot). Direct // callers of createServeApp (tests / embeds) omit it and the // server canonicalizes itself. + // + // PR 19 — wire up `fsFactory` so the new read routes + // (`GET /file|/list|/glob|/stat`) consume a per-request boundary + // built against THIS daemon's bound workspace. Trust snapshot + // defaults to true; tests / future hardening flows pass an + // explicit `trustedWorkspace` to flip the gate. The audit-emit + // hook plugs into PR 21's SSE fan-out once that lands; until then + // the warning-emit fallback in `createServeApp` makes any silent + // drop visible in operator logs. + const trustedWorkspace = deps.trustedWorkspace ?? true; + const fsFactory = + deps.fsFactory ?? + createWorkspaceFileSystemFactory({ + boundWorkspace, + trusted: trustedWorkspace, + emit: + deps.fsAuditEmit ?? + ((event: BridgeEvent) => { + // Default emit: stderr warning so a wiring regression + // that orphans the audit channel is visible. Throttled + // by the inner factory in `createServeApp` when used as + // its built-in fallback, so this branch only fires when + // a caller explicitly opts into runQwenServe's emit + // wiring without supplying their own hook (rare). + writeStderrLine( + `qwen serve: fs audit emit pre-wire: type=${event.type}`, + ); + }), + }); const app = createServeApp(opts, () => actualPort, { bridge, boundWorkspace, + fsFactory, }); // Node's `app.listen()` wants the unbracketed IPv6 literal (`::1`) but diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index d5d0bf3d220..81f6357a9c3 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -4,7 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { realpathSync } from 'node:fs'; +import { realpathSync, promises as fsp } from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, it, expect, afterEach, vi } from 'vitest'; @@ -59,6 +60,7 @@ import type { ServeWorkspaceSkillsStatus, } from './status.js'; import { CAPABILITIES_SCHEMA_VERSION, type ServeOptions } from './types.js'; +import { FsError, type WorkspaceFileSystemFactory } from './fs/index.js'; const baseOpts: ServeOptions = { hostname: '127.0.0.1', @@ -3158,6 +3160,192 @@ describe('runQwenServe', () => { expect(bridge.shutdownCalls).toBe(1); }); + it('wires fsFactory + emit through to the read routes (#4175 PR 19 follow-up #2)', async () => { + // Pin the contract that `runQwenServe` constructs the workspace + // filesystem boundary, threads its emit hook through to + // `createServeApp`, and that boundary actually drives the new + // PR 19 read routes. A regression that drops the `fsFactory` + // injection (or that swaps in a different emit channel) shows + // up here as either a 500 response or a missing audit event. + const captured: BridgeEvent[] = []; + const bridge = fakeBridge(); + const wsRoot = await fsp.mkdtemp( + path.join(os.tmpdir(), 'qwen-runqwen-fs-'), + ); + await fsp.writeFile(path.join(wsRoot, 'a.txt'), 'hello'); + try { + handle = await runQwenServe( + { + hostname: '127.0.0.1', + port: 0, + mode: 'http-bridge', + workspace: wsRoot, + }, + { bridge, fsAuditEmit: (e) => captured.push(e) }, + ); + const port = (handle.server.address() as { port: number }).port; + const ok = await fetch(`http://127.0.0.1:${port}/file?path=a.txt`); + expect(ok.status).toBe(200); + expect( + captured.find( + (e) => + e.type === 'fs.access' && + (e.data as { intent?: string }).intent === 'read', + ), + ).toBeDefined(); + + const bad = await fetch(`http://127.0.0.1:${port}/file?path=../escape`); + expect(bad.status).toBe(400); + const denied = captured.find( + (e) => + e.type === 'fs.denied' && + (e.data as { errorKind?: string }).errorKind === + 'path_outside_workspace', + ); + expect(denied).toBeDefined(); + } finally { + await fsp.rm(wsRoot, { recursive: true, force: true }); + } + }); + + it('honors deps.fsFactory override (#4175 PR 19 follow-up #2)', async () => { + // The injection point exists so embedded callers (other tools + // wrapping the daemon, future runtime locality contracts) can + // swap in a remote-fronting factory. This test asserts + // `runQwenServe` does NOT silently shadow a caller-supplied + // factory with its built-in default. A regression that + // reverses the priority order would produce a 200 (built-in + // factory reads `process.cwd()/package.json`); the sentinel + // ensures we see 400 instead. + const sentinelMessage = 'sentinel-from-fake-factory'; + const fsFactory: WorkspaceFileSystemFactory = { + forRequest: () => ({ + resolve: async () => { + throw new FsError('parse_error', sentinelMessage); + }, + readText: async () => { + throw new Error('unreachable'); + }, + readBytes: async () => { + throw new Error('unreachable'); + }, + list: async () => { + throw new Error('unreachable'); + }, + glob: async () => { + throw new Error('unreachable'); + }, + stat: async () => { + throw new Error('unreachable'); + }, + writeText: async () => { + throw new Error('unreachable'); + }, + edit: async () => { + throw new Error('unreachable'); + }, + }), + }; + const bridge = fakeBridge(); + handle = await runQwenServe( + { + hostname: '127.0.0.1', + port: 0, + mode: 'http-bridge', + workspace: process.cwd(), + }, + { bridge, fsFactory }, + ); + const port = (handle.server.address() as { port: number }).port; + const res = await fetch(`http://127.0.0.1:${port}/file?path=a.txt`); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string; errorKind: string }; + expect(body.errorKind).toBe('parse_error'); + expect(body.error).toContain(sentinelMessage); + }); + + it('trust snapshot defaults to true (operator-chosen workspace)', async () => { + // The default trust value drives PR 20 write-route behavior + // even though PR 19 only exercises read intents. Pin the + // default here so a future contributor flipping it has to + // rewrite this test, surfacing the security-relevant change + // for review. + const bridge = fakeBridge(); + const wsRoot = await fsp.mkdtemp( + path.join(os.tmpdir(), 'qwen-runqwen-trust-'), + ); + try { + const captured: BridgeEvent[] = []; + handle = await runQwenServe( + { + hostname: '127.0.0.1', + port: 0, + mode: 'http-bridge', + workspace: wsRoot, + }, + { bridge, fsAuditEmit: (e) => captured.push(e) }, + ); + // Drive a read so the factory's `assertTrustedForIntent` + // gate fires. Read intents pass under both trusted and + // untrusted; the test signal is the absence of any + // `untrusted_workspace` denial event in the captured stream. + await fsp.writeFile(path.join(wsRoot, 'b.txt'), 'b'); + const port = (handle.server.address() as { port: number }).port; + const res = await fetch(`http://127.0.0.1:${port}/file?path=b.txt`); + expect(res.status).toBe(200); + expect( + captured.find( + (e) => + (e.data as { errorKind?: string }).errorKind === + 'untrusted_workspace', + ), + ).toBeUndefined(); + } finally { + await fsp.rm(wsRoot, { recursive: true, force: true }); + } + }); + + it('trust snapshot=false flows through deps.trustedWorkspace into the boundary (#4175 PR 19 follow-up #2)', async () => { + // PR 19 has no write routes, so the trust gate's effect on + // mutating intents can't be observed via HTTP. Instead, we + // construct the same factory that runQwenServe would build, + // with the same `trusted` value runQwenServe would pass, and + // assert the gate trips. The contract is: when + // `deps.trustedWorkspace = false`, the factory's + // `assertTrustedForIntent` rejects writes with + // `untrusted_workspace` — exactly what PR 20 will rely on. + const wsRoot = await fsp.mkdtemp( + path.join(os.tmpdir(), 'qwen-runqwen-untrust-'), + ); + try { + // Mirror runQwenServe's construction. If `runQwenServe` + // changes the call shape (different deps order, different + // fields), this test will start failing to type-check — + // which is the point: the failure is the audit trail. + const { createWorkspaceFileSystemFactory } = await import( + './fs/index.js' + ); + const factory = createWorkspaceFileSystemFactory({ + boundWorkspace: wsRoot, + trusted: false, + emit: () => undefined, + }); + const fsApi = factory.forRequest({ route: 'TEST /op' }); + // Read still passes — read intents are always trusted. + await fsp.writeFile(path.join(wsRoot, 'a.txt'), 'a'); + const r = await fsApi.resolve('a.txt', 'read'); + const out = await fsApi.readText(r); + expect(out.content).toBe('a'); + // Write throws untrusted_workspace. + const w = await fsApi.resolve('out.txt', 'write'); + await expect(fsApi.writeText(w, 'x')).rejects.toMatchObject({ + kind: 'untrusted_workspace', + }); + } finally { + await fsp.rm(wsRoot, { recursive: true, force: true }); + } + }); + it('handle.close() is idempotent — concurrent + repeat calls share one drain cycle', async () => { const bridge = fakeBridge(); handle = await runQwenServe( From d7efd9b0763b8188bb6e7cfdeaee8eaa49fdda04 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 18 May 2026 15:07:29 +0800 Subject: [PATCH 4/6] fixup(serve): address PR #4269 round-1 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes 8 findings from Copilot inline review + Codex review on PR #4269 (5 P0, 3 P1): P0 (correctness / privacy / operations) - runQwenServe.ts: throttle the default fsAuditEmit by reusing the exported `createDefaultFsAuditEmit` from server.ts. The earlier per-event `writeStderrLine` would print one line for every /file/list/glob/stat audit event under normal traffic. Now warns once + every 100th drop with payload context, so a wiring regression is still visible without flooding logs. (Copilot runQwenServe.ts:316; Codex runQwenServe.ts:305) - routes/workspaceFileRead.ts: probe glob with maxResults+1 and trim, so `truncated` reflects whether the boundary actually had more matches. Earlier `length === maxResults` heuristic false-positived when the workspace happened to hold exactly N matches. (Copilot workspaceFileRead.ts:399) - routes/workspaceFileRead.ts: glob `relMatches` now flows through the shared `workspaceRelative` helper. Root match (`pattern=.`) renders as "." rather than the empty string `path.relative` returns; helper also covers the boundWorkspace-undefined edge case so the route no longer carries its own fallback branch. (Copilot workspaceFileRead.ts:388; review summary HIGH-1) - fs/audit.ts: `pattern` field now rides on the same privacy gate as `relPath` / `message`. Glob patterns commonly carry workspace-relative or absolute path fragments (`src/secrets/*.env`, rejected `/Users/alice/ws/**`), so emitting them in privacy mode bypassed the same redaction the other path-bearing fields honor. Operators wanting full forensic context opt in via QWEN_AUDIT_RAW_PATHS=1. (Codex audit.ts:249) - routes/workspaceFileRead.ts: cwd resolves with intent='list' rather than 'glob'. The orchestrator's `recordAndWrap` auto-derives `data.pattern` from `intent === 'glob'`, which turned cwd-resolution failures into rows where the cwd string masqueraded as the glob pattern (`?cwd=../outside` → `pattern: ../outside` in audit). Switching to 'list' is the correct semantic shape (cwd is a directory we intend to walk) with identical trust + path-resolution behavior. (Codex workspaceFileSystem.ts:941) P1 (cosmetic / comment accuracy) - server.test.ts: `honors deps.fsFactory override` test comment rewritten to match the actual failure mode (a regression would 404 on a.txt, not 200 against package.json). (Copilot server.test.ts:3219) - routes/workspaceFileRead.ts: `limit` error message uses the MAX_LIST_ENTRIES constant instead of the literal 2000. (review summary MEDIUM) - fs/audit.ts: expanded the JSDoc explaining why the AuditPublisher request types Omit four fields and pass `pattern` through. (review summary MEDIUM) Test additions / adjustments - audit.test.ts: split the existing pattern tests into raw-paths and privacy-default cases; added two new privacy-mode assertions that strip pattern under default config. - workspaceFileSystem.test.ts: harness accepts `includeRawPaths`; glob audit suite runs with raw paths to observe `pattern`; new `glob audit privacy default` suite asserts pattern + relPath are stripped without the env opt-in. - workspaceFileRead.test.ts: new GET /glob cases for the truncated edge case (count == maxResults → false; count > maxResults → true) and root-match normalization. Not adopted (with rationale) - review summary HIGH-2 (glob pathHash uses boundWorkspace): this is the deliberate follow-up #4 contract from PR 18; pattern is the per-call signal, pathHash is the workspace marker. - review summary MEDIUM-1 (parseIntInRange three-state return): matches `parseMaxQueuedQuery` in server.ts; consistency wins. - review summary LOW-1/2/3 (capabilities comment length, CSP header, reverse truncated:false assertion): rationale already documented in code, CSP belongs in a hardening PR, the reverse assertion already exists. 518/518 serve tests pass; typecheck + eslint clean within src/serve/. --- packages/cli/src/serve/fs/audit.test.ts | 45 +++++++++++++++-- packages/cli/src/serve/fs/audit.ts | 40 +++++++++++++--- .../src/serve/fs/workspaceFileSystem.test.ts | 48 +++++++++++++++++-- .../serve/routes/workspaceFileRead.test.ts | 46 ++++++++++++++++++ .../cli/src/serve/routes/workspaceFileRead.ts | 42 +++++++++++----- packages/cli/src/serve/runQwenServe.ts | 28 +++++------ packages/cli/src/serve/server.test.ts | 10 ++-- packages/cli/src/serve/server.ts | 2 +- 8 files changed, 217 insertions(+), 44 deletions(-) diff --git a/packages/cli/src/serve/fs/audit.test.ts b/packages/cli/src/serve/fs/audit.test.ts index b1c3f75d254..8d8813ac3e2 100644 --- a/packages/cli/src/serve/fs/audit.test.ts +++ b/packages/cli/src/serve/fs/audit.test.ts @@ -149,8 +149,12 @@ describe('createAuditPublisher', () => { expect(events[0].data).not.toHaveProperty('hint'); }); - it('attaches pattern field for fs.access on glob intent', () => { - const { events, publisher, workspace } = setup(); + it('attaches pattern field for fs.access on glob intent in raw-paths mode', () => { + // `pattern` rides on the same privacy gate as `relPath` / + // `message` — glob patterns commonly carry path fragments + // (`src/secrets/*.env`, `/Users/alice/ws/**`), so they're + // suppressed unless the operator opted into raw paths. + const { events, publisher, workspace } = setup({ includeRawPaths: true }); publisher.recordAccess( { route: 'GET /glob' }, { @@ -169,8 +173,8 @@ describe('createAuditPublisher', () => { }); }); - it('attaches pattern field for fs.denied on glob intent', () => { - const { events, publisher } = setup(); + it('attaches pattern field for fs.denied on glob intent in raw-paths mode', () => { + const { events, publisher } = setup({ includeRawPaths: true }); publisher.recordDenied( { route: 'GET /glob' }, { @@ -188,6 +192,39 @@ describe('createAuditPublisher', () => { }); }); + it('strips pattern from fs.access in privacy mode (default)', () => { + // Default `includeRawPaths: false`. Even though the orchestrator + // passed a literal pattern, the publisher must not echo it — + // glob patterns can leak path content the operator opted out of + // logging. + const { events, publisher, workspace } = setup(); + publisher.recordAccess( + { route: 'GET /glob' }, + { + intent: 'glob', + absolute: workspace, + durationMs: 1, + pattern: 'src/secrets/*.env', + }, + ); + expect(events[0].data).not.toHaveProperty('pattern'); + expect(events[0].data).not.toHaveProperty('relPath'); + }); + + it('strips pattern from fs.denied in privacy mode (default)', () => { + const { events, publisher } = setup(); + publisher.recordDenied( + { route: 'GET /glob' }, + { + intent: 'glob', + input: '../../**', + errorKind: 'parse_error', + pattern: '../../**', + }, + ); + expect(events[0].data).not.toHaveProperty('pattern'); + }); + it('omits pattern when not provided', () => { const { events, publisher, workspace } = setup(); publisher.recordAccess( diff --git a/packages/cli/src/serve/fs/audit.ts b/packages/cli/src/serve/fs/audit.ts index a9ad3e7d2a2..b022fabcf5d 100644 --- a/packages/cli/src/serve/fs/audit.ts +++ b/packages/cli/src/serve/fs/audit.ts @@ -142,10 +142,22 @@ export interface AuditPublisher { } /** - * Per-payload `pattern` is part of the public schema but never - * usefully sourced from anywhere other than the orchestrator's - * own glob path — the request `Omit` keeps it surfaced so the - * orchestrator (and tests) can pass it without TS errors. + * Why the request types `Omit` four fields and pass `pattern` + * through: + * + * `recordAccess` / `recordDenied` callers describe the event in + * domain terms (intent, durationMs, errorKind, …); the publisher + * synthesizes the wire-shaped fields the schema needs — `kind` + * (the discriminator), `pathHash` (computed via SHA-256), `relPath` + * (env-gated), `route` (pulled off `AuditContext`). Hiding those + * four behind `Omit` prevents callers from fabricating values that + * don't match what the publisher will actually serialize. + * + * `pattern` is the one optional field that survives the Omit: + * only the orchestrator's glob path knows the literal pattern and + * tests need a typed way to forward it through. The publisher + * cannot synthesize it from anything else, so it stays on the + * request shape as an explicit passthrough. */ /** @@ -246,7 +258,16 @@ export function createAuditPublisher( if (record.sizeBytes !== undefined) payload.sizeBytes = record.sizeBytes; if (record.truncated) payload.truncated = true; if (record.matchedIgnore) payload.matchedIgnore = record.matchedIgnore; - if (record.pattern !== undefined) payload.pattern = record.pattern; + // `pattern` shares the same privacy gate as `relPath` and + // `message`. Glob patterns commonly embed workspace-relative + // or absolute path fragments (`src/secrets/*.env`, + // `/Users/alice/ws/**`), so emitting the literal pattern in + // privacy mode would bypass the same redaction the other + // path-bearing fields honor. Operators wanting full forensic + // context opt in via `QWEN_AUDIT_RAW_PATHS=1`. + if (record.pattern !== undefined && includeRawPaths) { + payload.pattern = record.pattern; + } if (includeRawPaths) { payload.relPath = relForAudit(absolute, boundWorkspace); } @@ -282,7 +303,14 @@ export function createAuditPublisher( if (record.message && includeRawPaths) { payload.message = record.message; } - if (record.pattern !== undefined) payload.pattern = record.pattern; + // Same privacy gate as the success-path `pattern` above + // (and as `relPath` / `message` here). Reject-pattern denials + // (`../**`, `/etc/**`) are themselves path content; emitting + // them in privacy mode would let the audit log echo exactly + // what the operator opted out of seeing. + if (record.pattern !== undefined && includeRawPaths) { + payload.pattern = record.pattern; + } if (includeRawPaths) { payload.relPath = relForAudit(record.input, boundWorkspace); } diff --git a/packages/cli/src/serve/fs/workspaceFileSystem.test.ts b/packages/cli/src/serve/fs/workspaceFileSystem.test.ts index 7bb938eb9b9..e0de5089a83 100644 --- a/packages/cli/src/serve/fs/workspaceFileSystem.test.ts +++ b/packages/cli/src/serve/fs/workspaceFileSystem.test.ts @@ -33,6 +33,7 @@ interface Harness { async function makeHarness(opts?: { trusted?: boolean; ignore?: Ignore; + includeRawPaths?: boolean; }): Promise { const scratch = await fsp.mkdtemp( path.join(os.tmpdir(), `qwen-wfs-${randomBytes(4).toString('hex')}-`), @@ -46,6 +47,7 @@ async function makeHarness(opts?: { trusted: opts?.trusted ?? true, emit: (e) => events.push(e), ignore: opts?.ignore, + includeRawPaths: opts?.includeRawPaths, }); const fs = factory.forRequest({ originatorClientId: 'client-x', @@ -758,9 +760,12 @@ describe('WorkspaceFileSystem - audit always emits on body errors', () => { }); describe('WorkspaceFileSystem - glob escape audit', () => { + // `pattern` rides on the same privacy gate as `relPath` / + // `message`. Use `includeRawPaths: true` so the orchestrator's + // pattern wiring is observable in the test harness. let h: Harness; beforeEach(async () => { - h = await makeHarness(); + h = await makeHarness({ includeRawPaths: true }); }); afterEach(async () => teardown(h)); @@ -789,7 +794,7 @@ describe('WorkspaceFileSystem - glob escape audit', () => { expect((denied!.data as { pattern?: string }).pattern).toBe('*.ts'); }); - it('records fs.access with workspace-hashed pathHash and pattern field on glob success', async () => { + it('records fs.access with workspace-hashed pathHash and pattern field on glob success (raw-paths mode)', async () => { await fsp.writeFile(path.join(h.workspace, 'one.ts'), 'a'); await h.fs.glob('*.ts'); const access = h.events.find( @@ -810,7 +815,7 @@ describe('WorkspaceFileSystem - glob escape audit', () => { expect(data.pathHash).toBe(expectedHash); }); - it('emits fs.denied with pattern when glob pattern is rejected as parse_error', async () => { + it('emits fs.denied with pattern when glob pattern is rejected as parse_error (raw-paths mode)', async () => { await expect(h.fs.glob('../../**')).rejects.toThrow(/'..' segments/); const denied = h.events.find( (e) => @@ -823,6 +828,43 @@ describe('WorkspaceFileSystem - glob escape audit', () => { }); }); +describe('WorkspaceFileSystem - glob audit privacy default', () => { + // Default factory has `includeRawPaths: false`. The orchestrator + // still passes the pattern, but the audit publisher must strip it + // — same gate as `relPath` / `message`. Locks the privacy regression + // surfaced by the round-1 review on PR #4269. + let h: Harness; + beforeEach(async () => { + h = await makeHarness(); + }); + afterEach(async () => teardown(h)); + + it('strips pattern from fs.access in privacy default', async () => { + await fsp.writeFile(path.join(h.workspace, 'one.ts'), 'a'); + await h.fs.glob('*.ts'); + const access = h.events.find( + (e) => + e.type === FS_ACCESS_EVENT_TYPE && + (e.data as { intent: string }).intent === 'glob', + ); + expect(access).toBeDefined(); + expect(access!.data).not.toHaveProperty('pattern'); + expect(access!.data).not.toHaveProperty('relPath'); + }); + + it('strips pattern from fs.denied in privacy default', async () => { + await expect(h.fs.glob('../../**')).rejects.toThrow(); + const denied = h.events.find( + (e) => + e.type === FS_DENIED_EVENT_TYPE && + (e.data as { intent: string }).intent === 'glob' && + (e.data as { errorKind: string }).errorKind === 'parse_error', + ); + expect(denied).toBeDefined(); + expect(denied!.data).not.toHaveProperty('pattern'); + }); +}); + describe('WorkspaceFileSystem - factory', () => { it('canonicalizes the workspace once at factory build', async () => { const scratch = await fsp.mkdtemp( diff --git a/packages/cli/src/serve/routes/workspaceFileRead.test.ts b/packages/cli/src/serve/routes/workspaceFileRead.test.ts index 4b86cda6aee..2fa22943ec8 100644 --- a/packages/cli/src/serve/routes/workspaceFileRead.test.ts +++ b/packages/cli/src/serve/routes/workspaceFileRead.test.ts @@ -308,6 +308,52 @@ describe('GET /glob', () => { expect(res.body.errorKind).toBe('parse_error'); }); + it('reports truncated=false when match count equals maxResults exactly', async () => { + // Probe-then-trim: pre-fixup the route inferred `truncated` from + // `length === maxResults`, false-positive when the workspace + // happens to hold exactly N matches. After the fixup the + // boundary probes `cap + 1` so `truncated` is only true when + // there really were more matches. + await fsp.writeFile(path.join(h.workspace, 'a.ts'), ''); + await fsp.writeFile(path.join(h.workspace, 'b.ts'), ''); + await fsp.writeFile(path.join(h.workspace, 'c.ts'), ''); + const res = await request(h.app) + .get('/glob?pattern=*.ts&maxResults=3') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.count).toBe(3); + expect(res.body.truncated).toBe(false); + }); + + it('reports truncated=true when boundary saw more matches than maxResults', async () => { + for (const name of ['a.ts', 'b.ts', 'c.ts', 'd.ts']) { + await fsp.writeFile(path.join(h.workspace, name), ''); + } + const res = await request(h.app) + .get('/glob?pattern=*.ts&maxResults=2') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.count).toBe(2); + expect(res.body.truncated).toBe(true); + }); + + it('normalizes a root match to "." instead of empty string', async () => { + // `pattern=.` resolves to the workspace itself; `path.relative` + // returns "" but the response shape expects "." (matches + // /file/list/stat). The route uses the shared + // `workspaceRelative` helper to normalize. + const res = await request(h.app) + .get('/glob?pattern=.') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + if (res.body.matches.length > 0) { + for (const m of res.body.matches) { + expect(m).not.toBe(''); + expect(typeof m).toBe('string'); + } + } + }); + it('returns 400 parse_error when maxResults is malformed', async () => { const res = await request(h.app) .get('/glob?pattern=*&maxResults=zero') diff --git a/packages/cli/src/serve/routes/workspaceFileRead.ts b/packages/cli/src/serve/routes/workspaceFileRead.ts index 91307cad347..7b50efaf2bd 100644 --- a/packages/cli/src/serve/routes/workspaceFileRead.ts +++ b/packages/cli/src/serve/routes/workspaceFileRead.ts @@ -224,7 +224,7 @@ async function handleGetFile( applyReadHeaders(res); res.status(400).json({ errorKind: 'parse_error', - error: '`limit` must be a positive integer in [1, 2000]', + error: `\`limit\` must be a positive integer in [1, ${MAX_LIST_ENTRIES}]`, status: 400, }); return; @@ -372,21 +372,39 @@ async function handleGetGlob( }); const start = performance.now(); try { + // Resolve `cwd` with `intent: 'list'` rather than `'glob'`. The + // request-level `pattern` query and the directory-level `cwd` + // are independent inputs; tagging the cwd resolve as a "glob + // intent" caused `recordAndWrap` (which auto-derives + // `data.pattern` from `intent === 'glob'`) to record the cwd + // string as the glob pattern on resolution failure — corrupting + // audit data for cases like `?cwd=../outside&pattern=*.ts`. + // `'list'` is the right semantic shape (cwd is a directory we + // intend to walk) and the trust + path-resolution behavior is + // identical (both are read-shaped intents under `policy.ts`). const cwdResolved = cwdString - ? await fs.resolve(cwdString, 'glob') + ? await fs.resolve(cwdString, 'list') : undefined; - const matches = await fs.glob(pattern, { + // Probe with `cap + 1` so `truncated` reflects whether the + // boundary actually had more results to give. Inferring + // truncation purely from `length === cap` false-positives when + // the workspace happens to contain exactly `cap` matches. + const cap = maxResults ?? DEFAULT_GLOB_MAX_RESULTS; + const probe = await fs.glob(pattern, { cwd: cwdResolved, includeIgnored, - maxResults: maxResults ?? DEFAULT_GLOB_MAX_RESULTS, + maxResults: cap + 1, }); - const boundWorkspace = (req.app.locals as { boundWorkspace?: string }) - .boundWorkspace; - const relMatches = matches.map((m) => - boundWorkspace - ? path.relative(boundWorkspace, m as string) - : (m as string), - ); + const truncated = probe.length > cap; + const trimmed = truncated ? probe.slice(0, cap) : probe; + // Use the shared `workspaceRelative` helper so a root match + // (e.g. `pattern=.` resolving to the workspace itself) renders + // as `'.'` rather than the empty string `path.relative` returns + // — keeps the response shape consistent with `/file`, `/list`, + // `/stat`. Helper also handles the `boundWorkspace` undefined + // edge case (test embeds that skip the fixture) so the route + // doesn't carry its own fallback branch. + const relMatches = trimmed.map((m) => workspaceRelative(req, m as string)); applyReadHeaders(res); res.status(200).json({ kind: 'glob', @@ -394,7 +412,7 @@ async function handleGetGlob( cwd: cwdString ?? '', matches: relMatches, count: relMatches.length, - truncated: relMatches.length === (maxResults ?? DEFAULT_GLOB_MAX_RESULTS), + truncated, durationMs: Math.round(performance.now() - start), }); } catch (err) { diff --git a/packages/cli/src/serve/runQwenServe.ts b/packages/cli/src/serve/runQwenServe.ts index 05096ffb021..097882f8740 100644 --- a/packages/cli/src/serve/runQwenServe.ts +++ b/packages/cli/src/serve/runQwenServe.ts @@ -15,7 +15,7 @@ import { type HttpAcpBridge, } from './httpAcpBridge.js'; import { isLoopbackBind } from './loopbackBinds.js'; -import { createServeApp } from './server.js'; +import { createDefaultFsAuditEmit, createServeApp } from './server.js'; import type { ServeOptions } from './types.js'; import { createWorkspaceFileSystemFactory, @@ -296,24 +296,24 @@ export async function runQwenServe( // the warning-emit fallback in `createServeApp` makes any silent // drop visible in operator logs. const trustedWorkspace = deps.trustedWorkspace ?? true; + // Reuse `createDefaultFsAuditEmit` so the throttle behavior here + // matches what `createServeApp`'s built-in fallback would emit. + // The earlier per-event `writeStderrLine` would print one line for + // every `/file` / `/list` / `/glob` / `/stat` audit event under + // normal traffic — a workspace scan can flood operator logs in + // seconds. The shared helper warns once + every 100th drop and + // includes payload context (errorKind / intent / pathHash), so a + // genuine wiring regression still surfaces but routine audit + // traffic stays quiet. Future PR 21 SSE fan-out replaces this + // default with the workspace-scoped event channel; until then the + // throttled stderr warning is the canonical "emit channel orphaned" + // breadcrumb. const fsFactory = deps.fsFactory ?? createWorkspaceFileSystemFactory({ boundWorkspace, trusted: trustedWorkspace, - emit: - deps.fsAuditEmit ?? - ((event: BridgeEvent) => { - // Default emit: stderr warning so a wiring regression - // that orphans the audit channel is visible. Throttled - // by the inner factory in `createServeApp` when used as - // its built-in fallback, so this branch only fires when - // a caller explicitly opts into runQwenServe's emit - // wiring without supplying their own hook (rare). - writeStderrLine( - `qwen serve: fs audit emit pre-wire: type=${event.type}`, - ); - }), + emit: deps.fsAuditEmit ?? createDefaultFsAuditEmit(), }); const app = createServeApp(opts, () => actualPort, { bridge, diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 81f6357a9c3..e1586ca3835 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -3213,10 +3213,12 @@ describe('runQwenServe', () => { // wrapping the daemon, future runtime locality contracts) can // swap in a remote-fronting factory. This test asserts // `runQwenServe` does NOT silently shadow a caller-supplied - // factory with its built-in default. A regression that - // reverses the priority order would produce a 200 (built-in - // factory reads `process.cwd()/package.json`); the sentinel - // ensures we see 400 instead. + // factory with its built-in default. A regression that ignored + // `deps.fsFactory` and fell back to the built-in factory would + // resolve `a.txt` against `process.cwd()`, find no such file, + // and return 404 `path_not_found`. The sentinel-throwing + // factory ensures we see 400 with `sentinel-from-fake-factory` + // in the body — proof the override actually drives the request. const sentinelMessage = 'sentinel-from-fake-factory'; const fsFactory: WorkspaceFileSystemFactory = { forRequest: () => ({ diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index c49953144d6..c089c106bc6 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -62,7 +62,7 @@ import { registerWorkspaceFileReadRoutes } from './routes/workspaceFileRead.js'; * per-session emit, so legitimate production traffic never hits * the warning. */ -function createDefaultFsAuditEmit(): (event: BridgeEvent) => void { +export function createDefaultFsAuditEmit(): (event: BridgeEvent) => void { const WARN_EVERY = 100; let droppedCount = 0; return (event: BridgeEvent) => { From 31c57c6468713f277d47fc8a956e78afb92f1f83 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 18 May 2026 15:51:45 +0800 Subject: [PATCH 5/6] fix(serve): address workspace file read review Co-authored-by: Qwen-Coder --- .../serve/routes/workspaceFileRead.test.ts | 77 +++++++++++++++++++ .../cli/src/serve/routes/workspaceFileRead.ts | 44 ++++++----- 2 files changed, 102 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/serve/routes/workspaceFileRead.test.ts b/packages/cli/src/serve/routes/workspaceFileRead.test.ts index 2fa22943ec8..71d72136dac 100644 --- a/packages/cli/src/serve/routes/workspaceFileRead.test.ts +++ b/packages/cli/src/serve/routes/workspaceFileRead.test.ts @@ -87,6 +87,24 @@ describe('GET /file', () => { expect(res.body.sizeBytes).toBe(12); }); + it('returns the requested line window', async () => { + await fsp.writeFile( + path.join(h.workspace, 'multiline.txt'), + 'one\ntwo\nthree\n', + ); + const res = await request(h.app) + .get('/file?path=multiline.txt&line=2&limit=1') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + kind: 'file', + path: 'multiline.txt', + content: 'two', + originalLineCount: 4, + truncated: true, + }); + }); + it('attaches Cache-Control: no-store and X-Content-Type-Options: nosniff', async () => { await fsp.writeFile(path.join(h.workspace, 'a.txt'), 'x'); const res = await request(h.app) @@ -145,6 +163,14 @@ describe('GET /file', () => { expect(res.body.errorKind).toBe('parse_error'); }); + it('rejects malformed line with parse_error 400', async () => { + const res = await request(h.app) + .get('/file?path=a.txt&line=abc') + .set('Host', loopbackHost()); + expect(res.status).toBe(400); + expect(res.body.errorKind).toBe('parse_error'); + }); + it('requires the path query param', async () => { const res = await request(h.app).get('/file').set('Host', loopbackHost()); expect(res.status).toBe(400); @@ -194,6 +220,25 @@ describe('GET /stat', () => { expect(res.body.type).toBe('directory'); }); + it('returns 404 path_not_found for missing entries', async () => { + const res = await request(h.app) + .get('/stat?path=nosuch.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(404); + expect(res.body.errorKind).toBe('path_not_found'); + }); + + it('returns 500 rather than leaking paths when boundWorkspace is missing', async () => { + await fsp.writeFile(path.join(h.workspace, 'a.txt'), 'hi'); + delete (h.app.locals as { boundWorkspace?: string }).boundWorkspace; + const res = await request(h.app) + .get('/stat?path=a.txt') + .set('Host', loopbackHost()); + expect(res.status).toBe(500); + expect(res.body.errorKind).toBe('internal_error'); + expect(JSON.stringify(res.body)).not.toContain(h.workspace); + }); + it('returns 400 symlink_escape when target points outside the workspace', async () => { const outside = path.join(h.scratch, 'evil.txt'); await fsp.writeFile(outside, 'x'); @@ -354,6 +399,38 @@ describe('GET /glob', () => { } }); + it('drops ignored glob matches by default and includes them with includeIgnored=1', async () => { + await teardown(h); + h = await makeHarness({ ignore: new Ignore().add(['secret.ts']) }); + await fsp.writeFile(path.join(h.workspace, 'public.ts'), ''); + await fsp.writeFile(path.join(h.workspace, 'secret.ts'), ''); + + const filtered = await request(h.app) + .get('/glob?pattern=*.ts') + .set('Host', loopbackHost()); + expect(filtered.status).toBe(200); + expect(filtered.body.matches).toContain('public.ts'); + expect(filtered.body.matches).not.toContain('secret.ts'); + + const all = await request(h.app) + .get('/glob?pattern=*.ts&includeIgnored=1') + .set('Host', loopbackHost()); + expect(all.status).toBe(200); + expect(all.body.matches.sort()).toEqual(['public.ts', 'secret.ts']); + }); + + it('scopes glob matches to cwd', async () => { + await fsp.mkdir(path.join(h.workspace, 'sub')); + await fsp.writeFile(path.join(h.workspace, 'root.ts'), ''); + await fsp.writeFile(path.join(h.workspace, 'sub', 'inside.ts'), ''); + const res = await request(h.app) + .get('/glob?pattern=*.ts&cwd=sub') + .set('Host', loopbackHost()); + expect(res.status).toBe(200); + expect(res.body.cwd).toBe('sub'); + expect(res.body.matches).toEqual(['sub/inside.ts']); + }); + it('returns 400 parse_error when maxResults is malformed', async () => { const res = await request(h.app) .get('/glob?pattern=*&maxResults=zero') diff --git a/packages/cli/src/serve/routes/workspaceFileRead.ts b/packages/cli/src/serve/routes/workspaceFileRead.ts index 7b50efaf2bd..5e5f4f6b6e4 100644 --- a/packages/cli/src/serve/routes/workspaceFileRead.ts +++ b/packages/cli/src/serve/routes/workspaceFileRead.ts @@ -29,6 +29,13 @@ import { */ export const MAX_LIST_ENTRIES = 2000; +/** + * Hard cap for `GET /file?limit=` line-window reads. Kept separate + * from `MAX_LIST_ENTRIES` so directory listing pagination changes do + * not accidentally alter file line slicing semantics. + */ +export const MAX_FILE_LINE_LIMIT = 2000; + /** * Default cap when the caller omits `?maxResults` on `GET /glob`. * Mirrors the orchestrator's default behavior (no cap) clipped to a @@ -86,7 +93,7 @@ export function sendFsError(res: Response, err: unknown, route: string): void { } writeStderrLine( `qwen serve: ${route} unexpected error: ${ - err instanceof Error ? (err.stack ?? err.message) : String(err) + err instanceof Error ? err.message : String(err) }`, ); res.status(500).json({ @@ -190,12 +197,12 @@ interface RegisterDeps { async function handleGetFile( req: Request, res: Response, - _deps: RegisterDeps, + deps: RegisterDeps, ): Promise { const ROUTE = 'GET /file'; const factory = getFsFactory(req, res); if (!factory) return; - const clientId = _deps.parseClientId(req, res); + const clientId = deps.parseClientId(req, res); if (clientId === null) return; const queryPath = requireStringQuery(res, req.query['path'], 'path', ROUTE); if (queryPath === null) return; @@ -219,12 +226,12 @@ async function handleGetFile( }); return; } - const limit = parseIntInRange(req.query['limit'], 1, MAX_LIST_ENTRIES); + const limit = parseIntInRange(req.query['limit'], 1, MAX_FILE_LINE_LIMIT); if (limit === null) { applyReadHeaders(res); res.status(400).json({ errorKind: 'parse_error', - error: `\`limit\` must be a positive integer in [1, ${MAX_LIST_ENTRIES}]`, + error: `\`limit\` must be a positive integer in [1, ${MAX_FILE_LINE_LIMIT}]`, status: 400, }); return; @@ -257,12 +264,12 @@ async function handleGetFile( async function handleGetStat( req: Request, res: Response, - _deps: RegisterDeps, + deps: RegisterDeps, ): Promise { const ROUTE = 'GET /stat'; const factory = getFsFactory(req, res); if (!factory) return; - const clientId = _deps.parseClientId(req, res); + const clientId = deps.parseClientId(req, res); if (clientId === null) return; const queryPath = requireStringQuery(res, req.query['path'], 'path', ROUTE); if (queryPath === null) return; @@ -289,12 +296,12 @@ async function handleGetStat( async function handleGetList( req: Request, res: Response, - _deps: RegisterDeps, + deps: RegisterDeps, ): Promise { const ROUTE = 'GET /list'; const factory = getFsFactory(req, res); if (!factory) return; - const clientId = _deps.parseClientId(req, res); + const clientId = deps.parseClientId(req, res); if (clientId === null) return; const queryPath = requireStringQuery(res, req.query['path'], 'path', ROUTE); if (queryPath === null) return; @@ -334,12 +341,12 @@ async function handleGetList( async function handleGetGlob( req: Request, res: Response, - _deps: RegisterDeps, + deps: RegisterDeps, ): Promise { const ROUTE = 'GET /glob'; const factory = getFsFactory(req, res); if (!factory) return; - const clientId = _deps.parseClientId(req, res); + const clientId = deps.parseClientId(req, res); if (clientId === null) return; const pattern = requireStringQuery( res, @@ -401,9 +408,7 @@ async function handleGetGlob( // (e.g. `pattern=.` resolving to the workspace itself) renders // as `'.'` rather than the empty string `path.relative` returns // — keeps the response shape consistent with `/file`, `/list`, - // `/stat`. Helper also handles the `boundWorkspace` undefined - // edge case (test embeds that skip the fixture) so the route - // doesn't carry its own fallback branch. + // `/stat`. const relMatches = trimmed.map((m) => workspaceRelative(req, m as string)); applyReadHeaders(res); res.status(200).json({ @@ -422,15 +427,16 @@ async function handleGetGlob( /** * Compute the workspace-relative form of a `ResolvedPath` for the - * response payload. Falls back to the absolute path when - * `boundWorkspace` is not on `app.locals` (older test embeds that - * skipped the fixture); production always sets it via - * `createServeApp`. + * response payload. Missing `boundWorkspace` means the app was + * misconfigured; never fall back to returning absolute filesystem + * paths to clients. */ function workspaceRelative(req: Request, resolved: string): string { const boundWorkspace = (req.app.locals as { boundWorkspace?: string }) .boundWorkspace; - if (!boundWorkspace) return resolved; + if (!boundWorkspace) { + throw new Error('bound workspace is not configured'); + } const rel = path.relative(boundWorkspace, resolved); return rel === '' ? '.' : rel; } From 370f2d4820507efc042f364d301868ae322815cc Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 18 May 2026 15:59:38 +0800 Subject: [PATCH 6/6] fix(serve): tighten workspace file read review followups Co-authored-by: Qwen-Coder --- packages/cli/src/serve/fs/audit.ts | 31 ++++++++----------- .../src/serve/fs/workspaceFileSystem.test.ts | 10 ++++++ .../cli/src/serve/fs/workspaceFileSystem.ts | 16 ++++++++-- .../serve/routes/workspaceFileRead.test.ts | 15 ++++----- .../cli/src/serve/routes/workspaceFileRead.ts | 30 +++++++++--------- 5 files changed, 59 insertions(+), 43 deletions(-) diff --git a/packages/cli/src/serve/fs/audit.ts b/packages/cli/src/serve/fs/audit.ts index b022fabcf5d..60404cb7397 100644 --- a/packages/cli/src/serve/fs/audit.ts +++ b/packages/cli/src/serve/fs/audit.ts @@ -141,24 +141,19 @@ export interface AuditPublisher { ): void; } -/** - * Why the request types `Omit` four fields and pass `pattern` - * through: - * - * `recordAccess` / `recordDenied` callers describe the event in - * domain terms (intent, durationMs, errorKind, …); the publisher - * synthesizes the wire-shaped fields the schema needs — `kind` - * (the discriminator), `pathHash` (computed via SHA-256), `relPath` - * (env-gated), `route` (pulled off `AuditContext`). Hiding those - * four behind `Omit` prevents callers from fabricating values that - * don't match what the publisher will actually serialize. - * - * `pattern` is the one optional field that survives the Omit: - * only the orchestrator's glob path knows the literal pattern and - * tests need a typed way to forward it through. The publisher - * cannot synthesize it from anything else, so it stays on the - * request shape as an explicit passthrough. - */ +// Why the request types `Omit` four fields and pass `pattern` +// through: +// +// `recordAccess` / `recordDenied` callers describe the event in +// domain terms (intent, durationMs, errorKind, ...); the publisher +// synthesizes the wire-shaped fields the schema needs: `kind`, +// `pathHash`, `relPath`, and `route`. Hiding those fields behind +// `Omit` prevents callers from fabricating values that do not match +// what the publisher serializes. +// +// `pattern` is the one optional field that survives the Omit: only +// the orchestrator's glob path knows the literal pattern, and the +// publisher cannot synthesize it from anything else. /** * SHA-256 over the canonical absolute path, truncated to 16 hex diff --git a/packages/cli/src/serve/fs/workspaceFileSystem.test.ts b/packages/cli/src/serve/fs/workspaceFileSystem.test.ts index e0de5089a83..36ce980e8fe 100644 --- a/packages/cli/src/serve/fs/workspaceFileSystem.test.ts +++ b/packages/cli/src/serve/fs/workspaceFileSystem.test.ts @@ -115,6 +115,7 @@ describe('WorkspaceFileSystem - readText', () => { const out = await h.fs.readText(r); expect(out.content).toBe('hello\nworld\n'); expect(out.meta.lineEnding).toBe('lf'); + expect(out.meta.sizeBytes).toBe(12); expect(out.meta.truncated).toBeUndefined(); }); @@ -253,6 +254,15 @@ describe('WorkspaceFileSystem - list', () => { const log = entries.find((e) => e.name === 'b.log'); expect(log?.ignored).toBe(true); }); + + it('stops collecting entries once maxEntries is reached', async () => { + const r = await h.fs.resolve('.', 'list'); + const entries = await h.fs.list(r, { + includeIgnored: true, + maxEntries: 2, + }); + expect(entries).toHaveLength(2); + }); }); describe('WorkspaceFileSystem - glob', () => { diff --git a/packages/cli/src/serve/fs/workspaceFileSystem.ts b/packages/cli/src/serve/fs/workspaceFileSystem.ts index 5bb9ff2441c..c58e1d8c236 100644 --- a/packages/cli/src/serve/fs/workspaceFileSystem.ts +++ b/packages/cli/src/serve/fs/workspaceFileSystem.ts @@ -71,6 +71,7 @@ export interface ReadMeta { encoding?: string; bom?: boolean; lineEnding: 'crlf' | 'lf'; + sizeBytes?: number; truncated?: boolean; matchedIgnore?: 'file' | 'directory'; originalLineCount?: number; @@ -94,6 +95,8 @@ export interface ReadTextOptions { export interface ListOptions { /** When true, ignored entries are returned with `ignored: true` rather than dropped. */ includeIgnored?: boolean; + /** Stop after this many returned entries have been collected. */ + maxEntries?: number; } export interface GlobOptions { @@ -359,6 +362,7 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { encoding: result._meta?.encoding, bom: result._meta?.bom, lineEnding: (result._meta?.lineEnding ?? 'lf') as 'crlf' | 'lf', + sizeBytes: st.size, originalLineCount: result._meta?.originalLineCount, }; let truncatedContent = result.content; @@ -471,9 +475,9 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { const start = performance.now(); try { assertTrustedForIntent(this.deps.trusted, 'list'); - const dirents = await fsp.readdir(p as string, { withFileTypes: true }); const entries: FsEntry[] = []; - for (const d of dirents) { + const dir = await fsp.opendir(p as string); + for await (const d of dir) { // `path.join(p, d.name)` is a shallow extension of an // already-canonical workspace path. Symlinked dirents are // tagged as `kind: 'symlink'` rather than auto-followed — @@ -490,12 +494,20 @@ class WorkspaceFileSystemImpl implements WorkspaceFileSystem { ); if (verdict.ignored && !opts.includeIgnored) continue; entries.push({ name: d.name, kind, ignored: verdict.ignored }); + if ( + opts.maxEntries !== undefined && + entries.length >= opts.maxEntries + ) { + break; + } } this.deps.audit.recordAccess(this.deps.ctx, { intent: 'list', absolute: p, durationMs: performance.now() - start, sizeBytes: entries.length, + truncated: + opts.maxEntries !== undefined && entries.length >= opts.maxEntries, }); return entries; } catch (err) { diff --git a/packages/cli/src/serve/routes/workspaceFileRead.test.ts b/packages/cli/src/serve/routes/workspaceFileRead.test.ts index 71d72136dac..f7122e6a52c 100644 --- a/packages/cli/src/serve/routes/workspaceFileRead.test.ts +++ b/packages/cli/src/serve/routes/workspaceFileRead.test.ts @@ -85,6 +85,7 @@ describe('GET /file', () => { matchedIgnore: null, }); expect(res.body.sizeBytes).toBe(12); + expect(res.body.returnedBytes).toBe(12); }); it('returns the requested line window', async () => { @@ -135,8 +136,7 @@ describe('GET /file', () => { }); it('returns 422 binary_file when target contains NULs', async () => { - const buf = Buffer.alloc(64); - buf[5] = 0; + const buf = Buffer.from([0x47, 0x49, 0x46, 0x38, 0x39, 0x00, 0x61, 0x01]); await fsp.writeFile(path.join(h.workspace, 'bin.dat'), buf); const res = await request(h.app) .get('/file?path=bin.dat') @@ -152,6 +152,8 @@ describe('GET /file', () => { .set('Host', loopbackHost()); expect(res.status).toBe(200); expect(res.body.truncated).toBe(true); + expect(res.body.sizeBytes).toBe(2048); + expect(res.body.returnedBytes).toBeLessThanOrEqual(512); expect(res.body.content.length).toBeLessThanOrEqual(512); }); @@ -391,11 +393,10 @@ describe('GET /glob', () => { .get('/glob?pattern=.') .set('Host', loopbackHost()); expect(res.status).toBe(200); - if (res.body.matches.length > 0) { - for (const m of res.body.matches) { - expect(m).not.toBe(''); - expect(typeof m).toBe('string'); - } + expect(res.body.matches).toContain('.'); + for (const m of res.body.matches) { + expect(m).not.toBe(''); + expect(typeof m).toBe('string'); } }); diff --git a/packages/cli/src/serve/routes/workspaceFileRead.ts b/packages/cli/src/serve/routes/workspaceFileRead.ts index 5e5f4f6b6e4..8a61b6cecb9 100644 --- a/packages/cli/src/serve/routes/workspaceFileRead.ts +++ b/packages/cli/src/serve/routes/workspaceFileRead.ts @@ -14,13 +14,12 @@ import { } from '../fs/index.js'; /** - * Hard cap on entries returned from `GET /list`. The directory walk - * runs in a single tick (`fsp.readdir`), so an unbounded listing of - * a `node_modules`-style directory pins the event loop and the - * resulting JSON payload (which the JSON.stringify pass already - * holds in memory) is too large to be useful. 2000 is generous for - * legitimate listings while staying well under the 10MB request - * limit when each entry serializes to ~80 bytes. + * Hard cap on entries returned from `GET /list`. The boundary probes + * with `MAX_LIST_ENTRIES + 1` and stops collecting once it knows the + * response is truncated, avoiding full materialization of very large + * directories. 2000 is generous for legitimate listings while staying + * well under the 10MB request limit when each entry serializes to ~80 + * bytes. * * Ties into the response's `truncated: true` flag so SDK consumers * can ask the daemon to paginate (PR 19 emits the flag; pagination @@ -243,6 +242,7 @@ async function handleGetFile( try { const resolved = await fs.resolve(queryPath, 'read'); const out = await fs.readText(resolved, { maxBytes, line, limit }); + const returnedBytes = Buffer.byteLength(out.content, 'utf-8'); applyReadHeaders(res); res.status(200).json({ kind: 'file', @@ -251,10 +251,11 @@ async function handleGetFile( encoding: out.meta.encoding ?? 'utf-8', bom: out.meta.bom === true, lineEnding: out.meta.lineEnding, - sizeBytes: Buffer.byteLength(out.content, 'utf-8'), + sizeBytes: out.meta.sizeBytes ?? returnedBytes, + returnedBytes, truncated: out.meta.truncated === true, matchedIgnore: out.meta.matchedIgnore ?? null, - originalLineCount: out.meta.originalLineCount, + originalLineCount: out.meta.originalLineCount ?? null, }); } catch (err) { sendFsError(res, err, ROUTE); @@ -312,16 +313,13 @@ async function handleGetList( }); try { const resolved = await fs.resolve(queryPath, 'list'); - const entries = await fs.list(resolved, { includeIgnored }); + const entries = await fs.list(resolved, { + includeIgnored, + maxEntries: MAX_LIST_ENTRIES + 1, + }); let truncated = false; let returned = entries; if (returned.length > MAX_LIST_ENTRIES) { - // Slice on the route side rather than passing a cap into - // `list()` so the boundary's audit row reports the FULL - // directory size (`sizeBytes` = entries.length) — operators - // see the directory's true cardinality, not just what the - // route handed back. PR 18's `list()` doesn't accept a cap - // anyway; pagination support lives inside the route. returned = returned.slice(0, MAX_LIST_ENTRIES); truncated = true; }