Skip to content

feat: add compliant TLS backend - #1353

Merged
leseb merged 11 commits into
praxis-proxy:mainfrom
rhdedgar:fips-tls-backend
Sep 30, 2026
Merged

leseb merged 11 commits into
praxis-proxy:mainfrom
rhdedgar:fips-tls-backend

Conversation

@rhdedgar

@rhdedgar rhdedgar commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds an option to build with a FIPS-adherent TLS backend.

Note: rmcp's reqwest feature internally activates reqwest?/rustls, so it had to be removed from the workspace declaration and routed through the callout features instead. Without this fix, the native-tls build would silently include both TLS backends.

Closes #1219

Summary by CodeRabbit

  • New Features
    • File URL resolution is now available in FIPS builds.
  • Security & Privacy
    • File downloads and Azure and Google Cloud token requests use shared outbound request handling with response-size and timeout limits.
    • Outbound request logs redact query parameter values, helping protect signed links and other secrets.
  • Bug Fixes
    • File URL fetching now handles stalled responses and blocked or oversized responses through clearer error categories.

@rhdedgar
rhdedgar requested review from a team and pierDipi September 24, 2026 15:26
@rhdedgar
rhdedgar force-pushed the fips-tls-backend branch 2 times, most recently from 04275e9 to 9d04081 Compare September 24, 2026 18:19
Signed-off-by: Doug Edgar <dedgar@redhat.com>

@leseb leseb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Both TLS backends can silently compile together. apis/src/lib.rs rejects no backend, but not both. --features full,callout-native-tls succeeds and includes native TLS plus Rustls/AWS-LC—invalid for the intended FIPS graph. Add a mutual-exclusion compile_error!.

P2 — Azure AD can compile without any TLS backend. Removing workspace Rustls in Cargo.toml leaves azure-ad-filter with HTTP-only reqwest. The build succeeds, but its mandatory HTTPS token requests fail and produce 503s. Extend the backend requirement to Azure AD.

P2 — Native TLS has no functional handshake test. Makefile only runs cargo check and dependency-tree assertions. Neither this target nor the FIPS image exercises a callout through native TLS/OpenSSL. That misses #1219’s functional TLS acceptance criterion.

…lout features

Signed-off-by: Doug Edgar <dedgar@redhat.com>
@rhdedgar

Copy link
Copy Markdown
Contributor Author

Ack, I had initially put a check for zero TLS backend, but not both, that's a valid addition. I've added code in a new commit to address all 3 items, and the CI tests are passing now.

@rhdedgar
rhdedgar requested a review from leseb September 24, 2026 22:35
@leseb

leseb commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

@rhdedgar can you check those two?

[MAJOR][Defect] Existing documented builds no longer compile
File: apis/src/lib.rs:16
Commands documented throughout examples/ omit the newly required backend.
Reproduced with --features openai-file-resolve-filter and --features gcp-adc-filter; both fail at the new guards.
Either preserve the default backend through feature wiring or update every supported command and example.

[MAJOR][Defect] Native test-utils profile is uncompilable
File: tests/utils/src/inference_fixture/mod.rs:35
callout-native-tls enables record.rs, which calls rustls-only use_rustls_tls().
Reproduction: cargo check -p praxis-test-utils --no-default-features --features callout-native-tls fails with E0599.

Signed-off-by: Doug Edgar <dedgar@redhat.com>
Signed-off-by: Doug Edgar <dedgar@redhat.com>
@rhdedgar

Copy link
Copy Markdown
Contributor Author

Ok, tests are passing once again after pulling in the latest changes from main to avoid a merge conflict.

I've also added settings for the documented build configs so those issues don't slip by CI again.

Wire callout-rustls into the default feature for apis, filters, and server
so documented build commands like --features openai-file-resolve-filter
compile without an explicit TLS backend.
FIPS builds drop it cleanly with --no-default-features.

@leseb

leseb commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Ok, tests are passing once again after pulling in the latest changes from main to avoid a merge conflict.

I've also added settings for the documented build configs so those issues don't slip by CI again.

Wire callout-rustls into the default feature for apis, filters, and server
so documented build commands like --features openai-file-resolve-filter
compile without an explicit TLS backend.
FIPS builds drop it cleanly with --no-default-features.

Thanks, can you look into these new ones?

  1. Native-only builds include both reqwest TLS backends; CI’s grep misses __rustls.
  2. Rustls builds reintroduce bundled AWS-LC despite the provider-free policy.
  3. No production-path native TLS handshake test exists.

@leseb

leseb commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

@rhdedgar for rmcp using reqwest we could do:

rmcp = {
  version = "3.4.0",
  default-features = false,
  features = ["client", "transport-streamable-http-client"]
}

Then remove reqwest-tls-no-provider and the rmcp?/reqwest* forwarding from the callout features.

Signed-off-by: Doug Edgar <dedgar@redhat.com>
Signed-off-by: Doug Edgar <dedgar@redhat.com>
Signed-off-by: Doug Edgar <dedgar@redhat.com>
@rhdedgar

Copy link
Copy Markdown
Contributor Author

Ok, I have updated the PR with the new approach mentioned in 1219. It moves away from reqwest, which simplifies things a bit, and addresses the recent comments in this PR.

@leseb

leseb commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Ok, I have updated the PR with the new approach mentioned in 1219. It moves away from reqwest, which simplifies things a bit, and addresses the recent comments in this PR.

Thanks!

A few new small findings:

  1. [MAJOR][Defect] Signed URL secrets are written to debug logs
    File: apis/src/openai/responses/file_resolve/resolve_url.rs:393
    execute_url converts the complete path and query into request.uri.
    Praxis core 0.7.1 logs uri = %request.uri at debug level after connecting.
    A URL such as ...?sig=SECRET therefore exposes its credential in logs.
    This violates the file_url requirement that signed query values be redacted from logs.
    The shared transport needs a redacted logging representation, with a regression test capturing the emitted event.

  2. [MAJOR][Defect] The FIPS artifact still excludes the migrated filters
    Files: docs/architecture/outbound-callouts.md:64, Makefile:318, Makefile:362
    The new documentation says all production callouts inherit the FIPS-capable process TLS provider.
    However, FIPS_FEATURES still excludes openai-file-resolve-filter and openai-mcp-tools, with the obsolete explanation that their reqwest rustls backend pulls AWS-LC.
    I verified that adding these groups introduces none of reqwest, aws-lc-rs, or ring.
    Update the FIPS feature set, mirrored container argument, and stale FIPS documentation so the compliance artifact actually ships and exercises the migrated nonexperimental callouts.

  3. x-praxis-callout-depth is obsolete and is stripped by SubRequestClient as a reserved header. Nothing consumes this header, and real loop prevention now uses x-praxis-iterative-depth through FrameworkHeaders::set_depth(). Please remove this header construction and its comment; no replacement is needed for the direct file URL fetch.

@praxis-bot
praxis-bot marked this pull request as draft September 29, 2026 15:48
Signed-off-by: Doug Edgar <dedgar@redhat.com>
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6e75d9c9-b2e4-45b8-ba99-f76aee394c6a

📥 Commits

Reviewing files that changed from the base of the PR and between ed55279 and 55a439b.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock, !Cargo.lock
📒 Files selected for processing (21)
  • .github/workflows/tests.yaml
  • Cargo.toml
  • Containerfile.fips
  • Makefile
  • apis/Cargo.toml
  • apis/src/openai/responses/file_resolve/mod.rs
  • apis/src/openai/responses/file_resolve/resolve.rs
  • apis/src/openai/responses/file_resolve/resolve_url.rs
  • apis/src/openai/responses/file_resolve/tests.rs
  • apis/src/subrequest.rs
  • docs/architecture/outbound-callouts.md
  • filters/Cargo.toml
  • filters/src/azure/azure_ad.rs
  • filters/src/gcp/filter.rs
  • filters/src/gcp/tests.rs
  • filters/src/gcp/token.rs
  • filters/src/lib.rs
  • filters/src/pinned_client.rs
  • filters/src/register.rs
  • tests/utils/src/inference_fixture/record.rs
  • tests/utils/src/net/tls.rs

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

@rhdedgar

Copy link
Copy Markdown
Contributor Author

Ok, the tests are passing once again after the updates made in response to the latest round of action items.

@leseb
leseb merged commit af08183 into praxis-proxy:main Sep 30, 2026
62 of 63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide a compliance-oriented TLS backend for outbound callouts

2 participants