feat(sdk): resolve iam token placeholders in network transform callbacks - #1616
Conversation
🦋 Changeset detectedLatest commit: 35062ec The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
PR SummaryMedium Risk Overview
Callbacks must return a plain transform object; promises and wrong return types are rejected. JS uses a proxied Reviewed by Cursor Bugbot for commit 35062ec. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 42a060b. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.38.4-network-transform-callback.0.tgzCLI ( npm install ./e2b-cli-2.16.2-network-transform-callback.0.tgzPython SDK ( pip install ./e2b-2.38.0+network.transform.callback-py3-none-any.whl |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e206170ace
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Review feedback on #1616: - The token proxy gated on `prop in target`, which also matches inherited Object.prototype members, so an unregistered token named `constructor` or `__proto__` resolved to a built-in instead of being reported — `Bearer function Object() { [native code] }` on the wire. It now checks own keys, with the four properties the runtime itself reads (`toJSON`, `then`, `toString`, `valueOf`) still resolving normally so serializing, awaiting or coercing the map does not trip the guard. Python's mapping already treated those names as unregistered. - The callback return check accepted any object, so a `Map`, `Date` or class instance was forwarded and serialized to `{}` — a rule created without the headers it exists for. It now requires a plain object, mirroring Python's `isinstance(transform, Mapping)`. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f0f8f35. Configure here.
A network rule's `transform` can now be a callback receiving a context of
placeholder strings, so a workload identity token registered in the `iam`
option can be injected into egress requests without the SDK ever seeing
its value: `iam.tokens.aws` resolves to the literal
`${e2b.identity.tokens.aws}`, which the egress proxy substitutes with a
freshly minted token per request.
Referencing a token that is not registered in `iam.tokens` fails with
InvalidArgumentError / InvalidArgumentException — the proxy drops
placeholders it cannot resolve, so a typo would otherwise silently strip
the header. The update-network endpoint carries no iam config, so its
token names are unknown client-side and any name resolves there.
Co-Authored-By: Claude <noreply@anthropic.com>
…e destination The deployed egress proxy forwards header values verbatim, so an unregistered token name reaches the destination as literal placeholder text today and is dropped once substitution ships. Either way it never becomes a token, which is what the client-side check is for. Co-Authored-By: Claude <noreply@anthropic.com>
Review pass on the callback path:
- Python's placeholder map was a dict subclass hooking __missing__, which
`.get()` never calls: `ctx.iam.tokens.get('awz')` returned None and
shipped a literal "Bearer None" header, and on the update path `.get()`
did that even for a correctly spelled token. It is now a Mapping whose
__getitem__ owns both resolution and validation, with membership
answering "is it registered?" without raising, like `in` in JS.
- The JS return-value guard accepted any object, so an async callback's
promise serialized to `"transform": {}` — a rule created without the
headers it exists for. Promises and arrays are now rejected, with a
dedicated "must be synchronous" error in both SDKs; the awaitable is
closed/caught so the caller does not also get an unawaited-coroutine
warning or an unhandled rejection.
- JS forwarded an explicit `transform: null` as `null` while Python read
it as no transform.
- The JS token proxy threw on the runtime's own `toJSON`/`then` probes, so
`JSON.stringify(iam.tokens)` inside a callback aborted create.
- Document the permissive update-network path in `SandboxNetworkUpdate`
and the changeset instead of only in an internal comment.
- Payload tests move to tests/shared/sandbox, which is what that directory
is for — they only exercise the shared builders, so the sync and async
copies were 173 duplicated lines.
Co-Authored-By: Claude <noreply@anthropic.com>
Review feedback on #1616: - The token proxy gated on `prop in target`, which also matches inherited Object.prototype members, so an unregistered token named `constructor` or `__proto__` resolved to a built-in instead of being reported — `Bearer function Object() { [native code] }` on the wire. It now checks own keys, with the four properties the runtime itself reads (`toJSON`, `then`, `toString`, `valueOf`) still resolving normally so serializing, awaiting or coercing the map does not trip the guard. Python's mapping already treated those names as unregistered. - The callback return check accepted any object, so a `Map`, `Date` or class instance was forwarded and serialized to `{}` — a rule created without the headers it exists for. It now requires a plain object, mirroring Python's `isinstance(transform, Mapping)`. Co-Authored-By: Claude <noreply@anthropic.com>
The `get` trap moved to own keys in the previous commit, but `in` still went through the default `has`, which walks the prototype chain: with a name from config, `'constructor' in iam.tokens` was true while `iam.tokens.constructor` threw. Membership answers "is this token registered?", so it now checks own keys too, matching Python's `_IamTokenPlaceholders.__contains__`. Co-Authored-By: Claude <noreply@anthropic.com>
The egress proxy reads a placeholder as everything between
`${e2b.identity.tokens.` and the next `}` (belt `substitute`), then looks
that name up in the registered tokens. Interpolating an unchecked name
into it corrupts the reference in both directions: `a}b` ends the
placeholder early, so the proxy mints the unrelated token `a` and leaves
`b}` as literal text, and a name carrying `{` can close its own
placeholder and open another one, minting a token the caller never
referenced.
Names are now checked where they are registered — a transform
placeholder is the only way to consume a workload token, so a name that
cannot appear in one is unusable — and again before interpolation, which
is what covers the update-network path, where any name the callable looks
up is turned into a placeholder without passing through the iam config.
Control characters are rejected with them: they cannot appear in an HTTP
header value, so the API would answer with an opaque 400.
Co-Authored-By: Claude <noreply@anthropic.com>
The placeholder spelling, the name rule it forces, and the token map the callbacks read are one concern with a backend contract behind it, so they get a named module next to `network`/`mcp`/`signature` rather than more lines in sandboxApi. `utils` is the other candidate, but that is where generic mechanism lives (runtime detection, shellQuote, class_method_variant), not domain knowledge. sandboxApi keeps what is network-shaped: building the transform context around the map, and checking names as tokens are registered. Co-Authored-By: Claude <noreply@anthropic.com>
bb0970b to
35062ec
Compare

Stacked on #1606 (
iam-sdk-feature) — merge that one first. This is the second half of SDK-245: it makes the workload tokens registered bySandbox.create'siamoption usable, by letting a network rule'stransformbe a callback that receives placeholder strings the egress proxy resolves per request.iam.tokens.awsis the literal string${e2b.identity.tokens.aws}(the frozen backend spelling — a placeholder can only select a persisted named token, never an inline audience or claim). The SDK never resolves it: the wire payload carries the placeholder and the proxy substitutes a freshly minted JWT-SVID when it forwards the request, so the token value never reaches SDK-side code or the sandbox.Referencing a name that isn't registered in
iam.tokensfails withInvalidArgumentError/InvalidArgumentExceptionlisting the names that are — the proxy never turns an unregistered name into a token, so a typo would otherwise surface as a confusing auth failure at the destination.updateNetwork/update_networkaccepts the same callbacks, but its payload carries noiamconfig, so token names can't be validated client-side there and any name resolves to its placeholder.Static
transform: { headers }objects keep working unchanged (including hand-written${e2b.identity.tokens.<name>}strings, which stay the escape hatch for tokens the SDK doesn't know about).Only
{ iam }is exposed on the context for now —${e2b.sandboxId}/${e2b.teamId}/${e2b.executionId}from the older prototype are not part of the current backend design, sosandboxcan be added later when there is something to resolve.Usage
Both send:
{ "iam": { "tokens": { "aws": { "audience": "sts.amazonaws.com", "tokenType": "JWT-SVID" } } }, "network": { "allowOut": ["api.internal.example.com"], "rules": { "api.internal.example.com": [ { "transform": { "headers": { "Authorization": "Bearer ${e2b.identity.tokens.aws}" } } } ] } } }Notes
allowOut/deny_outselectors run before transforms are resolved, soctx.rulesstill hands back the rules you passed — a rule'stransformthere is the union (object or callback), not the materialized object. ThegetInfoview keeps its own narrowedSandboxNetworkRuleInfotype.{,}or control characters. The proxy reads a placeholder up to its first}, soa}bwould mint the unrelated tokenaand leaveb}as literal text, and a{in a name can open a second placeholder. The interpolation check is what coversupdateNetwork, where any name the callback looks up becomes a placeholder without passing through theiamconfig.iam.tokensis guarded, not just[name]: Python's map is aMappingwhose__getitem__owns resolution (so.get('typo')raises instead of returningNone), and membership ('aws' in ctx.iam.tokens/'aws' in iam.tokens) answers "is it registered?" without raising so a callback can branch on it. Lookup checks own keys only, so an unregistered name colliding with an object member (constructor,__proto__) reports as unregistered instead of resolving to a built-in; the four properties the runtime itself reads (toJSON,then,toString,valueOf) still resolve normally, so serializing, awaiting or coercing the map does not trip the guard.asynccallback), an array, aMap/Date/class instance, or a missing return value is rejected with an actionable error rather than silently creating a rule with no headers. The awaitable is closed/caught so you don't also get an unawaited-coroutine warning or an unhandled rejection.Tests
New payload-level tests: JS
tests/sandbox/networkTransform.test.ts(msw), Pythontests/shared/sandbox/test_network_transform.py— shared rather than mirrored into the sync and async suites, since they only exercise the shared builders. They cover placeholder resolution, enumerating and membership-testing registered tokens,JSON.stringifyof the context not tripping the guard, static transforms staying byte-identical,transform: null, the unregistered-name rejection through both[name]and.get(), the no-iamrejection, non-transform andasyncreturn values, unusable token names (both braces, a smuggled placeholder, a newline, empty) at registration and on the update path, and the permissiveupdateNetworkpath.Verified against production on all three surfaces (JS, sync Python, async Python), where:
Bearer ${e2b.identity.tokens.aws}is accepted byvalidateNetworkRulesand round-trips throughgetInfo/get_infounchanged;400: Sandbox IAM workload tokens are not available for your team.), sinceiamis still feature-flagged;Proxy-side substitution of the placeholder ships separately in belt (EN-1864); until then the header value is forwarded verbatim, which is why there is no end-to-end injection test here.
Part of SDK-245.