fix(proxy): add authenticated WebSocket tunneling for CONNECT/TLS-intercept routes by christine-at-datadog · Pull Request #1443 · nolabs-ai/nono

christine-at-datadog · 2026-07-17T22:53:47Z

Summary

Codex-style credential-provider routes go through nono-proxy's CONNECT/TLS-intercept path, which had no concept of a WebSocket upgrade: it forced Connection: close on every request and handed it to a strictly request/response-shaped forwarder that can't carry the raw bytes after a 101. The upstream never completed the upgrade, and the client hung.

Before: Client --CONNECT+TLS--> nono --(forces Connection: close)--> forward_request_with_response_rewrite --> Upstream
        (can't carry post-101 bytes -> ~60s hang -> reconnect loop)

After:  Client --CONNECT+TLS--> nono
          classify_upgrade_attempt      -> malformed?              -> 400
          select_intercept_route (WS-aware) -> no allow-listed path -> 403 (upstream never contacted)
          SPIFFE / AWS SigV4 route?      -> unsupported             -> 501
          build_websocket_upstream_request (shared credential/nonce pipeline, keeps Upgrade headers)
            unresolved phantom nonce     -> fail closed             -> 403
          --> raw TCP+TLS dial, write request, read response
                101 + valid Upgrade headers -> copy_bidirectional (raw tunnel)
                anything else               -> relay verbatim, framing-aware body copy, close

Notable pieces:

Upgrade-path matching moved into route selection, so an unmatched WS path can never fall through to the wrong route.
Phantom-nonce resolution is now fail-closed: an unresolved nonce in a forwarded header used to be sent upstream verbatim; it's now a 403.
Non-101 upstream responses are relayed with framing-aware body copying (Content-Length/chunked/close-delimited) instead of a naive io::copy.
The per-route allow-list is declarative (upgrades: [{ path }]); origin and method aren't separately configurable since they're implied by the route itself.
RFC 8441 (WebSocket-over-HTTP/2) is out of scope — this client negotiates HTTP/1.1 for the upgrade.

This is an agent-assisted contribution

This PR was prepared with the assistance of Claude Code (Anthropic). The author (@christine-at-datadog) reviewed the approach, the diff, and the test runs before opening this PR.

Agent Compliance Check

I am not prohibited from contributing under this policy
An issue already exists (nono-proxy hangs on Codex websocket upgrades for CONNECT/TLS-intercept credential routes #1433)
I described my intent and approach in the issue discussion
I reviewed repository coding and security rules for the affected area
I provided required attribution for reused or adapted code (all reused
helpers are pre-existing project code, called as-is; no external code
adapted)
I did not use forbidden patterns such as unwrap/expect
I used NonoError where required
I validated and canonicalized all relevant paths
This PR matches the approved or disclosed issue scope

gemini-code-assist · 2026-07-17T22:53:50Z

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

github-actions · 2026-07-17T22:54:00Z

PR Review Summary

Size

Metric	Value
Lines added	+2411
Lines removed	-44
Total changed	2455
Classification	Large (> 300 lines)

Affected crates

crates/nono (core library) — careful review required. This is the security-critical sandbox primitive. A bug here bypasses OS-level isolation for every downstream user.
crates/nono-proxy — downstream consumers depend on this crate. API or behaviour changes will affect external callers; treat any breaking change with extra scrutiny.
crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Moderate

This PR touches: source code,configuration / policy files

_{Updated automatically on each push to this PR.}

…tercept routes Codex-style credential-provider routes hang for ~1min on WebSocket upgrades because they route through the CONNECT/TLS-intercept path, not the reverse-proxy path Part 1 covered. Adds fail-fast upgrade classification, a declarative per-route upgrade allow-list (UpgradeProtocol/UpgradeRuleConfig, compiled into LoadedRoute and CredentialRouteDef), and handle_websocket_upgrade in tls_intercept/handle.rs, which reuses the existing header filtering, credential injection, and phantom-nonce resolution pipeline before dialing the upstream raw and relaying bytes via copy_bidirectional after a validated 101 handshake. Signed-off-by: Christine Le <christine.le@datadoghq.com>

…P/1 framing Move WebSocket upgrade-rule matching into route selection itself so an unmatched upgrade path is rejected before any upstream is contacted, add an explicit 501 for SPIFFE/AWS-authenticated routes (WebSocket tunneling is unsupported for those credential mechanisms), and make phantom-nonce resolution fail closed: an unresolved phantom nonce in a forwarded header now yields a 403 instead of leaking the unresolved token upstream. Simplify the declarative upgrade-rule schema to just a path (protocol and method are implied for classic WebSocket), and split handshake parsing and strict, framing-aware response relaying into new http1.rs/websocket.rs modules shared between the WS and non-WS intercept paths. Signed-off-by: Christine Le <christine.le@datadoghq.com>

main gained a disjoint-credential-routes regression test (nolabs-ai#1437) written against the pre-refactor select_intercept_route signature. Update it to use InterceptRouteRequest/SelectedRoute so it compiles after the rebase. Signed-off-by: Christine Le <christine.le@datadoghq.com>

Two RouteConfig literals in tests gated by #[cfg(not(target_os = "macos"))] were missing the new upgrades field, so they only failed to compile on Linux CI (invisible on a macOS dev machine). Signed-off-by: Christine Le <christine.le@datadoghq.com>

lukehinds · 2026-07-18T18:24:47Z

nice fine @christine-at-datadog

…eling for CONNECT/TLS-intercept routes

nogent-nolabs-ai

nogent code review

No blocking issues; 2 potential security and path validation concerns identified.

_{Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.}

nogent-nolabs-ai · 2026-07-20T16:48:52Z

@@ -270,8 +270,11 @@ async fn handle_h2_stream(
        &ctx.route_store,
        &ctx.host,
        ctx.port,


Caution
🔒 [HIGH · security] The HTTP/2 forwarder (handle_h2_stream) completely bypasses the new WebSocket upgrade restrictions. It passes websocket_path: None to select_intercept_route, skipping the upgrade_rules validation check entirely. Under RFC 8441, clients can initiate a WebSocket upgrade over an HTTP/2 stream using the CONNECT method and a :protocol: websocket pseudo-header. Because the HTTP/2 path does not check for or propagate the WebSocket upgrade status, a client can negotiate HTTP/2 with the proxy and successfully tunnel un-allowlisted WebSocket upgrades to the upstream, completely bypassing the route's default-deny WebSocket policy. If WebSocket-over-HTTP/2 (RFC 8441) is out of scope, the proxy must explicitly detect and reject HTTP/2 streams containing the :protocol: websocket pseudo-header, or accurately parse and pass the path to select_intercept_route so it fails closed.

Extended-CONNECT now gets rejected before route selection instead of silently falling.

nogent-nolabs-ai · 2026-07-20T16:48:52Z

+    /// request path.
+    #[must_use]
+    pub fn matches(&self, path: &str) -> bool {
+        let normalized = normalize_path(path);


Warning
🔒 [MEDIUM · security] In CompiledUpgradeRules::matches and CompiledUpgradeRules::compile, path comparison is performed after calling normalize_path. If normalize_path does not resolve and collapse relative path segments (such as . and ..), a client can craft a WebSocket upgrade request containing directory traversal components (e.g., /allowed/../evil) to bypass the exact-path allow-list. The upstream server may subsequently collapse the traversal and route the upgraded WebSocket connection to an unapproved endpoint. Ensure that path segment canonicalization (collapsing . and .. segments) is explicitly performed at the path-matching boundary or within normalize_path.

Updated to collapse paths :)

nogent-nolabs-ai · 2026-07-20T16:48:52Z

+    let mut request = Zeroizing::new(format!(
+        "{} {} {}\r\nHost: {}\r\n",
+        req.method, req.path, req.version, upstream_authority
+    ));


Warning
🔒 [MEDIUM · security] In build_websocket_upstream_request, headers are constructed using request.push_str(&format!("{}: {}\r\n", field.name, resolved_value));. While raw HTTP headers are validated upon initial parsing, the resolved_value obtained from resolve_nonce_in_header_value is not checked for containing carriage return (\r) or line feed (\n) characters. If a compromised or misbehaving nonce resolver returns a value containing CRLF characters, it could lead to HTTP header injection or request splitting. Add defensive checks to guarantee that resolved_value does not contain \r or \n before formatting it into the upstream request string.

Added checks for CRLF characters!

…eling for CONNECT/TLS-intercept routes

kipz · 2026-08-03T22:48:04Z

One gap while trying this out: the new upgrades field isn't in the published JSON schema.

crates/nono-cli/data/nono-profile.schema.json defines credential_routes.items with additionalProperties: false, and its properties are [base_url_env_var, endpoint_policy, env_var, name, provider] — upgrades doesn't appear anywhere in the file. So a profile using the feature parses and passes nono profile validate --strict, but is rejected by anything validating against the published schema.

lukehinds · 2026-08-04T13:24:54Z

@christine-at-datadog , can you pick up @kipz schema related change?

kipz reported that profiles using the new upgrades field on credential_routes parse and pass `nono profile validate --strict` but fail validation against the published schema, since CredentialRouteDef there still only lists [base_url_env_var, endpoint_policy, env_var, name, provider]. Add upgrades (CredentialWebSocketRuleDef: origin, path) to match the Rust model, plus a schema_shape regression test.

…tion The prior commit's CredentialWebSocketRuleDef used bare unconstrained strings for origin/path, looser than both the sibling CredentialProviderTokenEndpoint schema (host/path) and the actual runtime checks in validate_provider_origin/validate_provider_path, which require an https-only origin with no path/query/fragment and an absolute path starting with '/'. Use the existing UrlOrigin $ref for origin and add minLength/pattern for path so malformed profiles fail schema validation instead of only failing later at `nono profile validate --strict`.

christine-at-datadog · 2026-08-04T19:38:38Z

One gap while trying this out: the new upgrades field isn't in the published JSON schema.

crates/nono-cli/data/nono-profile.schema.json defines credential_routes.items with additionalProperties: false, and its properties are [base_url_env_var, endpoint_policy, env_var, name, provider] — upgrades doesn't appear anywhere in the file. So a profile using the feature parses and passes nono profile validate --strict, but is rejected by anything validating against the published schema.

@kipz thank you for calling this out! Added :)

lukehinds

lgtm thanks @christine-at-datadog , appreciate the efforts that went into this. just some clippy stuff failing in ci, but lets get this shipped.

Resolutions: - websocket.rs: kept nolabs-ai#1469's bounded line reads, added RFC 6455 Sec-WebSocket-Accept validation from nolabs-ai#1443. - handle.rs: adopted nolabs-ai#1443's run_websocket_tunnel split; kept the 4-arg resolve_nonce_in_header_value (redeem_phantoms) from nolabs-ai#1469 and nolabs-ai#1489's templated-phantom rewrite path. - token.rs: CRLF/NUL fail-closed guard moved into rewrite_first_phantom so the grant-set/templated path is covered too. - reverse.rs: header_pairs replaced by http1::parse_header_fields. - Deduplicated RouteConfig 'upgrades' fields introduced twice by the merge. Signed-off-by: James Carnegie <me@kipz.org>

…eling for CONNECT/TLS-intercept routes

The approval-flow tests in tls_intercept::handle were written against a pre-merge version of select_intercept_route/SelectedRoute/RouteConfig and no longer compiled after merging main: the function now takes an InterceptRouteRequest struct instead of positional method/path args, SelectedRoute needs Debug for panic! formatting, and RouteConfig gained a required `upgrades` field. Signed-off-by: christine.le <christine.le@datadoghq.com>

…1605) RouteConfig gained an `upgrades` field for WebSocket tunneling (#1443) but two test-only struct literals in server.rs werent updated, breaking `make ci` (clippy on test targets fails to compile). Signed-off-by: Aleksy Siek <aleksy@nolabs.ai>

github-actions Bot added bug Something isn't working nono nono-proxy nono-cli size/large labels Jul 17, 2026

github-actions Bot added size/large and removed size/large labels Jul 17, 2026

christine-at-datadog added 3 commits July 17, 2026 17:18

christine-at-datadog force-pushed the fix/issue-1433-ws-tunnel-tls-intercept branch from 7989f69 to 4851f34 Compare July 18, 2026 00:26

github-actions Bot added size/large and removed size/large labels Jul 18, 2026

kipz added a commit to kipz/nono that referenced this pull request Jul 20, 2026

Merge PR nolabs-ai#1443: fix(proxy): add authenticated WebSocket tunn…

535bb23

…eling for CONNECT/TLS-intercept routes

christine-at-datadog marked this pull request as ready for review July 20, 2026 16:46

Merge branch 'main' into fix/issue-1433-ws-tunnel-tls-intercept

2396220

github-actions Bot added size/large and removed size/large labels Jul 20, 2026

nogent-nolabs-ai Bot reviewed Jul 20, 2026

View reviewed changes

kipz added a commit to kipz/nono that referenced this pull request Jul 20, 2026

Merge PR nolabs-ai#1443: fix(proxy): add authenticated WebSocket tunn…

14c7d34

…eling for CONNECT/TLS-intercept routes

kipz added a commit to kipz/nono that referenced this pull request Jul 20, 2026

Merge PR nolabs-ai#1443: fix(proxy): add authenticated WebSocket tunn…

cc2a5de

…eling for CONNECT/TLS-intercept routes

This was referenced Jul 21, 2026

feat(proxy): add redeem_phantoms for by-value nonce redemption kipz/nono#59

Closed

feat(proxy): add redeem_phantoms for by-value nonce redemption #1469

Open

kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026

Merge PR nolabs-ai#1443

ab47553

github-actions Bot added size/large and removed size/large labels Aug 4, 2026

Merge branch 'main' into fix/issue-1433-ws-tunnel-tls-intercept

b52f62c

github-actions Bot added size/large and removed size/large labels Aug 4, 2026

Merge branch 'main' into fix/issue-1433-ws-tunnel-tls-intercept

9c02bf9

github-actions Bot added size/large and removed size/large labels Aug 6, 2026

Merge branch 'main' into fix/issue-1433-ws-tunnel-tls-intercept

39671ec

github-actions Bot added size/large and removed size/large labels Aug 7, 2026

Merge branch 'main' into fix/issue-1433-ws-tunnel-tls-intercept

c74ed08

github-actions Bot added size/large and removed size/large labels Aug 8, 2026

lukehinds approved these changes Aug 8, 2026

View reviewed changes

kipz added a commit to kipz/nono that referenced this pull request Aug 9, 2026

Merge PR nolabs-ai#1443: fix(proxy): add authenticated WebSocket tunn…

57c9345

…eling for CONNECT/TLS-intercept routes

github-actions Bot added size/large and removed size/large labels Aug 10, 2026

lukehinds merged commit 0905d0d into nolabs-ai:main Aug 10, 2026
17 checks passed

SequeI mentioned this pull request Aug 10, 2026

fix(proxy): add missing upgrades field to test RouteConfig literals #1605

Merged

4 tasks

Uh oh!

Conversation

christine-at-datadog commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

This is an agent-assisted contribution

Agent Compliance Check

Uh oh!

gemini-code-assist Bot commented Jul 17, 2026

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Summary

Size

Affected crates

Blast radius — Moderate

Uh oh!

lukehinds commented Jul 18, 2026

Uh oh!

nogent-nolabs-ai Bot left a comment

Choose a reason for hiding this comment

nogent code review

Uh oh!

nogent-nolabs-ai Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

christine-at-datadog Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

nogent-nolabs-ai Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

christine-at-datadog Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nogent-nolabs-ai Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

christine-at-datadog Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

kipz commented Aug 3, 2026

Uh oh!

lukehinds commented Aug 4, 2026

Uh oh!

christine-at-datadog commented Aug 4, 2026

Uh oh!

lukehinds left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

christine-at-datadog commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading

christine-at-datadog Jul 28, 2026 •

edited

Loading