fix(discovery): expired bearer tokens fail closed on public endpoints (F-015)

backend/app/Services/Auth/OptionalBearerUserResolver.php

@@ -13,11 +13,12 @@
  * Behavior (F-013 characterization, not to be changed without a decision
  * recorded in docs/DECISIONS.md):
  * - A user already authenticated by the guard is returned as-is.
- * - A resolvable bearer token returns its owner.
- * - A bearer token that fails to resolve returns an empty, unpersisted
- *   `User` instance instead of `null`, so an authenticated-looking but
- *   invalid request is never widened to the anonymous public scope
- *   (see ADR-23).
+ * - A resolvable, non-expired bearer token returns its owner.
+ * - A bearer token that fails to resolve, or that resolves but has an
+ *   `expires_at` in the past, returns an empty, unpersisted `User`
+ *   instance instead of `null`, so an authenticated-looking but invalid
+ *   or lapsed request is never widened to the anonymous public scope
+ *   (see ADR-23, F-015). A token without `expires_at` never lapses here.
  * - No bearer token at all (including a header that is not a `Bearer`
  *   scheme, or an empty `Bearer` value) returns `null`, the anonymous
  *   public scope.
@@ -40,6 +41,12 @@ public function resolve(Request $request): ?User
         }
 
         $accessToken = PersonalAccessToken::findToken($token);
+
+        if ($accessToken?->expires_at?->isPast()) {
+            // Do not widen an authenticated-looking, lapsed request to the anonymous public scope.
+            return new User;
+        }
+
         $tokenable = $accessToken?->tokenable;
 
         if ($tokenable instanceof User) {

backend/tests/Feature/OptionalBearerUserResolutionTest.php

@@ -9,7 +9,6 @@
 use App\Models\User;
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Support\Facades\Artisan;
-use Laravel\Sanctum\PersonalAccessToken;
 use Meilisearch\Client;
 use Meilisearch\Exceptions\ApiException;
 
@@ -392,22 +391,77 @@ function bearerCases(array $graph): array
     }
 });
 
-it('accepts an expired bearer token as its owner on public discovery endpoints (pre-existing gap, not fixed here)', function (): void {
+it('rejects an expired bearer token, falling back to empty scope on public discovery endpoints (F-015)', function (): void {
     $graph = bearerResolutionGraph();
 
-    // findToken() from laravel/sanctum only hashes and looks up the token
-    // row; it does not check `expires_at`. That check lives in
-    // Laravel\Sanctum\Guard, which only runs behind the `auth:sanctum`
-    // middleware. CatalogController/SearchController::currentUser() call
-    // PersonalAccessToken::findToken() directly, bypassing that guard, so
-    // an expired token is currently still resolved to its owner here.
+    // OptionalBearerUserResolver now checks expires_at itself: findToken()
+    // from laravel/sanctum only hashes and looks up the token row, and the
+    // guard-level expiry check in Laravel\Sanctum\Guard never runs here
+    // because CatalogController/SearchController::currentUser() resolve
+    // outside the auth:sanctum middleware. An expired token is treated like
+    // any other bearer that fails to resolve: empty scope, never the
+    // token owner's scope and never the anonymous public scope.
     $expiredToken = $graph['member']->createToken('expired', ['*'], now()->subDay())->plainTextToken;
 
     $this->withHeaders(['Authorization' => 'Bearer '.$expiredToken])
         ->getJson('/api/v1/plugins/'.$graph['plugin']->slug)
-        ->assertOk();
+        ->assertNotFound();
 
     $this->withHeaders(['Authorization' => 'Bearer '.$expiredToken])
         ->getJson('/api/v1/plugins/'.$graph['foreignPlugin']->slug)
         ->assertNotFound();
 });
+
+it('rejects a bearer token that expired only seconds ago', function (): void {
+    $graph = bearerResolutionGraph();
+
+    $justExpiredToken = $graph['member']->createToken('just-expired', ['*'], now()->subSeconds(5))->plainTextToken;
+
+    $this->withHeaders(['Authorization' => 'Bearer '.$justExpiredToken])
+        ->getJson('/api/v1/plugins/'.$graph['plugin']->slug)
+        ->assertNotFound();
+
+    $this->withHeaders(['Authorization' => 'Bearer '.$justExpiredToken])
+        ->getJson('/api/v1/plugins/'.$graph['foreignPlugin']->slug)
+        ->assertNotFound();
+});
+
+it('still accepts a bearer token with no expires_at as its owner', function (): void {
+    $graph = bearerResolutionGraph();
+
+    $neverExpiresToken = $graph['member']->createToken('no-expiry')->plainTextToken;
+
+    $this->withHeaders(['Authorization' => 'Bearer '.$neverExpiresToken])
+        ->getJson('/api/v1/plugins/'.$graph['plugin']->slug)
+        ->assertOk();
+
+    $this->withHeaders(['Authorization' => 'Bearer '.$neverExpiresToken])
+        ->getJson('/api/v1/plugins/'.$graph['foreignPlugin']->slug)
+        ->assertNotFound();
+});
+
+it('leaves the other 7 header cases identical after the expired-token fix', function (): void {
+    $graph = bearerResolutionGraph();
+
+    foreach (bearerCases($graph) as $label => $case) {
+        $ownResponse = $this->withHeaders($case['headers'])
+            ->getJson('/api/v1/plugins/'.$graph['plugin']->slug);
+        $foreignResponse = $this->withHeaders($case['headers'])
+            ->getJson('/api/v1/plugins/'.$graph['foreignPlugin']->slug);
+
+        match ($case['group']) {
+            'public' => [
+                $ownResponse->assertOk(),
+                $foreignResponse->assertOk(),
+            ],
+            'member' => [
+                $ownResponse->assertOk(),
+                $foreignResponse->assertNotFound(),
+            ],
+            'empty' => [
+                $ownResponse->assertNotFound(),
+                $foreignResponse->assertNotFound(),
+            ],
+        };
+    }
+});

brain/audits/2026-06-13-system-audit.md

@@ -398,6 +398,8 @@ Monitor now; upgrade when compatible.
 
 Severity: Medium
 
+Status: Closed by ADR-32 in branch `fix/expired-bearer-fail-closed`. An expired token now returns the empty scope. `sanctum.expiration` stays null by decision, so a token without an explicit `expires_at` still never lapses; that is a product question, not this fix.
+
 Found on 2026-09-23 while characterizing F-013. Pre-existing, not introduced by that refactor.
 
 Evidence:

brain/canonico/CURRENT_STATE.md

@@ -49,7 +49,7 @@ Core patterns:
 
 - Controllers stay thin and delegate domain rules to services, policies, resources, jobs, and models.
 - API is versioned under `/api/v1`.
-- Public discovery endpoints are readable anonymously, but authenticated-looking requests with invalid tokens are not widened to public scope. The rule lives in a single place, `App\Services\Auth\OptionalBearerUserResolver`, injected into `CatalogController` and `SearchController` and locked by characterization tests (ADR-31). Two properties of it are deliberate and documented rather than fixed: a non-bearer `Authorization` header and an empty `Bearer` value fall into the public scope, not the empty scope, and an expired token is still accepted as its owner because these endpoints resolve the token outside the `auth:sanctum` guard (F-015).
+- Public discovery endpoints are readable anonymously, but authenticated-looking requests with invalid tokens are not widened to public scope. The rule lives in a single place, `App\Services\Auth\OptionalBearerUserResolver`, injected into `CatalogController` and `SearchController` and locked by characterization tests (ADR-31). An expired token gets the empty scope too, checked in the resolver because these endpoints resolve the token outside the `auth:sanctum` guard (ADR-32); `sanctum.expiration` is null, so a token without an explicit `expires_at` never lapses. One property is deliberate and documented rather than fixed: a non-bearer `Authorization` header and an empty `Bearer` value fall into the public scope, not the empty scope.
 - Mutating and operational endpoints require Sanctum auth and permission checks.
 - Ingestion is asynchronous-capable through jobs and Horizon, but can be exercised through services in tests.
 - Natural keys protect ingestion idempotency:

brain/canonico/NEXT_ACTIONS.md

@@ -299,7 +299,7 @@ Risks:
 - Decide whether `X-Request-Id` must appear on API responses for requests that match no route. `AssignCorrelationId` lives in the `api` middleware group, so 404 and 405 responses for unrouted paths carry no correlation ID, while the global `SecurityHeaders` still applies.
 - On Windows worktrees, Jest loads `frontend/e2e/*.spec.ts` despite `testPathIgnorePatterns: ['<rootDir>/e2e/']` and reports 2 failed suites with 0 failed tests.
 - Fix the two `runtime-config.spec.ts` Jest failures caused by Docker Compose environment variables leaking into the test `process.env`.
-- Decide F-015: public discovery endpoints accept an expired bearer token as its owner, because they resolve it through `PersonalAccessToken::findToken` outside the `auth:sanctum` guard and `sanctum.expiration` is null. Closing it changes endpoint responses, so it needs its own branch, its own decision record, and an update to the characterization test that currently locks the behavior.
+- Decide whether Sanctum tokens should expire at all. `sanctum.expiration` is null, so a token issued without an explicit `expires_at` never lapses, on public and private endpoints alike. ADR-32 closed the narrower F-015 (an expired token no longer widens the public scope) but deliberately left this product question open, because setting a lifetime changes login and session behavior everywhere.
 - Keyboard-first power-user UX for command palette actions.
 - Saved searches and team-level curated collections.
 - Export command/document references.

brain/handoffs/2026-09-23-expired-bearer-fail-closed.md

@@ -0,0 +1,30 @@
+# Handoff - Expired Bearer Fails Closed
+
+Date: 2026-09-23
+Branch: `fix/expired-bearer-fail-closed`
+
+## Scope
+
+Closes F-015, the gap recorded while characterizing F-013: the public discovery endpoints resolve the optional bearer outside the `auth:sanctum` guard, and `PersonalAccessToken::findToken()` does not check `expires_at`, so an expired token still granted its owner's scope.
+
+## Implemented
+
+- `App\Services\Auth\OptionalBearerUserResolver` short-circuits between `findToken()` and the `tokenable` check: a resolved token whose `expires_at` is past returns an unpersisted `User`, the same empty scope as any non-empty bearer that fails to resolve. A token with no `expires_at` is untouched.
+- The characterization case that locked the old behavior was inverted in the same commit, not deleted, and renamed to state the new contract. Two cases added: a token expired five seconds ago, and a token with no `expires_at` still resolving to its owner. One case pins the other seven header cases as unchanged.
+- Decision recorded in ADR-32.
+
+## Not changed, deliberately
+
+`config('sanctum.expiration')` stays null, so a token issued without an explicit `expires_at` still never lapses, on every surface. Setting a lifetime changes login and session behavior through the guard; it is a product decision, queued in `NEXT_ACTIONS.md`. Routes, private middleware, policies, and `DiscoveryAccess` untouched.
+
+## Validation
+
+- Independent adversarial audit: APPROVED. It probed the resolver directly inside the container against a copy of main's class, case by case: only the expired row differs (owner to empty scope). The `expires_at` null edge was proven by execution, not by reading `?->`.
+- The audit proved the test bites: with main's resolver and the branch's test file, exactly the two expired-token cases fail (`Expected response status code [404] but received 200`) and the other ten pass. So the inverted case is stronger, not looser: 404 on the owner's own plugin separates empty scope from both owner (200) and public (200).
+- Assertion inventory main to branch: nothing shrank (`assertNotFound` 11 to 18, `assertOk` 17 to 20, the rest equal). No `skip`, `markTestSkipped`, `->todo(`, or `xit(`.
+- Gates on the rebuilt image, hashes confirmed to match the branch: Pest 100 passed, Pint 134 files. Assertion counts drift between runs (564 and 566 observed) because of the Meilisearch `retry()` loop; the test count is the stable figure.
+- A dead `use Laravel\Sanctum\PersonalAccessToken;` was removed from the test file. Verified dead on `main`: its only other occurrence there is inside a comment.
+
+## Open
+
+- Whether Sanctum tokens should expire at all. Until that is decided, this fix only reaches tokens that were given an explicit `expires_at`.

docs/DECISIONS.md

@@ -396,3 +396,22 @@ A regra de escopo dos endpoints públicos passa a ter um único ponto de manuten
 
 1. `Authorization: Basic ...` e `Bearer` vazio caem no escopo público, e não no escopo vazio. O guard clause devolve `null` assim que `bearerToken()` é nulo ou vazio, então o escopo vazio só existe para bearer não vazio que falha ao resolver.
 2. `findToken` não verifica `expires_at`, e `config('sanctum.expiration')` é `null`. Um token expirado continua aceito como seu dono nestes endpoints públicos, porque eles resolvem o token fora do guard `auth:sanctum`. Os endpoints privados não são afetados: ali quem valida é o guard. A lacuna é pré-existente, está travada por teste nomeado como tal e documentada no docblock do serviço; fechá-la é tarefa própria, com decisão registrada, porque muda resposta de endpoint.
+
+## ADR-32 — Token expirado falha fechado nos endpoints públicos de descoberta
+
+### Contexto
+Os endpoints públicos de catálogo e busca resolvem o bearer opcional por `PersonalAccessToken::findToken()`, fora do guard `auth:sanctum` (ADR-31). Esse método faz apenas hash e lookup: a verificação de validade temporal vive em `Laravel\Sanctum\Guard`, que só roda atrás do guard. Como `config('sanctum.expiration')` é nulo, nada mais compensava a ausência dessa checagem, e um token com `expires_at` no passado continuava valendo como seu dono nesses endpoints (F-015). O efeito era anular a expiração como mecanismo de revogação justamente na superfície que não passa pelo guard. A lacuna foi encontrada ao caracterizar a F-013 e deixada registrada de propósito, para não misturar mudança funcional a um refactor.
+
+### Decisão
+Rejeitar o token expirado dentro do próprio resolver, entre o `findToken()` e a checagem do `tokenable`: um token resolvido cujo `expires_at` já passou devolve `new User` não persistido, o mesmo escopo vazio de qualquer bearer não vazio que não resolve. Nunca o escopo público, porque a requisição tem credencial e não pode ser promovida a anônima (ADR-23).
+
+Um token sem `expires_at` continua válido, sem alteração: a checagem é estritamente sobre uma data já vencida.
+
+`config('sanctum.expiration')` permanece nulo, por decisão explícita. Defini-lo faria todo token passar a vencer, inclusive nos endpoints privados via guard, o que muda login e sessão — é questão de produto, com tarefa e decisão próprias, e segue na fila.
+
+### Consequências
+A expiração volta a valer como revogação em toda a superfície pública: um token vencido deixa de ver o catálogo do seu dono e passa a não ver nada, em vez de cair no escopo público. Endpoints privados não mudam, porque ali quem valida continua sendo o guard.
+
+O caso de caracterização que travava o comportamento antigo foi invertido no mesmo commit da correção, com asserção mais discriminante: `404` no plugin do próprio dono distingue escopo vazio tanto de dono (`200`) quanto de público (`200`), o que uma asserção de `200` não faria. A auditoria independente confirmou que os dez casos anteriores continuam passando contra o resolver da `main`, e que os dois casos novos falham contra ele — ou seja, o teste morde a mudança em vez de apenas acompanhá-la.
+
+Com `sanctum.expiration` nulo, a proteção só alcança tokens que receberam `expires_at` explícito. Enquanto essa decisão de produto não for tomada, a maioria dos tokens emitidos não vence, e este ADR não deve ser lido como se tokens caducassem sozinhos.