fix(core): keep no-follow reads protected where O_NOFOLLOW is missing

O_NOFOLLOW does not exist on Windows: fs.constants.O_NOFOLLOW is undefined, so the `(O_RDONLY | (O_NOFOLLOW ?? 0))` flag expressions silently collapse into a plain open that follows symlinks, dropping the symlink/TOCTOU hardening added for @-referenced file reads (#7206). Add a cross-platform open helper that uses the kernel flag where present and otherwise compensates with an lstat -> open -> fstat identity check, refusing symlinked paths, identity races, and zero-inode filesystems (fail-closed, matching #8290/#9857). Route the confirmed no-follow read call sites through it: validated @-file reads, session metadata reads, background-shell output tails, untracked diff line counts, the workspace registration store, and session-artifact workspace status. Co-authored-by: Qwen-Coder <[email protected]>
wenshao · Aug 31, 2026 · Aug 25, 2026 · Aug 25, 2026 · Aug 25, 2026 · Aug 25, 2026
commit c067b924cc02f2db15b27a8390dbfb6bc1724082
diff --git a/packages/acp-bridge/src/sessionArtifacts.ts b/packages/acp-bridge/src/sessionArtifacts.ts
@@ -5,7 +5,7 @@
  */
 
 import { createHash } from 'node:crypto';
-import { constants as fsConstants, promises as fs, type Stats } from 'node:fs';
+import { promises as fs, type Stats } from 'node:fs';
 import type { FileHandle } from 'node:fs/promises';
 import path from 'node:path';
 import {
@@ -17,6 +17,7 @@ import {
   MAX_DIRECTORY_ARTIFACT_DEPTH,
   MAX_DIRECTORY_ARTIFACT_FILES,
   metadataBudgetBytes,
+  openNoFollow,
   SESSION_ARTIFACT_PERSISTENCE_VERSION,
   pathHasSkippedDirectoryComponent,
   stableSessionArtifactId,
@@ -3108,10 +3109,10 @@ async function getWorkspaceStatus(
       return { status: 'missing', escaped: true };
     }
     const preOpenStat = await fs.lstat(realPath);
-    const handle = await fs.open(
-      realPath,
-      fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW,
-    );
+    // Where O_NOFOLLOW does not exist (Windows) the helper compensates
+    // with an lstat/open/fstat identity check instead of collapsing to a
+    // plain open that follows symlinks (#8227).
+    const handle = await openNoFollow(realPath);
     try {
       const stat = await handle.stat();
       if (!isSameFile(preOpenStat, stat)) {

diff --git a/packages/cli/src/serve/workspace-registration-store.test.ts b/packages/cli/src/serve/workspace-registration-store.test.ts
@@ -472,9 +472,16 @@ describe('WorkspaceRegistrationStore', () => {
         }),
       },
     }));
-    vi.doMock('@qwen-code/qwen-code-core', () => ({
-      atomicWriteFile: vi.fn().mockRejectedValue(writeError),
-    }));
+    // Keep the real barrel exports (the store now reads through
+    // `openNoFollow` from core) and only stub the write path under test.
+    vi.doMock('@qwen-code/qwen-code-core', async (importOriginal) => {
+      const actual =
+        await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
+      return {
+        ...actual,
+        atomicWriteFile: vi.fn().mockRejectedValue(writeError),
+      };
+    });
     try {
       const storeModule = await import('./workspace-registration-store.js');
       const store = new storeModule.WorkspaceRegistrationStore(

diff --git a/packages/cli/src/serve/workspace-registration-store.ts b/packages/cli/src/serve/workspace-registration-store.ts
@@ -5,11 +5,11 @@
  */
 
 import { createHash } from 'node:crypto';
-import { constants } from 'node:fs';
 import * as fs from 'node:fs/promises';
 import * as os from 'node:os';
 import * as path from 'node:path';
 import lockfile from 'proper-lockfile';
+import { openNoFollow } from '@qwen-code/qwen-code-core';
 import { MAX_WORKSPACE_PATH_LENGTH } from '@qwen-code/acp-bridge/workspacePaths';
 import { getGlobalQwenDirLite } from '../config/storage-paths-lite.js';
 import { MAX_REGISTERED_WORKSPACES } from './workspace-inputs.js';
@@ -355,10 +355,10 @@ export class WorkspaceRegistrationStore {
     }
     let file: Awaited<ReturnType<typeof fs.open>>;
     try {
-      file = await fs.open(
-        this.filePath,
-        (constants.O_RDONLY ?? 0) | (constants.O_NOFOLLOW ?? 0),
-      );
+      // Where O_NOFOLLOW does not exist (Windows) the helper compensates
+      // with an lstat/open/fstat identity check instead of collapsing to a
+      // plain open that follows symlinks (#8227).
+      file = await openNoFollow(this.filePath);
     } catch (err) {
       if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
         return emptySnapshot(this.primaryWorkspace);

@@ -151,6 +151,7 @@ export {
 export { atomicWriteFile } from './utils/atomicFileWrite.js';
 export { nextFireTime, parseCron } from './utils/cronParser.js';
 export { isWsl } from './utils/terminal-env.js';
+export { openNoFollow, openSyncNoFollow } from './utils/no-follow-open.js';
 export * from './services/session-organization-service.js';
 
 // Backward-compatible type re-exports for tool classes removed from eager loading.

@@ -559,6 +559,50 @@ describe('BackgroundShellRegistry', () => {
       expect(modelText).toContain('<output-tail error="unreadable"');
     });
 
+    const itNoSymlink = process.platform === 'win32' ? it.skip : it;
+
+    itNoSymlink(
+      'does not follow symlinked output files when O_NOFOLLOW is unavailable (Windows flag set)',
+      async () => {
+        // Cross-product the test above misses: Windows has no O_NOFOLLOW
+        // (the constant is `undefined` and `| (O_NOFOLLOW ?? 0)` collapses
+        // to a plain open), so stub the constant away and pin that the
+        // compensating check still refuses to read through the link (#8227).
+        const dir = makeTempDir();
+        const secretPath = join(dir, 'secret.txt');
+        const outputPath = join(dir, 'shell.output');
+        writeFileSync(secretPath, 'secret credentials');
+        symlinkSync(secretPath, outputPath);
+
+        vi.resetModules();
+        vi.doMock('node:fs', async (importOriginal) => {
+          const actual = await importOriginal<typeof import('node:fs')>();
+          return {
+            ...actual,
+            constants: { ...actual.constants, O_NOFOLLOW: undefined },
+          };
+        });
+
+        try {
+          const { BackgroundShellRegistry: RegistryWithoutNoFollow } =
+            await import('./backgroundShellRegistry.js');
+          const reg = new RegistryWithoutNoFollow();
+          const callback = vi.fn();
+          reg.setNotificationCallback(callback);
+          reg.register(makeEntry({ shellId: 'a', outputPath }));
+
+          reg.complete('a', 0, 2000);
+
+          const [, modelText] = callback.mock.calls[0];
+          expect(modelText).not.toContain('secret credentials');
+          expect(modelText).toContain('<output-tail error="unreadable"');
+        } finally {
+          vi.doUnmock('node:fs');
+          vi.resetModules();
+        }
+      },
+    );
+
     it('skips output-tail when the output file does not exist', () => {
       // Guards the catch branch in `readOutputTail`. If the try/catch
       // ever regresses to throwing, `complete()` would propagate the

@@ -23,6 +23,7 @@ import * as fs from 'node:fs';
 import type { TaskBase, TaskRegistration } from '../agents/tasks/types.js';
 import { atomicWriteFileSync } from '../utils/atomicFileWrite.js';
 import { createDebugLogger } from '../utils/debugLogger.js';
+import { openSyncNoFollow } from '../utils/no-follow-open.js';
 import { todoWorkChainContext } from '../utils/promptIdContext.js';
 import {
   isBidiControlChar,
@@ -61,7 +62,10 @@ type OutputTailResult =
 function readOutputTail(outputFile: string): OutputTailResult {
   let fd: number | undefined;
   try {
-    fd = fs.openSync(outputFile, getReadOutputOpenFlags());
+    // O_NOFOLLOW (or the compensating identity check where the flag does
+    // not exist, e.g. Windows) refuses a symlink planted over the output
+    // file, so the tail can never be read through it (#8227).
+    fd = openSyncNoFollow(outputFile);
     const stat = fs.fstatSync(fd);
     if (!stat.isFile() || stat.size <= 0) return undefined;
 
@@ -107,11 +111,6 @@ function readOutputTail(outputFile: string): OutputTailResult {
   }
 }
 
-function getReadOutputOpenFlags(): number {
-  const constants = fs.constants;
-  return (constants?.O_RDONLY ?? 0) | (constants?.O_NOFOLLOW ?? 0);
-}
-
 function truncateCommandForModel(command: string): {
   text: string;
   truncated: boolean;

@@ -23,6 +23,7 @@ import {
 } from '../utils/fileUtils.js';
 import { hasVerifiableInode } from '../utils/file-identity.js';
 import { getFolderStructure } from '../utils/getFolderStructure.js';
+import { openNoFollow } from '../utils/no-follow-open.js';
 
 /**
  * Options for reading multiple files.
@@ -295,10 +296,11 @@ async function readValidatedTextFileContent(
   signal: AbortSignal | undefined,
   displayPath: string,
 ): ReturnType<typeof readFileContent> {
-  const source = await fs.promises.open(
-    filePath,
-    (fs.constants.O_RDONLY ?? 0) | (fs.constants.O_NOFOLLOW ?? 0),
-  );
+  // Where O_NOFOLLOW does not exist (Windows) the helper compensates with
+  // an lstat/open/fstat identity check instead of collapsing to a plain
+  // open that follows symlinks (#8227); the validated-identity re-check
+  // below remains the second layer.
+  const source = await openNoFollow(filePath);
   try {
     const stats = await source.stat();
     if (!fileStatsMatchValidatedIdentity(stats, expected)) {
@@ -374,10 +376,9 @@ async function snapshotValidatedFile(
     | undefined;
   try {
     signal?.throwIfAborted();
-    const source = await fs.promises.open(
-      filePath,
-      (fs.constants.O_RDONLY ?? 0) | (fs.constants.O_NOFOLLOW ?? 0),
-    );
+    // See readValidatedTextFileContent: the helper keeps the no-follow
+    // guarantee on platforms without O_NOFOLLOW (#8227).
+    const source = await openNoFollow(filePath);
     try {
       const stats = await source.stat();
       if (!fileStatsMatchValidatedIdentity(stats, expected)) {

@@ -5,19 +5,12 @@
  */
 
 import { execFile } from 'node:child_process';
-// Namespace import (vs `import { constants }`) so vitest tests that
-// `vi.mock('node:fs', ...)` without supplying every named export don't
-// blow up in strict-mock mode just because they transitively load this
-// file via `@qwen-code/qwen-code-core`. The `constants?.X ?? 0` accesses
-// below absorb a missing `constants` field by falling through to plain
-// `O_RDONLY` (= 0 on POSIX) — harmless in mock environments where no
-// real `open()` ever runs.
-import * as nodeFs from 'node:fs';
-import { access, lstat, open, readFile, stat } from 'node:fs/promises';
+import { access, lstat, readFile, stat } from 'node:fs/promises';
 import * as path from 'node:path';
 import { promisify } from 'node:util';
 import type { Hunk } from 'diff';
 import { findGitRoot, readFirstLineNoFollow } from './gitUtils.js';
+import { openNoFollow } from './no-follow-open.js';
 
 /** Re-export so consumers don't need to depend on `diff` directly. */
 export type GitDiffHunk = Hunk;
@@ -90,25 +83,6 @@ const UNTRACKED_READ_CAP_BYTES = MAX_DIFF_SIZE_BYTES;
 const UNTRACKED_READ_CHUNK_BYTES = 64 * 1024;
 /** Scan the first N bytes for NUL to detect binary files (matches git's heuristic). */
 const BINARY_SNIFF_BYTES = 8 * 1024;
-/** Memoized open flags for line counting. `O_NOFOLLOW` closes the TOCTOU
- *  window between the `lstat` symlink check and `open` — if the path is
- *  replaced with a symlink in that gap, `open` rejects with `ELOOP` instead
- *  of silently dereferencing it. Falls back to plain `O_RDONLY` on platforms
- *  that don't expose the flag (Windows constants omit `O_NOFOLLOW`).
- *
- *  Computed lazily on first call (rather than at module load) so test files
- *  that `vi.mock('node:fs', ...)` without supplying `constants` can still
- *  load this module transitively via `@qwen-code/qwen-code-core` without
- *  vitest's strict-mock proxy throwing on the property access. Tests that
- *  do not actually exercise `countUntrackedLines` never trigger the lookup. */
-let untrackedOpenFlagsCache: number | undefined;
-function getUntrackedOpenFlags(): number {
-  if (untrackedOpenFlagsCache === undefined) {
-    untrackedOpenFlagsCache =
-      (nodeFs.constants?.O_RDONLY ?? 0) | (nodeFs.constants?.O_NOFOLLOW ?? 0);
-  }
-  return untrackedOpenFlagsCache;
-}
 
 /**
  * Fetch numstat-based git diff stats (files changed, lines added/removed) and
@@ -449,7 +423,10 @@ async function synthesizeUntrackedHunk(
   }
   let fh;
   try {
-    fh = await open(absPath, getUntrackedOpenFlags());
+    // O_NOFOLLOW closes the TOCTOU window between the lstat above and the
+    // open — where the flag does not exist (Windows) the helper compensates
+    // with an identity re-check (#8227).
+    fh = await openNoFollow(absPath);
   } catch {
     return null;
   }
@@ -968,7 +945,10 @@ async function countUntrackedLines(
   }
   let fh;
   try {
-    fh = await open(absPath, getUntrackedOpenFlags());
+    // O_NOFOLLOW closes the TOCTOU window between the lstat above and the
+    // open — where the flag does not exist (Windows) the helper compensates
+    // with an identity re-check (#8227).
+    fh = await openNoFollow(absPath);
   } catch {
     // ELOOP from O_NOFOLLOW (path raced into a symlink between lstat and
     // open) and any other open error all collapse to a binary row so the