fix(codex): demote scope validation to warn-only (raphaeltm#772)

* fix(codex): demote scope validation to warn-only to unblock token refresh

The scope validation added in PR raphaeltm#756 (MEDIUM #6) blocks Codex OAuth token
refresh with 502 when OpenAI returns any scopes beyond the hardcoded allowlist
(openid, profile, email, offline_access). Since we don't yet know the full set
of scopes OpenAI returns in practice, this was silently breaking all Codex
token refreshes — expired tokens couldn't be renewed, causing "Authentication
required" errors.

Changes:
- Default CODEX_SCOPE_VALIDATION_MODE to 'warn' (log but allow)
- Add CODEX_SCOPE_VALIDATION_MODE env var to opt back into 'block' mode
- Update tests to cover both warn and block behaviors
- The unexpected_scopes_allowed log line will reveal what scopes OpenAI
  actually returns, so we can update the allowlist and re-enable blocking

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: trigger CI with updated PR body

---------

Co-authored-by: Raphaël Titsworth-Morin <raphael@raphaeltm.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: 3 people

apps/api/src/durable-objects/codex-refresh-lock.ts

@@ -58,6 +58,11 @@ interface CodexRefreshEnv {
    * Empty string disables scope validation. Unset uses DEFAULT_EXPECTED_SCOPES.
    */
   CODEX_EXPECTED_SCOPES?: string;
+  /**
+   * 'warn' (default) or 'block'. Controls whether unexpected scopes block
+   * the refresh (502) or just log a warning and allow it to proceed.
+   */
+  CODEX_SCOPE_VALIDATION_MODE?: string;
   /**
    * Rate limit: max refresh requests per user per window. Defaults to 30.
    */
@@ -290,16 +295,30 @@ export class CodexRefreshLock extends DurableObject<CodexRefreshEnv> {
     // Parse new tokens from OpenAI response.
     const newTokens: Record<string, unknown> = await upstreamResponse.json();
 
-    // Scope validation (MEDIUM #6) — block instead of warn-only when upstream returns
-    // unexpected scopes. A scope change signals either provider drift or an escalation
-    // attempt; either way we refuse to store the new tokens and return 502 so the
-    // caller stays on the previous (validated) credential.
+    // Scope validation (MEDIUM #6) — warn-only mode.
+    // Previously this blocked with 502 when upstream returned unexpected scopes,
+    // but we don't yet know the full set of scopes OpenAI returns in practice.
+    // If the allowlist was incomplete, every refresh would silently fail, leaving
+    // Codex with expired tokens and "Authentication required" errors.
+    // Demoted to warn-only until we capture real-world scope data from logs.
+    // Re-enable blocking by setting CODEX_SCOPE_VALIDATION_MODE=block.
     const scopeResult = this.validateUpstreamScopes(newTokens, userId);
     if (!scopeResult.ok) {
-      return new Response(
-        JSON.stringify({ error: 'upstream_unexpected_scope', message: scopeResult.reason }),
-        { status: 502, headers: { 'Content-Type': 'application/json' } }
-      );
+      const validationMode = this.env.CODEX_SCOPE_VALIDATION_MODE ?? 'warn';
+      if (validationMode === 'block') {
+        return new Response(
+          JSON.stringify({ error: 'upstream_unexpected_scope', message: scopeResult.reason }),
+          { status: 502, headers: { 'Content-Type': 'application/json' } }
+        );
+      }
+      // Warn-only: log the unexpected scopes but allow the refresh to proceed.
+      // This lets us discover what scopes OpenAI actually returns without
+      // breaking token refresh for all users.
+      log.warn('codex_refresh.unexpected_scopes_allowed', {
+        userId,
+        reason: scopeResult.reason,
+        validationMode,
+      });
     }
 
     // Update the stored auth.json with new tokens.
@@ -363,13 +382,12 @@ export class CodexRefreshLock extends DurableObject<CodexRefreshEnv> {
   /**
    * Validate scopes in the upstream token response.
    *
-   * Default behavior (MEDIUM #6 fix): enforce a conservative allowlist derived from
-   * known Codex OAuth flow scopes. Unexpected scopes cause the refresh to fail with
-   * 502 — the previous token remains valid so the caller can continue operating
-   * until an admin investigates.
+   * Returns { ok: false } when scopes don't match the allowlist. The caller
+   * decides whether to block (502) or warn based on CODEX_SCOPE_VALIDATION_MODE.
+   * Default mode is 'warn' — unexpected scopes are logged but refresh proceeds.
    *
-   * Override with CODEX_EXPECTED_SCOPES (comma-separated). Empty string disables
-   * validation entirely (escape hatch for provider rollouts that add new scopes).
+   * Override allowlist with CODEX_EXPECTED_SCOPES (comma-separated). Empty
+   * string disables validation entirely.
    */
   private validateUpstreamScopes(
     tokens: Record<string, unknown>,

apps/api/src/env.ts

@@ -316,7 +316,8 @@ export interface Env {
   CODEX_REFRESH_UPSTREAM_URL?: string;             // OpenAI token endpoint (default: https://auth.openai.com/oauth/token)
   CODEX_REFRESH_UPSTREAM_TIMEOUT_MS?: string;      // Upstream request timeout (default: 10000)
   CODEX_CLIENT_ID?: string;                        // OpenAI OAuth client_id (default: app_EMoamEEZ73f0CkXaXp7hrann)
-  CODEX_EXPECTED_SCOPES?: string;                  // Comma-separated scope allowlist; unset = default allowlist enforced (openid,profile,email,offline_access); empty string disables validation; unexpected scopes block refresh with 502 (MEDIUM #6)
+  CODEX_EXPECTED_SCOPES?: string;                  // Comma-separated scope allowlist; unset = default allowlist enforced (openid,profile,email,offline_access); empty string disables validation
+  CODEX_SCOPE_VALIDATION_MODE?: string;            // 'warn' (default) or 'block' — controls whether unexpected scopes block refresh (502) or just log a warning
   // Google OAuth (for GCP OIDC integration)
   GOOGLE_CLIENT_ID?: string;
   GOOGLE_CLIENT_SECRET?: string;

apps/api/tests/unit/durable-objects/codex-refresh-lock.test.ts

@@ -8,7 +8,7 @@
  *    fallback, absent row falls through to user-scoped
  *  - MEDIUM #5: rate-limit state held in `ctx.storage` (atomic per-user), 429 with
  *    Retry-After on exceed
- *  - MEDIUM #6: upstream scope validation blocks (502) instead of warn-only
+ *  - MEDIUM #6: upstream scope validation (warn-only by default; block when CODEX_SCOPE_VALIDATION_MODE=block)
  *  - Decrypt/parse failure handling
  *  - Upstream error paths (timeout, network, filtered error body)
  */
@@ -599,13 +599,45 @@ describe('CodexRefreshLock', () => {
   });
 
   // -----------------------------------------------------------------------
-  // MEDIUM #6 — Scope validation now BLOCKS (502), not warn-only
+  // MEDIUM #6 — Scope validation (warn-only by default, block when opted in)
   // -----------------------------------------------------------------------
 
-  describe('MEDIUM #6 — scope validation blocks', () => {
-    it('blocks with 502 on unexpected scope in upstream response', async () => {
+  describe('MEDIUM #6 — scope validation', () => {
+    it('warns but allows refresh on unexpected scope by default (warn mode)', async () => {
       const { do: doInstance, env } = createDO({
         CODEX_EXPECTED_SCOPES: 'openid,offline_access',
+        // CODEX_SCOPE_VALIDATION_MODE not set — defaults to 'warn'
+      });
+      setupCredentialFound(env);
+
+      vi.mocked(fetch).mockResolvedValue(
+        new Response(
+          JSON.stringify({
+            access_token: 'new-access',
+            refresh_token: 'new-refresh',
+            id_token: 'new-id',
+            scope: 'openid offline_access admin:write',
+          }),
+          { status: 200, headers: { 'Content-Type': 'application/json' } },
+        ),
+      );
+
+      const res = await doInstance.fetch(
+        makeRequest({ refreshToken: 'stored-refresh', userId: 'user-1' }),
+      );
+      // Warn mode: refresh succeeds despite unexpected scopes
+      expect(res.status).toBe(200);
+      expect(vi.mocked(encrypt)).toHaveBeenCalled();
+      expect(mockLogWarn).toHaveBeenCalledWith(
+        'codex_refresh.unexpected_scopes_allowed',
+        expect.objectContaining({ validationMode: 'warn' }),
+      );
+    });
+
+    it('blocks with 502 on unexpected scope when CODEX_SCOPE_VALIDATION_MODE=block', async () => {
+      const { do: doInstance, env } = createDO({
+        CODEX_EXPECTED_SCOPES: 'openid,offline_access',
+        CODEX_SCOPE_VALIDATION_MODE: 'block',
       });
       setupCredentialFound(env);
 
@@ -628,7 +660,7 @@ describe('CodexRefreshLock', () => {
       const json = await res.json();
       expect(json.error).toBe('upstream_unexpected_scope');
 
-      // MUST NOT persist tokens that fail validation.
+      // MUST NOT persist tokens that fail validation in block mode.
       expect(vi.mocked(encrypt)).not.toHaveBeenCalled();
       expect(mockLogWarn).toHaveBeenCalledWith(
         'codex_refresh.unexpected_scopes_blocked',
@@ -660,7 +692,7 @@ describe('CodexRefreshLock', () => {
       expect(vi.mocked(encrypt)).toHaveBeenCalled();
     });
 
-    it('uses default scope allowlist when CODEX_EXPECTED_SCOPES is unset', async () => {
+    it('warns by default when CODEX_EXPECTED_SCOPES is unset (uses default allowlist in warn mode)', async () => {
       // Omit CODEX_EXPECTED_SCOPES entirely — DO must apply DEFAULT_EXPECTED_SCOPES.
       const env = createMockEnv();
       // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
@@ -683,8 +715,13 @@ describe('CodexRefreshLock', () => {
       const res = await doInstance.fetch(
         makeRequest({ refreshToken: 'stored-refresh', userId: 'user-1' }),
       );
-      // With defaults active, admin:write blocks.
-      expect(res.status).toBe(502);
+      // Default is warn mode — refresh succeeds, warning logged
+      expect(res.status).toBe(200);
+      expect(vi.mocked(encrypt)).toHaveBeenCalled();
+      expect(mockLogWarn).toHaveBeenCalledWith(
+        'codex_refresh.unexpected_scopes_allowed',
+        expect.objectContaining({ validationMode: 'warn' }),
+      );
     });
 
     it('allows all scopes when CODEX_EXPECTED_SCOPES is set to empty string (escape hatch)', async () => {
@@ -709,9 +746,10 @@ describe('CodexRefreshLock', () => {
       expect(vi.mocked(encrypt)).toHaveBeenCalled();
     });
 
-    it('blocks non-string scope values', async () => {
+    it('blocks non-string scope values in block mode', async () => {
       const { do: doInstance, env } = createDO({
         CODEX_EXPECTED_SCOPES: 'openid',
+        CODEX_SCOPE_VALIDATION_MODE: 'block',
       });
       setupCredentialFound(env);
 
@@ -737,6 +775,35 @@ describe('CodexRefreshLock', () => {
       expect(vi.mocked(encrypt)).not.toHaveBeenCalled();
     });
 
+    it('warns but allows non-string scope values in default warn mode', async () => {
+      const { do: doInstance, env } = createDO({
+        CODEX_EXPECTED_SCOPES: 'openid',
+      });
+      setupCredentialFound(env);
+
+      vi.mocked(fetch).mockResolvedValue(
+        new Response(
+          JSON.stringify({
+            access_token: 'new-access',
+            refresh_token: 'new-refresh',
+            scope: 42, // non-string scope
+          }),
+          { status: 200, headers: { 'Content-Type': 'application/json' } },
+        ),
+      );
+
+      const res = await doInstance.fetch(
+        makeRequest({ refreshToken: 'stored-refresh', userId: 'user-1' }),
+      );
+      // Warn mode: refresh succeeds despite non-string scope
+      expect(res.status).toBe(200);
+      expect(mockLogWarn).toHaveBeenCalledWith(
+        'codex_refresh.scope_validation_nonstring',
+        expect.objectContaining({ scopeType: 'number' }),
+      );
+      expect(vi.mocked(encrypt)).toHaveBeenCalled();
+    });
+
     it('allows responses with no scope field', async () => {
       const { do: doInstance, env } = createDO({
         CODEX_EXPECTED_SCOPES: 'openid,offline_access',