Skip to content

Per-command network allow lists, and deny reasons from the ask callback - #591

Merged
ronleizrowice-ant merged 7 commits into
mainfrom
feat/per-command-network-lists
Oct 7, 2026
Merged

ronleizrowice-ant merged 7 commits into
mainfrom
feat/per-command-network-lists

Conversation

@ronleizrowice-ant

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

Copy link
Copy Markdown
Collaborator

Adds a per-command network allow list, and lets the ask callback deny with a reason.

What changes

  • SandboxManager.registerCommandNetworkLists(commandId, { allowedDomains }) and unregisterCommandNetworkLists(commandId). Entries are checked like network.allowedDomains; an invalid one, or an id under 22 characters (unpaired surrogates not counted), throws.
  • Order: deniedDomains denies, allowedDomains allows, strictAllowlist denies, the list for the proxy username's id allows, the ask callback decides, else deny.
  • The callback may resolve to { allow: false, reason }; the reason (sanitized, 500 characters) goes in the violation line. Only === true allows. SandboxManager.askCallbackDenyReason marks a release that reads it.

Who sees a difference

  • A callback resolving to a truthy non-boolean used to allow and now denies: release-note it.
  • A wrapper testing the callback's result for truthiness still type-checks, and reads { allow: false, reason } as an allow.
  • Implementers of ISandboxManager add three members.

Known limits

  • The id alone binds a connection to a list, so it must be unguessable: attribution, not a boundary.
  • No list applies under an external network.httpProxyPort (registering succeeds, with one SRT_DEBUG line), nor under a non-ASCII id too long for the proxy username.
  • No CLI; no version bump.

Merge order

Conflicts with #584 and #615 in one comment-only hunk, the JSDoc of commandId (sandbox-manager.ts ~1755): keep both paragraphs. #615 logs commandId under debug; this PR keeps ids out of error text.

Testing

68 new cases in test/sandbox/update-config.test.ts (CONNECT, SOCKS5, sandboxed curl); same failing names as main.

…llback

registerCommandNetworkLists(commandId, { allowedDomains }) gives one
invocation an allow list of its own for as long as it runs, in addition
to the configured one; unregisterCommandNetworkLists(commandId) removes
it and reset() removes all. Until now the only way to let one command
reach one more host was updateConfig, which widens every command and
the default injectHosts scope. For a connection the order is:
deniedDomains denies, allowedDomains allows, strictAllowlist denies,
the list registered for the id in the proxy username allows, then the
ask callback, else deny. Entries get the same check as
network.allowedDomains; the predicate moves to domain-pattern.ts so the
manager does not load the schema module. An invalid entry throws, and
so does a commandId shorter than 22 characters, because the id is the
only thing that ties a connection to a list.

SandboxAskCallback may now resolve to { allow: false, reason }, and the
reason is reported in the violation line in place of "user denied". It
is sanitized like the rest of the line and cut to 500 UTF-16 code
units. Only the value true allows: a callback that resolved to some
other truthy value used to allow and now denies.
SandboxManager.askCallbackDenyReason is true so a caller can tell
whether a reason object is understood before returning one.
64 cases through the live proxy against a loopback listener: the order
of evaluation, entry validation, how the id is read from the proxy
username, each shape the callback can answer with, what the reason is
reduced to before it is stored, and the warning under SRT_DEBUG.
@ronleizrowice-ant ronleizrowice-ant mentioned this pull request Sep 21, 2026
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.
… a real wrap

Under an external `network.httpProxyPort` the wrap hands the proxy no
username, so no connection names an id and a registered list never
applies. It fails closed; the README and the doc comment now say so,
and registering in that state logs one debug line.

The 22-character floor is measured on the key the map holds, with
U+FFFD left out: every unpaired surrogate becomes one, so an id made
of them passed the floor while carrying nothing.

New tests: a list applies over SOCKS5 as over CONNECT, and it reaches a
sandboxed curl through the commandId its wrap was given.
No conflicts. The README said "the first release after v0.0.77" of the ask
callback allowing on `true` alone; 0.0.78 has been cut since, without it.
No conflicts. The README said the ask callback's change comes in 0.0.79,
which was cut without it.

@imichaelmoore imichaelmoore 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.

/stamp

@ronleizrowice-ant
ronleizrowice-ant merged commit 8f8623e into main Oct 7, 2026
16 checks passed
@ronleizrowice-ant
ronleizrowice-ant deleted the feat/per-command-network-lists branch October 7, 2026 21:50
@ronleizrowice-ant ronleizrowice-ant mentioned this pull request Oct 7, 2026
ronleizrowice-ant added a commit that referenced this pull request Oct 9, 2026
New to the bundle: #663, #665, #666 and #662. #656, #591 and #585 come as
squashes of what it held. In the two blocks git marks, a doc comment of the
manager and the order suite, it keeps its own side: the comment has both
paragraphs, and the suite leaves the manifest's name out of its comparison.
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.

2 participants