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".
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 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 437f74777b
ℹ️ 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".
| // (2) an approved propagating local function (the parameter was tainted in Phase A) | ||
| if (resolveLocalFunction(p.expression) !== null) return true; |
There was a problem hiding this comment.
When a proven request/response is passed to a local function whose matching parameter is a rest or destructuring parameter, Phase A deliberately does not propagate the capability, but this branch still approves the argument escape solely because the callee resolves locally. For example, function use(...values) { const req = values[0]; const s = req[(req.url ?? '').slice(1)]; ... } called as use(req) from a createServer listener makes usesOutboundNetwork return false; a /socket request can then destroy and reconnect the inbound socket outbound. Only allow the call when the corresponding parameter was actually classified, or fail closed for unsupported parameter shapes.
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| // ---- Phase A — bounded fixpoint over provenance (roots, aliases, factories, propagation). ---- | ||
| for (let iter = 0; iter < D3_ITERATION_CAP; iter++) { |
There was a problem hiding this comment.
When capability propagation requires more than 64 iterations, this loop exits without distinguishing convergence from exhaustion and Phase B proceeds with incomplete provenance. With more than 64 local forwarding functions declared in reverse source order, each pass advances the request capability only one hop; the final function parameter remains unclassified, so a runtime-key socket read and reconnect there makes usesOutboundNetwork return false. If the last permitted iteration still changes the state, reject the source rather than using the partial result.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5b7c3c974
ℹ️ 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".
| // (2) an identifier value read: HTTP_CLIENT is forbidden; HTTP_NS must sit in a safe | ||
| // position (else escape); a FREE network global (`fetch`/`WebSocket`) is rejected. | ||
| if (ts.isIdentifier(node) && isBinderValueReference(node)) { | ||
| const symbol = checker.getSymbolAtLocation(node); |
There was a problem hiding this comment.
When a capability is forwarded through a shorthand property, such as const client = { fetch }; client.fetch(url) or const client = { request }; client.request(url), checker.getSymbolAtLocation(node) resolves the shorthand property symbol rather than its value symbol. The capability is therefore classified as local/NONE, and the later member call is accepted while performing outbound egress. Use checker.getShorthandAssignmentValueSymbol, as the provenance pass already does for shorthand reads.
Useful? React with 👍 / 👎.
| // grant. This is a closed two-name family on the installed Node 24 API (`ServerOptions.IncomingMessage` | ||
| // / `ServerOptions.ServerResponse`); reserving the option KEYS at the createServer acquisition site | ||
| // closes it with no class-body, constructor, or alias inspection. | ||
| const CONSTRUCTOR_INJECTION_OPTIONS: ReadonlySet<string> = new Set(['IncomingMessage', 'ServerResponse']); |
There was a problem hiding this comment.
On the targeted Node 24.12 runtime, the shouldUpgradeCallback create-server option is invoked with the upgrade request's IncomingMessage, but the option scan reserves only the two constructor-injection keys and never classifies this callback parameter as a request. Consequently, an options object can define this callback, derive a runtime key from req.url, recover the inbound socket through req[key], and reconnect it outbound while usesOutboundNetwork returns false. Treat this option as another request-delivery root or reject it at the boundary.
Useful? React with 👍 / 👎.
| if (ts.isFunctionDeclaration(decl)) { | ||
| return decl.body !== undefined && !isInAmbientContext(decl) ? decl : null; |
There was a problem hiding this comment.
When a local function declaration is reassigned through an assertion such as (use as unknown as Fn) = replacement, TypeScript and the configured ESLint accept the write, but this resolver still returns the original declaration. Phase A therefore taints the original parameter and Phase B permits use(req), while at runtime the unchecked replacement receives the request and can recover its socket through a runtime key; the detector returns false for this path. Prove the declaration has no writes or restrict approved callees to immutable const function expressions.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5b7c3c974
ℹ️ 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".
| return; | ||
| } | ||
| if (ts.isArrowFunction(source) || ts.isFunctionExpression(source) || ts.isFunctionDeclaration(source)) { | ||
| for (const param of source.parameters) { |
There was a problem hiding this comment.
When a valid request listener declares an optional third parameter, this loop marks it as a request/response capability even though Node's requestListener supplies only request and response. For example, (req, res, extra?: any) => void extra?.[runtimeKey] is rejected by usesOutboundNetwork solely because extra is added to reqResSymbols, despite being undefined at runtime and providing no network authority. Track or scan only parameter indexes 0 and 1 here, matching the later provenance-root logic.
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.