refactor(server): isolate gateway listener context - #2542
Conversation
|
Label |
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This PR is project-valid as a small gateway/server refactor supporting the validated callback-endpoint work in #2215.
Head SHA: e0b68166fc1f398028fdaa7771917218f291f86f
Review findings:
- One warning remains about wildcard-covered callback addresses inheriting
Primarylistener scope if this scope becomes an authorization boundary.
Docs: Not needed; this is internal listener plumbing with no direct user-facing CLI, config, API, or Fern navigation change.
Checks: DCO and vouch are passing; Branch Checks are still pending. test:e2e is applied and the E2E workflow help says the existing run needs to be re-run to execute with that label.
Next state: gator:in-review
| for address in extra_addresses { | ||
| if !specs | ||
| .iter() | ||
| .any(|existing| listener_covers(existing.address, *address)) |
There was a problem hiding this comment.
gator-agent
Warning: listener_covers collapses a driver callback address covered by a wildcard primary listener into Primary scope. If GatewayListenerScope becomes an authorization boundary, callbacks to the covered address will be classified as primary traffic. Please either track covered callback addresses and derive scope from the accepted socket/local address, or explicitly document/assert that covered addresses intentionally inherit Primary scope.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
26dd8a8 to
a8b41f9
Compare
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This PR remains project-valid as a small gateway/server refactor supporting the validated callback-endpoint work in #2215.
Head SHA: a8b41f909ef3c7503e8bcfcbc1a34d64ae8b9f4d
Review findings:
- One warning remains: covered compute-driver callback addresses are still collapsed into
Primarylistener scope when the primary listener is wildcard-bound.
Docs: Not needed; this is internal listener plumbing with no direct user-facing CLI, config, API, or Fern navigation change.
Checks: DCO and Helm Lint are passing; Branch Checks and test:e2e are still pending on the current head.
Next state: gator:in-review
| scope: GatewayListenerScope::Primary, | ||
| }]; | ||
| for address in extra_addresses { | ||
| if !specs |
There was a problem hiding this comment.
gator-agent
Warning: listener_covers skips a compute-driver callback address when it is covered by a wildcard primary listener, so connections to that callback IP will be tagged GatewayListenerScope::Primary. If this scope becomes an authorization boundary, callback traffic can be misclassified as primary traffic (CWE-863). Preserve the requested callback addresses on the bound wildcard listener and derive the effective scope from the accepted socket local address, for example by comparing stream.local_addr() against covered callback addresses before calling spawn_gateway_connection; otherwise explicitly document and assert that covered callback addresses intentionally inherit Primary.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This PR remains project-valid as a small gateway/server refactor supporting the validated callback-endpoint work in #2215.
Head SHA: 9d1d5351243fd822423bc9cf360df25f343a2904
Review findings:
- No blocking code-review findings remain. The independent reviewer confirmed that the prior wildcard-covered callback scope concern is addressed by
covered_addressesplus accepted-connectionlocal_addr()scope resolution.
Docs: Not needed; this is internal listener plumbing with no direct user-facing CLI, config, API, workflow, or Fern navigation change.
Checks: Branch Checks, Helm Lint, DCO, and vouch are passing. Required test:e2e is failing because e2e / E2E (rust-podman-rootless, ubuntu-24.04) aborts before the test body with ERROR: expected Podman 4.x, found 5.8.4. That appears to be an E2E runner/workflow environment blocker rather than a regression in this PR.
Next state: gator:blocked
Next action: a maintainer or CI owner needs to update or rerun the Ubuntu 24.04 rootless Podman E2E path so the required E2E gate can complete for this head.
Summary
Isolate gateway listener selection and binding from the server entry point before adding driver-requested listener discovery. Preserve each listener's authorization scope through connection serving without changing which addresses are bound or which requests are accepted.
Related Issue
Supports #2215
Depends on #2544
Changes
BoundGatewayListenerTesting
mise run pre-commitpassesFocused validation:
cargo test -p openshell-server gateway_listenerChecklist