Repository navigation
Conversation
An embedder that runs several commands at once through the one
process-wide proxy cannot tell, inside SandboxAskCallback, which of them
opened the connection it is asked about. The proxy already knows: the
username carries the invocation's attribution key, and the same function
hands it to recordOutboundDeny and to the per-command list lookup. Without
it the embedder cannot ask on behalf of the right command, and cannot
answer differently for different kinds of process sharing the proxy.
The callback now receives a second argument, { attributionKey }: the
decoded key, cut to 100 characters like every carrier cuts it, or
undefined when the username names none. It is the key the violation store
records a denial under and registerCommandNetworkLists is looked up by.
The key is chosen by the sandboxed client, so it is documented as a label
to compare with ids the embedder issued, not a credential. It is passed
whether or not this process wrapped a command under it, so a long-running
command's key does not depend on the bounded command-text registry.
The parameter is optional in the type, so an existing one-argument callback
and code that invokes a callback directly both still type-check. A release
without this change calls the callback with one argument, which an embedder
can detect as context === undefined.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #734.
What
filterNetworkRequestpasses a second argument to the ask callback:SandboxAskContext = { attributionKey: string | undefined }is new insandbox-schemas.tsand exported from the package root.SandboxAskCallbackgainscontext?: SandboxAskContext. The library alwayspasses it. It is optional so that a one-argument callback, or code that
calls a callback directly, still type-checks.
it inline. Its behaviour does not change.
Why this value
It is the value the proxy already treats as "which invocation". It is the key
the violation store records the denial under (
encodedCommand, decoded) andthe key
registerCommandNetworkListsis looked up by. An embedder can useone id for all three. It is cut to 100 characters, as every carrier cuts it.
The key is passed even when no wrap in this process registered it. The
callback does not go through
resolveCommandText, for two reasons:commandTextsByKeyis a bounded FIFO, so a long-running command's entry canage out while it still makes connections.
Because the key is the client's choice, the type doc and the README call it a
label, not a credential, as #591 does for the per-command lists.
Compatibility
and TypeScript accepts a one-parameter function for a two-parameter type.
so
contextisundefinedthere. UnlikeaskCallbackDenyReason, anembedder can detect this from the call itself, so no feature flag is added.
A flag can be added if you would rather have one.
order and the strict-allowlist path are untouched.
Tests
New
describe('ask callback attribution')intest/sandbox/update-config.test.ts, against a real proxy and a loopbacklistener:
commandIdthe username names (HTTP CONNECT);srtusername givesattributionKey: undefined;encodedCommandof the recorded violation;Six of the seven fail without the change. The seventh guards the decision
order.
bun test test/sandbox/update-config.test.ts test/public-exports.test.ts:100 pass.
npm run typecheck,eslintandprettier --checkare clean on thechanged files.
Related: #591 (per-command lists and deny reasons), #722 (another
caller-supplied per-wrap option, for the pty a command is spawned on).