D3 network-egress purity — lexical binding enforcement - #64
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dd6a36411
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba636b23ae
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c71f3266c7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86e4b9fffe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9c0dc665c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2b4387cbc
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bf1d64764
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ag7ZrUfkkKnbh6YxLU1fLQ
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ca0a1fdc5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ag7ZrUfkkKnbh6YxLU1fLQ
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bff9a4081
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6781d8a28
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // rejects a shadowed `const g = globalThis`, and the escaped alias is never traced. | ||
| if ( | ||
| (ts.isPropertyAccessExpression(node) || ts.isElementAccessExpression(node)) && | ||
| isGlobalReceiver(node) && |
There was a problem hiding this comment.
Fresh evidence beyond the earlier direct constructed-hop fix is that this new forwarding branch still relies on the literal-only isGlobalReceiver: const g = globalThis['global' + 'This']; g.fetch('https://example.com') makes NET, RC, and HA all return false while g is the real global object and the call performs outbound egress. The computed key is considered statically harmless by the other checks, and the later local receiver g is not tracked, so apply the same bounded static-key resolution used by NET when recognizing a forwarded self-reference.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138GjJx3nbN2K7NhuMU2poe
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4835b19798
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isCreateServerCall(node)) { | ||
| for (const arg of (node as ts.CallExpression).arguments) { | ||
| const handler = binderUnwrap(arg); | ||
| if (ts.isArrowFunction(handler) || ts.isFunctionExpression(handler)) { |
There was a problem hiding this comment.
When createServer receives an IncomingMessage class in its options, Node constructs that class with the live connection socket, but this pass records only parameters of function arguments passed directly to createServer. Consequently, http.createServer({ IncomingMessage: Capture } as any, ...) can use Capture's constructor parameter to call destroy() and then connect() outbound without mentioning any banned socket property, and all three current purity detectors accept the source. Treat the custom request constructor as another socket-delivery entry point.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fj88FKnrXXioWzdngyDcSG
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e40e8c9d81
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isCreateServerCall(node)) { | ||
| for (const arg of (node as ts.CallExpression).arguments) { | ||
| const handler = binderUnwrap(arg); | ||
| if (ts.isArrowFunction(handler) || ts.isFunctionExpression(handler)) { |
There was a problem hiding this comment.
When the request listener is passed by reference, this collector ignores its parameters because it only recognizes inline arrow/function expressions. For example, function handler(req) { const key = (req.url ?? '').slice(1); const s = req[key]; s.destroy(); setTimeout(() => s.connect(80, 'example.com'), 50) } http.createServer(handler) passes all three purity detectors; a request for /socket then recovers the live inbound socket and reconnects it outbound. Resolve binder-visible function declarations used as listeners, or reject non-inline listeners so Rule A2 cannot be bypassed.
Useful? React with 👍 / 👎.
| const optionsObject = binderUnwrap(arg); | ||
| if (!ts.isObjectLiteralExpression(optionsObject)) continue; |
There was a problem hiding this comment.
Fresh evidence beyond the prior direct-object constructor comment is that storing the options first still bypasses the new rule: const options = { IncomingMessage: Capture }; http.createServer(options as any, ...) passes NET, RC, and HA because the argument is an identifier rather than an object literal, while Node still invokes Capture with the live socket and its constructor can destroy and reconnect it outbound. Trace binder-resolved constant option objects, or fail closed on non-literal createServer options.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdLcbXZMib9vDjZ7Lgr8gy
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 430f11d370
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const optionsObject = resolveCreateServerArgument(arg); | ||
| if (!ts.isObjectLiteralExpression(optionsObject)) continue; | ||
| for (const prop of optionsObject.properties) { | ||
| if (!ts.isPropertyAssignment(prop) && !ts.isShorthandPropertyAssignment(prop)) continue; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier direct/aliased constructor-option reports is that this property-kind filter skips getters: http.createServer({ get IncomingMessage() { return Capture; } } as any, () => {}) passes all three purity detectors. Node reads the getter and constructs the returned class with the live connection socket, so Capture can destroy and reconnect that socket outbound; inspect accessor keys such as IncomingMessage and ServerResponse alongside ordinary property assignments.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0168Ff8rrLX6K8yWx8WhwrUY
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 345b9e34a2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // resolve to an ObjectLiteralExpression and stays outside the proof (frozen positive model). | ||
| const optionsObject = resolveCreateServerArgument(arg); | ||
| if (!ts.isObjectLiteralExpression(optionsObject)) continue; | ||
| for (const prop of optionsObject.properties) { |
There was a problem hiding this comment.
Fresh evidence after the direct, aliased, and getter option fixes is that http.createServer({ __proto__: { IncomingMessage: Capture } } as any, ...) still passes usesOutboundNetwork: this loop examines only the top-level object properties, while Node reads the inherited options.IncomingMessage and constructs Capture with the live connection socket. The constructor can then destroy and reconnect that socket outbound, so reject prototype-bearing option shapes or inspect statically supplied prototypes for the reserved constructor keys.
Useful? React with 👍 / 👎.
| for (const arg of (node as ts.CallExpression).arguments) { | ||
| const handler = resolveCreateServerArgument(arg); | ||
| if (ts.isArrowFunction(handler) || ts.isFunctionExpression(handler) || ts.isFunctionDeclaration(handler)) { | ||
| for (const param of handler.parameters) { |
There was a problem hiding this comment.
When a non-arrow request listener has no declared parameters and reads arguments[0], this collector records no request/response symbol because it only iterates handler.parameters. For example, createServer(function () { const req = arguments[0]; const s = req[(req.url ?? '').slice(1)]; ... }) passes usesOutboundNetwork, yet a request for /socket recovers the live socket and can reconnect it outbound. Treat the listener's arguments binding as another request-delivery source or reject this access form.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121zTdfx8cyXQCQLJtN6Uzq
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4569d6e37c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const SOCKET_DELIVERY_MEMBERS: ReadonlySet<string> = new Set([ | ||
| 'on', | ||
| 'once', | ||
| 'addListener', | ||
| 'prependListener', |
There was a problem hiding this comment.
When code can mutate the returned http.Server, overriding server.emit intercepts the internal emit('connection', socket) call without using any name in this set. For example, saving const emit = server.emit, replacing it with a function that takes values[0] when the event is 'connection', and then destroying/reconnecting that socket performs outbound TCP; I verified that usesOutboundNetwork, usesRuntimeCodeGeneration, and acquiresHiddenBuiltin all return false for this witness. Include emit in the socket-delivery reservation (and its existing member/destructuring coverage) so the permitted server cannot expose its delivered socket this way.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AiozMuvvnzkYJgStQH6Qdo
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e99d12a1c4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'prependListener', | ||
| 'prependOnceListener', | ||
| 'setTimeout', | ||
| 'emit', |
There was a problem hiding this comment.
emit replacement
Fresh evidence beyond the earlier direct-assignment report is that adding emit to this member-name set only catches syntactic accesses: Object.defineProperty(server, 'emit', { value(event, ...args) { /* use args[0] on 'connection' */ } }) contains no server.emit node, and usesOutboundNetwork, usesRuntimeCodeGeneration, and acquiresHiddenBuiltin all return false. Node invokes this replacement for its internal connection event and supplies the live socket, which can then be destroyed and reconnected outbound, so reflective mutation APIs such as Object.defineProperty/Reflect.defineProperty must receive equivalent coverage.
Useful? React with 👍 / 👎.
Add one canonical, target-gated meta-mutation circuit breaker to the D3 cockpit-host purity analyzer: a free built-in meta-mutation API (Object.defineProperty/defineProperties/assign/setPrototypeOf, Reflect.defineProperty/setPrototypeOf/set) applied to a proven privileged target (a createServer result — direct or via the bounded unique-const spine — or a tracked req/res root) is denied. The gate is on the target, never the member key, so future member names deny identically without any emit/on/constructor enumeration. Reads, capability-removing APIs, and unprovable targets stay allowed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EczoNDvhWL5HRnF5CTPcHz
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b863191ad
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const SOCKET_DELIVERY_MEMBERS: ReadonlySet<string> = new Set([ | ||
| 'on', | ||
| 'once', | ||
| 'addListener', | ||
| 'prependListener', | ||
| 'prependOnceListener', | ||
| 'setTimeout', | ||
| 'emit', |
There was a problem hiding this comment.
When host code calls server.__defineGetter__('emit', () => replacement), the inherited Object.prototype.__defineGetter__ installs an accessor directly on the proven http.Server. Node's subsequent internal server.emit('connection', socket) invokes the getter and then passes the live socket to replacement, which can destroy and reconnect it outbound. Unlike the already-covered Object.defineProperty form, this is accepted because __defineGetter__ is absent from the reserved member family and the meta-mutation branch only recognizes direct Object/Reflect APIs; reserve this mutator or otherwise apply the privileged-target check to it.
Useful? React with 👍 / 👎.
1 participant
Stacked validation PR — OPEN / READY / unmerged. Merge is out of scope for this reconciliation gate.
Stack
cockpit/d3-readonly-dashboard-host5ae2b786ad6dc4653286d4c2b50e1fd705daa974(untouched)repair/d3-network-egress-puritya6781d8a2859cce8282a61724770fedf3967972dtests/cockpit-host/purity.test.tsScope
Bounded static source-policy enforcement for the D3 read-only cockpit host boundary (
src/cockpit-host/): a single-parse, finite AST guard that decides importless network globals (fetch/WebSocket), thenode:httppositive model (onlycreateServerallowed), runtime code-generation (RC), and hidden-builtin acquisition (HA) by lexical binding identity, not identifier text. This is a development-time source policy, not a runtime sandbox.History (condensed)
The detector began by replacing name-text decisions with a bounded lexical environment/scope-stack model (fixing shadowing false-positives NET-S1 and the missed-egress false-negative NET-S2), then closed a long series of laundering variants surfaced by review: node:http client/
Agent/ClientRequestmembers, dynamic-import bindings, nested/assignment destructuring, computed & template-substitution socket-registrar keys, statically-folded global self-reference hops (globalThis.globalThis,globalThis['global'+'This']), reflective reads (Reflect.get), and — most recently — forwarding of a recognized free-global self-reference value (const g = [globalThis.globalThis][0]; g.fetch(…)), closed by the RC v4 global-object self-reference forwarding closure in the head commita6781d8("fix(cockpit): reject forwarded global authority").Latest exact-head evidence
33443908648), head_shaa6781d8…, jobverify→ SUCCESS.a6781d8(Manual request).tests/cockpit-host/purity.test.ts: SHA25625d1115c774555be5d546ba2a1407f0333cdabde5ef756968438e5f7deae063e, 452778 bytes.git diff --checkall PASS.Review-thread reconciliation (exact HEAD
a6781d8)All 45 review threads resolved (0 unresolved). Of the 17 previously-unresolved threads, 14 are FIXED — each witness was re-run through the current detectors and is now rejected (
usesOutboundNetwork/usesRuntimeCodeGeneration= true), covered by the committed regression suite. BLOCKING_CURRENT_IN_BOUNDARY_COUNT = 0 — no current, in-boundary blocking egress defect remains.Explicit carried obligations (non-blocking, deferred)
globalThis.valueOf().fetch(…): CURRENT / P2, but outside the frozen finite boundary. Sound closure needs CallExpression / intrinsic method-return semantic modeling (an open-ended family, defeated by monkey-patching), which the adopted bounded source policy excludes. Deferred; the.call/.applyidentity variants are already closed by the forwarding rule.import type { request }+typeof requestis over-rejected: CURRENT / P3, a false-positive precision item (not an egress path), used by no production host source. Deferred.typeof EventSource === 'undefined'unless--experimental-eventsource, which the cockpit-host/CI launch paths do not enable). The finite network-global surface tracks only runtime-present importless globals; adding it on the@types/nodedeclaration alone would exceed the actual runtime. Flag-conditional / deferred.All in-boundary blocking findings have been repaired; remaining items are the explicitly-scoped carried obligations above. This PR remains unmerged.