Skip to content

fix(file-resolve): make outbound_chain optional to match callout filters - #1235

Merged
leseb merged 1 commit into
praxis-proxy:mainfrom
leseb:leseb/fix-1186-file-resolve-chain
Sep 18, 2026
Merged

leseb merged 1 commit into
praxis-proxy:mainfrom
leseb:leseb/fix-1186-file-resolve-chain

Conversation

@leseb

@leseb leseb commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #1186 made openai_file_resolve's outbound_chain effectively required — registration failed the pipeline build whenever the key was omitted — which diverged from the sibling SSRF-hardening filter openai_file_search_callout (PR #1185) that treats the chain as optional and defaults it to an empty inline passthrough. This change makes outbound_chain optional on openai_file_resolve too (#[serde(default)] → empty inline passthrough chain), so all three outbound-callout filters (openai_web_search, openai_file_search_callout, openai_file_resolve) share one identical pattern. An omitted chain still routes configured file_id callouts through the bound outbound pipeline with allow_private_upstreams enforcement, so SSRF protection is unchanged; a provided-but-unbuildable chain still fails the build. file_resolve continues to accept named chains because it runs at the top pipeline level, not nested inside an iterative_request_router step.

Related issue

Refs #1186

Validation

  • Unit tests — cargo test -p praxis-ai-apis -- file_resolve (252 passed), cargo test -p praxis-ai-filters -- register (7 passed, incl. new file_resolve_binds_when_outbound_chain_omitted and file_resolve_rejects_unbuildable_outbound_chain)
  • Functional/registration tests — pipeline build_with_chains proves an omitted chain binds and an unbuildable chain fails closed; existing examples/configs/openai/responses/file-resolve.yaml remains valid
  • make lint (exit 0), make doc (exit 0), make build (exit 0)

Checklist

  • I reviewed every changed line and can explain the change.
  • No new capability; the existing example config already exercises outbound_chain and stays valid.
  • User-facing behavior and generated documentation are updated (regenerated docs/filters/openai_file_resolve.md).
  • Not performance-sensitive; no benchmark needed.
  • Commit is SSH-signed and carries a single Signed-off-by trailer.

Breaking changes

None — strictly permissive. A configuration that previously failed the build (omitting outbound_chain) now succeeds with an empty inline passthrough default; configurations that already specify outbound_chain are unaffected.

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>
@leseb
leseb requested review from a team and franciscojavierarceo September 18, 2026 08:49
@leseb
leseb enabled auto-merge September 18, 2026 08:53

@franciscojavierarceo franciscojavierarceo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed the current head against main and the existing review context. the config defaulting, registration path, and focused tests are consistent with the file-search callout pattern. no actionable correctness, security, or architectural issues found, and no inference recording is needed for this deterministic config/registration change.

@leseb
leseb added this pull request to the merge queue Sep 18, 2026
Merged via the queue into praxis-proxy:main with commit fdf08b9 Sep 18, 2026
31 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.

2 participants