Skip to content

Commit 81610a7

Browse files
authored
🏯 fix: Guard Worker Credentials From Local Accounts (LibreChat-AI#112)
Everything here defends a threat the merged owner-only fix does not: another account on the same host. It is separate deliberately, because BYOM's stated model is a worker on the user's own machine or VM with the sandboxed command as the adversary, and these checks buy that defence by trading deployment flexibility for it. Ownership. Mode bits do not establish trust: a 0600 file owned by another account is unreadable by others yet fully rewritable by its owner, who then controls the credential the worker loads - or, for a quarantine marker, can delete it and let mutations resume. The containing directory is judged the same way, since an owner lacking write bits today can grant them tomorrow. Root counts as the trust root. `--default-workspace` is application-owned by contract, so a pre-existing one under another account is refused too. Containers. A 0600 file in a directory others can write can be unlinked and replaced. Publishing goes through `rename`, which replaces the named entry, so the write path judges the entry's directory; reading follows the link, so both ends are judged. The sticky bit counts as protection, keeping /tmp-style parents usable. Only the immediate container is inspected. Pairing preflight. `pair` redeemed the one-time code before the destination was known usable, so an unusable path cost the code and left an orphaned remote pairing. The destination is now validated - rejecting a directory, a foreign-owned file under a sticky bit that `rename` could not replace, and a parent that denies the sibling temporary file the publish needs - and then claimed, so another account cannot take the name while the pairing request is in flight. The claim records its inode and is released only if the file is still that inode and still empty, so a concurrent pairing that published a real identity over the name is never destroyed by another invocation's unwind. Known gaps, left explicit rather than half-done: only the immediate container is checked, so a writable ancestor could still rename a private directory out from under the worker; a bind-mounted destination still fails at `rename` with EBUSY because every way to detect it either false-positives on btrfs subvolumes or races; and macOS extended ACLs and Windows ACLs are both outside what a mode check can see, which needs real ACL inspection on a host that can validate it.
1 parent 6a3bb9c commit 81610a7

3 files changed

Lines changed: 578 additions & 55 deletions

File tree

‎packages/code/src/cli.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { pairBridgeWorker } from './pairing.js';
88
import { startFileRelay } from './relay.js';
99
import { DockerFileRelaySupervisor } from './relay-runtime.js';
1010
import {
11+
assertIdentityPathIsPrivate,
1112
assertWorkspaceMutationQuarantineOwner,
1213
clearWorkspaceMutationQuarantine,
1314
defaultBridgeIdentityPath,
@@ -128,8 +129,14 @@ async function pair(args: string[]): Promise<void> {
128129
option(args, '--identity') ??
129130
process.env.LIBRECHAT_CODE_IDENTITY_FILE ??
130131
defaultBridgeIdentityPath(workerId);
131-
const identity = await pairBridgeWorker({ codeApiUrl, workerId, code });
132-
await saveBridgeIdentity(identityPath, identity);
132+
const reservation = await assertIdentityPathIsPrivate(identityPath);
133+
try {
134+
const identity = await pairBridgeWorker({ codeApiUrl, workerId, code });
135+
await saveBridgeIdentity(identityPath, identity);
136+
} catch (error) {
137+
await reservation.release();
138+
throw error;
139+
}
133140
process.stdout.write(
134141
`Paired worker ${workerId}. Identity saved to ${identityPath}\n`,
135142
);

0 commit comments

Comments
 (0)