← Back to PRs

#21326: Security/UI: harden Control UI gatewayUrl URL overrides

by bmendonca3 open 2026-02-19 22:23 View on GitHub →
docs app: web-ui size: L
## Summary - harden URL-param `gatewayUrl` handling in Control UI to only allow loopback or same-host endpoints - block insecure non-loopback `ws://` connections in the browser gateway client before any auth/device payloads are sent - add repeated `4008` security-close telemetry/alerting to surface suspicious reconnect patterns - add regression tests for URL security helpers and URL-param override integration behavior - update docs and changelog with new restrictions and sensitive-deployment rotation guidance ## Security impact This closes a high-impact trust-boundary gap where untrusted URL params could steer the Control UI websocket connection toward attacker-controlled endpoints and capture auth context. ## Changes - `ui/src/ui/app-settings.ts` - sanitize URL-param `gatewayUrl` via strict policy before setting `pendingGatewayUrl` - `ui/src/ui/gateway.ts` - fail closed on insecure gateway websocket targets with code `4008` - `ui/src/ui/app-gateway.ts` - add repeated security-close detection and operator-facing alert message/logging - `ui/src/ui/gateway-url-security.ts` - central URL security and override sanitation helpers - `ui/src/ui/gateway-security-alert.ts` - telemetry counter for repeated `4008` security-close reasons - tests: - `ui/src/ui/gateway-url-security.node.test.ts` - `ui/src/ui/gateway-security-alert.node.test.ts` - `ui/src/ui/app-settings.gateway-url-override.node.test.ts` - docs: - `docs/web/control-ui.md` - changelog: - `CHANGELOG.md` ## Verification - `pnpm exec oxfmt --check ui/src/ui/app-gateway.ts ui/src/ui/app-settings.ts ui/src/ui/gateway.ts ui/src/ui/gateway-url-security.ts ui/src/ui/gateway-url-security.node.test.ts ui/src/ui/gateway-security-alert.ts ui/src/ui/gateway-security-alert.node.test.ts ui/src/ui/app-settings.gateway-url-override.node.test.ts docs/web/control-ui.md CHANGELOG.md` - `pnpm --dir ui exec vitest run --config vitest.node.config.ts src/ui/gateway-url-security.node.test.ts src/ui/gateway-security-alert.node.test.ts src/ui/app-settings.gateway-url-override.node.test.ts` ## Post-deploy (sensitive environments) - rotate `gateway.auth.token` - rotate/reissue Control UI device-auth tokens <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR implements critical security hardening for the Control UI's gateway URL handling to prevent URL parameter injection attacks that could redirect websocket connections to attacker-controlled endpoints. **Key security improvements:** - `gatewayUrl` URL parameter overrides now restricted to same-host or loopback targets only via `sanitizeGatewayUrlForUrlOverride()` in `gateway-url-security.ts:84` - Insecure `ws://` connections to non-loopback hosts blocked before any auth payloads are sent in `gateway.ts:98-103` - Security close code `4008` used to signal rejected connections, with repeated-attempt detection and alerting after 3 attempts within 60 seconds in `gateway-security-alert.ts` **Implementation quality:** - Comprehensive unit tests covering loopback detection (IPv4 127.x.x.x, IPv6 ::1, IPv6-mapped IPv4), URL sanitization, security alerting, and integration with `applySettingsFromUrl` - Proper hostname normalization handles edge cases like IPv6 brackets and `::ffff:` prefixes - Defense-in-depth approach with validation at both URL override ingestion and connection establishment - Documentation updated with clear security restrictions and rotation guidance for sensitive deployments **Test coverage:** - `gateway-url-security.node.test.ts`: validates loopback detection, secure URL checking, and sanitization rules - `gateway-security-alert.node.test.ts`: validates alert thresholds and time windows - `app-settings.gateway-url-override.node.test.ts`: validates end-to-end URL override behavior The changes are focused, well-tested, and close a legitimate trust-boundary vulnerability where malicious URL parameters could capture authentication context. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge - it hardens security without breaking existing functionality - Score of 5 reflects thorough security hardening with comprehensive test coverage, proper defense-in-depth implementation, clear documentation, and no identified bugs or regressions. The changes are well-scoped, follow established patterns, and close a real trust-boundary vulnerability. - No files require special attention - all changes are well-implemented with appropriate test coverage <sub>Last reviewed commit: 0d0e6c9</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs