feat(security): add bounded outbound fetch API - #113
Merged
seonghobae merged 2 commits intoAug 26, 2026
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
merged commit Aug 26, 2026
55d8025
into
feat/issue-79-destination-policy
6 of 7 checks passed
Comment on lines
+766
to
+788
| for redirects in 0..=OUTBOUND_FETCH_MAX_REDIRECTS { | ||
| let decision = state.resolve_outbound(url.as_str()).await.map_err(|_| { | ||
| ( | ||
| StatusCode::BAD_REQUEST, | ||
| "destination_denied", | ||
| "destination policy denied the URL", | ||
| ) | ||
| })?; | ||
| // A request-local pin board prevents a concurrent evaluation of the same | ||
| // hostname from replacing the addresses between policy and connect. | ||
| let request_pins = Arc::new(destination::DestinationPins::default()); | ||
| request_pins.record(&decision.host, &decision.ips); | ||
| let request_http = outbound_http_client(request_pins); | ||
| let response = request_http.get(url.clone()).send().await.map_err(|_| { | ||
| ( | ||
| StatusCode::BAD_GATEWAY, | ||
| "upstream_request_failed", | ||
| "upstream request failed", | ||
| ) | ||
| })?; | ||
|
|
||
| if response.status().is_redirection() { | ||
| if redirects == OUTBOUND_FETCH_MAX_REDIRECTS { |
There was a problem hiding this comment.
📝 Info: Redirect bound is correct (no off-by-one)
for redirects in 0..=OUTBOUND_FETCH_MAX_REDIRECTS follows at most 3 redirects across up to 4 requests; a 3xx at the final index returns too_many_redirects. Matches the documented bound, and the trailing unreachable! is genuinely unreachable.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+767
to
+778
| let decision = state.resolve_outbound(url.as_str()).await.map_err(|_| { | ||
| ( | ||
| StatusCode::BAD_REQUEST, | ||
| "destination_denied", | ||
| "destination policy denied the URL", | ||
| ) | ||
| })?; | ||
| // A request-local pin board prevents a concurrent evaluation of the same | ||
| // hostname from replacing the addresses between policy and connect. | ||
| let request_pins = Arc::new(destination::DestinationPins::default()); | ||
| request_pins.record(&decision.host, &decision.ips); | ||
| let request_http = outbound_http_client(request_pins); |
There was a problem hiding this comment.
📝 Info: resolve_outbound writes shared pin board unnecessarily
resolve_outbound records into the shared state.pins, but the fetch handler connects through a request-local request_pins and a fresh client. The shared write is an unused side effect that can evict other pins under the 4096 cap. Benign, but at odds with the request-local intent noted at lib.rs.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+704
to
+715
| async fn outbound_fetch( | ||
| State(state): State<AppState>, | ||
| headers: HeaderMap, | ||
| Json(request): Json<OutboundFetchRequest>, | ||
| ) -> Response { | ||
| if !admin_authorized(&state, &headers) { | ||
| return outbound_fetch_error( | ||
| StatusCode::UNAUTHORIZED, | ||
| "unauthorized", | ||
| "missing or invalid X-Admin-Token", | ||
| ); | ||
| } |
There was a problem hiding this comment.
📝 Info: Malformed JSON bypasses stable error envelope
Json<OutboundFetchRequest> is extracted before the handler runs, so a malformed body returns axum's default plain-text 400 rather than the documented {code, error} envelope, and before the auth check. Consistent with other endpoints, but the fetch contract promises stable error fields.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
added a commit
that referenced
this pull request
Aug 26, 2026
* feat(security): fail-closed destination policy for outbound HTTP One DestinationPolicy mediates gateway upstreams, threat-intel fetches, Clearfolio, SOC LLM, and the Coraza sidecar URL. Private, loopback, link-local, CGNAT, and metadata classes are denied unless DESTINATION_ALLOWLIST (or loopback development) permits them; DESTINATION_DENYLIST wins. Clients ignore ambient HTTP proxies and do not follow redirects. Refs #79. * docs: record PR #96 in the product-technical gap baseline * feat(security): harden destination policy per-IP CIDR and readiness order CIDR allowlist matches apply per resolved address, authorize non-default ports, and reject prefixes outside the address-family width. IPv6 site-local is a denied class. Hostnames that merely contain 0x are not hex IP literals. AppState constructors default to production policy; seeded fixtures opt into development. Blocking DNS runs on spawn_blocking with a timeout. Persistence and destination-list validation complete before the readiness line. * feat(security): pin outbound HTTP to evaluated destination addresses After destination policy allows a host, the reqwest client resolves only those IPs so a rebinding answer cannot reach loopback, private, or metadata classes. Host and SNI stay on the original name. Unpinned hostnames fail closed instead of falling back to OS DNS. * feat(waf): evaluate live gateway transactions with in-process libcoraza Issue #86 remainder: dlopen operator-supplied libcoraza and drive the C ABI on each /gateway request. Missing library or empty ruleset fail closed before bind. CI stays hermetic with a fixture cdylib that exports the same symbols. * docs: record PR #97 in the product-technical gap baseline * feat(waf): forward bounded client headers into in-process libcoraza In-process transactions now receive the same forwarded-header allowlist as the sidecar path (host, user-agent, accept, content-type, referer, origin, x-requested-with, x-forwarded-for, x-real-ip, cookie — never Authorization; 32 headers / 8 KiB caps enforced by proven_engine::engine_forwarded_headers). Each header crosses the C ABI via coraza_add_request_header before process_request_headers, so CRS rules that inspect headers evaluate real client input instead of a synthetic Host only. Brings in the PR #95 sidecar hardening via merge so both engines share one allowlist implementation and one status/bound contract. Behavioral header-battery evidence lands with the issue-11 battery fixture (PR #110); this slice ships the plumbing and keeps the stub contract unchanged. * feat(store): require PostgreSQL as the production control plane (#98) * feat(store): require PostgreSQL as the production control plane Non-loopback binds fail closed without CONTROL_PLANE_DATABASE_URL. Migrations create 3NF two-word tables with default-deny RLS. Snapshot persist commits policy rows and audit records in one transaction. Loopback still uses the JSON file or memory adapter. * feat(store): transactional outbox and leased workers Issue #81 first slice on the PostgreSQL control plane. Security events append with an outbox row in one transaction instead of rewriting the snapshot. Workers claim with SKIP LOCKED, retry, dead-letter, and record unique receipts. Stdout SIEM is at-least-once; the receipt is the exactly-once ack. Also deterministic ORDER BY on postgres loads (still-valid #98 finding). Do not re-implement the postgres gate. * feat(store): rustls for production PostgreSQL sslmode=require (#100) * feat(store): rustls for production PostgreSQL sslmode=require Issue #80 remainder. sslmode=require/verify-ca/verify-full connect with rustls and Mozilla roots; certificates are always verified. allow/prefer are still rejected so the process cannot silently drop to plaintext. Live test against plaintext CI postgres proves fail-closed. Do not re-implement the postgres gate or the outbox. * docs: record PR #100 in the product-technical gap baseline * fix(store): rewrite verify-full sslmode for tokio-postgres 0.7 Still-valid #100 Devin finding. tokio-postgres 0.7 only parses disable/prefer/require. Map verify-ca/verify-full to require before connect; rustls still verifies certificates. Password query-lookalikes are left untouched. * feat(store): bound outbox listing and prune processed rows (#101) * feat(store): bound outbox listing and prune processed rows Still-valid #99 finding. GET /api/outbox returns at most EVENT_LIMIT rows (dead letters, then pending, then leased, then processed). Processed outbox_message rows prune to that cap; receipts and dead letters stay. Do not re-implement the outbox, postgres gate, or rustls. * docs: record PR #101 in the product-technical gap baseline * fix(store): prune processed outbox to EVENT_LIMIT on save and ack Still-valid #101 Devin finding. Snapshot save and worker ack used LIST_LIMIT (1000) while append used operator EVENT_LIMIT. Store the configured cap on PostgresPlane so all three paths retain the same processed-row bound. Receipts and dead letters stay. * feat(store): logical backup and isolated restore drill (#102) * feat(store): logical backup and isolated restore drill Issue #80 remainder stacked on #101. GET /api/backup exports a hashed tenant snapshot; POST /api/backup restores after schema and payload-hash checks; POST /api/backup/drill restores into an isolated tenant, compares unmasked invariants, and drops the drill rows. Declared RPO is last successful export; declared RTO is 60s. File/memory adapters report backup=disabled. Do not re-implement rustls, outbox, or retention. * docs: record PR #102 in the product-technical gap baseline * feat(store): non-owner PostgreSQL runtime role after migrate (#103) * feat(store): non-owner PostgreSQL runtime role after migrate Still-valid #98 finding. CI connects as a superuser, which bypasses FORCE RLS. Migrations stay on the login role, then SET ROLE wardnet_runtime (NOSUPERUSER, NOBYPASSRLS, not table owner). Missing tenant GUC yields no rows; DROP TABLE and DISABLE RLS are denied. Do not re-implement rustls, outbox, retention, or backup/restore. * docs: record PR #103 in the product-technical gap baseline * fix(store): restore logical backups across role-only schema versions v3 only provisions wardnet_runtime and does not change table shape. verify() accepts schema 2 through the current migration version so a role-only upgrade cannot void the last pre-upgrade logical backup. * feat(store): HASH-partition security_event by tenant (#104) * feat(store): HASH-partition security_event by tenant Convert unpartitioned security_event to PARTITION BY HASH (tenant_id) with eight children under pg_advisory_lock. Rows keep unmasked client IPs and paths. /healthz.event_partitions reports the child count. Logical restore still accepts schema 2 through the current version. * docs: record PR #104 in the product-technical gap baseline * feat(security): add bounded outbound fetch API (#113) * feat(security): add bounded outbound fetch API * fix(security): isolate fetch DNS pins per request * feat(security): route browser DNS and HTTPS through Wardnet (#116) * feat(security): add bounded outbound fetch API * feat(security): route browser DNS and HTTPS through Wardnet * fix(security): isolate fetch DNS pins per request * fix(dns): bound concurrent UDP query handling * fix(security): close destination policy review gaps * fix(runtime): make worker shutdown durable
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
1 participant
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.
Summary
Contract
Request: {"url":"https://example.com/privacy","max_bytes":524288}
Success: {"status":200,"content_type":"text/html; charset=utf-8","final_url":"https://example.com/privacy","body_base64":"...","redirects":0}
Verification
Stacked on #96 so the API owns no duplicate DNS or destination-policy logic.