security(request): snapshot untrusted extension mappings before timeout access - #210
security(request): snapshot untrusted extension mappings before timeout access#210seonghobae wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 57 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
|
@coderabbitai review Review exact current head |
Rate Limit Exceeded
|
seonghobae
left a comment
There was a problem hiding this comment.
Current-head source review for b7021ef272170664abd81c22b38a2a5e39bfe497 found no blocking defect in the two-file request-extension snapshot scope. _bind_bounded_request_timeouts now detaches the outer Mapping once through the existing exception-masking copy seam, performs timeout lookup only on the built-in dictionary snapshot, separately snapshots nested timeout mappings, and retains generic non-leaking denial for copy and value failures. This removes hostile get() dispatch and split-snapshot behavior without changing allowed extension keys or finite timeout policy. This is author-side review evidence only, not independent approval. Keep Draft until corrected Dependency Review and authoritative Strix evidence execute on the unchanged head and an independent current-head review is present.
Purpose
Repair a request-boundary fail-closed gap proven against exact protected
main7faf7a3b8a47980113982914000e724ab6a6cda5.Before this PR,
_bind_bounded_request_timeouts(...)calledextensions.get("timeout")directly on the caller-providedMappingbefore detaching it, then independently materialized the same mapping later. A custom mapping could therefore execute its ownget()implementation and leak an arbitrary exception across the generic denial boundary; a stateful mapping could also present two different snapshots during one decision.RCA and test-first proof
10569788099d0eb869befa65003b2d47f7d8c9c9.31643103797, Python 3.12 job94270211704. Exact checkout, dependency installation and Ruff passed;coverage run -m pytest -qfailed because the hostile outer mapping's overriddenget()raisedRuntimeError("secret extensions get failure")instead of remaining inside the generic request boundary.c758f11764456d9584175d5f2e806ffd6ab4c1dc.38b6e4c561d4e5fbf2a339254e5363d9f681d22c. A valid customMappingis accepted from its detached built-in snapshot; production code must not dynamically dispatch its hostileget()method. Rejecting all mapping subclasses was rejected as a wrong-layer compatibility regression.b7021ef272170664abd81c22b38a2a5e39bfe497, adding a hostile outer-copy regression that requires the stable generic denial with no cause/context.Root-cause remedy
Detach the outer request-extension mapping exactly once through the existing exception-masking copy seam, then perform timeout lookup and replacement only against that built-in detached dictionary. This removes attacker-controlled
Mapping.get()dispatch and makes one authorization decision observe one extension snapshot. The change does not widen allowed extension keys, timeout semantics, SNI/authority, DNS/TLS/proxy behavior, request/response limits, credentials, redirects, Unix sockets, or release authority.Exact-head validation
On unchanged exact current head
b7021ef272170664abd81c22b38a2a5e39bfe497over live protected base7faf7a3b8a47980113982914000e724ab6a6cda5:31643563698: success; Python 3.10–3.14 tests and coverage reports pass, including Python 3.14; package wheel/sdist build and installed-wheel smoke verification pass;31643563689: success;31643563652: aggregate/wrapper success; OSV, Trivy and Scorecard pass, but the actual immutable-pinnedDependency reviewaction is skipped, so supply-chain evidence remains non-passing;94271801977completed wrapper success but emitted theStrix backend unavailablewarning and explicitly produced no vulnerability report. The workflow treated that outcome as a neutral skip; under EgressWeave fail-closed evidence semantics this is non-passing semantic security evidence;Remaining acceptance / ownership
The product fix itself has exact-head CI, package, Python 3.14, exact coverage, SAST and GHAS evidence. The remaining blockers are evidence/control-plane boundaries rather than a defect in these two changed files:
ContextualWisdomLab/.github#897or a freshly verified protected successor). Do not add an EgressWeave-local substitute or weaken the gate.ContextualWisdomLab/.github#891or a freshly verified protected successor). Do not churn this clean source head merely to retrigger provider capacity.Keep this PR Draft. Immediately before any integration, refetch the exact head, independently resolved live base, ancestry, live ruleset, every required workflow and actual semantic step, reviews/findings/threads, and writer evidence. Merge only an unchanged accepted tree under normal governance; no predecessor, synthetic-merge, skipped, neutral, queued, rate-limited, wrapper-only, model-only, status-only or stale evidence transfers.