Repository navigation
[P1][Review] Reject raw paths that change the authenticated API origin #102
Description
Review-only / approval gate
No implementation, PR merge, package publication or production mutation until the owner approves the full sprint plan.
Evidence
Reviewed main 31b8d80686495c5d6dcac0846d6e5a2c8d4bce8b.
oilpriceapi/client.py:234-237 and oilpriceapi/async_client.py:205-208. Local mock-transport reproduction using only a fixture key: request('GET', '//fixture.invalid/v1/prices') resolves the destination to fixture.invalid and attaches Authorization. Python uses urljoin with a network-path reference; the other clients concatenate the base URL with an unchecked path, allowing the base host to become URL userinfo. No external request or real credential was used. This requires a caller to pass an untrusted/malformed raw path; it is not a demonstrated unauthenticated remote exploit.
Preferred smallest change
Enforce the documented API-path contract centrally before adding credentials: require a single leading slash and ensure the resolved origin equals the configured base origin. Reject scheme-relative references, authority/userinfo changes, and parser-normalized backslashes. Preserve explicit custom base URLs for intentional proxies/test servers. No new URL abstraction or arbitrary-host feature.
Acceptance / negative-path tests
Mock transport must observe zero calls for off-origin paths, including //host, @host, backslashes and parser-normalized variants; valid /v1 paths, encoded path segments, query parameters and explicit custom base URLs continue working. Exercise every raw entry point, sync/async where present. No real key in fixtures, logs or issue output.
Risk and PR gate
High-risk credential transport boundary; Friday/Saturday only.
One focused issue PR; independent adversarial review must challenge deletion safety, public compatibility, failure recovery and whether a smaller deletion suffices. Include red/green test evidence and packed/installable artifact proof for any published SDK/MCP change. Merge only in the approved priority order after reviewer objections are resolved. Do not publish a breaking change as a patch. If already fixed at execution time, record evidence and skip without a cosmetic PR.
Activity
karlwaldman commentedon Sep 5, 2026
Adversarial sprint admission — rank 11/70
Scheduled Saturday, 2026-09-05 (America/New_York) in the approval-gated seven-night plan. Planning only; no implementation or merge yet.
Narrow outcome: Constrain authenticated raw requests to the configured origin.
Required issue → PR → independent review → ordered merge evidence
Risk window: High-risk window, including prerequisites to the related high-risk correction.
karlwaldman commentedon Sep 5, 2026
Coverage-audit revision — current sprint admission
Plan v2: oilpriceapi-api#8015 supersedes earlier scheduling comments. Rank 12/70; Saturday, 2026-09-05, America/New_York. Friday/Saturday high-risk window.
Intended outcome: Constrain authenticated raw requests to the configured origin.
Coverage ledger: #8056. Inventory/pattern screening is complete; exhaustive semantic/runtime sign-off is not. Before implementation, review the complete affected file set and callers, revalidate against current main, supply the missing negative-path proof, and retain prior scope corrections. Prefer deletion, then simplification; correct behavior needs no PR.
No implementation is approved yet. After Karl approves: focused reversible PR → independent adversarial review → passing affected checks → merge in plan order → separately approved release/publish and artifact/customer-path verification. Maximum ten planned slots per night, not ten forced merges. If the dated window elapses before approval, re-anchor the calendar rather than backdate execution.
fix(security): reject raw paths that change the authenticated API ori…