feat(serve): safe workspace file read routes (#4175 PR 19)

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

packages/cli/src/serve/fs/audit.test.ts

@@ -149,6 +149,95 @@ describe('createAuditPublisher', () => {
     expect(events[0].data).not.toHaveProperty('hint');
   });
 
+  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' },
+      {
+        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 in raw-paths mode', () => {
+    const { events, publisher } = setup({ includeRawPaths: true });
+    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('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(
+      { 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';

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,20 @@ 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`,
+// `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
  * chars. The truncation matches claude-code's privacy model: long
@@ -227,6 +253,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;
+      // `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);
       }
@@ -262,6 +298,14 @@ export function createAuditPublisher(
       if (record.message && includeRawPaths) {
         payload.message = record.message;
       }
+      // 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);
       }

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,
@@ -33,6 +33,7 @@ interface Harness {
 async function makeHarness(opts?: {
   trusted?: boolean;
   ignore?: Ignore;
+  includeRawPaths?: boolean;
 }): Promise<Harness> {
   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',
@@ -113,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();
   });
 
@@ -251,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', () => {
@@ -758,9 +770,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));
 
@@ -786,6 +801,77 @@ 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 (raw-paths mode)', 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 (raw-paths mode)', 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('../../**');
+  });
+});
+
+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');
   });
 });