[freeqaz]

Problem

Since #2980, Agent network-policy admission (preview, save, and the worker recheck before a live apply) validates grants only with the selected Sandbox Driver's sandbox-network-policy contribution. The shared SandboxNetworkPolicyConfiguration from #2599 (unique IDs, normalized executable paths, DNS labels, ordered port ranges) no longer runs in OCC. RFC-0022 requires a Sandbox Driver to contribute exactly that definition, but nothing checks it does.

With a package Sandbox Driver whose contribution is looser and whose admitNetworkPolicy does not recheck:

• preview (POST .../sandbox-policy/networking/validate) returns success: true for a grant with no port range on an unlisted host, or with a reversed range such as { from: 65535, to: 1 } on a delegated executable and host;
• save then fails late: PostgreSQL's sandbox_policies_document_valid check refuses the row, and the API answers 404 NOT_FOUND instead of field details (the in-memory store answers 500).

sandboxNetworkGrantsWithinCeiling is part of why preview passes: an empty port list or a reversed range counts as covered. Storage stays protected by the database check. The bundled OpenShell Driver repeats the shared rules itself and is unaffected.

Change

• admitAgentNetworkPolicy (preview and save) and reconcileSandboxPolicy (worker) run the shared grant rules first. A Driver contribution can then only narrow them.
• sandboxNetworkGrantsWithinCeiling refuses a grant that has no port range or a reversed one.
• The policy flow doc states the order.

For the bundled Driver, refusals and their error.details are unchanged, because its contribution is the shared definition.

Verification

• New PostgreSQL case in tests/integration/postgres-agent-sandbox-policy.test.mjs. It registers a looser package contribution through the existing createPolicyApp helper. It fails on main (preview reports success; save answers 404) and passes with the fix: preview returns the minItems detail, and save answers 409 with the INVALID_RANGE detail and keeps version 1. The whole file passes 4/4 locally against a disposable PostgreSQL.
• Ceiling assertions added to tests/conformance/contracts.test.mjs: 20/20 pass, and the new assertion fails on main.
• tsc --build, ESLint and Prettier pass on the touched files.

Risks

Low. Grants that the shared rules refuse could never be stored in PostgreSQL anyway. The only change in behavior is that such grants are now refused at preview and save with field details, instead of later with a 404.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR validates Agent network grants against shared rules before Driver contributions and rejects empty or reversed port ranges during ceiling checks.

Example: A loose Sandbox Driver accepts /usr/bin/python3.11 accessing example.org with ports [{"from":65535,"to":1}].

• Before: Preview can report success, while saving fails later at PostgreSQL validation with a misleading 404.
• After: Saving returns 409 with an INVALID_RANGE detail and leaves the saved policy unchanged.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	A focused, source-supported repair with relevant integration coverage; confidence is bounded by the supplied in-process evidence.
Proof confidence	🦐 gold shrimp (3/6)	The ordinary contributor proof gate excludes this collaborator-authored PR; its supplied validation is an in-process PostgreSQL regression report, and the inspected historical screenshots do not demonstrate this repair.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Policy preview can accept invalid grants that saving later rejects with a misleading resource error.
Reason: The maintainer-authored repair restores an existing RFC requirement and provides early field diagnostics without expanding the product contract.

Regression provenance

Regression provenance — suspected predecessor, not a causality claim: source commit 141515257737 by Derek Carr; no PR verified.

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it repairs a source-confirmed admission regression with a focused change and relevant PostgreSQL coverage; no actionable introduced defect was found.

Priority: P2
Reviewed head: 335b270341b8949597a872cd7b200535ccd72f60

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: The supplied regression uses Fastify injection with PostgreSQL; no shipped-server trace specific to the loose-Driver repair is supplied.

Agent review details

How this fits together

OCC admits proposed network grants before storing policy or deployment snapshots; the durable worker rechecks policy before invoking the selected Sandbox Driver.

flowchart TB
  A[Agent policy request] --> B[Exact resource authorization]
  B --> C[Shared grant validation]
  C --> D[Driver and ceiling admission]
  D --> E[PostgreSQL policy and work]
  E --> F[Worker authority and policy recheck]
  F --> G[Sandbox provider operations]

Loading

Technical review

Best possible solution:

Retain the shared validator in OCC admission and worker rechecks, with Driver contributions permitted to narrow the accepted grants.

Do we have a high-confidence way to reproduce the issue?

Current main can accept empty or reversed port ranges through a loose Driver contribution before PostgreSQL rejects persistence; the author reports a failing-main/passing-head regression, but this review ran no tests.

Is this the best way to solve the issue?

The patch restores the existing shared contract at its owning admission boundaries and hardens the common containment helper without adding a parallel validator or configuration option.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 4b019d47b58f.

Provenance checked

• Shared validation and Driver contribution admission keeps the original intent (774b3d6: Configuration discovery unified Driver definitions while RFC-0022 retained the existing sandbox grant rules; this repair preserves registry dispatch and reinstates their unconditional enforcement.)
• Network delegation containment keeps the original intent (1415152: Per-Agent additions must remain inside the Installation delegation ceiling; refusing empty and reversed ranges preserves that invariant.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Pinned introduced change: The introduced diff adds shared validation at OCC admission and worker recheck, hardens ceiling containment, and extends existing tests and the owning flow document. (packages/occ/src/index.ts:7126, 335b270341b8)
• Existing contract and regression provenance: RFC-0022 requires shared ID, path, DNS, and range rules. The merged registry refactor removed the direct shared-validator call from OCC admission; this PR restores that invariant while retaining contribution validation. (specs/rfcs/0022-driver-configuration-contributions.md:116, 774b3d6760f9)
• Existing-state compatibility: Existing PostgreSQL constraints already reject empty ports, reversed ranges, duplicate IDs, invalid executable paths, and invalid DNS names; application snapshots must contain the saved additions. The PR changes neither these constraints nor serialized policy shape. (migrations/0058_sandbox_policies.sql:5, 335b270341b8)
• Target-native regression coverage: The added case exercises actual Fastify routes, OCC, Native IAM, and PostgreSQL with a deliberately loose Driver boundary, checking field diagnostics and unchanged persisted policy. The existing file is registered in the PostgreSQL CI lane. The author reports failure on main and success after the fix; tests were not executed during this read-only review. (tests/integration/postgres-agent-sandbox-policy.test.mjs:428, 335b270341b8)
• Security boundary: The worker retains exact-Agent and exact-policy reauthorization, lifecycle and ceiling checks, and Driver identity checks before provider I/O; the added shared validation executes before observeNetworkPolicy or applyNetworkPolicy. (apps/controller/src/worker/sandbox-policy.ts:194, 335b270341b8)
• Discussion and inspected media: The four prepared screenshots belong to feat: add per-agent sandbox network policies and denial review #2599 and show historical simulated policy UI states. They do not exercise this PR's loose-Driver validation repair. This PR's current body supplies regression-test results rather than new runtime media.

Likely related people:

• Kevin: Raw commit 774b3d6 adds packages/contracts/src/sandbox-policy.ts:111 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 774b3d6760f9; files: packages/contracts/src/sandbox-policy.ts)
• Derek Carr: Raw commit 1415152 adds packages/contracts/src/sandbox-policy.ts:92 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 141515257737; files: packages/contracts/src/sandbox-policy.ts)
• Free Wortley: Raw commit c8f73c8 adds docs/flows/agent-sandbox-policy.md:141 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: c8f73c8b1d7a; files: docs/flows/agent-sandbox-policy.md)

Review metrics

Metric	Value	Why it matters
Change size	Production +9/-0; tests +70/-2; documentation +10/-7	Small production changes reuse existing owners; most growth establishes the loose-Driver regression scenario.

Labels

Label changes:

• add P2: The failure affects admission diagnostics with a loose package Driver; PostgreSQL protects stored policy and bundled OpenShell already applies the shared rules.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P2: The failure affects admission diagnostics with a loose package Driver; PostgreSQL protects stored policy and bundled OpenShell already applies the shared rules.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 22, 2026, 9:47 AM ET / 13:47 UTC.