fix: rate-limit pre-auth and unauthenticated paths (SEC_REVIEW F27)

RateLimitMiddleware previously bailed out and forwarded the request
unrate-limited whenever `$principal` was missing. Combined with the
middleware's position *inside* TokenAuthenticationMiddleware, this meant
any request that returned 401 (invalid bearer, malformed token, missing
header) reached the DB token-table lookup with no backoff — an attacker
could pin the connection budget by hammering a fake bearer.

The handler now picks a namespaced bucket key:
  - principal present  → `token:<tokenId>`  (post-auth, original behaviour)
  - principal missing  → `ip:<REMOTE_ADDR>` (pre-auth, new fail-closed path)

The middleware is registered twice on /api/v1/* and /api/v1/auth/*: once
as the outermost layer (consumes the IP bucket before TokenAuth runs)
and once after TokenAuth (consumes the token bucket once a principal is
attached). The two namespaces draw on independent buckets so legitimate
authenticated bursts are not amplified by the IP cap. RateLimiter keys
are now arbitrary strings; existing call sites pass the namespaced form.

Tests cover the IP fallback, the token/IP namespace isolation, and an
end-to-end burst with an invalid bearer that must produce 429s after
the bucket drains.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

api/src/App/AppFactory.php

@@ -50,10 +50,14 @@ use Slim\Routing\RouteCollectorProxy;
  * add them in reverse on each route group.
  *
  * Route groups in M04:
- *  - Public    /api/v1/report               TokenAuth → RateLimit → controller (kind check inside)
+ *  - Public    /api/v1/report               RateLimit(ip) → TokenAuth → AuditContext → RateLimit(token) → controller (kind check inside)
  *  - Admin     /api/v1/admin/{reporters,consumers,tokens}  TokenAuth → Impersonation → Rbac(Admin)
  *  - Admin     /api/v1/admin/me             TokenAuth → Impersonation → Rbac(Viewer)
- *  - Auth      /api/v1/auth/*               TokenAuth → AuditContext → RateLimit (controller checks kind=service)
+ *  - Auth      /api/v1/auth/*               RateLimit(ip) → TokenAuth → AuditContext → RateLimit(token) → controller (kind=service inside)
+ *
+ * The two RateLimit positions (SEC_REVIEW F27) draw on independent
+ * `ip:` and `token:` buckets so an invalid-bearer-token flood is
+ * throttled before TokenAuthenticationMiddleware ever queries the DB.
  */
 final class AppFactory
 {
@@ -175,7 +179,8 @@ final class AppFactory
         })
             ->add($rateLimit)
             ->add($auditContext)
-            ->add($tokenAuth);
+            ->add($tokenAuth)
+            ->add($rateLimit);
 
         // Public API: ingest endpoint. Auth → rate limit → controller. The
         // controller rejects non-reporter kinds itself (uniform 401 per
@@ -191,7 +196,8 @@ final class AppFactory
         })
             ->add($rateLimit)
             ->add($auditContext)
-            ->add($tokenAuth);
+            ->add($tokenAuth)
+            ->add($rateLimit);
 
         // Admin API: token auth → impersonation → role check.
         $app->group('/api/v1/admin', function (RouteCollectorProxy $admin) use ($container, $rf): void {

api/src/Infrastructure/Http/Middleware/RateLimitMiddleware.php

@@ -13,10 +13,18 @@ use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 
 /**
- * Per-token-id rate limit. Runs after TokenAuthenticationMiddleware so we
- * have a `tokenId` to bucket on. If the principal is somehow missing
- * (defensive), the limiter is bypassed — auth/RBAC will reject the
- * request shortly anyway.
+ * Rate-limit gate, used in two positions per route group (SEC_REVIEW F27):
+ *
+ *  1. Outermost — runs before TokenAuthenticationMiddleware. The principal
+ *     is absent so the bucket key falls back to `ip:<REMOTE_ADDR>`. This
+ *     throttles invalid-bearer-token floods (every miss costs an
+ *     `tokens.findByHash()` query and, on rare hash collisions, a
+ *     `last_used_at` write) and 401-only paths that previously bypassed
+ *     the limiter entirely.
+ *
+ *  2. After auth — runs once a principal exists, bucketed on
+ *     `token:<tokenId>` so a single token cannot saturate the API even
+ *     from a fleet of egress IPs.
  *
  * On exhaustion: 429 with `Retry-After: 1` per SPEC §6.
  */
@@ -30,12 +38,9 @@ final class RateLimitMiddleware implements MiddlewareInterface
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
-        $principal = $request->getAttribute(TokenAuthenticationMiddleware::ATTR_PRINCIPAL);
-        if (!$principal instanceof AuthenticatedPrincipal) {
-            return $handler->handle($request);
-        }
+        $key = $this->bucketKey($request);
 
-        if (!$this->limiter->tryConsume($principal->tokenId)) {
+        if (!$this->limiter->tryConsume($key)) {
             $response = $this->responseFactory->createResponse(429)
                 ->withHeader('Content-Type', 'application/json')
                 ->withHeader('Retry-After', '1');
@@ -46,4 +51,17 @@ final class RateLimitMiddleware implements MiddlewareInterface
 
         return $handler->handle($request);
     }
+
+    private function bucketKey(ServerRequestInterface $request): string
+    {
+        $principal = $request->getAttribute(TokenAuthenticationMiddleware::ATTR_PRINCIPAL);
+        if ($principal instanceof AuthenticatedPrincipal) {
+            return 'token:' . $principal->tokenId;
+        }
+
+        $params = $request->getServerParams();
+        $remote = $params['REMOTE_ADDR'] ?? null;
+
+        return 'ip:' . (is_string($remote) && $remote !== '' ? $remote : 'unknown');
+    }
 }

api/src/Infrastructure/Http/RateLimiter.php

@@ -7,7 +7,13 @@ namespace App\Infrastructure\Http;
 use App\Domain\Time\Clock;
 
 /**
- * In-process token-bucket rate limiter, keyed by token id.
+ * In-process token-bucket rate limiter, keyed by an arbitrary string.
+ *
+ * Callers namespace their keys (`token:42`, `ip:1.2.3.4`) so authenticated
+ * and unauthenticated requests draw from independent buckets. The
+ * unauthenticated `ip:*` bucket is what throttles invalid-bearer-token
+ * floods that would otherwise hit `TokenRepository::findByHash()` for
+ * every attempt (SEC_REVIEW F27).
  *
  * Capacity = `API_RATE_LIMIT_PER_SECOND × 2`, refill rate = same per second.
  * Refill is computed lazily on each `tryConsume()` call, so an idle bucket
@@ -19,7 +25,7 @@ use App\Domain\Time\Clock;
  */
 final class RateLimiter
 {
-    /** @var array<int, array{tokens: float, updated: float}> */
+    /** @var array<string, array{tokens: float, updated: float}> */
     private array $buckets = [];
 
     public function __construct(
@@ -29,7 +35,7 @@ final class RateLimiter
     ) {
     }
 
-    public function tryConsume(int $key): bool
+    public function tryConsume(string $key): bool
     {
         $now = (float) $this->clock->now()->getTimestamp() + ((float) $this->clock->now()->format('u') / 1_000_000.0);
 

api/tests/Integration/Public/RateLimitTest.php

@@ -141,4 +141,43 @@ final class RateLimitTest extends AppTestCase
         $limited = count(array_filter($statuses, static fn (int $s): bool => $s === 429));
         self::assertGreaterThan(0, $limited, 'upsert-local must rate-limit');
     }
+
+    /**
+     * SEC_REVIEW F27. Auth-failed paths (401 from
+     * TokenAuthenticationMiddleware) used to bypass the rate limiter
+     * because RateLimitMiddleware sat *inside* TokenAuth. The handler
+     * now also runs as the outermost layer, keying on `ip:` when no
+     * principal exists, so invalid-bearer-token floods are throttled
+     * before the DB lookup.
+     */
+    public function testInvalidBearerTokenFloodIsRateLimitedBeforeAuth(): void
+    {
+        $headers = ['Authorization' => 'Bearer ' . str_repeat('a', 32)];
+
+        $statuses = [];
+        for ($i = 0; $i < 20; $i++) {
+            $statuses[] = $this->request('POST', '/api/v1/report', $headers)->getStatusCode();
+        }
+
+        $unauthorized = count(array_filter($statuses, static fn (int $s): bool => $s === 401));
+        $limited = count(array_filter($statuses, static fn (int $s): bool => $s === 429));
+
+        // Capacity is 4, so the first 4 attempts incur a token-table lookup
+        // and return 401; the rest drain the IP bucket and return 429.
+        self::assertSame(4, $unauthorized, 'first 4 attempts hit the auth path');
+        self::assertSame(16, $limited, 'remaining attempts must 429 before auth');
+    }
+
+    public function testMissingBearerHeaderIsAlsoRateLimitedByIp(): void
+    {
+        // No Authorization header at all — same protection applies, the
+        // request must not reach TokenAuth on every retry.
+        $statuses = [];
+        for ($i = 0; $i < 10; $i++) {
+            $statuses[] = $this->request('GET', '/api/v1/blocklist')->getStatusCode();
+        }
+
+        $limited = count(array_filter($statuses, static fn (int $s): bool => $s === 429));
+        self::assertGreaterThan(0, $limited, 'pre-auth flood must hit 429');
+    }
 }

api/tests/Unit/Http/RateLimitMiddlewareTest.php

@@ -0,0 +1,140 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\Http;
+
+use App\Domain\Auth\AuthenticatedPrincipal;
+use App\Domain\Auth\TokenKind;
+use App\Domain\Time\FixedClock;
+use App\Infrastructure\Http\Middleware\RateLimitMiddleware;
+use App\Infrastructure\Http\Middleware\TokenAuthenticationMiddleware;
+use App\Infrastructure\Http\RateLimiter;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use Slim\Psr7\Factory\ResponseFactory;
+use Slim\Psr7\Factory\ServerRequestFactory;
+
+/**
+ * SEC_REVIEW F27. Two invariants:
+ *
+ * 1. With a principal: bucket key is `token:<tokenId>`, no IP draw.
+ * 2. Without a principal: bucket key falls back to `ip:<REMOTE_ADDR>`
+ *    so 401 paths and pre-auth flooding are throttled instead of
+ *    bypassing the limiter.
+ *
+ * The `token:` and `ip:` namespaces draw on independent buckets — see
+ * {@see RateLimiterTest::testTokenAndIpNamespacesDoNotShareABucket}.
+ */
+final class RateLimitMiddlewareTest extends TestCase
+{
+    public function testAuthenticatedRequestKeysOnTokenId(): void
+    {
+        $clock = FixedClock::at('2026-04-29T00:00:00Z');
+        // capacity=2, refill=1/s. Token 42 should drain in two requests.
+        $limiter = new RateLimiter($clock, 1.0, 2.0);
+        $middleware = new RateLimitMiddleware($limiter, new ResponseFactory());
+
+        $principal = new AuthenticatedPrincipal(
+            tokenKind: TokenKind::Reporter,
+            userId: null,
+            role: null,
+            reporterId: 1,
+            consumerId: null,
+            tokenId: 42,
+        );
+
+        $request = $this->request('1.2.3.4')
+            ->withAttribute(TokenAuthenticationMiddleware::ATTR_PRINCIPAL, $principal);
+
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(429, $middleware->process($request, $this->okHandler())->getStatusCode());
+
+        // Confirm the bucket actually was `token:42` — a different IP-only
+        // request still has a full ip-bucket and must succeed.
+        $unauth = $this->request('1.2.3.4');
+        self::assertSame(200, $middleware->process($unauth, $this->okHandler())->getStatusCode());
+    }
+
+    public function testUnauthenticatedRequestKeysOnRemoteAddr(): void
+    {
+        $clock = FixedClock::at('2026-04-29T00:00:00Z');
+        $limiter = new RateLimiter($clock, 1.0, 2.0);
+        $middleware = new RateLimitMiddleware($limiter, new ResponseFactory());
+
+        $request = $this->request('1.2.3.4');
+
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+
+        // Third request from same IP, still no principal — bucket is drained.
+        $third = $middleware->process($request, $this->okHandler());
+        self::assertSame(429, $third->getStatusCode());
+        self::assertSame('1', $third->getHeaderLine('Retry-After'));
+        self::assertSame('application/json', $third->getHeaderLine('Content-Type'));
+        self::assertStringContainsString('rate_limited', (string) $third->getBody());
+    }
+
+    public function testDifferentRemoteIpsHaveIndependentBuckets(): void
+    {
+        $clock = FixedClock::at('2026-04-29T00:00:00Z');
+        $limiter = new RateLimiter($clock, 1.0, 2.0);
+        $middleware = new RateLimitMiddleware($limiter, new ResponseFactory());
+
+        // Drain 1.2.3.4.
+        $a = $this->request('1.2.3.4');
+        $middleware->process($a, $this->okHandler());
+        $middleware->process($a, $this->okHandler());
+        self::assertSame(429, $middleware->process($a, $this->okHandler())->getStatusCode());
+
+        // 5.6.7.8 is unaffected.
+        $b = $this->request('5.6.7.8');
+        self::assertSame(200, $middleware->process($b, $this->okHandler())->getStatusCode());
+        self::assertSame(200, $middleware->process($b, $this->okHandler())->getStatusCode());
+    }
+
+    public function testMissingRemoteAddrFallsBackToUnknownBucket(): void
+    {
+        $clock = FixedClock::at('2026-04-29T00:00:00Z');
+        $limiter = new RateLimiter($clock, 1.0, 2.0);
+        $middleware = new RateLimitMiddleware($limiter, new ResponseFactory());
+
+        $request = (new ServerRequestFactory())->createServerRequest('GET', 'https://api.test/x');
+
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(429, $middleware->process($request, $this->okHandler())->getStatusCode());
+    }
+
+    public function testEmptyRemoteAddrFallsBackToUnknownBucket(): void
+    {
+        $clock = FixedClock::at('2026-04-29T00:00:00Z');
+        $limiter = new RateLimiter($clock, 1.0, 2.0);
+        $middleware = new RateLimitMiddleware($limiter, new ResponseFactory());
+
+        $request = $this->request('');
+
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(200, $middleware->process($request, $this->okHandler())->getStatusCode());
+        self::assertSame(429, $middleware->process($request, $this->okHandler())->getStatusCode());
+    }
+
+    private function request(string $remoteAddr): ServerRequestInterface
+    {
+        return (new ServerRequestFactory())
+            ->createServerRequest('GET', 'https://api.test/x', ['REMOTE_ADDR' => $remoteAddr]);
+    }
+
+    private function okHandler(): RequestHandlerInterface
+    {
+        return new class () implements RequestHandlerInterface {
+            public function handle(ServerRequestInterface $request): ResponseInterface
+            {
+                return (new ResponseFactory())->createResponse(200);
+            }
+        };
+    }
+}

api/tests/Unit/Http/RateLimiterTest.php

@@ -13,6 +13,8 @@ use PHPUnit\Framework\TestCase;
  *  - capacity = 2 × refill, so 6 immediate consumes succeed at refill=3.
  *  - the 7th fails, then advancing 1s restores 3 tokens.
  *  - per-key isolation: two keys do not share a bucket.
+ *  - keys are arbitrary strings so callers can namespace tokens vs IPs
+ *    (SEC_REVIEW F27).
  */
 final class RateLimiterTest extends TestCase
 {
@@ -22,9 +24,9 @@ final class RateLimiterTest extends TestCase
         $limiter = new RateLimiter($clock, refillPerSecond: 3.0, capacity: 6.0);
 
         for ($i = 0; $i < 6; $i++) {
-            self::assertTrue($limiter->tryConsume(1), "consume #{$i} should succeed");
+            self::assertTrue($limiter->tryConsume('token:1'), "consume #{$i} should succeed");
         }
-        self::assertFalse($limiter->tryConsume(1), 'consume #7 should be denied');
+        self::assertFalse($limiter->tryConsume('token:1'), 'consume #7 should be denied');
     }
 
     public function testRefillRestoresTokensOverTime(): void
@@ -33,17 +35,17 @@ final class RateLimiterTest extends TestCase
         $limiter = new RateLimiter($clock, refillPerSecond: 3.0, capacity: 6.0);
 
         for ($i = 0; $i < 6; $i++) {
-            $limiter->tryConsume(1);
+            $limiter->tryConsume('token:1');
         }
-        self::assertFalse($limiter->tryConsume(1));
+        self::assertFalse($limiter->tryConsume('token:1'));
 
         $clock->advance('+1 second');
 
         // After 1s @ 3/s we should have 3 fresh tokens.
-        self::assertTrue($limiter->tryConsume(1));
-        self::assertTrue($limiter->tryConsume(1));
-        self::assertTrue($limiter->tryConsume(1));
-        self::assertFalse($limiter->tryConsume(1));
+        self::assertTrue($limiter->tryConsume('token:1'));
+        self::assertTrue($limiter->tryConsume('token:1'));
+        self::assertTrue($limiter->tryConsume('token:1'));
+        self::assertFalse($limiter->tryConsume('token:1'));
     }
 
     public function testKeysAreIsolated(): void
@@ -51,12 +53,31 @@ final class RateLimiterTest extends TestCase
         $clock = FixedClock::at('2026-04-29T00:00:00Z');
         $limiter = new RateLimiter($clock, refillPerSecond: 1.0, capacity: 2.0);
 
-        self::assertTrue($limiter->tryConsume(1));
-        self::assertTrue($limiter->tryConsume(1));
-        self::assertFalse($limiter->tryConsume(1));
+        self::assertTrue($limiter->tryConsume('token:1'));
+        self::assertTrue($limiter->tryConsume('token:1'));
+        self::assertFalse($limiter->tryConsume('token:1'));
 
         // Token 2's bucket is untouched.
-        self::assertTrue($limiter->tryConsume(2));
-        self::assertTrue($limiter->tryConsume(2));
+        self::assertTrue($limiter->tryConsume('token:2'));
+        self::assertTrue($limiter->tryConsume('token:2'));
+    }
+
+    public function testTokenAndIpNamespacesDoNotShareABucket(): void
+    {
+        // SEC_REVIEW F27: namespace-prefixed keys mean an authenticated
+        // user's `token:42` bucket cannot drain the same shared `ip:…`
+        // bucket that throttles unauthenticated traffic from elsewhere.
+        $clock = FixedClock::at('2026-04-29T00:00:00Z');
+        $limiter = new RateLimiter($clock, refillPerSecond: 1.0, capacity: 2.0);
+
+        self::assertTrue($limiter->tryConsume('token:42'));
+        self::assertTrue($limiter->tryConsume('token:42'));
+        self::assertFalse($limiter->tryConsume('token:42'));
+
+        // Same numeric tail under a different namespace must be its own
+        // bucket — string keys are not silently coerced to int.
+        self::assertTrue($limiter->tryConsume('ip:42'));
+        self::assertTrue($limiter->tryConsume('ip:42'));
+        self::assertFalse($limiter->tryConsume('ip:42'));
     }
 }