fix(network): refuse protocol upgrades on GraphQL endpoints - #3841
Conversation
Refuse Upgrade headers before forwarding GraphQL-over-HTTP requests. Share the protocol refusal table with JSON-RPC and MCP, and close unexpected protocol switches before relaying frames. Keep GraphQL-over-WebSocket inspection on separate WebSocket endpoints. Cover upgrade refusal, audit mode, subscription handshakes, and ordinary HTTP and WebSocket controls. Update the current policy documentation. Signed-off-by: Shiju <shiju@nvidia.com>
Preserve upstream architecture documentation removal and retain GraphQL upgrade guidance in the published policy pages. Signed-off-by: Shiju <shiju@nvidia.com>
johntmyers
added
the
gator:in-review
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
A sandboxed workload using the supported single-endpoint GraphQL path can send an upgrade request with an oversized or incomplete body; the relay inspects that body before applying this PR's new refusal, so it can return no documented 403 or wait for missing bytes instead of rejecting immediately.
Action required: move the single-endpoint GraphQL upgrade check ahead of body inspection and add the regression test described inline.
Blocking findings:
GATOR-e1e76599-01: the single-endpoint GraphQL relay rejects upgrades too late
Carried findings:
- None
Non-blocking suggestions:
- None
Gator metadata
- Validation: Project-valid localized network-policy security correction from a repository maintainer
- Docs: Fern policy documentation updated for the changed user-visible behavior
- Checks: Existing current-head required checks are green; E2E has not been dispatched
- E2E:
test:e2ewill be required after review feedback is resolved - Head SHA:
e1e7659978329b83574a80617f610ac0e1aa7234 - Base SHA:
33a8eac196a21aff3125dea46462a4591bcb5d0b - Merge base SHA:
33a8eac196a21aff3125dea46462a4591bcb5d0b - Patch ID:
41a19ef33300612608f9343b4d6286b40ddd450d - Gator payload:
9 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
Validate the HTTP head and endpoint authority before upgrade refusal, then inspect ordinary GraphQL bodies. Preserve missing-authority credential rejection after body inspection. Signed-off-by: Shiju <shiju@nvidia.com>
|
Label |
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @shiju-nv. I checked the new single-endpoint GraphQL flow and its withheld-body regressions: the relay now validates the request head and endpoint authority, refuses unsupported upgrades before reading the body, and preserves ordinary GraphQL body inspection. The prior finding is resolved, and the follow-up review found no new blockers.
Blocking findings:
- No blocking findings remain
Carried findings:
GATOR-e1e76599-01: resolved by the current head; the Gator-owned thread has been closed
Gator metadata
- Validation: Project-valid localized network-policy security correction from a repository maintainer
- Docs: Relevant Fern policy documentation is updated
- Checks: Existing required checks are green; the required current-head E2E attempt is running
- E2E:
test:e2eapplied; Branch E2E Checks run36686663426, attempt 2, is in progress - Head SHA:
71346cb1834e3ffe41cf0b02d613a4ca43747a0a - Base SHA:
33a8eac196a21aff3125dea46462a4591bcb5d0b - Merge base SHA:
33a8eac196a21aff3125dea46462a4591bcb5d0b - Patch ID:
bf5e22f167555d7086b52f2389d02f8cf3739512 - Gator payload:
9 - Review mode:
follow_up - Previous reviewed SHA:
e1e7659978329b83574a80617f610ac0e1aa7234 - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:watch-pipeline
johntmyers
added
gator:watch-pipeline
johntmyers
added
gator:merge-ready
and removed
gator:approval-needed
Monitoring CompleteMonitoring is complete because this PR has merged. Final status: Gator review found no remaining blockers on the final head, the required core E2E suite passed, and maintainer approval was present before merge. I removed the active Gator metadata
|
2 participants
Summary
On a
protocol: graphqlendpoint, an accepted protocol upgrade can switch the connection to a raw relay, where later messages bypass GraphQL operation checks. Refuse requests carrying anUpgradeheader with403 Forbiddenafter validating the HTTP head and endpoint authority, before reading the body or forwarding the request, in both enforce and audit mode. Ordinary GraphQL HTTP requests continue through body inspection and operation policy.Related Issue
No issue required: this is a localized correction to protocol-upgrade handling, extending the refusal already merged in #3753 to GraphQL endpoints.
Changes
101 Switching Protocolsresponses.protocol: websocketwith GraphQL operation rules on a separate server path or port. Both inspected transports cannot share the same host, port, and path.Testing
mise run pre-commitpassesLocal verification passed package formatting, changed-file license checks,
git diff --check,cargo check -p openshell-supervisor-network --locked, and seven exact library regressions. The regressions cover withheld bodies, framing and authority validation, ordinary POST inspection, existing upgrade refusals, credential denial, and chunked request sequencing. The retained reproduction failed on the previous head and its base.Hosted Branch Checks and standard runtime E2E passed on
71346cb1834e3ffe41cf0b02d613a4ca43747a0a. The optional GPU and Kubernetes HA/credential-driver suites were skipped.Checklist