fix(code): close remaining owner-only credential storage gaps #135

Activity

1. danny-avila

   added

   bugSomething isn't workingSomething isn't working

   on Sep 7, 2026
2. lia-by-librechat commented on Sep 17, 2026

   

   lia-by-librechatbot

   on Sep 17, 2026 – with Lia by LibreChat

   Contributor

   More actions

   Re-checked all three gaps against main at 672e19522900. Each one now has an implementation and tests, so this reads as fixed rather than open. Evidence per bullet, and one scoping question at the end.

   1. Windows and macOS ACL validation

   Mode-only checks can no longer accept a foreign-readable file, because unsupported platforms fail closed instead of falling back to mode bits. assertPrivateStorageSupported (packages/code/src/private-storage.ts:9) throws unless the platform is linux or darwin and process.getuid exists, and it guards the head of every storage entry point (packages/code/src/storage.ts:258, :359, :426, :471, :504, :537, :572).

   macOS extended ACLs are verified natively rather than inferred: assertPrivateStorageAcl calls verifyMacOsAcl(handle.fd, path, directory) through the Koffi binding (private-storage.ts:27), with removePrivateStorageAcl limited to application-owned entries. Directory ACLs are checked during the ancestor walk under an open handle, and a dev/ino comparison rejects a directory swapped mid-verification (private-storage.ts:85).

   2. Writable-ancestor replacement

   Path validation no longer stops at the immediate container. assertPrivateStorageAncestors (private-storage.ts:54) walks from / one component at a time, resolving symlinks as it goes under a 40-link bound, and rejects any entry owned by another account (uid neither self nor root) as well as any directory writable by others without the sticky bit. Both the parent and the target are walked (storage.ts:166, :170), and it returns every traversed entry so callers can enforce containment without resolving intermediate links away.

   The TOCTOU window that made ancestor checks weak is closed separately: assertIdentityPathIsPrivate claims the destination itself with open(path, 'wx', 0o600) and holds it across the pairing request, rather than probing a sibling and letting go (storage.ts:365, comment at :368).

   3. Bind-mounted destination failing after the code is spent

   This is the one I checked hardest, since the complaint is about ordering rather than detection. It now fails validation before redemption:

   • assertIdentityIsNotMountPoint (packages/code/src/identity-mount.ts:31) parses /proc/self/mountinfo, which covers same-device bind mounts, rejects a malformed table rather than guessing, bounds the read at 4 MiB, decodes octal-escaped mount points, and resolves the parent rather than the leaf so a leaf symlink is not mistaken for a mount. macOS goes through macOsMountPoint.
   • It is called from assertIdentityDestinationIsReplaceable (storage.ts:294, :304), which also rejects a directory destination and an unreplaceable foreign-owned file in a sticky directory.
   • That runs inside assertIdentityPathIsPrivate (storage.ts:357), and pair() calls it before pairBridgeWorker redeems the code, releasing the reservation if pairing then fails (packages/code/src/cli.ts:265).

   So the EBUSY-at-rename-after-redemption sequence in the report is no longer reachable through the CLI pairing path. assertSiblingPublishable additionally exercises a real sibling write, so a 0500 directory that passes every check on the file itself also fails before redemption instead of at save.

   Tests

   identity-mount.test.ts, private-storage.test.ts, storage-ancestors.test.ts, macos-storage.test.ts and storage.test.ts cover these paths, including cross-account ownership and preflight release. They are green in CI on #233, whose head is main plus workflow-only changes: Code Package Tests on Node 20.11.0, 22.21.0 and 24.16.0 all pass, as does the dedicated macOS Storage ACL Tests lane.

   Recommendation

   Close as fixed, unless one of these is the intent, in which case re-scoping the issue to it would be clearer than leaving all three bullets open:

   • Native Windows support. Windows currently fails closed by design, with the message pointing at WSL2 on a native Linux filesystem. That is a deliberate scope decision, so if the issue means "make Windows work", it is a feature rather than one of these gaps.
   • Linux POSIX ACLs beyond the mode mask. The Linux path relies on mode bits carrying the ACL mask, which holds for a mask entry but does not surface named entries that grant no permissions. If enumerating named entries on Linux is wanted, that is a distinct piece of work.

   I did not verify behaviour on a real bind mount or a real macOS host; the above is source and CI evidence, not a live reproduction of the original failure.