Skip to content

Warn when the library is installed where a sandboxed command can write it - #595

Merged
ronleizrowice-ant merged 10 commits into
mainfrom
fix/deny-writes-to-the-library-itself
Oct 10, 2026
Merged

ronleizrowice-ant merged 10 commits into
mainfrom
fix/deny-writes-to-the-library-itself

Conversation

@ronleizrowice-ant

@ronleizrowice-ant ronleizrowice-ant commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Installed as a dependency, this package lives in <project>/node_modules/, usually inside allowWrite. A sandboxed command can rewrite it there, and the next srt run in that project executes the result unsandboxed.

Earlier commits on this branch write-denied the library's own files. That made every sandboxed npm install fail on Linux and did not cover pnpm layouts, so it is gone: no deny, no mount, no new config field, and both backends are as on main.

What remains is a warning:

  • ownInstallWarning() in src/sandbox/own-files.ts finds the package directory this copy is loaded from and tells whether it lies at or under an allowed write path, as spelled or where it really is. Nothing for a checkout (not under a node_modules) or a copy compiled into an application.
  • checkDependencies() returns it as a warning on Linux and macOS, unless filesystem.disabled is set. The first wrap with a filesystem policy logs it once under SRT_DEBUG.
  • It never throws. It only advises, so whatever goes wrong in it (a malformed entry, a working directory that is gone) counts as no warning, and initialize(), checkDependencies() and the wrap behave as on main.
  • main is merged in. The write lists hold entries now, so both call sites read them through writeRootsOf(): a root marked { path, literal: true } is judged too.
  • Cost: 3 existsSync and two realpathSync per write root for each evaluation (once in initialize(), once per checkDependencies(), once in the first wrap). From the second wrap on, nothing.
  • README, Security Limitations: install the package outside every allowed write path.

Tests: npm and pnpm layouts, a checkout, an install outside the write paths, a write path that does not cover the copy, the source loaded from a project for both call sites, a write path marked literal, a malformed entry, a removed working directory, and a check that is made to throw.

Driven through an installed copy of the built package, beside main's build: one warning with the copy inside a write path, none outside, and the wrapped command is the same in every case.

Installed as a dependency of a project, this package lives in
<project>/node_modules/, inside what allowWrite usually names. A wrapped
command could rewrite dist/*.js there, the bundled seccomp helper, or
one of the packages this one loads, and the next `srt` run in that
project executed the result on the host, outside any sandbox.

The directories this copy was loaded from are now write-denied to every
wrapped command whenever they lie inside an allowed write path: the
package's own directory and that of every package it depends on at run
time, found the way the module loader finds them (nearest node_modules
on the way up, dependencies of dependencies included). They join the
caller's own denyWrite entries in both places a write configuration is
built. Nothing else under node_modules is touched.

Nothing is denied for a copy that is not installed under a node_modules
(a checkout of this repository), for one that lies outside every write
path, or where the library has been compiled into an application and
has no file on disk.
…e it

Unit cases for finding what a copy was loaded from (hoisted and nested
dependencies, dependencies of dependencies, a cycle, a checkout, no
package on disk) and for which of those a wrap denies (inside a write
path only, judged where the paths really are). Then the real thing: the
built package copied into a temporary project's node_modules with what
it depends on, run from there under node with the project writable. The
command writes the project, and cannot change dist/, a dependency's
files, move the package, its scope directory or node_modules aside; it
can still add another package beside them.
Three ways round the denies on the library's own files.

A scoped package is asked for its dependency in
node_modules/@scope/node_modules/ ahead of the hoisted copy, and an ES
module also in node_modules/node_modules/. Neither exists, so a wrapped
command could create one and be loaded in place of the real package.
Every place the loader looks before the place a dependency is found is
now denied with it, by its first missing component, which is what would
have to be made.

The package's launchers in node_modules/.bin/ are links in a directory
the command may write, and are what npx and npm run start. They are
denied by name. Seatbelt holds such a name; on Linux a mount lands on
what a link leads to, so there a launcher can still be re-pointed, and
the README says how to start the library without going through it.

These paths are names the library read off the disk, not spellings a
caller wrote, and went to the macOS profile through the list that is
read for pattern syntax: a project directory with `[` or `*` in its name
compiled to a filter that matched nothing. They now travel in a new
optional FsWriteRestrictionConfig.literalDenyWithinAllow, which both
platforms take as literal paths. And where an allowed write path is
itself a pattern, containment cannot be judged, so all of them are
denied rather than none.
The places looked at before a hoisted dependency, in a plain project and
in a workspace; the launchers a manifest declares, by object and by
string; every path denied when an allowed write path is a pattern; a
project directory with brackets in its name reaching the macOS profile
as a subpath and not as a regex. End to end, the wrapped command cannot
create a package where the loader looks first (beside the library in
its scope, or one level above), cannot reach the library's file through
a hard link made where writing is allowed, and leaves no placeholder
behind; on macOS it cannot re-point the launcher either. It can still
add another launcher and another package.
A declared dependency that is on disk nowhere (an optional one, say) was
passed over, and with it everywhere it had been looked for. The loader
would take a copy from any of those levels, so a wrapped command could
supply one. The places are now kept and denied like those looked at
before an installed dependency: by their first missing component, and
only where a wrap could write them.
Comments only: the JavaScript emitted with comments removed is
byte-identical for every file, and the test names are unchanged.

Kept: every invariant and what could be done without it, the
documentation of exported names, and the reasons for what is not done
the obvious way. Cut: how the code came to be as it is, narration of
the lines that follow, and explanations repeated at several sites.
…eny nothing

The deny mounts on the library's own files broke every sandboxed
`npm install` on Linux (masking the absent
`node_modules/node_modules` makes npm fail with EBUSY) and did not
cover a pnpm layout, where the launcher is a real script and the links
can be re-pointed. Covering each package manager's layout is not a rule
the library can keep.

All of them are removed, with `literalDenyWithinAllow`. What stays is
one check: when the package directory this copy is loaded from lies at
or under an allowed write path, as spelled or where it really is,
checkDependencies() returns a warning and the first wrap logs it under
SRT_DEBUG. A checkout and a copy compiled into an application are not
judged. README: install the package outside every allowed write path.
@ronleizrowice-ant ronleizrowice-ant changed the title Deny a wrapped command the library's own files Warn when the library is installed where a sandboxed command can write it Sep 28, 2026
dylan-conway
dylan-conway previously approved these changes Sep 28, 2026
…throw

The write lists hold entries now, so both call sites go through
writeRootsOf(): read as strings, a root marked literal was missed, and that
type-checked.

The check only advises. A malformed entry, or a working directory that is
gone, made it throw out of checkDependencies() and initialize(), where main
returns. Whatever goes wrong in it now counts as no warning: whoever
enforces a config says what is wrong with it.

@antdres antdres left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No blockers from me at 0ecb488. I built main's merge-base and this head, placed each build as an installed copy under a project's node_modules, and ran both. The warning shows up once in checkDependencies() when the project, a parent of it, a symlink to it, "." or a { path, literal: true } entry is in allowWrite, and for a pnpm-style layout; it stays silent for an install outside the write paths, a sibling write path, and an empty allowWrite. The wrapped command is the same as main's apart from the random socket names and proxy tokens, the CLI prints nothing extra without SRT_DEBUG, and the new tests fail when I neuter the function or remove either call site (12 pass as written).

Worth knowing, not blocking: this only advises. A sandboxed command can still append to the installed dist/cli.js when the project is writable, same as on main. On Linux an allowWrite glob gives no warning (it is not bound there either); macOS was not run.


Generated by Claude Code

@antdres antdres left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 0ecb488. No blocking issues; what I ran is in my comment above.


Generated by Claude Code

@ronleizrowice-ant
ronleizrowice-ant merged commit 451b950 into main Oct 10, 2026
16 checks passed
@ronleizrowice-ant
ronleizrowice-ant deleted the fix/deny-writes-to-the-library-itself branch October 10, 2026 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants