fix(keychain): secure credentials fallback writes

[Kewe63]

Summary

Fixes #585.

The wallet-password .credentials fallback previously used fs.writeFileSync(path, ..., { mode: 0o600 }). On POSIX, the mode option applies only when creating a file, so an existing 0644 file remained broadly readable. The same path-based write also followed a final-component symlink and overwrote its target.

Changes

• use a POSIX file-descriptor write path with O_NOFOLLOW to reject credential symlinks
• use O_NONBLOCK and verify the opened descriptor is a regular file before modifying it
• enforce 0600 before truncating/writing an existing credential file and again after writing
• close the descriptor on every success or failure path
• preserve the existing Windows fallback, where POSIX mode and O_NOFOLLOW semantics are not portable
• add regression coverage for newly created permissions, existing 0644 files, symlink rejection, target preservation, and normal retrieval
• add a patch changeset

Security properties

The POSIX path validates the opened descriptor rather than doing a separate path check followed by a path-based write. This keeps the final-component symlink check and subsequent write attached to the same opened object.

The scope remains local: this prevents unsafe fallback-file state from preserving broad permissions or redirecting the credential write through a symlink.

Regression proof

With the new tests applied to the original implementation:

Test Files  1 failed (1)
Tests       2 failed | 23 passed (25)

The failures showed:

• existing mode remained decimal 420 (0644) instead of 384 (0600)
• the symlink write returned { stored: true, method: 'file' } instead of failing

With this fix:

Test Files  1 passed (1)
Tests       25 passed (25)

Full suite:

Test Files  66 passed (66)
Tests       2680 passed | 16 skipped (2696)
Duration    156.67s

Additional checks:

• npm run lint — passed
• git diff --check — passed
• npm pack --ignore-scripts --dry-run — passed; 85 package files, no test or audit artifacts included

All security regressions used a temporary HOME, fake passwords, mocked OS-keychain failure, and no external network access.

Compatibility and risk

• no public API, command, schema, or credential format changes
• normal fallback creation and retrieval remain covered
• unsupported POSIX credential path types now fail closed
• Windows keeps the prior file-write behavior because the POSIX descriptor guarantees are not portable there

Checklist

• Tests pass (npm test)
• src/schema.json updated if new commands or flags were added — no command or flag changes
• README.md updated if new top-level commands or categories were added — no command changes
• Changeset added (.changeset/tighten-credentials-permissions.md)

[nansen-pr-reviewer]

pr-reviewer Summary for #d61493f

✅ No issues found

The code review completed successfully with no findings.

Review effort: 3/5 (Moderate)

Summary

This PR hardens the POSIX fallback credential write path against two vulnerabilities: (1) permissions not being enforced when overwriting an existing 0644 file, and (2) a final-component symlink redirect. The fix is well-scoped and technically sound.

Overall assessment: The implementation is correct. The fd lifecycle is clean — writeFileSync does not close a file descriptor when one is passed, so the finally block's closeSync is the sole close point on every code path. The nested try/finally (no inner catch) correctly propagates errors to the outer catch { return false }, meaning all failure modes fail closed. The O_NONBLOCK flag is appropriate: it prevents blocking on FIFOs that have no reader (which would also fail the isFile() guard), and it is a no-op for regular files per POSIX. The fstatSync guard after openSync correctly rejects device files, directories, and FIFOs that O_NOFOLLOW alone wouldn't block. The double fchmodSync (before truncate and after write) is a defensible belt-and-suspenders approach given that the mode set at openSync time is only effective for newly created files. The Windows fallback is correctly preserved unchanged.

The regression tests are solid: they use a temporary HOME, mock the OS keychain to force the file path, verify the exact 0644→0600 transition, confirm the symlink target is untouched, and cover normal round-trip retrieval. The changeset is present and correctly typed as patch.

---

Token usage: 21 input, 4,954 output, 339,491 cache read, 35,369 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.