fix(security): do not follow redirects off the validated origin (#22)

[karlwaldman]

Closes #22.

The defect

src/Http/CurlTransport.php set CURLOPT_FOLLOWLOCATION => true. The #17
origin guard validates the URL the SDK sends; it has nothing to say about
where the server points us next.

Reproduced on origin/main @ 21339d43e, PHP 8.5.8, libcurl 8.21.0 — two
loopback servers, the first 302-ing to the second:

$client = new Client($key, 'http://127.0.0.1:18081', 5.0, 0);
$client->latest('BRENT_CRUDE_USD');

RETURNED code=PWNED price=1
capture log: {"uri":"/v1/prices/latest?by_code=BRENT_CRUDE_USD","host":"127.0.0.1:18082","authorization":null}

Fabricated data from a foreign origin, handed back as an authoritative price.
The authorization: null confirms the reviewer's note: libcurl 8.21.0 strips
Authorization across an origin change. It has only done so since 7.58.0
(the CVE-2018-1000007 hardening), and composer.json required
"ext-curl": "*" with no floor — so on an older host this was also key
disclosure.

The approach I chose, and why

Do not follow redirects. Not "follow and re-validate the effective URL".

Re-validating afterwards fixes the data half but cannot fix the credential
half: the request carrying Authorization: Token <key> is already on the wire
to the new host before there is any effective URL to inspect. On a host with
libcurl < 7.58.0 that is the leak, and the SDK has no way to detect the
version gap at the moment it matters. Not following is the only option that
keeps the key inside the origin the client already validated.

It also costs nothing. Production does not redirect — verified just now:

$ curl -o /dev/null -w "%{http_code} num_redirects=%{num_redirects}\n" \
    https://api.oilpriceapi.com/v1/prices/latest?by_code=BRENT_CRUDE_USD
401 num_redirects=0
$ ... /v1/demo/prices
429 num_redirects=0

An intentional proxy is still reachable: point $baseUrl at the final URL.

What landed:

• CURLOPT_FOLLOWLOCATION => false, CURLOPT_MAXREDIRS => 0.
• A 3xx raises TransportException naming the Location, so a redirect
  surfaces as a redirect rather than as "API returned invalid JSON" three
  frames later. That matters: the old path would have sent an integrator
  hunting a JSON bug.
• CURLINFO_EFFECTIVE_URL is still compared against the requested origin, as
  a tripwire. Under FOLLOWLOCATION => false it cannot fire today; it exists
  so that re-enabling following, or a future libcurl default change, fails
  loudly instead of silently. A source-level regression test fences the
  setting itself.
• composer.json declares "lib-curl": ">=7.58.0". ext-curl's own version
  is PHP's version (8.5.8 here), so it cannot express a libcurl floor —
  lib-curl is the honest constraint, and 7.58.0 is exactly where the
  cross-origin Authorization strip landed. composer validate --strict
  passes.

TDD evidence

Test written first, against unmodified origin/main source, and it drives the
real CurlTransport against real loopback servers — a mock transport
cannot see this defect, it lives inside cURL. New fixtures:
tests/fixtures/redirector.php and tests/fixtures/foreign-origin.php.

RED (./vendor/bin/phpunit --filter RedirectOriginTest, source at origin/main):

FFFFFF.F.FF                                                       11 / 11 (100%)

There were 9 failures:

1) RedirectOriginTest::testCrossOriginRedirectNeitherReturnsDataNorReachesTheForeignHost@301 moved permanently with data (301)
A 301 to a foreign origin returned price data: code=PWNED price=1

2) ...@302 found with data (302)
A 302 to a foreign origin returned price data: code=PWNED price=1

3) ...@303 see other with data (303)
A 303 to a foreign origin returned price data: code=PWNED price=1

4) ...@307 temporary redirect with data (307)
A 307 to a foreign origin returned price data: code=PWNED price=1

5) ...@308 permanent redirect with data (308)
A 308 to a foreign origin returned price data: code=PWNED price=1

6) RedirectOriginTest::testTheApiKeyNeverReachesTheRedirectTarget
The redirect target was contacted at all: {"uri":"/v1/prices/latest?by_code=BRENT_CRUDE_USD","host":"127.0.0.1:63342","authorization":null}

8) RedirectOriginTest::testRedirectRaisesAnErrorThatNamesTheRedirect
10) RedirectOriginTest::testTransportDoesNotEnableFollowLocation
11) RedirectOriginTest::testComposerDeclaresALibcurlFloor

FAILURES!
Tests: 11, Assertions: 52, Failures: 9.

Re-proved red-capable after the fix by
git checkout origin/main -- src/Http/CurlTransport.php composer.json:
Tests: 11, Assertions: 52, Failures: 9. — then restored.

GREEN (this branch):

...........                                                       11 / 11 (100%)
OK (11 tests, 59 assertions)

Same standalone repro, now:

THREW: OilPriceAPI\Exception\TransportException: HTTP request to
http://127.0.0.1:18081/v1/prices/latest?by_code=BRENT_CRUDE_USD was answered
with a 302 redirect to http://127.0.0.1:18082/... This SDK does not follow
redirects, because a redirect moves the request - and the API key - to an
origin the client never validated.
capture log: (empty)

Full suite, PHP 8.5.8 / PHPUnit 13.3.3:

• baseline on clean main @ 21339d43e: OK (108 tests, 478 assertions)
• this branch: OK (119 tests, 537 assertions)

No pre-existing test changed.

Compatibility

Any caller pointing $baseUrl at a URL that redirects now gets a
TransportException instead of a silently-followed hop. Production does not
redirect, so this only affects a custom $baseUrl, and the message says
exactly what to do about it.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d9611269-63a0-4f77-affd-982d3dfadb62

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}