security: refuse to write credentials through pre-existing symlinks

[garagon]

Summary

writeCredentialFile (packages/cli/src/utils/credential-output.ts, added in #67) is vulnerable to a TOCTOU exfiltration on shared filesystems. The previous flow was:

fs.access(resolved);                                  // follows symlinks
fs.writeFile(resolved, ..., { mode: 0o600 });         // follows symlinks;
                                                      // mode is only applied
                                                      // when the file is
                                                      // created, not when it
                                                      // already exists
fs.chmod(resolved, 0o600);                            // follows symlinks;
                                                      // chmod's the target

If the operator runs spend-request retrieve <id> --output-file <path> --force and an attacker on the same filesystem (CI runner, multi-tenant host, container with shared /tmp) pre-plants a symbolic link at <path> pointing at a file the attacker can already read, the attacker can:

1. Plant <path> as a symbolic link to /tmp/<readable>.
2. Open a read fd on /tmp/<readable> while it is still world-readable.
3. Operator runs the command — writeFile follows the symlink and writes the full card credential (PAN, CVC, billing address, valid_until) to the symlink target.
4. Operator's fs.chmod then locks the target down to 0o600 — too late, the attacker's fd was opened before the chmod and survives it (open fds keep their access mode regardless of subsequent permission changes).

Verified end-to-end by a Node reproduction. Pre-fix, the attacker fd reads the JSON-encoded credential immediately after the operator's writeCredentialFile call returns:

attacker planted symlink → target
attacker pre-opened fd while target was 0o644
victim running writeCredentialFile with --force ...
attacker fd read after victim wrote:
{
  "card_number": "4242-4242-4242-4242",
  "cvv": "123"
}
target file mode after victim chmod: 600     ← chmod ran but fd was already open
symlink still a symlink: true

Fix

Replace fs.access + fs.writeFile + fs.chmod with a single atomic open:

fs.open(resolved,
  constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY | constants.O_NOFOLLOW,
  0o600);

• The mode is set at create time, so the chmod follow-up step is unnecessary (and the symlink-following bug it introduced is gone).
• O_NOFOLLOW makes open fail with ELOOP if the final path component is a symbolic link.
• O_EXCL makes open fail with EEXIST if the file already exists.
• Force-mode unlinks any pre-existing entry first via fs.unlink (operating on the symlink itself rather than the target via fs.writeFile), then takes the same atomic-create path. A re-planted symlink that races into place between the unlink and the open causes open to fail-closed instead of writing through.

Post-fix verification:

attacker planted symlink → target
attacker pre-opened fd to target
--- attempt without force ---
refused: OUTPUT_FILE_EXISTS: /tmp/link-poc-symlink2
--- attempt with force=true (after force unlink, target should be untouched) ---
attacker fd read of target after victim wrote with --force:
  contents: BEFORE                                   ← target untouched
  ↑ if "BEFORE" — target untouched (fix works)
symlinkPath now:
  isSymlink: false                                   ← real file, not symlink
  isFile: true

Tests

packages/cli/src/utils/__tests__/credential-output.test.ts — four new cases:

• refuses to write through a symbolic link without force — symlink at the target path causes OUTPUT_FILE_EXISTS, and the link target is unchanged.
• refuses to write through a symbolic link with force — --force unlinks the symlink and atomically creates a fresh regular file at the path. The original link target is unchanged. The new file is a regular file (not a symlink) with mode 0o600.
• refuses to write through a symlink that races into place between unlink and create — sanity check on the --force race-shape. The fail-closed semantics are guaranteed by O_EXCL | O_NOFOLLOW.
• produces a 0o600 file even when force is set — regression guard for the removed fs.chmod step. Mode is now set at open() time via the third argument.

$ pnpm vitest run src/utils/__tests__/credential-output.test.ts
Test Files  1 passed (1)
     Tests  8 passed (8)

$ pnpm test
@stripe/link-sdk:test Test Files  10 passed (10)
@stripe/link-sdk:test      Tests  82 passed (82)
@stripe/link-cli:test  Test Files  11 passed (11)
@stripe/link-cli:test       Tests  117 passed (117)

Negative control: with the implementation reverted, two of the new tests fail because the symlink target is overwritten by the credential JSON (the existing it('overwrites existing file with force', …) test still passes — the previous behavior was technically "compatible" with that assertion).

Test plan

• pnpm vitest run src/utils/__tests__/credential-output.test.ts green (8/8)
• pnpm test green
• pnpm typecheck green
• Negative control: revert the implementation, the two symlink tests fail with the credential JSON appearing in the symlink target's contents
• End-to-end reproduction against the patched module: a pre-planted symlink + pre-opened attacker fd no longer expose the credential. Without --force, the call refuses up front. With --force, the symlink is replaced by a fresh regular file at the target path; the original symlink target is unmodified and the attacker fd reads the pre-existing contents.

Files

packages/cli/src/utils/credential-output.ts        | 58 +++++++++++++--
packages/cli/src/utils/__tests__/credential-output.test.ts | 85 ++++++++++++++++++++++
2 files changed, 135 insertions(+), 8 deletions(-)

[raubrey-stripe]

Thank you for this, some quick feedback + windows support!

[raubrey-stripe]

Could we cut down on the comment size here to make this more concise?

[garagon]

Addressed in cc1a135: comment now captures the operational why only. The full attack chain stays in the PR body and tests.

[raubrey-stripe]

It looks like O_NOFOLLOW is not supported on windows.

You can see in a similar vulnerability in a filelock python package here took a Windows specific patch in the "Patches" section.

If you're able we'd very much appreciate an investigation into the windows specific fix as well! But in the meantime, could you document this fix only applies to Posix? We can do a check for constants.O_NOFOLLOW !== 0 and make a decision about whether we support --auth-file as well here 🤔 cc @danhill-stripe

[garagon]

Addressed in cc1a135: flag is now constants.O_NOFOLLOW ?? 0 with an inline note on the Windows gap (O_EXCL still prevents overwriting a pre-existing entry, but no full no-follow semantics there). The proposed Windows fix and the plan to validate it on Windows CI before promising it are in the top-level comment.

[garagon]

Thanks for the review and the GHSA reference. Both points make sense.

In-file comment trimmed to the operational "why". Attack chain stays in the PR body and tests.

On Windows: agreed. The flag is now constants.O_NOFOLLOW ?? 0 with an inline note. On Windows the OR is a no-op, so O_EXCL still prevents overwriting a pre-existing entry but no longer provides full no-follow protection.

For the Windows fix itself, I'd rather not promise it without validating on Windows CI first. The pattern I have in mind is open(O_CREAT | O_EXCL) → lstat(path) → handle.stat() → fail-closed on mismatch, but the fd-vs-reparse-point semantics are subtle. If it holds up I'll add it here. If not, I'll keep this PR scoped to the POSIX fix + documentation and file a follow-up issue so you can decide whether to support --output-file on Windows.

[raubrey-stripe]

Thanks for the updates! Ack on the windows front. One more piece of feedback, let's focus on this first if (force) block. How are you changing the behavior there?