feat(policy): establish direct TCP egress foundation - #2711
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-2711.docs.buildwithfern.com/openshell |
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: Project-valid as the explicitly authorized first delivery slice of accepted RFC #2155 and the authoritative policy-DNS/transparent-TCP series plan. This review evaluates only stage-1 responsibilities and does not require DNS, mapping-store, capture, or native transparent-TCP functionality deferred to later PRs.
Head SHA: c9ee490d4c2ea7ae7e93a1eee723425022414fed
Base SHA: 0310cbed6c809e8950fc513d0a25c2ec03946198
Merge base SHA: 0310cbed6c809e8950fc513d0a25c2ec03946198
Patch ID: 64c0cd43a0b601c291aab47f2c53c7bd1a8217f6
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no
Blocking findings:
GATOR-c9ee490d-01: The authoritative authorization result and ambiguity model must preserve explicit TCP eligibility and a generation-stable matched endpoint identity, even though explicit TCP and omitted protocol remain equivalent for CONNECT.GATOR-c9ee490d-02: A pinned resolved-address destination mode must construct an unopened connector from policy-DNS-pinned addresses without performing connect-time DNS resolution.GATOR-c9ee490d-03: Explicit protocol: tcp must reject fields that promise L7-only enforcement or transformation rather than accepting and silently ignoring them.GATOR-c9ee490d-04: Published policy and CLI reference documentation must list protocol: tcp and accurately describe its current L4-only constraints when the public parser accepts it.
Carried findings:
- None
Non-blocking suggestions:
- None
Docs: Missing canonical Fern updates for the newly accepted public protocol: tcp policy and CLI syntax.
Next state: gator:in-review
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
Re-check After Author Update
Thanks @johntmyers. I reviewed the new implementation commit and your note that the Fern documentation is being deferred to a separate branch. I checked the stage-1 code obligations against the authoritative policy-DNS/transparent-TCP series plan and checked the canonical published policy and CLI pages.
Validation: This remains project-valid as PR 1 of the authorized series. DNS, mapping-store, capture, and native transparent-TCP runtime functionality remain correctly deferred to later PRs.
Head SHA: 2dedd9c4323f4378fac828232bc8c44b3d7a8e5b
Base SHA: 0310cbed6c809e8950fc513d0a25c2ec03946198
Merge base SHA: 0310cbed6c809e8950fc513d0a25c2ec03946198
Patch ID: 544beca42375b85776ff2da0101389b02c7d4998
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: c9ee490d4c2ea7ae7e93a1eee723425022414fed
Review budget exhausted: no
Maintainer decision required: no
Resolved findings:
GATOR-c9ee490d-01: the authoritative result now preserves matched endpoint identity and explicit TCP eligibility, with ambiguity coverage.GATOR-c9ee490d-02: the pinned-address validation plan now constructs an unopened connector without DNS resolution and is covered by deterministic tests.GATOR-c9ee490d-03: TCP activation now rejects the represented L7-only field family across policy and provider-profile paths.
Blocking findings:
GATOR-c9ee490d-04remains open.docs/reference/policy-schema.mdxstill instructs users to omit the protocol for TCP passthrough, whiledocs/sandboxes/policies.mdxstill omitstcpfrom the values accepted bypolicy update. Because this PR directly ships the public policy and CLI syntax and the stage-1 plan requires documenting its current L4-only semantics, a separate in-flight docs branch does not make this PR's published contract accurate and is not a maintainer waiver. Please update those two existing Fern pages here, or obtain an explicit maintainer waiver for deferring them. No navigation change is needed.
Carried findings:
GATOR-c9ee490d-04: canonical Fern policy and CLI documentation must describe explicitprotocol: tcpand its current L4-only constraints.
Docs: Missing for the direct public policy/CLI syntax change.
Next state: gator:in-review
|
@ gator, as mentioned in the reply to your comment we will defer the docs. #2711 (comment) |
|
Label |
Maintainer Review NudgeThis PR has been in @NVIDIA/openshell-maintainers @NVIDIA/openshell-codeowners @mrunalp @maxamillion @derekwaynecarr, can someone review and either approve, request changes, or close this out? |
Author Follow-Up NudgeThis PR has been in @johntmyers, please either update |
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
2dedd9c to
d73b4c5
Compare
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: Project-valid as stage 1 of the accepted policy-DNS/transparent-TCP series. This review is bounded to the authoritative stage-1 plan and does not require DNS, mapping-store, capture, resolver-injection, or native transparent-TCP functionality deferred to later PRs.
Head SHA: d73b4c5b25a99c1a309b8bf1b35479d4a0ece1ab
Base SHA: 7547edc7ffda6d8a18cf164ea0f4e8d9966f1dbe
Merge base SHA: 7547edc7ffda6d8a18cf164ea0f4e8d9966f1dbe
Patch ID: d9fa17fd02e3aeb13bd9caba86a7b3920f266c68
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: 2dedd9c4323f4378fac828232bc8c44b3d7a8e5b
Review budget exhausted: no
Maintainer decision required: no
Blocking findings:
- No new blocking findings in the author-only rebase delta.
Carried findings:
GATOR-c9ee490d-04remains open: the canonical Fern policy schema and policy-update documentation must describe explicitprotocol: tcp, its current equivalence to omitted L4 CONNECT behavior, and its rejected L7-only fields, or a maintainer must explicitly waive deferral. @johntmyers's earlier note that the docs would be deferred does not itself provide that maintainer waiver.
Docs: docs/reference/policy-schema.mdx and docs/sandboxes/policies.mdx remain inconsistent with the accepted public policy and CLI syntax. No navigation change is needed.
Next state: gator:in-review
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
Re-check After Author Update
Thanks @johntmyers. I checked the current-head documentation update against the remaining stage-1 obligation and reviewed the latest author-only delta under the required critical-only convergence mode.
Validation: This remains project-valid as stage 1 of the accepted policy-DNS/transparent-TCP series. DNS service, mapping-store, capture, resolver injection, and native transparent-TCP functionality remain correctly deferred to later PRs.
Head SHA: 94c3804056d2c8f496100352b5b0bb76667ac829
Base SHA: 7547edc7ffda6d8a18cf164ea0f4e8d9966f1dbe
Merge base SHA: 7547edc7ffda6d8a18cf164ea0f4e8d9966f1dbe
Patch ID: 9f6f6956a26f278c942bf766f74b07286659d204
Gator payload: 4
Review mode: critical_only
Previous reviewed SHA: d73b4c5b25a99c1a309b8bf1b35479d4a0ece1ab
Review budget exhausted: yes
Maintainer decision required: no
Blocking findings:
- No new Critical findings in the latest author-only delta.
Resolved findings:
GATOR-c9ee490d-04:docs/reference/policy-schema.mdxanddocs/sandboxes/policies.mdxnow document explicitprotocol: tcp, its current L4-only equivalence to omitted protocol, and the incremental CLI syntax.
Carried findings:
- None. The earlier implementation findings remain resolved, and no unresolved or unwaived obligation remains.
Docs: Canonical Fern policy and CLI documentation is updated; no navigation change is needed.
Tests: The existing test:e2e label is present, and current-head Branch Checks and Branch E2E workflows are dispatched.
Next state: gator:watch-pipeline
Summary
Establish the policy and proxy foundations for RFC 0005's direct DNS and transparent TCP work without enabling either runtime path yet. Explicit
protocol: tcpis accepted as the current L4 host/port behavior, while proxy authorization now returns one generation-consistent decision snapshot that later adapters can consume safely.Related Issue
Implements the first delivery slice of the accepted RFC in #2155. The RFC was merged without a separate tracking issue.
Changes
protocol: tcpin policy YAML, profile validation, ambiguity checks, Rego evaluation, and incrementalpolicy updateparsingTesting
mise run pre-commitpassesmise run docspassesmise run testpassesChecklist