fix(security): credential writers follow existing symlinks for auth.json / JSON stores

[1837620622]

Description

Credential and state writers (FSUtil.writeJson, Filesystem.write used for auth/MCP/plugin stores) open the target path with normal write APIs. If auth.json (or similar) is replaced by a symlink, writes follow the link and can overwrite an attacker-chosen file (or plant secrets outside the intended store).

Expected

Refuse to write through an existing symlink for credential/state JSON writers; leave the link target untouched.

Actual

writeFile / writeFileString follow symlinks and write the target.

[cazcik]

If the goal is hardening auth.json there may be a better way to do it here; I investigated a narrower atomic-replacement approach related to this issue. Instead of relying on an lstat check before writing, FSUtil.writeJson can write complete JSON to an exclusive, same-directory temporary file with the requested mode and then rename it over the destination.

Something like:

packages/core/src/fs-util.ts

const writeJson = Effect.fn("FileSystem.writeJson")(function* (
  path: string,
  data: unknown,
  mode?: number,
) {
  const content = JSON.stringify(data, null, 2);
  const replacementMode =
    mode ??
    (yield* fs.stat(path).pipe(
      Effect.map((info) => info.mode),
      Effect.catch(() => Effect.succeed(undefined)),
    ));
  const temporary = join(dirname(path), `.${crypto.randomUUID()}.tmp`);
  yield* fs
    .writeFileString(temporary, content, { flag: "wx", mode: replacementMode })
    .pipe(
      Effect.andThen(
        replacementMode === undefined
          ? Effect.void
          : fs.chmod(temporary, replacementMode),
      ),
      Effect.andThen(fs.rename(temporary, path).pipe(Effect.uninterruptible)),
      Effect.ensuring(
        fs.remove(temporary, { force: true }).pipe(Effect.ignore),
      ),
    );
});

This would avoid following a final-component symlink or hard link during the write, ensure credential files are private from creation, and preserve the previous JSON if writing fails or is interrupted. It would not address cross-process read-modify-write races from #30396 or malformed-file recovery from #29628.

Would maintainers prefer this to replace the lstat guard proposed in #36360, build on top of it, or be tracked as a separate issue? If a separate issue is appropriate, I’m happy to open it and work on the patch once assigned.