Skip to content

feat(file-resolve): route file_id callouts through a filtered outbound chain - #1186

Merged
leseb merged 1 commit into
praxis-proxy:mainfrom
leseb:leseb/fix-960-from-pr-1166-approved
Sep 17, 2026
Merged

leseb merged 1 commit into
praxis-proxy:mainfrom
leseb:leseb/fix-960-from-pr-1166-approved

Conversation

@leseb

@leseb leseb commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Route openai_file_resolve's configured Files API (file_id) metadata and content callouts through a filtered outbound chain executed via the FilteredSubrequestExecutor, replacing the direct subrequest path. The chain's filters observe and can mutate each callout before it is dialed. Client-controlled file_url downloads stay on the credential-free hardened resolver and never traverse the chain.

Closes #960.

What changed

  • New outbound_chain config key on openai_file_resolve — an inline filter chain applied to configured file_id metadata + content callouts.
  • allow_private_files_api_url removed. SSRF protection for files_api_url now derives from the pipeline's allow_private_upstreams, enforced both when the callout target is pinned (prepare_url_target) and again at connect time.
  • New internal filter openai_callout_seed_upstream projects the validated, pinned upstream onto the callout context (the executor dials ctx.upstream, which only a chain filter can set).
  • Example configs updated (drop the removed flag, add the outbound chain + allow_private_upstreams); generated filter docs regenerated.

Design: head + tail upstream seed

The seed is installed at both ends of the inline chain:

  • the head projects the pinned upstream so an operator's own filters observe the real destination;
  • the tail reasserts the pinned upstream after those filters run, so an operator filter (e.g. an endpoint_selector) cannot silently retarget a callout that still carries Files API credentials.

The seed clones rather than consumes the staged upstream so both instances see it. Named outbound chains are rejected at build time — the seed cannot be injected into a chain resolved by name inside the binding context, so the field is inline-only.

⚠️ Draft: dependency status

This PR consumes praxis-core APIs that landed in praxis PR #1166 (URL-aware target preparation for subrequests + the chain-binding callout executor), which is not yet in a crates.io release. Locally it builds against a [patch.crates-io] git-rev pin of praxis main; that dev-only patch (and its Cargo.lock churn) is intentionally excluded from the commit, so CI will be red until praxis publishes a release containing #1166 and we bump the [workspace.dependencies] version. No functional code change is needed at that point — just the version bump.

Known praxis-executor limitations (fail-closed, documented in-code)

Two behaviors from the previous direct-subrequest path cannot be fully preserved against the merged callout executor and are documented as honest, fail-closed limitations:

  1. Multi-address DNS fallback. praxis Upstream carries a single address and the executor dials only it — there is no connect-fallback list like the old subrequest::execute_with_addresses. Dual-stack / multi-A hosts fail closed if the first validated address is unreachable. Minimal praxis fix: let Upstream/the executor carry and try multiple validated addresses.
  2. Typed oversized-response result. TransportFailure::ResponseTooLarge is pub(crate) and collapsed into an opaque 502 inside executor.run(), so the typed 413/ResponseTooLarge classification is lost. The common case is mitigated by the existing metadata pre-check (metadata.bytes vs max_content_bytes, which rejects before download); the residual case fails closed as 502. Minimal praxis fix: surface the typed overflow result from run().

Tests

  • Unit: openai_callout_seed_upstream head/tail reassert (incl. a mid-chain operator-override defeat test); registration installs seed at head+tail and rejects named chains.
  • Integration: file_id runs through the outbound chain while file_url does not (observable via a marker header).
  • SDK: new OpenAI-client integration test (test_file_id_resolved_via_outbound_chain_not_file_url) proving file_id → chain-resolved base64 content and file_url → data-URI (chain not run).

Preserved: caching, ref limits, rewritten-body/resolved budgets, deadlines, on_missing, DNS-rebind/SSRF validation, TLS/SNI/Host, static creds, forward_headers.

@leseb
leseb force-pushed the leseb/fix-960-from-pr-1166-approved branch 3 times, most recently from a50e5cd to 2130075 Compare September 17, 2026 09:42
…d chain

Route openai_file_resolve's configured Files API (file_id) metadata and
content callouts through a filtered outbound chain, executed via the
FilteredSubrequestExecutor, replacing the direct subrequest path. The
chain's filters observe and can mutate each callout before it is dialed.
Client-controlled file_url downloads stay on the credential-free hardened
resolver and never traverse the chain.

Build the callout destination with Praxis core's StagedUpstream and
StagedUpstreamFallback::from_prepared_target: the executor seeds and
reasserts the prepared, validated destination while retaining every
resolved fallback address, so named outbound chains bind directly without
any injected seed filter. Drive the callout through run_classified so a
ResponseTooLarge outcome is preserved as the existing file-resolution 413
response.

Remove the allow_private_files_api_url opt-in. SSRF protection for
files_api_url now derives from the pipeline's allow_private_upstreams,
enforced both when the callout target is pinned (prepare_url_target) and
again at connect time.

Closes praxis-proxy#960

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
leseb force-pushed the leseb/fix-960-from-pr-1166-approved branch from 2130075 to ab9d7c4 Compare September 17, 2026 13:51
@leseb
leseb marked this pull request as ready for review September 17, 2026 13:58
@leseb
leseb requested review from a team and liavweiss September 17, 2026 13:58
@leseb
leseb enabled auto-merge September 17, 2026 13:58

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review (praxis-bot) -- 1 finding.

[Medium] tests/utils/src/proxy.rs: duplicate set_allow_private_upstreams call. The new line (added before add_pipeline_extension) and the existing line (after it) both call pipeline.set_allow_private_upstreams(config.insecure_options.allow_private_upstreams). The second call is redundant since set_allow_private_upstreams does not interact with extensions. Remove one of the two calls to match the server's configure_pipeline, which has a single call.

@leseb
leseb added this pull request to the merge queue Sep 17, 2026
Merged via the queue into praxis-proxy:main with commit 4378321 Sep 17, 2026
35 checks passed
@leseb
leseb deleted the leseb/fix-960-from-pr-1166-approved branch September 18, 2026 07:34
eoinfennessy pushed a commit to eoinfennessy/praxis-ai that referenced this pull request Sep 18, 2026
…ers (praxis-proxy#1235)

PR praxis-proxy#1186 made openai_file_resolve's `outbound_chain` effectively
required: registration failed the pipeline build when the key was
omitted. The sibling SSRF-hardening filter openai_file_search_callout
(PR praxis-proxy#1185) treats the chain as optional, defaulting to an empty inline
passthrough. Align file_resolve with that pattern so all three outbound
callout filters share identical handling.

- config: `outbound_chain` is now `#[serde(default = ...)] ChainRef`
  defaulting to an empty inline passthrough chain (was
  `Option<ChainRef>`).
- mod: `outbound_chain_ref` returns the resolved `ChainRef`, not an
  `Option`.
- register: drop the "outbound_chain is required" gate; the defaulted
  chain binds and callouts still route through the bound pipeline. A
  provided-but-unbuildable chain still fails the build.
- Regenerated the openai_file_resolve filter docs.

An omitted chain still routes file_id callouts through the bound
outbound pipeline with allow_private_upstreams enforcement, identical to
file_search_callout, so SSRF protection is unchanged. file_resolve
continues to accept named chains because it runs at the top pipeline
level, not nested inside an iterative_request_router step.

Refs praxis-proxy#1186

Signed-off-by: Sébastien Han <seb@redhat.com>
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.

feat(callout): route OGX file-resolution requests through filtered subrequests

3 participants