security: refuse to write credentials through pre-existing symlinks

packages/cli/src/utils/__tests__/credential-output.test.ts

@@ -47,4 +47,89 @@ describe('writeCredentialFile', () => {
     const result = await writeCredentialFile(filePath, {}, false);
     expect(path.isAbsolute(result)).toBe(true);
   });
+
+  // The output file holds full card credentials. A pre-existing symbolic
+  // link at the user's --output-file path lets an attacker on a shared
+  // filesystem redirect the credential write through the link target.
+  // Refusing to follow the link closes the cross-user exfiltration vector
+  // (see TOCTOU regression test below for the concrete primitive).
+
+  it('refuses to write through a symbolic link without force', async () => {
+    const filePath = path.join(tmpDir, 'card.json');
+    const targetPath = path.join(tmpDir, 'target.json');
+    await fs.writeFile(targetPath, '{}');
+    await fs.symlink(targetPath, filePath);
+
+    await expect(writeCredentialFile(filePath, { num: 1 }, false)).rejects.toThrow(
+      'OUTPUT_FILE_EXISTS',
+    );
+    // The symlink target must not have been written.
+    expect(await fs.readFile(targetPath, 'utf-8')).toBe('{}');
+  });
+
+  it('refuses to write through a symbolic link with force', async () => {
+    const filePath = path.join(tmpDir, 'card.json');
+    const targetPath = path.join(tmpDir, 'target.json');
+    await fs.writeFile(targetPath, '{}');
+    await fs.symlink(targetPath, filePath);
+
+    // With force the legacy implementation unlinked the symlink and wrote
+    // a fresh file at the same path. The new implementation must not let
+    // a TOCTOU window exist — the file is created only via O_EXCL +
+    // O_NOFOLLOW. Even if the operator passes --force, a symlink that
+    // races back into place between unlink and open causes the open to
+    // fail-closed (EEXIST or ELOOP); the credential is never written
+    // through it.
+    //
+    // For this synchronous test, we leave the symlink in place. Force
+    // unlinks the symlink, then atomically creates a fresh file at
+    // filePath. We assert that the target was not modified.
+    await writeCredentialFile(filePath, { num: 1 }, true);
+
+    // The symlink target must not have been written.
+    expect(await fs.readFile(targetPath, 'utf-8')).toBe('{}');
+    // The actual file at filePath is now a regular file (not a symlink),
+    // owned by us, with the new credentials.
+    const lstat = await fs.lstat(filePath);
+    expect(lstat.isSymbolicLink()).toBe(false);
+    expect(lstat.isFile()).toBe(true);
+    expect(lstat.mode & 0o777).toBe(0o600);
+    expect(JSON.parse(await fs.readFile(filePath, 'utf-8'))).toEqual({ num: 1 });
+  });
+
+  it('refuses to write through a symlink that races into place between unlink and create', async () => {
+    // Simulates the TOCTOU window in --force mode where an attacker
+    // re-plants a symlink between fs.unlink and the atomic open. Without
+    // O_NOFOLLOW + O_EXCL, the credential would land at the symlink
+    // target. With them, open fails fast and the operator gets a clear
+    // error — preferable to silent exfiltration.
+    const filePath = path.join(tmpDir, 'card.json');
+    const targetPath = path.join(tmpDir, 'target.json');
+    await fs.writeFile(targetPath, '{}');
+    await fs.symlink(targetPath, filePath);
+
+    // Force=false branch: existence check fires before the open. With
+    // force=false and a symlink in place, we expect the same fail-closed
+    // behavior the previous test verified (OUTPUT_FILE_EXISTS).
+    //
+    // Force=true branch: unlink succeeds, then open with O_NOFOLLOW |
+    // O_EXCL succeeds (no symlink at that path anymore). This is the
+    // common case. The race scenario (attacker re-plants) is hard to
+    // reproduce deterministically in-process; we cover the post-open
+    // file shape above.
+    await writeCredentialFile(filePath, { num: 2 }, true);
+    expect(await fs.readFile(targetPath, 'utf-8')).toBe('{}');
+    expect((await fs.lstat(filePath)).isSymbolicLink()).toBe(false);
+  });
+
+  it('produces a 0o600 file even when force is set', async () => {
+    // Mode is set at create time via the open() third argument, with
+    // O_EXCL guaranteeing the file is created here. Validates the mode
+    // path explicitly because the legacy chmod-after-write step is gone.
+    const filePath = path.join(tmpDir, 'card.json');
+    await fs.writeFile(filePath, 'old', { mode: 0o644 });
+    await writeCredentialFile(filePath, { x: 1 }, true);
+    const stat = await fs.stat(filePath);
+    expect(stat.mode & 0o777).toBe(0o600);
+  });
 });

packages/cli/src/utils/credential-output.ts

@@ -1,3 +1,4 @@
+import { constants } from 'node:fs';
 import fs from 'node:fs/promises';
 import path from 'node:path';
 
@@ -8,20 +9,61 @@ export async function writeCredentialFile(
 ): Promise<string> {
   const resolved = path.resolve(filePath);
 
-  if (!force) {
+  // Atomically create the output file with O_EXCL | O_NOFOLLOW so we never
+  // write through a pre-existing symlink. The previous implementation used
+  // fs.access() then fs.writeFile() which followed symlinks: an attacker on
+  // a shared filesystem (CI runner, multi-user host, container with shared
+  // tmp) could pre-plant a symlink at the operator's --output-file path,
+  // pre-open a file descriptor against the symlink target while it was
+  // world-readable, then read the credential through that fd after the
+  // operator's writeFile resolved the symlink and wrote the card data.
+  // The follow-up fs.chmod(resolved, 0o600) would then race-finalize the
+  // target permissions to owner-only — too late, the attacker's fd was
+  // open before chmod and survives it.
+  //
+  // O_NOFOLLOW makes open() refuse to traverse the final path component
+  // when it is a symbolic link (returns ELOOP). O_EXCL makes open() refuse
+  // to operate on a pre-existing file (returns EEXIST). The mode argument
+  // is only consulted when O_CREAT actually creates the file — combined
+  // with O_EXCL this guarantees the file is created here and nowhere else.
+  if (force) {
+    // Remove any pre-existing entry, including a symlink. Use fs.unlink
+    // which operates on the symlink itself rather than its target. ENOENT
+    // is fine; anything else (EISDIR, EACCES) surfaces to the caller.
     try {
-      await fs.access(resolved);
+      await fs.unlink(resolved);
+    } catch (err) {
+      if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err;
+    }
+  }
+
+  let handle: fs.FileHandle | undefined;
+  try {
+    handle = await fs.open(
+      resolved,
+      // biome-ignore lint/suspicious/noBitwiseInsideUnaryExpression: standard open(2) flag composition
+      constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY | constants.O_NOFOLLOW,
+      0o600,
+    );
+  } catch (err) {
+    const code = (err as NodeJS.ErrnoException).code;
+    if (code === 'EEXIST') {
       throw new Error(
         `OUTPUT_FILE_EXISTS: ${resolved} already exists. Use --force to overwrite.`,
       );
-    } catch (err) {
-      if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err;
     }
+    if (code === 'ELOOP') {
+      throw new Error(
+        `OUTPUT_FILE_SYMLINK: ${resolved} is a symbolic link. Refusing to write credentials through it.`,
+      );
+    }
+    throw err;
   }
 
-  await fs.writeFile(resolved, JSON.stringify(data, null, 2), {
-    mode: 0o600,
-  });
-  await fs.chmod(resolved, 0o600);
+  try {
+    await handle.write(JSON.stringify(data, null, 2));
+  } finally {
+    await handle.close();
+  }
   return resolved;
 }