security: refuse to write credentials through pre-existing symlinks

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

@@ -47,4 +47,56 @@ 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.
+  // --force does not authorize symlink replacement: an operator who wants
+  // to overwrite a regular output file gets that, but a symlink at the
+  // path is rejected outright in either mode.
+
+  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_SYMLINK');
+    // The symlink target must not have been written.
+    expect(await fs.readFile(targetPath, 'utf-8')).toBe('{}');
+    // The symlink itself must still be in place.
+    expect((await fs.lstat(filePath)).isSymbolicLink()).toBe(true);
+  });
+
+  it('refuses to overwrite a symbolic link even 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);
+
+    // --force is intended to replace an existing regular output file. It
+    // does not authorize destroying a symlink the operator may not have
+    // intended to remove, nor writing credentials over its target.
+    await expect(
+      writeCredentialFile(filePath, { num: 1 }, true),
+    ).rejects.toThrow('OUTPUT_FILE_SYMLINK');
+    // The symlink target must not have been written.
+    expect(await fs.readFile(targetPath, 'utf-8')).toBe('{}');
+    // The symlink itself must still be in place.
+    expect((await fs.lstat(filePath)).isSymbolicLink()).toBe(true);
+  });
+
+  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, type Stats } from 'node:fs';
 import fs from 'node:fs/promises';
 import path from 'node:path';
 
@@ -8,20 +9,68 @@ export async function writeCredentialFile(
 ): Promise<string> {
   const resolved = path.resolve(filePath);
 
-  if (!force) {
-    try {
-      await fs.access(resolved);
+  // Atomically create the credential file so we never write through a
+  // pre-existing output path. lstat first to inspect what is there: a
+  // symlink at the final path component is rejected outright (even with
+  // --force), since --force is meant to replace an existing output file,
+  // not to authorize writing credentials over or through a symlink. A
+  // regular file is overwritten only when --force is set. The atomic
+  // create below uses O_EXCL | O_NOFOLLOW so the race window between the
+  // precheck and the create is fail-closed.
+  let existing: Stats | undefined;
+  try {
+    existing = await fs.lstat(resolved);
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err;
+  }
+
+  if (existing?.isSymbolicLink()) {
+    throw new Error(
+      `OUTPUT_FILE_SYMLINK: ${resolved} is a symbolic link. Refusing to write credentials through it.`,
+    );
+  }
+
+  if (existing) {
+    if (!force) {
       throw new Error(
         `OUTPUT_FILE_EXISTS: ${resolved} already exists. Use --force to overwrite.`,
       );
-    } catch (err) {
-      if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err;
     }
+    await fs.unlink(resolved);
   }
 
-  await fs.writeFile(resolved, JSON.stringify(data, null, 2), {
-    mode: 0o600,
-  });
-  await fs.chmod(resolved, 0o600);
+  // O_NOFOLLOW is POSIX-only. On Windows, Node exposes it as 0, so the
+  // bitwise OR is a no-op there: O_EXCL still prevents overwriting a
+  // pre-existing entry, but open() does not provide full no-follow
+  // semantics on Windows.
+  const noFollowFlag = constants.O_NOFOLLOW ?? 0;
+
+  let handle: fs.FileHandle | undefined;
+  try {
+    handle = await fs.open(
+      resolved,
+      constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY | noFollowFlag,
+      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.`,
+      );
+    }
+    if (code === 'ELOOP') {
+      throw new Error(
+        `OUTPUT_FILE_SYMLINK: ${resolved} is a symbolic link. Refusing to write credentials through it.`,
+      );
+    }
+    throw err;
+  }
+
+  try {
+    await handle.write(JSON.stringify(data, null, 2));
+  } finally {
+    await handle.close();
+  }
   return resolved;
 }