fix(supervisor-network): L7 endpoint validation edge cases

[andrewwhitecdw]

Summary

Two small robustness fixes in L7 network policy handling:

1. validate_l7_policies assumed every entry in network_policies.<name>.endpoints was a JSON object, and expand_access_presets called ep.as_object_mut().unwrap(). A malformed non-object endpoint caused a panic. This change adds an object-shape validation error and replaces the unwraps with safe pattern matching.
2. The scalar port field was already validated to be > 0, but the ports array accepted 0 values silently. The same inconsistency existed in the agent-proposal endpoint parser. Both paths now filter out zero ports.

Related Issue

N/A — small fixes found during code review.

Changes

• l7/mod.rs: validate endpoint is an object; replace as_object_mut().unwrap() with safe matching; filter ports array to > 0
• policy_local.rs: filter zero entries from endpoint.ports

Testing

• mise run pre-commit passes (mise unavailable in this environment; ran equivalent cargo fmt + cargo clippy -p openshell-supervisor-network --all-targets — clean)
• Unit tests pass (cargo test -p openshell-supervisor-network --lib — 969 passed)
• E2E tests added/updated (if applicable)

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[andrewwhitecdw]

I have read the DCO document and I hereby sign the DCO.

[johntmyers]

gator-agent

PR Review Status

Validation: This is project-valid small, concentrated work in supervisor network-policy handling: it replaces a malformed-input panic with fail-closed validation and attempts to make zero-port handling consistent. No duplicate work was found.
Head SHA: 583f62ef7019a4c54b23fe91e6ef289c2ca02d4a

Review findings:

• One correctness issue is anchored inline: the raw-policy zero-port filter does not normalize the policy data later supplied to OPA, so mixed arrays still retain zero.
• Focused regression coverage is missing for all three changed contracts. Please add unit tests showing that a non-object endpoint reports the expected validation error and preset expansion remains panic-free; raw-policy [0] and [0, 443] follow the intended contract; and agent proposals reject an all-zero array while retaining only positive entries from mixed arrays, including scalar fallback.

Docs: Fern docs are not needed for this malformed-input robustness fix; the existing policy reference already describes endpoint objects and concrete MCP ports.

E2E: The changed network-policy path requires test:e2e after review findings are resolved.

Next state: gator:in-review pending an author update.

[johntmyers]

@andrewwhitecdw Several of your PRs are awaiting feedback, please let us know if you plan on looking so we can plan accordingly

[johntmyers]

gator-agent

PR Review Status

Validation: This remains project-valid small, concentrated robustness work in supervisor network-policy handling.
Head SHA: 200ecd2f0444b5ef60d33c2332990c546e521dc5

Thanks @andrewwhitecdw. I checked the update you pushed after the earlier zero-port normalization and test feedback. The raw-YAML mixed-array case is now normalized and the added helper tests cover that path, but the production proto load/reload path still bypasses the new normalizer.

Review findings:

• One critical correctness/security-boundary issue is anchored inline: zero ports still reach OPA through OpaEngine::from_proto.
• One warning is anchored inline: the separate agent-proposal parser behavior still needs focused regression coverage.

Docs: Fern docs are not needed because this hardens malformed input without changing supported policy syntax or commands.

E2E: The network-policy path still requires test:e2e after review findings are resolved.

Next state: gator:in-review pending an author update.

[andrewwhitecdw]

Thanks for the nudge — I’ve gone through all five open PRs:

• fix(core): bracket IPv6 bind addresses in forward arg and access URL #2449: reused bracket_ipv6() in format_gateway_url() and added an IPv6 example to the Port Forwarding docs.
• fix: assorted numeric overflow fixes #2462: tightened Docker/Podman numeric boundary checks so inputs rounding to 2^63/2^64 are rejected instead of saturating, and replaced saturating_mul in the Podman health-check interval.
• fix: assorted small validation and robustness fixes #2450: made short_hash() slice on character boundaries, switched the policy revision table to use it, and added tests for the ephemeral-port boundary.
• fix(supervisor-network): L7 endpoint validation edge cases #2464 (this PR): normalized zero ports before OPA and added regression tests for non-object endpoints and mixed/all-zero port arrays.
• fix(cli): avoid panic on multi-byte UTF-8 in --since duration #2446: no actionable gator findings; I replied to elezar’s question about using an external duration crate.

All relevant test suites pass. Please re-review when you have a chance.

[andrewwhitecdw]

Both review items addressed:

• Critical (opa.rs): proto_to_opa_data_json now filters zero ports in the production path, same as the YAML preprocess path — ports: [0, 443] → [443], zero-only arrays fall back to a positive scalar port, and a zero scalar port is not promoted. Zero ports can no longer reach OPA from OpaEngine::from_proto.
• Warning (policy_local.rs): added proposal_chunks_from_body tests for mixed [0, 443], zero-only [0] rejection, and [0] with positive scalar fallback, asserting the resulting port/ports.

Verified: cargo test -p openshell-supervisor-network --lib → 977 passed.

[johntmyers]

gator-agent

PR Review Status

Validation: This remains project-valid small, concentrated robustness work in supervisor network-policy handling.
Head SHA: ed4a4b63649a5857e2405a8f72bbb62f99a93282

Thanks @andrewwhitecdw. I checked your update about filtering zero ports in the production proto-to-OPA path and adding focused proposal-parser coverage. Those prior gaps are addressed, but the independent review found two remaining normalization cases that can reinterpret malformed or zero-only authorization input as a scalar-port allow; both are anchored inline. A focused preset-expansion regression gap is also anchored inline.

Review findings:

• Two blocking CWE-20 warnings are anchored inline: malformed JSON array elements can be discarded before scalar fallback, and zero-only proto arrays can activate a shadowed scalar despite documented ports precedence.
• One focused test gap is anchored inline for the removed expand_access_presets unwraps.

Docs: Fern docs are not needed for the intended malformed-input robustness fix. If scalar fallback for an explicitly present zero-only ports array is intentional, the schema and proto precedence contract would need updating.

E2E: The network-policy path still requires test:e2e after review findings are resolved.

Next state: gator:in-review pending an author update.

[johntmyers]

gator-agent

Author Follow-Up Nudge

This PR has been in gator:in-review for more than 48 business hours with unresolved review feedback.

@andrewwhitecdw, please respond to the two blocking review comments about malformed array elements and zero-only proto-array precedence, or push an update. If this is no longer planned, please say so and a maintainer can close it out.

[johntmyers]

gator-agent

Author Follow-Up Nudge

This PR has been in gator:in-review for more than 48 business hours with unresolved review feedback.

@andrewwhitecdw, please respond to the two blocking review comments about malformed array elements and zero-only proto-array precedence, or push an update. If this is no longer planned, please say so and a maintainer can close it out.

[johntmyers]

/ok to test 805f189

[johntmyers]

gator-agent

PR Review Status

Validation: This remains project-valid small, concentrated robustness work in supervisor network-policy handling.
Head SHA: 805f189c9a35f241bcbf3395bbd0cea1bfaa5d7a
Base SHA: 0c7e59a95355cabc15ccaddb86fcbe6a1d30eaaa
Merge base SHA: 21da343c9f838bd9ac85dc61bf44889de1a72873
Patch ID: d8044cf19d18d7dd4ad2547d4467f81feea7a843
Gator payload: 3
Review mode: follow_up
Previous reviewed SHA: ed4a4b63649a5857e2405a8f72bbb62f99a93282

Thanks @andrewwhitecdw. I checked the update pushed after the malformed-array and zero-only proto-array feedback. The raw-policy normalizer now preserves malformed non-numeric entries for fail-closed validation, the proto conversion preserves explicit ports precedence, and focused regression coverage now exercises both cases plus preset expansion with a non-object endpoint.

Blocking findings:

• No blocking findings remain.

Carried findings:

• gator-inline-3679152284: resolved by preserving non-numeric array entries through normalization.
• gator-inline-3679152285: resolved by branching on whether the original proto ports array is present.

Docs: Fern docs are not needed because this hardens malformed input without changing supported policy syntax or user workflows.

E2E: test:e2e is applied. The mirror was absent, so an authorized maintainer command /ok to test 805f189c9a35f241bcbf3395bbd0cea1bfaa5d7a was posted.

Next state: gator:watch-pipeline

[github-actions]

Label test:e2e applied for 805f189. Open Branch E2E Checks, find the run for commit 805f189, and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

[johntmyers]

/ok to test 805f189

[andrewwhitecdw]

Pushed an additional fix (7d2795a) for the YAML-normalization path in normalize_endpoint_ports:

• A present ports array now takes precedence over scalar port, matching the proto-serialization path fixed in 805f189. Previously {port: 8080, ports: [0]} would filter the array to empty and then promote 8080; it now leaves ports: [] so downstream validation fails closed.
• Updated normalize_endpoint_ports_empty_array_after_filtering to assert the new precedence behavior.

The previously-flaky connect_handler_returns_ssrf_403_for_internal_address_not_503 test now passes locally; the only remaining local failure is test_dns_failure_returns_error, which fails because this-host-does-not-exist.invalid resolves to an internal address in this environment and is unrelated to these changes.

[johntmyers]

gator-agent

PR Review Status

Validation: This remains project-valid small, concentrated robustness work in supervisor network-policy handling.
Head SHA: 7d2795a07ebfff47d7d3385d7d73149bfe83c5c7
Base SHA: d85339d621e0e96697499a9d4c8780ee9b9c1324
Merge base SHA: 21da343c9f838bd9ac85dc61bf44889de1a72873
Patch ID: 1b46948db13a9f420d22f80cc84f0e36b8af8828
Gator payload: 3
Review mode: follow_up
Previous reviewed SHA: 805f189c9a35f241bcbf3395bbd0cea1bfaa5d7a

Thanks @andrewwhitecdw. I checked your update making an explicitly present YAML ports array take precedence over scalar port. The focused delta now matches the proto serialization path, preserves fail-closed handling for a zero-only array, and does not invalidate the prior review resolutions.

Blocking findings:

• No blocking findings remain.

Carried findings:

• gator-inline-3679152284: remains resolved; malformed non-numeric entries are preserved for fail-closed validation.
• gator-inline-3679152285: remains resolved; both YAML and proto paths now preserve explicit ports precedence.

Docs: Fern docs are not needed because this hardens malformed input without changing supported policy syntax or user workflows.

E2E: test:e2e is applied. The current head requires authorized test execution.

Next state: gator:watch-pipeline

[johntmyers]

/ok to test 7d2795a

[johntmyers]

gator-agent

Author Follow-Up Nudge

This PR has been in gator:in-review for more than 48 business hours and the required Branch Checks still need an author update.

@andrewwhitecdw, please fix the Clippy nonminimal_bool failure at crates/openshell-supervisor-network/src/opa.rs:1131 (the check suggests p.as_u64().is_none_or(|n| n != 0)) and push a new commit so the required checks can rerun.

[johntmyers]

gator-agent

Review Convergence Checkpoint

Head SHA: 3f451232644a3bc0ed1ade72a1d53a9ba84b0c81
Base SHA: d85339d621e0e96697499a9d4c8780ee9b9c1324
Merge base SHA: 21da343c9f838bd9ac85dc61bf44889de1a72873
Patch ID: aba86ebabccc7c0f27b4f86ab62c0fc87dacf748
Gator payload: 3

Three finding-bearing review rounds have completed.

Thanks @andrewwhitecdw. I checked your latest update replacing the Clippy-rejected boolean expression at opa.rs:1131; the new expression is behaviorally equivalent, clears the reported lint shape, and the bounded checkpoint review found no newly introduced Critical defect.

Root-cause findings:

• gator-inline-3647350480, gator-inline-3668842601, and gator-inline-3668842604: zero-port normalization across raw policy, production proto-to-OPA conversion, and agent proposals, plus focused coverage; addressed by the later production-path and parser changes.
• gator-inline-3679152284 and gator-inline-3679152285: malformed array elements must remain fail-closed and an explicit ports array must retain precedence over scalar port; resolved by the 805f189c and 7d2795a0 updates.
• gator-inline-3679152289: preset expansion regression coverage; addressed by the focused non-object endpoint test and remained non-blocking.

Scope growth:

• None. The remediation remained within supervisor network-policy normalization and its focused tests.

Reviewer-quality signals:

• No duplicate finding IDs, waived-finding re-raises, or unchanged-code proposals were recorded. The checkpoint reviewer proposed no findings.

Maintainer action: accept the current scope, split follow-up work, waive a finding, or explicitly authorize another autonomous review round. Given the resolved obligations and no new Critical finding, accepting the current scope will allow gator to resume pipeline monitoring.

Next state: gator:blocked

[johntmyers]

/ok to test 3f45123

[johntmyers]

/ok to test e71e6af

[johntmyers]

gator-agent

Review Convergence Checkpoint

Head SHA: e71e6af814aae046be1b93fc31e8818ff214aedb
Base SHA: 0310cbed6c809e8950fc513d0a25c2ec03946198
Merge base SHA: 0310cbed6c809e8950fc513d0a25c2ec03946198
Patch ID: aba86ebabccc7c0f27b4f86ab62c0fc87dacf748
Gator payload: 3

Three finding-bearing review rounds have completed.

Thanks @andrewwhitecdw. I checked the force-rebased current head and the final Clippy-motivated boolean rewrite. The author-series range-diff preserves the six previously reviewed commits, the current patch ID matches the prior checkpoint patch, and the bounded checkpoint reviewer found no newly introduced Critical defect.

Root-cause findings:

• gator-inline-3647350480, gator-inline-3668842601, and gator-inline-3668842604: zero-port normalization across raw policy, production proto-to-OPA conversion, and agent proposals, plus focused coverage; addressed by the later production-path and parser changes.
• gator-inline-3679152284 and gator-inline-3679152285: malformed array elements must remain fail-closed and an explicit ports array must retain precedence over scalar port; resolved by the later normalization updates.
• gator-inline-3679152289: preset expansion regression coverage; addressed by the focused non-object endpoint test and remained non-blocking.

Scope growth:

• None. The remediation remained within supervisor network-policy normalization and focused tests.

Reviewer-quality signals:

• No duplicate finding IDs, waived-finding re-raises, unchanged-code proposals, or Critical/Warning proposals lacking a reproducer. The checkpoint reviewer proposed no findings.

CI:

• test:e2e is applied, authorized test execution was posted for this head, and the required Branch Checks, Helm Lint, and E2E gates are green.

Maintainer action: accept the current scope, split follow-up work, waive a finding, or explicitly authorize another autonomous review round. Given the resolved obligations and no new Critical finding, accepting the current scope will allow gator to resume pipeline monitoring.

Next state: gator:blocked

[johntmyers]

gator-agent

Merge Decision Nudge

This PR has been in gator:merge-ready for more than 48 business hours with maintainer approval present and no merge or close decision.

@NVIDIA/openshell-maintainers @NVIDIA/openshell-codeowners @mrunalp @maxamillion @derekwaynecarr, can someone merge this PR or close/request changes if it should not proceed?

[andrewwhitecdw]

Closing this sweep-generated PR: PR has 7 commits; sweep requires exactly one commit per PR. It does not meet the sweep requirements (single signed-off commit).

[johntmyers]

gator-agent

Monitoring Complete

Head SHA: e71e6af814aae046be1b93fc31e8818ff214aedb
Gator payload: 4

Monitoring is complete because this PR has been closed without merge.

Final status: The durable finding ledger records all review findings addressed or non-blocking, no remediation scope growth, and no newly introduced Critical defect. Required Branch Checks, Helm Lint, and E2E status contexts were green, and maintainer approval was present. The author closed the PR because the sweep requires exactly one signed-off commit and this PR contains seven commits.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.