fix(supervisor-network): L7 endpoint validation edge cases#2464
fix(supervisor-network): L7 endpoint validation edge cases#2464andrewwhitecdw wants to merge 2 commits into
Conversation
…orts validate_l7_policies assumed every endpoint entry was a JSON object and used ep.as_object_mut().unwrap() in expand_access_presets. A non-object endpoint would panic. Add an object-shape validation error and replace the unwraps with safe pattern matching. Also filter zero values out of the ports array to match the scalar port validation (which already rejects port == 0). Signed-off-by: Andrew White <andrewh@cdw.com>
…arser network_endpoint_from_json accepted port 0 from the ports array without filtering, creating an endpoint with no usable ports. Drop zero entries so the array behaves consistently with the scalar port field. Signed-off-by: Andrew White <andrewh@cdw.com>
|
I have read the DCO document and I hereby sign the DCO. |
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
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.
| |arr| { | ||
| arr.iter() | ||
| .filter_map(serde_json::Value::as_u64) | ||
| .filter(|p| *p > 0) |
There was a problem hiding this comment.
gator-agent
Warning: This filters only a temporary vector whose sole consumer already checks any(|port| *port > 0). Consequently, [0] was already rejected, while [0, 443] still passes and the zero remains in the JSON supplied to OPA. Please either reject any zero-valued ports member or remove zeros during endpoint normalization/proto serialization, then cover all-zero and mixed arrays with regression tests.
johntmyers
added
the
gator:in-review
2 participants
Summary
Two small robustness fixes in L7 network policy handling:
validate_l7_policiesassumed every entry innetwork_policies.<name>.endpointswas a JSON object, andexpand_access_presetscalledep.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.portfield was already validated to be> 0, but theportsarray accepted0values 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; replaceas_object_mut().unwrap()with safe matching; filterportsarray to> 0policy_local.rs: filter zero entries fromendpoint.portsTesting
mise run pre-commitpasses (mise unavailable in this environment; ran equivalentcargo fmt+cargo clippy -p openshell-supervisor-network --all-targets— clean)cargo test -p openshell-supervisor-network --lib— 969 passed)Checklist