Repository navigation
fix(network): align REST and WebSocket endpoint handling - #5336
Conversation
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
🌿 Preview your docs: https://nvidia-preview-pr-5336.docs.buildwithfern.com/openshell |
Compatibility review (advisory)New or updated breaking changes; see the full report for all findings. 1. policy — No explicit risk acceptance foundREST endpoints previously relayed allowed WebSocket upgrades and supported
Review details
Advisory review; publication gates are unchanged. |
|
Label |
PR Review StatusReviewed the complete REST/WebSocket handling change and found no blocking findings. REST upgrades are refused across relay and forwarding paths, credential-rewrite validation is consistent across policies, provider profiles, and runtime configuration, and the docs explain the migration to explicit WebSocket endpoints. Required checks are still running. I applied Blocking findings: None. Carried findings: None. Gator metadata
|
russellb
left a comment
There was a problem hiding this comment.
lgtm, thanks!
I reviewed this prior to posting and it has since been updated to address the one finding I had.
Monitoring CompleteMonitoring is complete because this PR has merged. The prior Gator review found no blocking findings, and the last active state was I removed the active Gator metadata
|
Summary
Keep REST endpoints on HTTP request handling and require explicit WebSocket endpoints for WebSocket upgrades. Apply consistent validation for credential-rewrite settings across policies, provider profiles, and runtime configuration.
Changes
Release Notes
REST endpoints return
403for HTTP upgrade requests in both enforce and audit modes. Configure WebSocket traffic withprotocol: websocketand explicit handshake and message rules.websocket_credential_rewrite: truerequires a WebSocket endpoint.Testing
Checklist