Skip to content

fix(core): restrict daemon and plugin worker socket access to the owning user - #36370

Merged
AgentEnder merged 142 commits into
masterfrom
pie-leotard
Aug 7, 2026
Merged

fix(core): restrict daemon and plugin worker socket access to the owning user#36370
AgentEnder merged 142 commits into
masterfrom
pie-leotard

Conversation

@AgentEnder

@AgentEnder AgentEnder commented Jul 15, 2026

Copy link
Copy Markdown
Member

Current Behavior

The Nx daemon and isolated plugin workers communicate over unix domain sockets (named pipes on Windows).

On POSIX the socket directory and files are created under the ambient umask with no explicit mode, and plugin worker sockets sit directly in the shared system temp root. Socket paths are also unpredictable: each process derives its own hashed directory under os.tmpdir(), so there is no single path a sandbox or container policy can allowlist.

The native binding is copied into a file cache before being loaded. That cache lives in a hash-named directory under the system temp root, and the loader accepts an existing entry on byte size alone, so a rebuild that happens to produce an identical size is not picked up.

Separately, a daemon error is tagged as internal only when its log file can be read, so a daemon that fails before ever writing a log aborts the command instead of falling back to a daemonless graph build.

Expected Behavior

Layout on macOS and Linux

Every runtime artifact moves under one fixed root, with a per-user directory beneath it. <tmp> is os.tmpdir()/var/folders/… on macOS, /tmp on Linux:

Today After
<tmp>/<hash20>/d.sock /tmp/.nx/<uid>/sockets/<hash20>/d.sock
<tmp>/<hash20>/fp<pid>-<n>.sock /tmp/.nx/<uid>/sockets/<hash20>/fp<pid>-<n>.sock
<tmp>/plugin<pid>-<n>-<perfNow>.sock /tmp/.nx/<uid>/sockets/<hash8>/p<pid>-<n36>-<rand8>.sock
<tmp>/nx-native-file-cache-<hash7>/<file> /tmp/.nx/<uid>/native-cache/<nxVersion>/<file>

The root is a literal /tmp/.nx rather than os.tmpdir(), which honors $TMPDIR — per-user on macOS, rewritten by sandboxes, and stripped from the daemon's environment. A fixed path is identical on every machine, so a sandbox allowlist entry for it can be committed and shared with a team.

The uid sits directly beneath the container, so /tmp/.nx is the only level that can ever be shared between users, and sockets and native-cache are inside the per-user directory rather than beside it. That is what makes a single install -d -m 1777 -o root -g root /tmp/.nx sufficient provisioning: there is no /tmp/.nx/sockets for the first user to create and own.

When the container cannot be used, sockets move to ~/.nx/sockets before falling back into the workspace. Home needs no administrator — there is no shared level to own — so a peer holding /tmp/.nx no longer costs the short, fixed-length socket path.

The native cache deliberately gets no second tier. Sockets need somewhere to live or the daemon and plugin workers cannot run at all, which is what justifies chaining locations for them. The cache is an optimisation with a working fallback already in hand — load the binding in place from node_modules — so a second location would add a directory to guard and a .node to trust for no capability that is otherwise lost. Skipping it is also the fail-closed answer: the only thing worse than no cache is a cache another user can write to.

The home tier is skipped when HOME makes ~/.nx the shared container itself (HOME=/tmp); otherwise a tier-1 failure would point the owner-only guard at /tmp/.nx and take a root-owned 1777 directory to 0700.

Layout on Windows

Named pipes are not filesystem objects, so there is nothing to allowlist and no shared directory to separate users in, and %TMP% is already per-user. Neither reason for the shared root applies, so sockets stay directly under %TMP% and no socket path is longer than it is today:

Today After
%TMP%\<hash20>\d.sock %TMP%\<hash20>\d.sock (unchanged)
%TMP%\<hash20>\fp<pid>-<n>.sock %TMP%\<hash20>\fp<pid>-<n>.sock (unchanged)
%TMP%\plugin<pid>-<n>-<perfNow>.sock %TMP%\<hash8>\p<pid>-<n36>-<rand8>.sock (3 chars shorter)
%TMP%\nx-native-file-cache-<hash7>\<file> %TMP%\.nx\native-cache\<nxVersion>\<file>

There is no per-uid segment on Windows — %TMP% is already per-user, and the segment would only spend path budget. This matters because assertValidSocketPath rejects paths over 95 characters and has no platform guard, so it applies to named pipes too, and %TMP% already contains the username. Longest username that still fits, with the default %TMP%:

socket today after
daemon ≤39 ≤39
plugin ≤30 ≤33
forked / pty ≤29 ≤29

The native cache is not subject to that limit.

Shared roots

/tmp/.nx is 1777 sticky, like /tmp itself, so every user on a machine can create their own directory beneath it. It is the only shared level: /tmp/.nx/<uid> and everything under it is 0700, created and re-checked on every use.

Before creating anything beneath a shared root, Nx checks that it is a real directory, that it carries the sticky bit if anyone beyond its owner can write to it, and that it belongs to the current user or to root. The ownership half is not redundant: sticky restricts renaming an entry to that entry's owner and the directory's owner, so a root belonging to another regular user is unsafe at any mode. /tmp itself is safe for the same reason — root owns it.

Several users therefore share the container only when something trusted created it first — a container image, or an administrator running sudo install -d -m 1777 -o root -g root /tmp/.nx. That one command is the whole remedy, because no descendant is shared. Otherwise everyone after the first moves to ~/.nx, which needs no provisioning at all; the error names the chown that would restore sharing.

On a single-user machine, in a container, and in a sandbox, none of this applies: the first Nx run creates the container and owns it, so isSafeSharedRoot accepts it with no setup.

NX_SOCKET_DIR still overrides the socket location and is used as given — it names the socket directory itself. Pointing it at one of Nx's own roots throws InvalidSocketDirConfigured rather than silently substituting a default, with two distinct messages: the system temp dir and /tmp/.nx are reachable by other users, while the per-user roots are refused because Nx manages and cleans what lives beneath them. A directory nested under any of them is still accepted.

Directory handling

  • ensureOwnedPrivateDir accepts a directory only if it is a real directory (lstat, so a symlink does not qualify), owned by the current user, and carries no group or other permissions — read and search are enough to reach a socket inside it, so 0755 is tightened to 0700 rather than accepted. A directory that fails a check it cannot repair is not used.
  • Shared roots are created one level at a time, so a symlink at any level is caught rather than resolved through. The verdict comes from the resulting mode rather than from whether the chmod succeeded, since only a root's owner can chmod it.
  • Directory modes are changed through a descriptor opened O_NOFOLLOW | O_NONBLOCK and confirmed with fstat to be a directory, rather than by classifying errnos — the errno for a given condition is not stable across flag combinations or kernels, and O_NONBLOCK avoids a FIFO blocking the open.
  • The daemon socket file is set to 0600 after listen. On Linux, connecting requires write permission on the socket file; on macOS/BSD the (already 0700) directory is what applies.
  • Anything that cannot be established is skipped rather than approximated: sockets try ~/.nx/sockets and then a directory inside the workspace, and the native cache is bypassed so the binding loads in place from node_modules.

Message scoping

Every message carries the sender's workspaceRoot, stamped centrally by the transport rather than by each message constructor. A receiver scoped to a different workspace refuses it: the daemon returns the mismatch and stays alive for its own workspace, and a plugin worker drops the message. This is what catches two workspaces accidentally sharing an NX_SOCKET_DIR.

Daemon failure classification

A daemon failure is tagged as internal whether or not a log file exists yet, so a daemon that cannot start degrades to a daemonless graph build instead of aborting. Previously the tag was set only inside the block that read the log, so a first run — exactly when startup is most likely to fail — aborted the command.

A connect EPERM/EACCES is the one exception, and is tagged separately rather than as internal. It degrades the same way, but is deliberately not disabled until nx reset: a socket owned by someone else stops being there once it is removed or the machine reboots, so a sticky disable would outlive the cause and hide the fix from anyone who followed the advice. The message names the socket, both ways out, and keeps the errno on its first line — EACCES and EPERM need opposite remedies and it is the only token that tells them apart.

Scope

Socket paths, permissions, and the native binary cache only.

The AI-agent and sandbox work previously folded in here is split into #36586, stacked on this branch (it replaces #36463, which was closed): Codex/Superset agent detection, nx configure-ai-agents writing the sandbox allowances, the sandbox remediation hint, and the changes that re-enable the daemon and plugin isolation inside sandboxes. On this branch both still auto-disable under a detected sandbox, exactly as on master.

The Rust side is deliberately not covered, and NXC-4025 is therefore only partly met. packages/nx/src/native/utils/socket_path.rs derives the Nx Console socket independently — std::env::temp_dir() plus a workspace hash, created with create_dir_all at the ambient umask — and is reached from native/ide/nx_console/messaging.rs on every TUI run that talks to Nx Console. It also reads NX_SOCKET_DIR/NX_DAEMON_SOCKET_DIR and applies none of the refusal list added here, so a value this branch rejects is still used as-is by the Rust path.

That means the ticket's "all Nx sockets under one allowlistable root" is not yet true, and the sandbox allowlist this branch emits (/tmp/.nx or ~/.nx) does not cover the Nx Console socket. It is pre-existing code on a different runtime with its own path handling, so folding it in would widen this change materially; it is tracked as a follow-up instead.

Behavior changes worth calling out

  • Socket paths moved on macOS and Linux. Anything that located Nx sockets by scanning the system temp root must look under /tmp/.nx/<uid>/sockets, ~/.nx/sockets, or NX_SOCKET_DIR instead. On macOS this is a different filesystem, not just a different name: $TMPDIR is /var/folders/… while the new root is the literal /tmp. Windows socket directories are unchanged.
  • The native cache directory moved on both platforms, from <tmp>/nx-native-file-cache-<hash7> to /tmp/.nx/<uid>/native-cache/<nxVersion> (%TMP%\.nx\native-cache\<nxVersion> on Windows). Tooling that excluded or cleaned the old path by name needs updating.
  • Previously published NX_DAEMON_SOCKET_DIR guidance is reversed. The daemon docs told users to point it at a shared directory so several long-running containers could share one daemon socket. That is exactly what this change refuses, so the page now says the opposite: never share a socket directory, and give each container its own daemon. Anyone following the old documented setup will see InvalidSocketDirConfigured or a refused directory rather than a silent behaviour change — that is intended, but it is a documented workflow being withdrawn, not just a docs edit.
  • The 0700 mode is applied to directories you already own, not only ones Nx creates — in practice the workspace-local fallback (<workspaceRoot>/.nx/workspace-data/d, routinely 0755) and an explicitly configured NX_SOCKET_DIR. This ends the docker-compose pattern of sharing one socket directory across containers running as different users, and today it happens without a message.
  • A container owned by another regular user is no longer used. On a multi-user machine where someone else ran Nx first, sockets move to ~/.nx/sockets and the native binding loads in place. Pre-creating /tmp/.nx as root restores the shared layout for everyone.
  • NX_NATIVE_FILE_CACHE_DIRECTORY and NX_SOCKET_DIR now behave differently on a bad value. A socket directory Nx refuses throws InvalidSocketDirConfigured, because there is no safe substitute and silently relocating sockets resurfaces later as a path-length error about a directory the user never set. A native cache directory Nx refuses warns and loads the binding in place, because that fallback is complete. Both are loud; only one is fatal.
  • The native cache is keyed per uid and Nx version, with each file additionally keyed by a hash of the resolved binding path so multiple source checkouts (all reporting 0.0.1) do not collide. A cache entry older than its source is refreshed.

Related Issue(s)

Fixes NXC-4658
Fixes NXC-4025


View session information ↗

@netlify

netlify Bot commented Jul 15, 2026

Copy link
Copy Markdown

Deploy Preview for nx-dev ready!

Name Link
🔨 Latest commit 6fb74f7
🔍 Latest deploy log https://app.netlify.com/projects/nx-dev/deploys/6a76044083fc7b0008755f60
😎 Deploy Preview https://deploy-preview-36370--nx-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Jul 15, 2026

Copy link
Copy Markdown

Deploy Preview for nx-docs ready!

Name Link
🔨 Latest commit 6fb74f7
🔍 Latest deploy log https://app.netlify.com/projects/nx-docs/deploys/6a760440b95650000921823c
😎 Deploy Preview https://deploy-preview-36370--nx-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@nx-cloud

nx-cloud Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit 6fb74f7

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 50m 5s View ↗
nx run-many -t check-imports check-lock-files c... ✅ Succeeded 4s View ↗
nx-cloud record -- pnpm nx-cloud conformance:check ✅ Succeeded 55s View ↗
nx build workspace-plugin ✅ Succeeded <1s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 18s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 8s View ↗

☁️ Nx Cloud last updated this comment at 2026-08-07 17:08:16 UTC

nx-cloud[bot]

This comment was marked as outdated.

nx-cloud[bot]

This comment was marked as outdated.

@AgentEnder
AgentEnder marked this pull request as ready for review July 16, 2026 19:50
@AgentEnder
AgentEnder requested a review from a team as a code owner July 16, 2026 19:50
@AgentEnder
AgentEnder requested a review from leosvelperez July 16, 2026 19:50
nx-cloud[bot]

This comment was marked as outdated.

Comment on lines +51 to +56
The Nx Daemon uses a unix socket to communicate between the daemon and the Nx processes. By default this socket gets placed in a temp directory. If you are using Nx in a docker-compose environment, however, you may want to run the daemon manually
and control its location to enable sharing the daemon among your docker containers. To do so, set the `NX_SOCKET_DIR` environment variable to a shared directory.
and control its location to enable sharing the daemon among your docker containers. To do so, set the `NX_SOCKET_DIR` environment variable to a directory shared by those containers.

{% aside type="caution" title="Keep the socket directory private" %}
Do not point `NX_SOCKET_DIR` (or `NX_DAEMON_SOCKET_DIR`) at a directory that other users on the machine can access, such as a world-writable temp directory. The daemon drives work inside a long-lived process, so another user who can reach its socket could execute code in your daemon. Share the socket directory only between processes that belong to the same user and the same workspace; the daemon rejects messages whose workspace root does not match its own.
{% /aside %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the primary usecase of this variable to be a workaround for file permissions. Do not recommend the whole docker container sharing daemon thing. Still have a caution that points out that it should not be shared because it is a remote for code execution.

| `NX_SKIP_PROVENANCE_CHECK` | boolean | If set to `true`, skips `npm` provenance verification when installing packages during `nx migrate`. This is a security-sensitive check. Only disable it if you understand the implications. |
| `NX_SKIP_VSCODE_EXTENSION_INSTALL` | boolean | If set to `true`, skips the automatic installation of the Nx Console extension for supported editors. Set this in environments where the temp file that we store this information in otherwise isn't accessible. |
| `NX_SOCKET_DIR` | string | Directory for all Nx sockets (daemon, forked process, and plugin). Set this to a shorter path when the default temp directory produces a socket path that exceeds the OS limit. Takes precedence over `NX_DAEMON_SOCKET_DIR`. |
| `NX_SOCKET_DIR` | string | Directory for all Nx sockets (daemon, forked process, and plugin). Set this to a shorter path when the default temp directory produces a socket path that exceeds the OS limit. Do not set it to a location shared with other users or other workspaces: the daemon drives work inside a long-lived process, so a shared socket directory would let another user connect to your daemon. Takes precedence over `NX_DAEMON_SOCKET_DIR`. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `NX_SOCKET_DIR` | string | Directory for all Nx sockets (daemon, forked process, and plugin). Set this to a shorter path when the default temp directory produces a socket path that exceeds the OS limit. Do not set it to a location shared with other users or other workspaces: the daemon drives work inside a long-lived process, so a shared socket directory would let another user connect to your daemon. Takes precedence over `NX_DAEMON_SOCKET_DIR`. |
| `NX_SOCKET_DIR` | string | Directory for all Nx sockets (daemon, forked process, and plugin). Be sure to read [Nx Daemon documentation](blah) before using this. |

daemonWorkspaceRoot: string
): boolean {
return (
msg.workspaceRoot !== undefined &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return false when workspaceRoot is not defined.

): boolean {
return (
msg.workspaceRoot !== undefined &&
msg.workspaceRoot.toLowerCase() !== daemonWorkspaceRoot.toLowerCase()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't sanitize this. They should be identical.

Comment thread packages/nx/src/daemon/server/server.ts Outdated
// respond with an error but keep the daemon alive for its own workspace.
if (
isDaemonMessage(payload) &&
isForeignWorkspaceMessage(payload, workspaceRoot)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change this to a try catch { respondWithError } so that the error can utilize the information in the check.

Call it assertValidDaemonMessage

// Skipped on Windows, where `socketPath` is a named pipe.
if (!isWindows) {
try {
chmodSync(socketPath, 0o600);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider if we can pass these permissions in where we create the directory / create the socket

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed socket creation can't create files w/ given perms.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed not possible for either half, so this stayed as a post-listen chmod.

net.Server.listen() takes no mode option, and the socket is created by the bind inside it — there is no point between creation and the file existing where a mode could be supplied. umask is the only other lever and it is process-global, so narrowing it around the bind would race every other write in the process.

The directory is created with an explicit mode (mkdirSync(dir, { mode: 0o700 }), non-recursive so the mode is not subject to recursive:true's parent handling), which is the half that can be done at creation time. The 0600 on the socket file is defence-in-depth on top of that: Linux gates connect() on write permission to the socket file, macOS/BSD gate on the directory, which is already 0700. The comment at the call site now says exactly this.

Comment thread packages/nx/src/daemon/tmp-dir.ts Outdated
// cannot connect to the sockets inside it (which would let them execute
// code in the daemon or a plugin worker). We never touch the shared system
// temp dir, which other processes rely on.
if (dir !== tmpdir) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw when the directory is the tmpdir which we assume can be utilized by any user on the machine and thus unsafe. Then change this logic appropriately.

Comment thread packages/nx/src/daemon/tmp-dir.spec.ts Outdated
// the machine can access it, so it can never be locked down to us. We fail
// closed by falling back to the owner-controlled workspace data dir instead
// of trusting the shared root.
process.env.NX_SOCKET_DIR = tmpdir;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, no we should just throw if we detect this instead of fall back to the default value. Its invalid configuration.

Comment thread packages/nx/src/daemon/tmp-dir.ts Outdated
restrictToOwner(dir);
return dir;
} catch (e) {
// Fail closed: fall back to the owner-controlled workspace data dir rather

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should catch the resolve(dir) == resolve(tmpdir), or if we do we should rethrow... We can extract a InvalidSocketDirConfigured error type or similar for control flow.

@AgentEnder
AgentEnder requested a review from FrozenPandaz July 21, 2026 23:42
Comment thread packages/nx/src/daemon/tmp-dir.ts Outdated
export class InvalidSocketDirConfigured extends Error {
constructor(public readonly dir: string) {
super(
`The configured Nx socket directory ('${dir}') resolves to the shared system temp directory ('${tmpdir}'), which every user on the machine can access. Pointing the Nx socket directory there is unsafe: another local user could connect to the daemon or plugin worker sockets and execute code in them. Set NX_SOCKET_DIR (or NX_DAEMON_SOCKET_DIR) to a directory that only your user can access.`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`The configured Nx socket directory ('${dir}') resolves to the shared system temp directory ('${tmpdir}'), which every user on the machine can access. Pointing the Nx socket directory there is unsafe: another local user could connect to the daemon or plugin worker sockets and execute code in them. Set NX_SOCKET_DIR (or NX_DAEMON_SOCKET_DIR) to a directory that only your user can access.`
`The configured Nx socket directory ${dir} cannot be the shared system temp directory. Pointing the Nx socket directory there is unsafe as another local user could connect to the daemon or plugin worker sockets and execute code in them. Set NX_SOCKET_DIR to a directory that only your user can access.`

* processed. Both values are produced from the same workspace-root resolution,
* so they are compared directly.
*/
export function isForeignWorkspaceMessage(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do this for plugin workers as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes — this is shared with the plugin worker now, which is what assertValidDaemonMessage was extracted for.

plugin-worker.ts:103 calls it with its own receiverDescription, so the two receivers differ only in what they do with the throw: the daemon catches it and responds with the mismatch, the worker catches it and drops the message. WorkspaceScopedMessage is structural precisely so neither side has to import the other's message type.

Worth being explicit about the limit, since it reads stronger than it is: isForeignWorkspaceMessage treats an absent workspaceRoot as not foreign, so anyone who simply omits the field bypasses this. It is a detector for two workspaces accidentally sharing an NX_SOCKET_DIR, not a control against an attacker — the 0700 directory and 0600 socket are the control.

export type DaemonMessage = {
type: string;
env?: Record<string, string>;
workspaceRoot?: string;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this required?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left optional deliberately, for two reasons now recorded in the docblock above it.

The field is stamped centrally by DaemonSocketMessenger.sendMessage (and sendMessageOverSocket on the plugin-worker side) rather than by the ~30 sites that construct messages, so making it required would mean touching all of them to set a value the transport immediately overwrites.

It also has to tolerate absence on the wire: a client from a previous version sends no workspaceRoot, and isForeignWorkspaceMessage returning false for undefined is what keeps that working rather than having the daemon reject every message during a version skew.

The type-level fix that would give both — an outgoing type without the field and a stamped type where it is required — is worth doing, but it splits the message types across the daemon and plugin-worker protocols, so I would rather not land it inside this one.

nx-cloud[bot]

This comment was marked as outdated.

@AgentEnder
AgentEnder requested a review from FrozenPandaz July 22, 2026 19:03
// in another workspace (e.g. one that reached this worker's socket via a
// shared NX_SOCKET_DIR); do not process it. An undefined workspaceRoot is
// treated as "not foreign", consistent with the daemon.
if (isForeignWorkspaceMessage(message, hostWorkspaceRoot)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come we don't reuse assertValidDaemonMessage?

nx-cloud[bot]

This comment was marked as outdated.

nx-cloud[bot]

This comment was marked as outdated.

nx-cloud[bot]

This comment was marked as outdated.

@AgentEnder

Copy link
Copy Markdown
Member Author

Supersedes #36258 — the stable socket root work (sockets + native file cache under /tmp/.nx) was folded into this PR via a6b83b9, alongside the daemon/plugin socket access restrictions. Closing #36258 in favor of this combined PR.

nx-cloud[bot]

This comment was marked as outdated.

The one converted call site that kept a hand-written sentence. Three
structurally different refusals printed the same text, and for a `not-created`
refusal it was false — the directory was never created, and the reason was the
errno. The comment four lines above states the goal this defeated: "A typo,
EACCES or EROFS is otherwise indistinguishable from a working cache."

Throws the guard's own refusal instead, as tmp-dir.ts already does.
Destroying only the returned socket was not enough. A regression that connects
but never hands the socket back leaves the poll's attempts open, and
server.close() does not call back while any connection is open — so jest
printed the failure and never exited. Measured unbounded: killed at 300s and
again at 401s. Tracking accepted connections brings the same regression to a
1s failure.

net.Server has no closeAllConnections(); that is http.Server only.
Three corrections to the remedy split, all found by re-reviewing it.

`remedyFor` told the user to "Remove it if it is stale". They usually cannot.
This branch is reached only beneath a container isSafeSharedRoot accepted, and
that container is sticky whenever a peer could have written to it — sticky
restricts unlink to the entry's owner, the container's owner, or root. Staged
exactly as the KB page provisions (`install -d -m 1777 -o root -g root
/tmp/.nx`) with a peer-planted per-uid directory, rm, rmdir and mv all return
EPERM for the victim, and the warning then repeats verbatim on the next run.
NX_SOCKET_DIR now leads, since it is the lever that always works.

The `uid === 0` exemption sat in the shared gate and suppressed the per-user
advice too, so a root-owned ~/.nx — routine after one `sudo nx`, or a
root-provisioned image run as a non-root user — got nothing at all. It belongs
inside the shared branch: root owning the shared container is the goal, not a
problem. Only safe to move now that the per-user sentence no longer leads with
advice that directory's owner cannot follow.

The module preamble said four guards return GuardResult<T>. Three do
(isSafeSharedRoot, ensureSafeSharedRoot, ensureOwnedPrivateDir); the count
double-counted isOwnedRealDirectory, the exception the same sentence names. And
remedyFor's docstring claimed the per-user case returns undefined, which stopped
being true when that branch started returning a message.

Tests: the per-user remedy was pinned only negatively — replacing the whole
sentence with one that dropped both the path and the escape hatch left the suite
green. It now asserts both, plus the absence of "Remove it" and the uid-0 case.
shellQuote's inner-quote escaping gains a row. Three tmp-dir fixtures mocked
ensureSafeSharedRoot returning foreign-owner without `shared`, a shape the real
guard cannot produce — the same fidelity problem a comment two tests below warns
about.
The permission error told sandboxed users to "allow unix sockets under the Nx
socket root". The KB page this PR ships says that is not enough: a scoped
allowlist permits connecting but not creating, and a fresh daemon, plugin
workers and forked tasks need `allowAllUnixSockets: true`.

This branch is reached from startInBackground *after* it has spawned a daemon,
so it is exactly the case the KB page excludes. A user who followed the message
unblocked connect and still had a daemon that could not bind.

Links the page rather than restating it. Verified the URL resolves — the site
routes it under /docs, and the relative form the .mdoc pages use is not
meaningful in a terminal.
The advice named `remove`, which fails on every directory this branch reaches:
a foreign-owned directory Nx created is 0700, so we cannot empty it. What does
work is moving it aside, and whether that is ours to do depends on who owns the
parent — measured across the three reachable shapes:

  container root-owned 1777   rm EPERM   mv EPERM   (an administrator is needed)
  container ours 1777         rm EACCES  mv ok
  $HOME ours 0755             rm EACCES  mv ok

So the sentence now names the action that works and the condition that decides
who can take it, rather than routing everyone to an administrator.

The comments justifying the old sentence were wrong in the same way each time —
generalising from `/tmp/.nx/<uid>`, the one directory that motivated them, to a
branch that serves five. "reached only under a container isSafeSharedRoot
accepted" is true of one of the five; the home tier reaches it with no container
guard at all. "never us" is false because sticky permits the container's owner,
which ensureSafeSharedRoot's own mkdirSync(0o1777) routinely makes us. Both are
deleted rather than reworded: per CLAUDE.md a comment earns its place by stating
what the code cannot, and why a change is correct belongs here instead.

The uid-0 arm's comment claimed it handles a live case. isSafeSharedRoot accepts
a root-owned container rather than refusing it, so no guard emits `shared` with
uid 0 and the arm cannot fire. Kept as belt and braces, now labelled as such.

Tests: a title said "should offer no remedy" while asserting a remedy is
offered. The condition clause was unpinned — dropping it left the suite green.
And the guard against the old advice was over-specified to a phrase that appears
nowhere in the repo, so it passed against the exact wording it was meant to
forbid; `/remove it/i` catches it.
The message asserted that "a scoped allowlist only permits connecting" as a
property of sandboxes generally. That is Claude Code's `allowUnixSockets`
specifically — the setting is named nowhere else in the repo — and it is not
true of an AppArmor rule, a firejail whitelist, or a bind mount, which permit
bind and connect alike. It also dropped the vendor-neutral instruction the
previous wording carried, so a user under any other sandbox was told to set a
key they do not have and lost the advice that applied to them.

Restores the general instruction and scopes the Claude Code detail to it.

The comment above it claimed the branch is reached after startInBackground has
spawned a daemon. daemonPermissionException has two call sites, and the other
fires from setUpConnection on a probe that already found a daemon and on
reconnect — neither spawns anything. Deleted rather than corrected; scoping the
create-claim to "starting a daemon" makes the message true at both sites without
needing the explanation.

Adds the only test that reads the sandbox half — reverting it to wording without
`allowAllUnixSockets` previously left the suite green.
The runtime now tells users a directory may need an administrator to clear, but
the page's only administrator guidance was the one-line container provisioning,
followed by "That one command is the entire setup" — so the advice pointed at a
procedure the canonical page said did not exist.

Placed after the paragraph explaining what happens without the container, so the
"Without it" antecedent still reads.
Both the `remedyFor` docstring and the test guarding it led with an
unqualified "`rm` cannot help", scoped only by the clause after it. The
branch is entered on ownership alone — `ensureOwnedPrivateDir` returns
`foreign-owner` before the mode check — so a foreign 0755 directory Nx
did not create is removable, and is exactly the shape the neighbouring
root-provisioned test calls reachable.

The remedy itself is unchanged: it does not know the mode, so it still
cannot tell anyone to remove anything. Only the justification narrows to
what it can support.

Also names the HOME condition on the sudo comment, matching the KB page
corrected for the same reason, and replaces the KB's warning sentence.
`logger.warn` fires only on the workspace fallback; a demotion from
/tmp/.nx to ~/.nx is silent, and its verbose line names the skipped tier
root /tmp/.nx/<uid>/sockets rather than the refused /tmp/.nx/<uid>.
tmp-dir.spec.ts:374 pins that silence.
"A foreign directory Nx created is 0700" is contradicted by this same
file 270 lines down: mkdir's mode is advisory on mounts that ignore it
(WSL2 drvfs without metadata, CIFS with dir_mode, FAT), which is why the
mode is re-lstat'ed and re-locked on every run and why `not-tightenable`
exists. The branch never learns the mode in any case — the foreign-owner
deny precedes the mode check.

The refusal carries neither the parent's owner nor the mode, so that is
what the comment now says. Both conclusions rest on the shape of
DirRefusal rather than on a claim about filesystems.

The KB warning sentence had the same defect. `remedyFor` returns
undefined for every kind but foreign-owner and the warning takes the
first that survives, so it names one refused directory or none;
tmp-dir.spec.ts pins both shapes. `--verbose` is what reliably names
them, so the sentence points there.
`isSafeSharedRoot` does not simply accept root-owned containers: uid 0 is
exempt from the ownership deny, but execution falls through to the mode
clause, which refuses a root-owned world-writable container without the
sticky bit as `peer-writable-not-sticky`. The spec next door pins both
halves.

The conclusion holds for a different reason — the one site that sets
`shared` is that ownership deny, and it is gated on `stats.uid !== 0`.

Also drops the "every location" quantifier from the KB sentence. It is
true when no root could be established, but `createOwnerOnlySocketDir`
forwards only its own refusal, so a leaf I/O failure under an established
root names one and drops the rest.
FrozenPandaz and others added 6 commits August 5, 2026 16:32
The governing rule is who owns the refused directory's parent, which the
preceding sentence already states. Naming `/tmp/.nx` instead misses the
case where root created something inside a `/tmp/.nx/<uid>` you own.
A demotion from /tmp/.nx to ~/.nx logs at verbose only, and named
`tiers[0].root` — /tmp/.nx/<uid>/sockets. That is one level below the
directory that was actually refused, and a user cannot stat it when the
parent is the foreign-owned one, so the KB paragraph telling them to move
that directory aside gave them no way to find it. The refusals array was
already in hand at the call site and was being dropped.

The docs sentence changes with it: --verbose now names the directory it
skipped and why, so "reports only that the default root was skipped" no
longer describes what it prints.

Also fixes the sandbox test added last commit. Its assertions were
`allowAllUnixSockets: true` and the KB URL, both of which the pre-fix
string already contained, so reverting client.ts to its previous bytes
left the suite green — it pinned what survived the change rather than the
change. It now pins the vendor-neutral instruction and the Claude Code
scoping, and its title and comment no longer restate the unscoped claim
that same commit removed from the message as false.

Verified by mutation against the real pre-fix bytes rather than an
invented wording: reverting client.ts reds 1 test, dropping the refusal
suffix reds 1 test.

Finally, sweeps the em dashes out of both docs pages this PR touches.
STYLE_GUIDE.md forbids them, no vale rule covers it, and the PR had
introduced six where master had none.
Three things the union was leaving to comments or to luck.

`shared?: true` decided whether Nx tells a user to `chmod 1777` a directory, and
the rule that only `isSafeSharedRoot` may set it lived in a comment. Adding it to
`ensureOwnedPrivateDir`'s deny — one property, on a deny site that already
exists — compiled clean and passed the whole suite, and would have told someone
whose `~/.nx` is root-owned after one `sudo nx` to widen their own home to 1777.
Split into `foreign-owner` and `foreign-shared-container`, so `remedyFor`
branches on the discriminant it trusts everywhere else and `describeRefusal`'s
`never` arm forces the second wording to exist.

`not-a-directory` gave a planted symlink — the attack this module exists to
detect — the same message as a stray file or a fifo, and no remedy, while the
strictly less alarming foreign-owner case got a paragraph. `lstat` already knows;
the refusal now carries it, and says so.

`ensureOwnedPrivateDir` took `fchmod` not throwing as proof the mode changed,
where its sibling re-reads. On the mounts its own comment names — WSL2 `drvfs`,
CIFS, FAT — `chmod` succeeds and changes nothing, so the guard detected `0777`
and branded it private anyway: the reported bug still present, on the check
standing in front of the socket directory and the `.node` load path. It re-reads
now, one extra stat and only on the already-loose path.
`socketDirUnderFirstUsableRoot` builds the list of why each tier was skipped, and
it reached the user on two of three exits. On the third — a tier establishes and
the per-run directory beneath it then fails — the catch built its own
one-element list from the leaf error and the caller's was dropped, while the
warning still told the user `--verbose` would explain why the others were
rejected.

Not an exotic path: reaching the home tier at all means `/tmp/.nx` was refused,
and the mounts `ensureOwnedPrivateDir`'s comment names produce exactly this
leaf. That user was told to run `--verbose`, did, and learned nothing about
`/tmp/.nx` — including the one actionable remedy in the whole scenario.

The chain is threaded in and appended to instead, so both `remedyFor` and the
aggregate cause see the whole thing.

Also fixes two tests. One asserted the fallback cause contains no `chown` while
staging a refusal no arm of `describeRefusal` could ever render `chown` for — it
held for every kind, including the chownable one it was named after. It now
stages the chownable refusal, so it fails if the remedy leaks into the cause.
The other was named for the fallback cause and asserts the warning.
…ng it

The field's doc justified keeping it on the instance with "`_daemonStatus`
serializes that region, so there is exactly one writer and one reader in
flight". `_daemonStatus` serializes `startDaemonIfNecessary` against itself. It
does not serialize the five public callers of `isServerAvailable`, each of which
cleared the field on entry and wrote it from the socket error handler outside
any guard — the same shape the poll's refusal was moved off instance state for
last round.

All five are `await`ed sequentially in their own flows, so nothing is reachable
today. What was wrong is a comment asserting a structural invariant that isn't
one, on the field whose lifetime was last round's bug.

`probeServer()` returns `{available, refusal}`, `isServerAvailable()` delegates
and discards the refusal, and `startDaemonIfNecessary` hands the probe's own
value to `startInBackground`. The field is gone, and the shape matches
`waitForServerToBeAvailable`.

Also corrects a spec comment's baseline. It reads against the wording the commit
it documents replaced, not against the start of the review round — where the
neutral sentence was also present and briefly removed, which makes the same
sentence look like the added half rather than the surviving one.
`nx-daemon.mdoc` was rewritten last round to say `NX_SOCKET_DIR` replaces the
list of socket locations rather than joining it. The KB page still numbered it
as item 1 of four, and its own item 1 then said "It sits outside the order
below" — so a reader counted four tiers on one page and three on the other, with
the list contradicting itself in the process. Lifted into the sentence that
introduces the list, leaving three ordered entries matching the concept page.

Its `NX_SOCKET_DIR` table row claimed a per-run default. Plugin worker
directories hash the workspace root only, which is what the concept page was
corrected to say last round, and the row also restated a default the list above
it owns. It now says what the variable does instead of repeating where the
defaults are.

Two more:

- `nx-daemon.mdoc` used a semicolon, which STYLE_GUIDE forbids outright. It was
  the only one in the file, and Vale has no punctuation rule, so nothing would
  have caught it.
- The `NX_SOCKET_DIR` reference row had been reduced to "be sure to read" plus a
  link — a shape the guide bans, in a section defined as exhaustive facts. The
  two facts it dropped (the path-length use case, and precedence over
  `NX_DAEMON_SOCKET_DIR`) are back.

@nx-cloud nx-cloud Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nx Cloud has identified a flaky task in your failed CI:

🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.

Nx Cloud View detailed reasoning in Nx Cloud ↗

🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.


🎓 Learn more about Self-Healing CI on nx.dev

… back

The fallback warning showed only the first remedy in the refusal chain, and the
chain runs tier roots first with the leaf appended last, so a planted-symlink
leaf never reached the user whenever a tier above it had any advice. Report each
distinct remedy instead. Give `not-tightenable` a remedy of its own: it is
terminal on the workspace last resort, and was the only fatal refusal with no
way out.

Three tmp-dir.spec fixtures still staged the shared container as `foreign-owner`
carrying the deleted `shared` flag; one of them pinned a kind and a message
`isSafeSharedRoot` can no longer emit there. Type the spec's `reject` helper as
`DirRefusal` so the next drift is a compile error rather than a silent pass.

Pin the scoping that the allowAllUnixSockets test is named for — its assertions
survived a message that generalised the advice away from Claude Code.

Cut design history, reviewer-facing justification and narration from the socket
guard comments per .claude/agents/comment-analyzer.md, and correct the ones this
branch left stale: `shared` in remedyFor's JSDoc, the orphaned comment in
reset(), and the isServerAvailable references that probeServer replaced.
…ints the cut lost

The remedy added last commit named a cause the code never determined. Reproduced
on ordinary ext4 at uid 1000: a directory the user owns at mode 0070 fails
`openSync(O_RDONLY|O_NOFOLLOW)` with EACCES, so `chmodRealDirectory` returns
false and the refusal claimed the filesystem does not support POSIX permissions
and told the user to relocate — when `chmod 0700` fixes it and the guard then
returns ok. Name the mode, offer the chmod the owner can actually run, and keep
relocation as the fallback for when it does not stick.

Give `remedyFor` the same `never` arm `describeRefusal` has. Adding a member to
`DirRefusal` previously errored in only one of the two, which is how
`not-tightenable` shipped with a sentence and no advice.

Carry the remedy into `establishWorkspaceSocketDir`'s throw. That is the one
refusal path with nowhere left to fall and the only one that cannot reach the
warning where every other remedy is printed.

Annotate the `const` refusal fixtures as `DirRefusal`. Excess-property checking
only reaches fresh object literals, so the typing added last commit missed two of
the three sites it was added for, and its comment claimed otherwise.

Restore constraints the comment cut removed without a home: the Windows carve-out
on `NX_TMP_DIR` (the constant *is* `os.tmpdir()` there), the throw anchor on the
native cache contrast, why `reset()` is avoided in the stale-refusal test (a
one-line swap to it blinds that test), the deny-list-fails-open reason in
`chmodRealDirectory`, the umask precondition on the EACCES race, and that a
failed plugin-worker bind surfaces through the error handler.
@AgentEnder
AgentEnder merged commit 3298fd8 into master Aug 7, 2026
25 checks passed
@AgentEnder
AgentEnder deleted the pie-leotard branch August 7, 2026 17:54
polygraph-snapshot-app Bot pushed a commit that referenced this pull request Aug 11, 2026
…nstall (#36557)

## Current Behavior

Step 3 of the `review-pr` skill installs the workspace once, up front,
so the review agents can run tests, mutate sources to prove a test can
fail, and execute the repo's own eslint and tsc.

That block is the **only** `docker exec` in the skill without the mise
PATH export that every other one carries:

```bash
docker exec "$CONTAINER" bash -lc '
  cd /work/nx
  mise install >/dev/null 2>&1
  if   pnpm install --frozen-lockfile >/dev/null 2>&1; then
```

`bash -lc` does not put the mise shims on `PATH` by itself, so `pnpm` is
not found and the block falls through to:

```
workspace install FAILED — agents cannot run tests or the repo eslint
```

That message reads as a problem with the PR being reviewed. It isn't —
the real error is `pnpm: command not found`, and it is invisible because
all three branches redirect to `/dev/null`.

The consequence is not just a confusing message. The skill's own
guidance on a failed install is to tell the agents the workspace is
unavailable and restrict them to reading, so a whole review silently
degrades to a read-only pass with no one noticing why.

Observed on a real run of the skill against #36370.

## Expected Behavior

The install block exports `PATH` exactly like every other `docker exec`
in the skill, so `pnpm` resolves and the install actually runs.

Verified in a `nx-review-sandbox` container, running the shipped bytes
both ways:

| | `pnpm --version` |
| --- | --- |
| without the export (as shipped) | `bash: line 3: pnpm: command not
found` |
| with the export (this change) | `11.20.0` |

Install output now also goes to a log whose tail is printed on failure.
The log lives inside a container that Step 9 destroys, so a bare
"FAILED" previously left nothing to diagnose from — which is exactly
what made this take two runs to spot.

Documentation-only change to a `.claude/skills` file. No product code,
no tests affected.

## Related Issue(s)

None — found while running the skill.

<!-- polygraph-session-start -->
---
<p><picture><source media="(prefers-color-scheme: dark)"
srcset="https://static.ops.cloud.nx.app/polygraph/session-logo-v4-dark.svg"><img
src="https://static.ops.cloud.nx.app/polygraph/session-logo-v4-light.svg"
width="16" height="22" align="middle" alt="Polygraph"></picture> <a
href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Fix-review-pr-skills-workspace-install-missing-the-mise-PATH-export-7640b31d">View
session ↗</a></p>
<!-- polygraph-session-end -->
polygraph-snapshot-app Bot pushed a commit that referenced this pull request Aug 11, 2026
… it exists (#36560)

## Current Behavior

`review-pr`'s pre-flight asks whether the sandbox image exists:

```bash
test -n "$(docker images -q "$SANDBOX_IMAGE" 2>/dev/null)" && echo "image OK" || echo "image MISSING"
```

An image built from **any** older revision answers that identically. So
a capability added to the Dockerfile never reaches an image that already
exists, and nothing surfaces it — the only symptom is a review that is
slower or quietly weaker.

That is not hypothetical. The pnpm-store warming (`pnpm fetch`) landed
in `889f4cd45d` on **2026-07-31**; the local image was built
**2026-07-16**. `docker history` showed no `pnpm fetch` layer at all and
`/root/.local/share/pnpm` was 0 bytes, so the "warm store" the skill
promises had never existed on that machine. Every review in the two
weeks between downloaded ~4200 packages instead of linking them — about
**25 minutes each** — and the skill's only hint was a symptom you had to
notice yourself (*"If it is unexpectedly slow, the image predates the
warm store"*).

`setup-review-sandbox` had the same gate, plus a manual *"check the
`created` date against the Dockerfile"* that nobody does.

## Expected Behavior

Build unconditionally via a shared
`tools/review-sandbox/build-image.sh`, and let Docker's layer cache
decide what that costs. Both skills call it, so the image is kept
current by every review rather than by remembering to re-run setup.

Measured on the real image:

| situation | cost |
| --- | --- |
| nothing changed | **0.66 s** — prints `sandbox image up to date` |
| missing store layer (the bug above) | **2 m 47 s** — apt/mise layers
stayed cached |
| resulting store | **2.6 G**, matching the documented figure |

A `pnpm-lock.yaml` change re-runs `pnpm fetch`, which is the point: it
keeps the warm store matching the lockfile reviews actually install
from.

### No lock, because BuildKit already has one

`review-prs` drives up to five parallel `/review-pr` panes, so the
obvious worry is five simultaneous multi-GB builds. Measured instead of
assumed — 5 concurrent identical builds of a Dockerfile with a 20 s
step:

```
pane3 done at 21s   pane2 done at 21s   pane5 done at 22s
pane1 done at 22s   pane4 done at 22s

$ docker run --rm bktest cat /marker.txt
slow step running at 1785862632781210805      <- one line, not five
```

The step ran **once** and all five returned in ~21 s rather than 100 s.
An external lock would only duplicate that.

### Notes

- The build script writes to `tmp/review-sandbox-ctx` (gitignored) and
keeps the same minimal five-entry context — never the repo root.
- `allowed-tools` updated in both skills, or every run prompts.
- Documentation/tooling only. No product code, no tests affected.

## Related Issue(s)

Follow-up to #36557, found while running the skill against #36370.

<!-- polygraph-session-start -->
---
<p><picture><source media="(prefers-color-scheme: dark)"
srcset="https://static.ops.cloud.nx.app/polygraph/session-logo-v4-dark.svg"><img
src="https://static.ops.cloud.nx.app/polygraph/session-logo-v4-light.svg"
width="16" height="22" align="middle" alt="Polygraph"></picture> <a
href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Rebuild-the-review-sandbox-image-instead-of-probing-that-it-exists-c14dad6f">View
session ↗</a></p>
<!-- polygraph-session-end -->
polygraph-snapshot-app Bot pushed a commit that referenced this pull request Aug 11, 2026
…ing user (#36370)

## Current Behavior

The Nx daemon and isolated plugin workers communicate over unix domain
sockets (named pipes on Windows).

On POSIX the socket directory and files are created under the ambient
umask with no explicit mode, and plugin worker sockets sit directly in
the shared system temp root. Socket paths are also unpredictable: each
process derives its own hashed directory under `os.tmpdir()`, so there
is no single path a sandbox or container policy can allowlist.

The native binding is copied into a file cache before being loaded. That
cache lives in a hash-named directory under the system temp root, and
the loader accepts an existing entry on byte size alone, so a rebuild
that happens to produce an identical size is not picked up.

Separately, a daemon error is tagged as internal only when its log file
can be read, so a daemon that fails *before* ever writing a log aborts
the command instead of falling back to a daemonless graph build.

## Expected Behavior

### Layout on macOS and Linux

Every runtime artifact moves under one fixed root, with a per-user
directory beneath it. `<tmp>` is `os.tmpdir()` — `/var/folders/…` on
macOS, `/tmp` on Linux:

| Today | After |
| --- | --- |
| `<tmp>/<hash20>/d.sock` | `/tmp/.nx/<uid>/sockets/<hash20>/d.sock` |
| `<tmp>/<hash20>/fp<pid>-<n>.sock` |
`/tmp/.nx/<uid>/sockets/<hash20>/fp<pid>-<n>.sock` |
| `<tmp>/plugin<pid>-<n>-<perfNow>.sock` |
`/tmp/.nx/<uid>/sockets/<hash8>/p<pid>-<n36>-<rand8>.sock` |
| `<tmp>/nx-native-file-cache-<hash7>/<file>` |
`/tmp/.nx/<uid>/native-cache/<nxVersion>/<file>` |

The root is a literal `/tmp/.nx` rather than `os.tmpdir()`, which honors
`$TMPDIR` — per-user on macOS, rewritten by sandboxes, and stripped from
the daemon's environment. A fixed path is identical on every machine, so
a sandbox allowlist entry for it can be committed and shared with a
team.

**The uid sits directly beneath the container**, so `/tmp/.nx` is the
only level that can ever be shared between users, and `sockets` and
`native-cache` are inside the per-user directory rather than beside it.
That is what makes a single `install -d -m 1777 -o root -g root
/tmp/.nx` sufficient provisioning: there is no `/tmp/.nx/sockets` for
the first user to create and own.

When the container cannot be used, sockets move to `~/.nx/sockets`
before falling back into the workspace. Home needs no administrator —
there is no shared level to own — so a peer holding `/tmp/.nx` no longer
costs the short, fixed-length socket path.

**The native cache deliberately gets no second tier.** Sockets need
*somewhere* to live or the daemon and plugin workers cannot run at all,
which is what justifies chaining locations for them. The cache is an
optimisation with a working fallback already in hand — load the binding
in place from `node_modules` — so a second location would add a
directory to guard and a `.node` to trust for no capability that is
otherwise lost. Skipping it is also the fail-closed answer: the only
thing worse than no cache is a cache another user can write to.

The home tier is skipped when `HOME` makes `~/.nx` the shared container
itself (`HOME=/tmp`); otherwise a tier-1 failure would point the
owner-only guard at `/tmp/.nx` and take a root-owned `1777` directory to
`0700`.

### Layout on Windows

Named pipes are not filesystem objects, so there is nothing to allowlist
and no shared directory to separate users in, and `%TMP%` is already
per-user. Neither reason for the shared root applies, so sockets stay
directly under `%TMP%` and **no socket path is longer than it is
today**:

| Today | After |
| --- | --- |
| `%TMP%\<hash20>\d.sock` | `%TMP%\<hash20>\d.sock` (unchanged) |
| `%TMP%\<hash20>\fp<pid>-<n>.sock` | `%TMP%\<hash20>\fp<pid>-<n>.sock`
(unchanged) |
| `%TMP%\plugin<pid>-<n>-<perfNow>.sock` |
`%TMP%\<hash8>\p<pid>-<n36>-<rand8>.sock` (3 chars shorter) |
| `%TMP%\nx-native-file-cache-<hash7>\<file>` |
`%TMP%\.nx\native-cache\<nxVersion>\<file>` |

There is no per-uid segment on Windows — `%TMP%` is already per-user,
and the segment would only spend path budget. This matters because
`assertValidSocketPath` rejects paths over 95 characters and has no
platform guard, so it applies to named pipes too, and `%TMP%` already
contains the username. Longest username that still fits, with the
default `%TMP%`:

| socket | today | after |
| --- | --- | --- |
| daemon | ≤39 | ≤39 |
| plugin | ≤30 | ≤33 |
| forked / pty | ≤29 | ≤29 |

The native cache is not subject to that limit.

### Shared roots

`/tmp/.nx` is `1777` sticky, like `/tmp` itself, so every user on a
machine can create their own directory beneath it. It is the **only**
shared level: `/tmp/.nx/<uid>` and everything under it is `0700`,
created and re-checked on every use.

Before creating anything beneath a shared root, Nx checks that it is a
real directory, that it carries the sticky bit if anyone beyond its
owner can write to it, and that it belongs to the current user or to
`root`. The ownership half is not redundant: sticky restricts renaming
an entry to that entry's owner *and the directory's owner*, so a root
belonging to another regular user is unsafe at any mode. `/tmp` itself
is safe for the same reason — `root` owns it.

Several users therefore share the container only when something trusted
created it first — a container image, or an administrator running `sudo
install -d -m 1777 -o root -g root /tmp/.nx`. That one command is the
whole remedy, because no descendant is shared. Otherwise everyone after
the first moves to `~/.nx`, which needs no provisioning at all; the
error names the `chown` that would restore sharing.

On a single-user machine, in a container, and in a sandbox, none of this
applies: the first Nx run creates the container and owns it, so
`isSafeSharedRoot` accepts it with no setup.

`NX_SOCKET_DIR` still overrides the socket location and is used as given
— it names the socket directory itself. Pointing it at one of Nx's own
roots throws `InvalidSocketDirConfigured` rather than silently
substituting a default, with two distinct messages: the system temp dir
and `/tmp/.nx` are reachable by other users, while the per-user roots
are refused because Nx manages and cleans what lives beneath them. A
directory *nested* under any of them is still accepted.

### Directory handling

- `ensureOwnedPrivateDir` accepts a directory only if it is a real
directory (`lstat`, so a symlink does not qualify), owned by the current
user, and carries no group or other permissions — read and search are
enough to reach a socket inside it, so `0755` is tightened to `0700`
rather than accepted. A directory that fails a check it cannot repair is
not used.
- Shared roots are created one level at a time, so a symlink at any
level is caught rather than resolved through. The verdict comes from the
resulting mode rather than from whether the `chmod` succeeded, since
only a root's owner can `chmod` it.
- Directory modes are changed through a descriptor opened `O_NOFOLLOW |
O_NONBLOCK` and confirmed with `fstat` to be a directory, rather than by
classifying errnos — the errno for a given condition is not stable
across flag combinations or kernels, and `O_NONBLOCK` avoids a FIFO
blocking the open.
- The daemon socket file is set to `0600` after `listen`. On Linux,
connecting requires write permission on the socket file; on macOS/BSD
the (already `0700`) directory is what applies.
- Anything that cannot be established is skipped rather than
approximated: sockets try `~/.nx/sockets` and then a directory inside
the workspace, and the native cache is bypassed so the binding loads in
place from `node_modules`.

### Message scoping

Every message carries the sender's `workspaceRoot`, stamped centrally by
the transport rather than by each message constructor. A receiver scoped
to a different workspace refuses it: the daemon returns the mismatch and
stays alive for its own workspace, and a plugin worker drops the
message. This is what catches two workspaces accidentally sharing an
`NX_SOCKET_DIR`.

### Daemon failure classification

A daemon failure is tagged as internal whether or not a log file exists
yet, so a daemon that cannot start degrades to a daemonless graph build
instead of aborting. Previously the tag was set only inside the block
that read the log, so a first run — exactly when startup is most likely
to fail — aborted the command.

A `connect EPERM`/`EACCES` is the one exception, and is tagged
separately rather than as internal. It degrades the same way, but is
deliberately *not* disabled until `nx reset`: a socket owned by someone
else stops being there once it is removed or the machine reboots, so a
sticky disable would outlive the cause and hide the fix from anyone who
followed the advice. The message names the socket, both ways out, and
keeps the errno on its first line — `EACCES` and `EPERM` need opposite
remedies and it is the only token that tells them apart.

## Scope

Socket paths, permissions, and the native binary cache only.

The AI-agent and sandbox work previously folded in here is split into
**#36586**, stacked on this branch (it replaces #36463, which was
closed): Codex/Superset agent detection, `nx configure-ai-agents`
writing the sandbox allowances, the sandbox remediation hint, and the
changes that re-enable the daemon and plugin isolation inside sandboxes.
On this branch both still auto-disable under a detected sandbox, exactly
as on `master`.

**The Rust side is deliberately not covered, and NXC-4025 is therefore
only partly met.** `packages/nx/src/native/utils/socket_path.rs` derives
the Nx Console socket independently — `std::env::temp_dir()` plus a
workspace hash, created with `create_dir_all` at the ambient umask — and
is reached from `native/ide/nx_console/messaging.rs` on every TUI run
that talks to Nx Console. It also reads
`NX_SOCKET_DIR`/`NX_DAEMON_SOCKET_DIR` and applies none of the refusal
list added here, so a value this branch rejects is still used as-is by
the Rust path.

That means the ticket's "all Nx sockets under one allowlistable root" is
not yet true, and the sandbox allowlist this branch emits (`/tmp/.nx` or
`~/.nx`) does not cover the Nx Console socket. It is pre-existing code
on a different runtime with its own path handling, so folding it in
would widen this change materially; it is tracked as a follow-up
instead.

## Behavior changes worth calling out

- **Socket paths moved on macOS and Linux.** Anything that located Nx
sockets by scanning the system temp root must look under
`/tmp/.nx/<uid>/sockets`, `~/.nx/sockets`, or `NX_SOCKET_DIR` instead.
On macOS this is a different filesystem, not just a different name:
`$TMPDIR` is `/var/folders/…` while the new root is the literal `/tmp`.
Windows socket directories are unchanged.
- **The native cache directory moved on both platforms**, from
`<tmp>/nx-native-file-cache-<hash7>` to
`/tmp/.nx/<uid>/native-cache/<nxVersion>`
(`%TMP%\.nx\native-cache\<nxVersion>` on Windows). Tooling that excluded
or cleaned the old path by name needs updating.
- **Previously published `NX_DAEMON_SOCKET_DIR` guidance is reversed.**
The daemon docs told users to point it at a shared directory so several
long-running containers could share one daemon socket. That is exactly
what this change refuses, so the page now says the opposite: never share
a socket directory, and give each container its own daemon. Anyone
following the old documented setup will see `InvalidSocketDirConfigured`
or a refused directory rather than a silent behaviour change — that is
intended, but it is a documented workflow being withdrawn, not just a
docs edit.
- **The `0700` mode is applied to directories you already own**, not
only ones Nx creates — in practice the workspace-local fallback
(`<workspaceRoot>/.nx/workspace-data/d`, routinely `0755`) and an
explicitly configured `NX_SOCKET_DIR`. This ends the docker-compose
pattern of sharing one socket directory across containers running as
different users, and today it happens without a message.
- **A container owned by another regular user is no longer used.** On a
multi-user machine where someone else ran Nx first, sockets move to
`~/.nx/sockets` and the native binding loads in place. Pre-creating
`/tmp/.nx` as `root` restores the shared layout for everyone.
- **`NX_NATIVE_FILE_CACHE_DIRECTORY` and `NX_SOCKET_DIR` now behave
differently on a bad value.** A socket directory Nx refuses throws
`InvalidSocketDirConfigured`, because there is no safe substitute and
silently relocating sockets resurfaces later as a path-length error
about a directory the user never set. A native cache directory Nx
refuses warns and loads the binding in place, because that fallback is
complete. Both are loud; only one is fatal.
- **The native cache is keyed per uid and Nx version**, with each file
additionally keyed by a hash of the resolved binding path so multiple
source checkouts (all reporting `0.0.1`) do not collide. A cache entry
older than its source is refreshed.

## Related Issue(s)

Fixes NXC-4658
Fixes NXC-4025

<!-- polygraph-session-start -->
---
[View session information
↗](https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Tighten-Nx-daemon-RPC-socket-security-0700-0600-perms-1fee7ebf)
<!-- polygraph-session-end -->

---------

Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com>
Co-authored-by: AgentEnder <AgentEnder@users.noreply.github.com>
Co-authored-by: FrozenPandaz <jasonjean1993@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants