Feat/http callout#484
Draft
usize wants to merge 17 commits into
Draft
Conversation
Move the HTTP callout filter from praxis (draft PR #730) into the AI repo as filters/src/callout/. The filter makes outbound HTTP requests during request processing, extracts values from responses via JSONPath, and writes them to FilterResultSet for branch-chain evaluation. Addresses all six code review comments from praxis-bot on PR #730: - Remove on_denied_headers (dead code: CalloutResult::Rejected carries no response headers, so build_rejection always received None) - Cap max_body_bytes at 100 MiB to prevent unbounded memory allocation - Add private/loopback IP warning in validate_callout_url for SSRF awareness (warning-only since internal callouts are a valid use case) - Add non_json_response_body_skips_extraction test for the untested non-JSON fallback path - Add config_rejects_unknown_fields test for deny_unknown_fields - Rename separator from "Helpers" to "Test Utilities" per convention Assisted by Opus 4.6
Add tests proving the old on_denied_headers config field is now rejected as unknown, and that rejection responses carry no headers since CalloutResult::Rejected has no response headers to forward. Assisted by Opus 4.6
Add boundary test at exactly one byte above the limit (100 MiB + 1) and a test confirming the default value (1 MiB) is accepted. Assisted by Opus 4.6
Expand IP range coverage to include link-local (169.254.x.x / cloud metadata endpoint), all three RFC 1918 ranges, IPv6 loopback (::1), and public IP non-match. Add is_private_or_loopback unit test for direct range validation. Assisted by Opus 4.6
…is-proxy#4) Add empty response body variant alongside the existing plain-text test to cover both non-JSON edge cases in the extraction fallback path. Assisted by Opus 4.6
…-proxy#5) Add tests for unknown fields in target, request, response, and circuit_breaker config structs, extending the existing top-level unknown field test to cover all nested structures that use #[serde(deny_unknown_fields)]. Assisted by Opus 4.6
Confirmed via cargo xtask lint-separators that all separator comments in the lakera_guard integration test use "Test Utilities" (not "Helpers") per project convention. No code changes needed. Assisted by Opus 4.6
Branch evaluation clears ctx.filter_results after every filter in the headers-phase pipeline. Results written during StreamBuffer pre-read (the request_body phase) were wiped by any preceding filter before the callout's own branch_chains were evaluated, silently disabling result-driven branches at any chain position but the first. Stash extractions in filter_state during the body hook and publish them to filter_results in on_request, immediately before this filter's branch evaluation. Add a unit test pinning the stash/publish sequence and an integration test with token_usage_headers preceding the callout in the lakera-guard example chain. Assisted by Kimi K3
FilterResultSet caps values at 256 bytes and forbids control characters. Propagating those validation errors from the filter hook turned a successful callout into a downstream 500 whenever the callout response carried a long string, a serialized array/object, or a string with escaped control characters (e.g. a guard service's reason field). Skip the offending value with a warning instead, consistent with the existing null/no-match skip semantics, and flatten the now-infallible handle_success/handle_result/execute_callout helpers. Assisted by Kimi K3
A result_key violating FilterResultSet rules (e.g. 'lakera.flagged') parsed successfully and only failed at runtime on the first successful extraction. Validate the key during CompiledExtraction::compile by probing a real FilterResultSet, keeping the rule in sync with core and surfacing the misconfiguration at startup. Assisted by Kimi K3
parse_filter_config strips the failure_mode structural key before the filter config is deserialized, so the serde alias could never fire. Worse, an operator writing failure_mode: open would get the pipeline-level entry failure mode (swallow filter errors) while the callout silently stayed fail-closed — two opposite semantics under one spelling. Remove the alias and pin the behavior with a regression test. Assisted by Kimi K3
inject_headers wrote to extra_request_headers, which the protocol layer applies as insert (overwrite) in the headers phase but append in the body phase — a downstream client pre-seeding the same header name would produce two upstream values. Write to request_headers_to_set instead for consistent overwrite semantics in both phases, which also drops the lossy to_str conversion. Assisted by Kimi K3
The filter-docs generator discovers filter anchors by string literal in name(); returning the FILTER_NAME const made the filter invisible, so lint-filter-docs passed without docs existing. Return the literal (with a drift test against FILTER_NAME), lead the struct doc with a descriptive first paragraph, and generate http_callout.md plus the reference index entry. Assisted by Kimi K3
The mock guard flags every request, so a GET returning 200 from upstream proves the conditions: methods [POST] scoping prevents the callout from firing at all. Assisted by Kimi K3
Pin two fallback behaviors: a forward_headers entry absent from the downstream request is simply not sent to the callout target, and a non-JSON downstream body with target.body configured is forwarded raw (with a warning) rather than dropped or erroring the request. Assisted by Kimi K3
10 tasks
- Warn when target.body is configured with phase request_headers (dead config: no body is ever sent) - Correct the cast_possible_truncation expect reason (no config bound exists; truncation is physically implausible) - Document max_depth: 0 semantics and the SSRF heuristic's uncovered cases (IPv4-mapped IPv6, ULA/link-local, unresolved hostnames) - Note in the lakera-guard example that non-JSON bodies are forwarded raw and rejected fail-closed by Lakera - Restore the unrelated web-search README description trimmed in the feature commit - List filters/src/callout in AGENTS.md filter organization - Alphabetize HttpCalloutFilter methods per conventions Assisted by Kimi K3
Line-wrapping normalizations from cargo +nightly fmt over the tests added in this branch. Assisted by Kimi K3
Contributor
|
@usize Any reason this is not in core? It seems fairly unrelated to AI besides on a usecase level. |
praxis-bot
reviewed
Jul 23, 2026
praxis-bot
left a comment
praxis-bot
left a comment
Collaborator
There was a problem hiding this comment.
HTTP Callout Filter Review
Clean, well-structured filter with thorough test coverage. The two-phase execution model (body-phase results stashed via insert_filter_state then re-published in on_request) correctly handles the pipeline result-clearing issue -- the integration test lakera_guard_rejects_flagged_with_preceding_filter validates this end-to-end. Good use of deny_unknown_fields on all config structs, pre-compiled JSONPath at config time, and the failure_mode vs on_failure distinction is well-documented and tested.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 2 |
Inline comments below.
| fn is_private_or_loopback(addr: &IpAddr) -> bool { | ||
| match addr { | ||
| IpAddr::V4(v4) => v4.is_loopback() || v4.is_private() || v4.is_link_local(), | ||
| IpAddr::V6(v6) => v6.is_loopback(), |
Collaborator
There was a problem hiding this comment.
[Medium] IPv6 branch only matches ::1. An attacker can use IPv4-mapped IPv6 (::ffff:127.0.0.1 or ::ffff:169.254.169.254) to bypass this check entirely, reaching loopback and cloud-metadata endpoints without even a warning. Since Rust's IpAddr parser accepts these forms, consider extracting the mapped IPv4 address and re-checking:
IpAddr::V6(v6) => {
if let Some(mapped) = v6.to_ipv4_mapped() {
return mapped.is_loopback() || mapped.is_private() || mapped.is_link_local();
}
v6.is_loopback()
}Ipv6Addr::to_ipv4_mapped is stable since Rust 1.75, well within the project's MSRV of 1.96.
| pub on_failure: FailureModeConfig, | ||
|
|
||
| /// HTTP status code returned when rejecting on failure. | ||
| pub status_on_error: Option<u16>, |
Collaborator
There was a problem hiding this comment.
[Medium] Option<u16> allows values outside the valid HTTP status code range (100--599). Per project conventions, constrained numerics should use #[serde(try_from)]. A newtype wrapper or a validation check in from_config would catch invalid values like 0 or 65535 at config time with a clear error message, rather than deferring to the core callout layer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An HTTP Callout filter, utilizing core's reqwest based callout client.
This allows us to make arbitrary API calls, passing selected fields from payloads and to parse any results. It's particularly useful for integrating with third party guardrails services.