security: close three pre-pen-test findings (S-2, S-3, S-1 hardening)#205
Merged
Conversation
Pre-pen-test audit (audit_security_preaudit_2026-07-18) findings, verified fixes:
- S-2 (P2) engine-error leakage: the entity/metric/NL routes and batch items
returned `str(e)` where e wraps a BackendExecutionError — raw ClickHouse/DuckDB
text (engine type, SQL fragments, table/column names) to any authenticated
caller. A ValueError whose __cause__ is a BackendExecutionError now returns a
generic detail carrying only the correlation id; the real error is logged
server-side. Plain request-validation messages stay verbatim (status codes
unchanged), so useful 4xx feedback is preserved.
- S-3 (P2) unauthenticated health disclosure: /v1/health and /health/ready are
auth-exempt, yet leaked `f"Kafka unavailable: {exc}"` (internal hostnames),
broker/topic counts, cluster sizes and backend identity. The public views now
expose overall status + per-component name/status/source, a fail-closed
allowlist of operational gauges (pool utilization, data freshness), and drop
message/topology metrics and the readiness detail/backend. Pool-utilization
observability (its only exposure — not on /metrics) is preserved.
- S-1 feeder (defense-in-depth): _ip_is_public now unwraps IPv4-mapped IPv6 and
6to4 addresses and judges the embedded IPv4, so the SSRF guard does not hinge
on the CPython version of the prod image (3.11 catches these via is_private,
but that was tightened in 3.13). The S-1 DNS-rebinding TOCTOU itself (pinning
the resolved IP through httpx while preserving TLS/SNI) needs live verification
against a rebinding harness and stays the documented accepted residual.
Verified: ruff check + format clean; 219 unit tests pass across egress/error/
health/pool/batch/webhook areas (live PG/CH suites run in CI).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DORA Metrics
|
…able The first sanitizer pass genericised BackendMissingTableError too (it subclasses BackendExecutionError), turning the actionable "serving table not materialized — run provisioning" 503 into "backend query failed" and breaking two integration tests that assert on that operator signal. Missing-table is a provisioning hint, not engine internals; return a clean fixed detail that keeps the "not materialized yet" phrasing but drops the scoped-SQL table reference the upstream message embeds. Arbitrary BackendExecutionError text stays genericised. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 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.
Closes three findings from the internal pre-pen-test audit of the serving/control-plane surface. Each fix is verified with unit tests; the audit file itself is not committed (working note).
S-2 (P2) — engine-error leakage → generic detail
The entity/metric/NL routes (
agent_query.py) and batch items (batch.py) returnedstr(e)/str(exc)where the exception wraps aBackendExecutionError— i.e. raw ClickHouse/DuckDB error text (engine type, SQL fragments, table/column names) reaching any authenticated caller and fingerprinting the backend. AValueErrorwhose__cause__is aBackendExecutionErrornow returns a generic detail carrying only the correlation id; the real text is logged server-side. Plain request-validationValueErrors stay verbatim and status codes are unchanged, so useful 4xx feedback is kept.S-3 (P2) — unauthenticated health disclosure → sanitized public view
/v1/healthand/health/readyare auth-exempt but leakedf"Kafka unavailable: {exc}"(internal hostnames/URLs), broker/topic counts, cluster sizes and backend identity. Public views now expose overall status + per-componentname/status/sourceplus a fail-closed allowlist of operational gauges (pool utilization, data freshness), and dropmessage, topology metrics, and the readinessdetail/backend. Pool-utilization observability — whose only exposure is this endpoint, not/metrics— is preserved.S-1 feeder (defense-in-depth) — IPv4-mapped/6to4 unwrap
_ip_is_publicnow unwraps IPv4-mapped IPv6 (::ffff:169.254.169.254) and 6to4 (2002::/16) and judges the embedded IPv4, so the SSRF egress guard does not hinge on the CPython version of the prod image (3.11 catches these viais_private; tightened in 3.13). The core S-1 DNS-rebinding TOCTOU (pinning the resolved public IP through httpx while preserving TLS/SNI/cert verification) needs live verification against a rebinding harness and remains the documented accepted residual — not addressed here.Verified: ruff check + format clean; 219 unit tests pass across egress/error/health/pool/batch/webhook (live PG/CH suites run in CI).
🤖 Generated with Claude Code