fix: codex refresh proxy security hardening (#603)

* chore: move task to active

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

* fix: address deferred security findings from codex refresh proxy

- Add per-workspace rate limiting to /api/auth/codex-refresh endpoint
  (default: 30/hour, configurable via RATE_LIMIT_CODEX_REFRESH)
- Add scope validation on upstream token responses with warning log
  (configurable via CODEX_EXPECTED_SCOPES, non-blocking)
- Document token-in-URL accepted risk in secrets-taxonomy.md with
  mitigations (short-lived JWT, scope enforcement, rate limiting)
- Document JWT token lifetimes and callback token design rationale

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

* fix: use structured logger for scope validation warnings

Replace console.warn with log.warn from lib/logger to comply with
no-console lint rule for API code.

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

* chore: update task checklist with completed items

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

* fix: address validator findings — doc sync and edge case test

- Add RATE_LIMIT_CODEX_REFRESH and CODEX_EXPECTED_SCOPES to .env.example
- Add test for non-string scope edge case in upstream response

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

* chore: archive completed task

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

* fix: add env var override for codex refresh rate limit window

RATE_LIMIT_CODEX_REFRESH_WINDOW_SECONDS allows configuring the rate
limit window per constitution Principle XI (no hardcoded values).

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

* docs: address review findings — self-hosting vars, KV race note

- Add RATE_LIMIT_CODEX_REFRESH, RATE_LIMIT_CODEX_REFRESH_WINDOW_SECONDS,
  and CODEX_EXPECTED_SCOPES to self-hosting.md configurable variables table
- Document KV rate limiter's non-atomic read-modify-write as a known
  limitation (pre-existing pattern across all rate-limited endpoints)

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

---------

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/.env.example

@@ -345,3 +345,6 @@ BASE_DOMAIN=simple-agent-manager.org
 # CODEX_REFRESH_UPSTREAM_URL=https://auth.openai.com/oauth/token  # OpenAI token endpoint
 # CODEX_REFRESH_UPSTREAM_TIMEOUT_MS=10000              # Upstream request timeout (default: 10s)
 # CODEX_CLIENT_ID=app_EMoamEEZ73f0CkXaXp7hrann        # OpenAI OAuth client_id
+# RATE_LIMIT_CODEX_REFRESH=30                          # Max refresh requests per window per workspace (default: 30)
+# RATE_LIMIT_CODEX_REFRESH_WINDOW_SECONDS=3600         # Rate limit window in seconds (default: 3600 = 1 hour)
+# CODEX_EXPECTED_SCOPES=openid,offline_access          # Comma-separated expected scopes for upstream validation (warning log only)

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

@@ -13,6 +13,7 @@
  */
 import { DurableObject } from 'cloudflare:workers';
 
+import { log } from '../lib/logger';
 import { getCredentialEncryptionKey } from '../lib/secrets';
 import { decrypt, encrypt } from '../services/encryption';
 
@@ -31,6 +32,7 @@ interface CodexRefreshEnv {
   CODEX_REFRESH_UPSTREAM_TIMEOUT_MS?: string;
   CODEX_REFRESH_LOCK_TIMEOUT_MS?: string;
   CODEX_CLIENT_ID?: string;
+  CODEX_EXPECTED_SCOPES?: string;
 }
 
 const DEFAULT_UPSTREAM_URL = 'https://auth.openai.com/oauth/token';
@@ -207,16 +209,20 @@ export class CodexRefreshLock extends DurableObject<CodexRefreshEnv> {
     }
 
     // Parse new tokens from OpenAI response.
-    const newTokens: Record<string, string> = await upstreamResponse.json();
+    const newTokens: Record<string, unknown> = await upstreamResponse.json();
+
+    // Scope validation — warn (not block) when upstream returns unexpected scopes.
+    // This catches scope escalation or drift without breaking legacy tokens.
+    this.validateUpstreamScopes(newTokens, userId);
 
     // Update the stored auth.json with new tokens.
     if (!storedAuth.tokens || typeof storedAuth.tokens !== 'object') {
       storedAuth.tokens = {};
     }
     const authTokens = storedAuth.tokens as Record<string, string>;
-    if (newTokens.access_token) authTokens.access_token = newTokens.access_token;
-    if (newTokens.refresh_token) authTokens.refresh_token = newTokens.refresh_token;
-    if (newTokens.id_token) authTokens.id_token = newTokens.id_token;
+    if (typeof newTokens.access_token === 'string') authTokens.access_token = newTokens.access_token;
+    if (typeof newTokens.refresh_token === 'string') authTokens.refresh_token = newTokens.refresh_token;
+    if (typeof newTokens.id_token === 'string') authTokens.id_token = newTokens.id_token;
     authTokens.last_refresh = new Date().toISOString();
 
     // Re-encrypt with fresh IV and update the database.
@@ -232,14 +238,57 @@ export class CodexRefreshLock extends DurableObject<CodexRefreshEnv> {
     // Return the new tokens to Codex.
     return new Response(
       JSON.stringify({
-        access_token: newTokens.access_token ?? null,
-        refresh_token: newTokens.refresh_token ?? null,
-        id_token: newTokens.id_token ?? null,
+        access_token: (typeof newTokens.access_token === 'string' ? newTokens.access_token : null),
+        refresh_token: (typeof newTokens.refresh_token === 'string' ? newTokens.refresh_token : null),
+        id_token: (typeof newTokens.id_token === 'string' ? newTokens.id_token : null),
       }),
       { status: 200, headers: { 'Content-Type': 'application/json' } }
     );
   }
 
+  /**
+   * Validate scopes in the upstream token response.
+   * Logs a warning if unexpected scopes are present — does NOT block the refresh.
+   * This catches scope escalation or drift without breaking backward compatibility
+   * with legacy tokens that may not include a scope field.
+   */
+  private validateUpstreamScopes(tokens: Record<string, unknown>, userId: string): void {
+    const scope = tokens.scope;
+    if (scope === undefined || scope === null) {
+      // No scope in response — common for legacy tokens. Nothing to validate.
+      return;
+    }
+
+    if (typeof scope !== 'string') {
+      log.warn('codex_refresh.scope_validation', {
+        userId,
+        scopeType: typeof scope,
+        message: 'Non-string scope in upstream response',
+      });
+      return;
+    }
+
+    // Parse expected scopes from env (comma-separated) or use empty set (accept all).
+    const expectedScopesEnv = this.env.CODEX_EXPECTED_SCOPES;
+    if (!expectedScopesEnv) {
+      // No expected scopes configured — skip validation (accept all).
+      return;
+    }
+
+    const expectedScopes = new Set(expectedScopesEnv.split(',').map(s => s.trim()).filter(Boolean));
+    const returnedScopes = scope.split(' ').filter(Boolean);
+    const unexpected = returnedScopes.filter(s => !expectedScopes.has(s));
+
+    if (unexpected.length > 0) {
+      log.warn('codex_refresh.unexpected_scopes', {
+        userId,
+        expectedScopes: [...expectedScopes].join(','),
+        returnedScopes: returnedScopes.join(' '),
+        unexpectedScopes: unexpected.join(','),
+      });
+    }
+  }
+
   /**
    * Look up the active openai-codex oauth-token credential for the given user.
    */

apps/api/src/index.ts

@@ -127,6 +127,8 @@ export interface Env {
   RATE_LIMIT_ANONYMOUS?: string;
   RATE_LIMIT_IDENTITY_TOKEN?: string;
   RATE_LIMIT_IDENTITY_TOKEN_WINDOW_SECONDS?: string;
+  RATE_LIMIT_CODEX_REFRESH?: string;
+  RATE_LIMIT_CODEX_REFRESH_WINDOW_SECONDS?: string;
   IDENTITY_TOKEN_CACHE_BUFFER_SECONDS?: string;
   IDENTITY_TOKEN_CACHE_MIN_TTL_SECONDS?: string;
   // Hierarchy limits
@@ -335,6 +337,7 @@ 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 expected scopes for upstream validation (warning log only)
   // Google OAuth (for GCP OIDC integration)
   GOOGLE_CLIENT_ID?: string;
   GOOGLE_CLIENT_SECRET?: string;

apps/api/src/middleware/rate-limit.ts

@@ -37,6 +37,7 @@ export const DEFAULT_RATE_LIMITS = {
   CLIENT_ERRORS: 200,
   IDENTITY_TOKEN: 60,
   ANALYTICS_INGEST: 60,
+  CODEX_REFRESH: 30,
 } as const;
 
 /** Default time window (1 hour in seconds) */
@@ -94,6 +95,10 @@ export function getCurrentWindowStart(windowSeconds: number): number {
 
 /**
  * Check and update rate limit.
+ *
+ * NOTE: This read-increment-write pattern is not atomic. KV has no CAS primitive.
+ * Under concurrent requests from the same identifier, the true count may exceed
+ * `limit` by a small amount. For strict enforcement, use a Durable Object counter.
  */
 export async function checkRateLimit(
   kv: KVNamespace,
@@ -226,3 +231,25 @@ export function rateLimitAnonymous(env: Env): MiddlewareHandler<{ Bindings: Env
     useIp: true,
   });
 }
+
+/**
+ * Check rate limit for Codex refresh endpoint.
+ * Uses workspaceId as the identifier (extracted from verified callback token).
+ * Default: 30 requests per hour per workspace.
+ *
+ * Returns null if allowed, or a Response if rate-limited.
+ * This is a direct check (not middleware) because the codex-refresh endpoint
+ * uses callback token auth, not session auth.
+ */
+export async function checkCodexRefreshRateLimit(
+  env: Env,
+  workspaceId: string,
+): Promise<{ allowed: boolean; remaining: number; resetAt: number }> {
+  const limit = getRateLimit(env, 'CODEX_REFRESH');
+  const envWindow = parseInt(env.RATE_LIMIT_CODEX_REFRESH_WINDOW_SECONDS || '', 10);
+  const windowSeconds = Number.isFinite(envWindow) && envWindow > 0 ? envWindow : DEFAULT_WINDOW_SECONDS;
+  const windowStart = getCurrentWindowStart(windowSeconds);
+  const key = createRateLimitKey('codex-refresh', workspaceId, windowStart);
+
+  return checkRateLimit(env.KV, key, limit, windowSeconds);
+}

apps/api/src/routes/codex-refresh.ts

@@ -16,6 +16,7 @@ import { Hono } from 'hono';
 import * as schema from '../db/schema';
 import type { Env } from '../index';
 import { log } from '../lib/logger';
+import { checkCodexRefreshRateLimit } from '../middleware/rate-limit';
 import { verifyCallbackToken } from '../services/jwt';
 
 const codexRefreshRoutes = new Hono<{ Bindings: Env }>();
@@ -68,6 +69,17 @@ codexRefreshRoutes.post('/codex-refresh', async (c) => {
 
   const workspaceId = tokenPayload.workspace;
 
+  // Rate limit per workspace (prevents abuse via stolen callback tokens).
+  const rateLimitResult = await checkCodexRefreshRateLimit(c.env, workspaceId);
+  if (!rateLimitResult.allowed) {
+    const retryAfter = rateLimitResult.resetAt - Math.floor(Date.now() / 1000);
+    log.warn('codex_refresh.rate_limited', { workspaceId });
+    return c.json(
+      { error: 'rate_limit_exceeded', message: 'Too many refresh requests' },
+      { status: 429, headers: { 'Retry-After': Math.max(1, retryAfter).toString() } },
+    );
+  }
+
   // Parse the request body (Codex sends hardcoded format).
   let body: { client_id?: string; grant_type?: string; refresh_token?: string };
   try {

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

@@ -33,6 +33,16 @@ vi.mock('../../../src/lib/secrets', () => ({
   getCredentialEncryptionKey: vi.fn().mockReturnValue('test-encryption-key'),
 }));
 
+// Mock logger
+const mockLogWarn = vi.fn();
+vi.mock('../../../src/lib/logger', () => ({
+  log: {
+    info: vi.fn(),
+    warn: mockLogWarn,
+    error: vi.fn(),
+  },
+}));
+
 const { CodexRefreshLock } = await import(
   '../../../src/durable-objects/codex-refresh-lock'
 );
@@ -360,6 +370,158 @@ describe('CodexRefreshLock', () => {
     expect(json.error).toBe('upstream_error');
   });
 
+  // -----------------------------------------------------------------------
+  // Scope validation
+  // -----------------------------------------------------------------------
+
+  it('succeeds without warning when upstream returns no scope field', async () => {
+    const { do: doInstance, env } = createDO({
+      CODEX_EXPECTED_SCOPES: 'openid,offline_access',
+    });
+    setupCredentialFound(env);
+    mockLogWarn.mockClear();
+
+    vi.mocked(fetch).mockResolvedValue(
+      new Response(
+        JSON.stringify({
+          access_token: 'new-access',
+          refresh_token: 'new-refresh',
+          id_token: 'new-id',
+        }),
+        { status: 200, headers: { 'Content-Type': 'application/json' } },
+      ),
+    );
+
+    const res = await doInstance.fetch(
+      makeRequest({ refreshToken: 'stored-refresh', userId: 'user-1' }),
+    );
+    expect(res.status).toBe(200);
+    expect(mockLogWarn).not.toHaveBeenCalledWith(
+      'codex_refresh.unexpected_scopes',
+      expect.anything(),
+    );
+  });
+
+  it('warns when upstream returns unexpected scopes', async () => {
+    const { do: doInstance, env } = createDO({
+      CODEX_EXPECTED_SCOPES: 'openid,offline_access',
+    });
+    setupCredentialFound(env);
+    mockLogWarn.mockClear();
+
+    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' }),
+    );
+    // Should still succeed (warning only, not blocking)
+    expect(res.status).toBe(200);
+    const json = await res.json();
+    expect(json.access_token).toBe('new-access');
+
+    expect(mockLogWarn).toHaveBeenCalledWith(
+      'codex_refresh.unexpected_scopes',
+      expect.objectContaining({
+        unexpectedScopes: 'admin:write',
+      }),
+    );
+  });
+
+  it('does not warn when scopes match expected', async () => {
+    const { do: doInstance, env } = createDO({
+      CODEX_EXPECTED_SCOPES: 'openid,offline_access',
+    });
+    setupCredentialFound(env);
+    mockLogWarn.mockClear();
+
+    vi.mocked(fetch).mockResolvedValue(
+      new Response(
+        JSON.stringify({
+          access_token: 'new-access',
+          refresh_token: 'new-refresh',
+          scope: 'openid offline_access',
+        }),
+        { status: 200, headers: { 'Content-Type': 'application/json' } },
+      ),
+    );
+
+    const res = await doInstance.fetch(
+      makeRequest({ refreshToken: 'stored-refresh', userId: 'user-1' }),
+    );
+    expect(res.status).toBe(200);
+    expect(mockLogWarn).not.toHaveBeenCalledWith(
+      'codex_refresh.unexpected_scopes',
+      expect.anything(),
+    );
+  });
+
+  it('skips scope validation when CODEX_EXPECTED_SCOPES is not configured', async () => {
+    const { do: doInstance, env } = createDO();
+    // No CODEX_EXPECTED_SCOPES set
+    setupCredentialFound(env);
+    mockLogWarn.mockClear();
+
+    vi.mocked(fetch).mockResolvedValue(
+      new Response(
+        JSON.stringify({
+          access_token: 'new-access',
+          refresh_token: 'new-refresh',
+          scope: 'openid offline_access admin:write some:other:scope',
+        }),
+        { status: 200, headers: { 'Content-Type': 'application/json' } },
+      ),
+    );
+
+    const res = await doInstance.fetch(
+      makeRequest({ refreshToken: 'stored-refresh', userId: 'user-1' }),
+    );
+    expect(res.status).toBe(200);
+    expect(mockLogWarn).not.toHaveBeenCalledWith(
+      'codex_refresh.unexpected_scopes',
+      expect.anything(),
+    );
+  });
+
+  it('warns on non-string scope in upstream response', async () => {
+    const { do: doInstance, env } = createDO({
+      CODEX_EXPECTED_SCOPES: 'openid',
+    });
+    setupCredentialFound(env);
+    mockLogWarn.mockClear();
+
+    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' }),
+    );
+    expect(res.status).toBe(200);
+    expect(mockLogWarn).toHaveBeenCalledWith(
+      'codex_refresh.scope_validation',
+      expect.objectContaining({
+        scopeType: 'number',
+      }),
+    );
+  });
+
   // -----------------------------------------------------------------------
   // Configurable upstream URL and client_id
   // -----------------------------------------------------------------------