feat(runtime): encrypt TCP request dispatch (frontend → worker) by walkoss · Pull Request #12533 · ai-dynamo/dynamo

walkoss · 2026-08-01T16:09:16Z

Overview

Extends TLS encryption to the TCP request plane (egress/tcp_client / ingress/shared_tcp_endpoint), which carries user prompts from frontend to worker. This closes the security gap where inference request payloads were transmitted in plaintext while only the response stream was encrypted (#10921).

Details

Server side (ingress/shared_tcp_endpoint.rs):

Builds TlsAcceptor from DYN_TCP_TLS_CERT_PATH/KEY_PATH at startup
Per-connection TLS handshake with configurable timeout
Panics on invalid TLS config (fail-closed, not silent fallback)
BoxRead/BoxWrite unify TLS and plaintext through read_loop/write_loop

Client side (egress/tcp_client.rs):

OnceLock-cached TlsConnector from DYN_TCP_TLS_CA_CERT_PATH/INSECURE
Honors DYN_TCP_TLS_SERVER_NAME for SNI override
TLS handshake with timeout on each pooled connection
BoxRead/BoxWrite through writer_task/reader_task

Reuses the same DYN_TCP_TLS_* environment variables as the call-home transport (#10921). No new env vars needed.

Where should the reviewer start?

lib/runtime/src/pipeline/network/ingress/shared_tcp_endpoint.rs - TLS acceptor in new() + accept_loop()
lib/runtime/src/pipeline/network/egress/tcp_client.rs - TLS connector in connect() + get_request_plane_tls_connector()

Related Issues

Depends on feat(runtime): add TLS support to TCP request plane #10921 (TCP call-home TLS)
Relates to [CONTRIBUTION]: Add opt-in TLS and mTLS support to NATS and TCP transports #10809 (TLS contribution request)

Summary by CodeRabbit

New Features
- Added optional TLS encryption for TCP request and response traffic.
- Supports configurable certificates, CA validation, server names, insecure mode, and handshake timeouts.
- TLS connections are securely established per connection.
Bug Fixes
- Invalid or incomplete TLS settings now prevent startup with clear configuration errors.
- Failed or timed-out TLS handshakes are safely closed.
Documentation
- Expanded TLS guidance with supported traffic flows and configuration behavior.

copy-pr-bot · 2026-08-01T16:09:20Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-actions · 2026-08-01T16:09:24Z

🌿 Fern Docs Preview: https://nvidia-preview-019ff54c-b7b8-76c4-bdca-c98117506485.docs.buildwithfern.com/dynamo/dev

Extend TLS support to the shared TCP request plane that carries inference requests from frontend to worker. This closes the security gap where user prompts were transmitted in plaintext. Server side (shared_tcp_endpoint.rs): - Build TlsAcceptor from DYN_TCP_TLS_CERT_PATH/KEY_PATH at startup - Per-connection TLS handshake with configurable timeout - BoxRead/BoxWrite unify TLS and plaintext through read/write loops Client side (egress/tcp_client.rs): - OnceLock-cached TlsConnector from DYN_TCP_TLS_CA_CERT_PATH/INSECURE - TLS handshake with timeout on each pooled connection - BoxRead/BoxWrite through writer_task and reader_task Reuses the same DYN_TCP_TLS_* env vars as the call-home transport. No new env vars needed. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

…ane TLS config SharedTcpServer::new was the only TLS entry point that panicked on bad/partial cert config; every other path (tcp/server.rs build_tls_acceptor, tcp/client.rs, egress/tcp_client.rs, nats.rs) returns a fallible Result. Make new() return anyhow::Result<Arc<Self>> and propagate the error, matching the response-stream server, so misconfiguration fails startup cleanly instead of aborting the process. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com> (cherry picked from commit ac3623b) Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

Mirror the response-stream `build_tls_acceptor` tests from tcp/server.rs for the request-plane ingress: - new_no_tls_env_is_plaintext: no TLS env vars -> plaintext (no acceptor) - new_partial_tls_config_errors: only cert or only key -> returns Err (covers the panic->error fix, ensuring startup fails cleanly) - new_both_paths_enables_tls: cert + key -> TLS acceptor built Uses rcgen self-signed certs and temp_env, matching the existing pattern. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

devin-ai-integration

Devin Review found 2 potential issues.

View 1 additional finding in Devin Review.

coderabbitai · 2026-08-04T15:53:26Z

Walkthrough

Changes

TCP transport TLS

Layer / File(s)	Summary
Server TLS configuration and acceptance `lib/runtime/src/pipeline/network/ingress/shared_tcp_endpoint.rs`, `lib/runtime/src/pipeline/network/manager.rs`, `docs/fern/pages/reference/components/tls-configuration.mdx`	`SharedTcpServer` validates certificate and key settings, performs timeout-bounded handshakes, and returns initialization errors. Tests cover plaintext mode and TLS configuration cases. Documentation describes TLS coverage and encrypted paths.
Client TLS connector and stream integration `lib/runtime/src/pipeline/network/egress/tcp_client.rs`	The TCP client caches optional TLS configuration, performs timed handshakes, and passes boxed plaintext or TLS stream halves to reader and writer tasks.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: encrypting TCP request dispatch between the frontend and worker.
Description check	✅ Passed	The description covers the required overview, details, reviewer starting points, and related issues with specific implementation information.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/fern/pages/reference/components/tls-configuration.mdx`:
- Around line 153-158: Update the TLS lifecycle description to state that
REQUEST_PLANE_TLS_CONNECTOR is cached after the first client connection, while
SharedTcpServer::new creates the TlsAcceptor during server initialization; note
that the TLS server name and handshake timeout are read per connection, and
remove the conflicting claim that all TLS configuration is cached after the
first connection.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c0733a02-ee7f-4aa5-adcb-509b47c546ac

📥 Commits

Reviewing files that changed from the base of the PR and between 4860c46 and 16b2e6b.

📒 Files selected for processing (4)

docs/fern/pages/reference/components/tls-configuration.mdx
lib/runtime/src/pipeline/network/egress/tcp_client.rs
lib/runtime/src/pipeline/network/ingress/shared_tcp_endpoint.rs
lib/runtime/src/pipeline/network/manager.rs

…ckpressure writer_task wrote each batch via poll_write_vectored but never flushed the write half. With a plaintext WriteHalf<TcpStream> that was fine, but the request plane now writes through a BoxWrite that may wrap a tokio-rustls TlsStream — whose poll_write can report Ready(Ok(n)) with encrypted records still buffered in the session when the socket would block. Without a flush, a batch could stall until the next write, hanging its callers until timeout. Flush after each batch write (no-op for plaintext), failing the batch on error like a write failure. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

Per review feedback (sttts), the intro overstated coverage: TLS here encrypts only the frontend<->worker TCP request and response streams, not "all traffic between components". The KV event plane (ZMQ or NATS Core) is a separate transport and is not encrypted by this configuration. Also soften the cert- rotation note — restart is a consequence of caching, not a hard requirement; hot-reload via a rustls cert resolver is a possible future enhancement. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

sttts

For docs.

jthomson04

The transport implementation follows #10921 closely. I left targeted follow-ups on test coverage and small Rust/tracing cleanup.

jthomson04 · 2026-08-06T23:02:21Z

/ok to test d4786f4

Per PR review (jthomson04): - store Option<TlsAcceptor> instead of Option<Arc<TlsAcceptor>> — TlsAcceptor is cheaply Clone and already holds an Arc<ServerConfig>, so the outer Arc added a redundant pointer/refcount. - emit structured tracing fields (peer_addr, error) on the request-plane TLS handshake-failed and handshake-timeout warnings instead of formatting them into the message, keeping them queryable. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

sttts · 2026-08-11T17:30:01Z

/ok to test c4a66a4

Serve the server leaf certificate through a ReloadingCertifiedKey that reloads the cert/key from disk when their contents change, so certificate rotation (an in-place rewrite or an atomic symlink swap) takes effect without a process restart — addressing the cert-expiry outage risk raised in review (sttts): - Change is detected by hashing the file contents (blake3), not mtime, so rotations done by an atomic symlink swap are handled reliably. - The current identity is held in an ArcSwap; a single caller performs the rate-limited reload (>=30s, 1s retry after a failure) under try_lock, so a reload never blocks a handshake and a bad/partial file keeps the last valid identity. - CertifiedKey.keys_match() validates the cert and key belong together. Applies to every server built via tls_utils::server_tls_config — both the request-plane ingress and the response-stream server. The same type also implements ResolvesClientCert, so mTLS client identities reload the same way once wired on the mTLS branch. Initial load is still validated eagerly. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

Per PR review (jthomson04): the previous test built TlsAcceptor/TlsConnector directly, so it could not catch env parsing, connector wiring, SNI, boxed I/O, or reader/writer integration. - Split the env->connector logic into build_request_plane_tls_connector_from_env (behind the OnceCell) so it can be unit-tested directly. - Add TcpConnection::connect_with_connector so a test can drive the real connect/handshake/reader/writer path with an explicit connector, without initializing (and poisoning) the process-global REQUEST_PLANE_TLS_CONNECTOR. - request_plane_tls_end_to_end drives a real encrypted request (connect + TLS handshake with SNI from env + boxed I/O + send_request framing) against a TLS-wrapped echo server built from the production server_tls_config. - request_plane_tls_connector_from_env_parses covers the env parsing. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>

sttts · 2026-08-12T09:25:25Z

/ok to test e94247a

walkoss temporarily deployed to external_collaborator August 1, 2026 16:09 — with GitHub Actions Inactive

pull-request-size Bot added the size/XXL label Aug 1, 2026

github-actions Bot added external-contribution Pull request is from an external contributor feat documentation Improvements or additions to documentation frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` labels Aug 1, 2026

walkoss force-pushed the walid/request-plane-tls branch from bf9b03a to bce0da3 Compare August 1, 2026 16:14

walkoss temporarily deployed to external_collaborator August 1, 2026 16:14 — with GitHub Actions Inactive

walkoss changed the title ~~feat(runtime): add TLS to TCP request plane~~ feat(runtime): encrypt TCP request dispatch (frontend → worker) Aug 1, 2026

walkoss temporarily deployed to external_collaborator August 3, 2026 21:22 — with GitHub Actions Inactive

This comment has been minimized.

Sign in to view

walkoss mentioned this pull request Aug 4, 2026

[CONTRIBUTION]: Add opt-in TLS and mTLS support to NATS and TCP transports #10809

Open

walkoss added 2 commits August 4, 2026 11:09

walkoss force-pushed the walid/request-plane-tls branch from 0667773 to 09293f9 Compare August 4, 2026 09:12

pull-request-size Bot removed the size/XXL label Aug 4, 2026

walkoss temporarily deployed to external_collaborator August 4, 2026 09:12 — with GitHub Actions Inactive

pull-request-size Bot added the size/L label Aug 4, 2026

walkoss temporarily deployed to external_collaborator August 4, 2026 15:12 — with GitHub Actions Inactive

walkoss marked this pull request as ready for review August 4, 2026 15:46

walkoss requested review from a team as code owners August 4, 2026 15:46

devin-ai-integration Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread lib/runtime/src/pipeline/network/egress/tcp_client.rs

Comment thread lib/runtime/src/pipeline/network/egress/tcp_client.rs Outdated

coderabbitai Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread docs/fern/pages/reference/components/tls-configuration.mdx Outdated

walkoss force-pushed the walid/request-plane-tls branch from dd0b186 to e88abb1 Compare August 5, 2026 11:37

walkoss temporarily deployed to external_collaborator August 5, 2026 11:37 — with GitHub Actions Inactive

walkoss requested a review from sttts August 5, 2026 11:47

sttts reviewed Aug 5, 2026

View reviewed changes

Comment thread docs/fern/pages/reference/components/tls-configuration.mdx Outdated

walkoss force-pushed the walid/request-plane-tls branch from e88abb1 to d4786f4 Compare August 5, 2026 16:02

walkoss temporarily deployed to external_collaborator August 5, 2026 16:02 — with GitHub Actions Inactive

walkoss requested a review from sttts August 5, 2026 16:26

sttts approved these changes Aug 5, 2026

View reviewed changes

jthomson04 approved these changes Aug 6, 2026

View reviewed changes

Comment thread lib/runtime/src/pipeline/network/egress/tcp_client.rs Outdated

Comment thread lib/runtime/src/pipeline/network/ingress/shared_tcp_endpoint.rs Outdated

Comment thread lib/runtime/src/pipeline/network/ingress/shared_tcp_endpoint.rs

pull-request-size Bot removed the size/L label Aug 11, 2026

walkoss temporarily deployed to external_collaborator August 11, 2026 12:22 — with GitHub Actions Inactive

pull-request-size Bot added the size/XL label Aug 11, 2026

walkoss force-pushed the walid/request-plane-tls branch from 3bf00aa to c4a66a4 Compare August 11, 2026 12:53

walkoss temporarily deployed to external_collaborator August 11, 2026 12:53 — with GitHub Actions Inactive

sttts enabled auto-merge (squash) August 11, 2026 17:29

walkoss added 2 commits August 12, 2026 10:14

auto-merge was automatically disabled August 12, 2026 08:15
Head branch was pushed to by a user without write access

walkoss force-pushed the walid/request-plane-tls branch from c4a66a4 to e94247a Compare August 12, 2026 08:15

walkoss temporarily deployed to external_collaborator August 12, 2026 08:15 — with GitHub Actions Inactive

sttts merged commit 9f616ae into ai-dynamo:main Aug 12, 2026
109 checks passed

walkoss mentioned this pull request Aug 12, 2026

feat(runtime): add TLS support to NATS transport #13096

Merged

dagil-nvidia mentioned this pull request Aug 14, 2026

docs: backport main docs tree and docs CI to release/1.4.0 #12950

Merged

This was referenced Aug 19, 2026

feat(runtime): add mutual TLS (mTLS) support for TCP and NATS transports #13528

Merged

feat(operator): add TLS/mTLS auto-injection via InfrastructureConfiguration #13689

Merged

Conversation

walkoss commented Aug 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Details

Where should the reviewer start?

Related Issues

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Aug 1, 2026

Uh oh!

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

Uh oh!

Uh oh!

This comment has been minimized.

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 4, 2026

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sttts left a comment

Choose a reason for hiding this comment

Uh oh!

jthomson04 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jthomson04 commented Aug 6, 2026

Uh oh!

sttts commented Aug 11, 2026

Uh oh!

sttts commented Aug 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

walkoss commented Aug 1, 2026 •

edited by coderabbitai Bot

Loading

github-actions Bot commented Aug 1, 2026 •

edited

Loading