fix(security): close 6 OAuth backend + nx-claude script scan findings

Slack OAuth backend (apps/slack-oauth-backend):
- #1 (HIGH) CSRF: replace the length-only state check with a stateless
  HMAC-SHA256 signed state token (new src/oauth/state.ts). generateState
  signs a CSPRNG nonce + timestamp with config.sessionSecret and base64url-
  encodes it; validateState recomputes the HMAC with timingSafeEqual and a
  10-min freshness window. Stateless by design so it works on Vercel
  serverless (no shared store); base64url satisfies the existing callback
  validator charset/length bounds.
- #2 (MED) weak randomness: nonce now from crypto.randomBytes(16), not
  Math.random()/Date.now().
- #3 (MED) tokens in cacheable HTML: add Cache-Control no-store (+no-cache/
  Pragma/Expires) on all token-bearing /callback responses and the /refresh
  JSON; stop rendering the refresh token in the HTML fallback.
- #6 (MED) unauthenticated /slack/refresh: proportionate hardening (rate
  limit 10->5/min, no-store header, documented threat model). The
  refresh_token is itself the bearer credential in a stateless flow, so
  per-caller auth is intentionally not added.

nx-claude scripts (packages/ai-toolkit-nx-claude):
- #4 (MED) bash arithmetic command injection: source validate-numeric.sh and
  validate registry-derived values (^[0-9]+$) before $((...)) in
  reset-prerelease-version.sh.
- #5 (MED) shell injection in generated slack-env.sh: POSIX single-quote
  escaping (shSingleQuote) in createSlackEnvFile and updateRefreshToken; the
  latter also switches String.replace to a replacement function to neutralize
  $-substitution corruption.

Verification: 83/83 backend tests pass (integration suite updated to mint
real signed states); typecheck + lint clean on both projects; #4 and #5
functionally verified (injection payload rejected, metacharacters inert);
code-reviewer pass: READY TO MERGE.

Finding #7 (heredoc delimiter) was already fixed on next by PR #509.

Author: wkoutre

apps/slack-oauth-backend/src/oauth/state.spec.ts

@@ -0,0 +1,63 @@
+// Mock the config module with a deterministic session secret
+jest.mock('../config', () => ({
+  config: {
+    sessionSecret: 'test-session-secret-deterministic',
+  },
+}));
+
+import { generateState, validateState } from './state';
+
+describe('OAuth State Token', () => {
+  describe('generateState', () => {
+    it('should produce a base64url token with no padding or unsafe chars', () => {
+      const state = generateState();
+      expect(state).toMatch(/^[A-Za-z0-9_-]+$/);
+      expect(state).not.toContain('=');
+    });
+
+    it('should stay well under the 500-char callback limit', () => {
+      const state = generateState();
+      expect(state.length).toBeGreaterThanOrEqual(16);
+      expect(state.length).toBeLessThan(500);
+    });
+
+    it('should produce a different token on each call', () => {
+      expect(generateState()).not.toEqual(generateState());
+    });
+  });
+
+  describe('validateState', () => {
+    it('should validate a freshly generated state', () => {
+      const state = generateState();
+      expect(validateState(state)).toBe(true);
+    });
+
+    it('should reject an arbitrary 16+ char string', () => {
+      expect(validateState('aaaaaaaaaaaaaaaa')).toBe(false);
+    });
+
+    it('should reject a tampered token (flipped char)', () => {
+      const state = generateState();
+      const idx = Math.floor(state.length / 2);
+      const original = state[idx];
+      const replacement = original === 'A' ? 'B' : 'A';
+      const tampered = state.slice(0, idx) + replacement + state.slice(idx + 1);
+      expect(validateState(tampered)).toBe(false);
+    });
+
+    it('should reject an expired token via a tiny maxAgeMs', async () => {
+      const state = generateState();
+      await new Promise((resolve) => setTimeout(resolve, 5));
+      expect(validateState(state, 1)).toBe(false);
+    });
+
+    it('should reject an empty string', () => {
+      expect(validateState('')).toBe(false);
+    });
+
+    it('should reject malformed base64url that does not decode to 3 parts', () => {
+      const malformed = Buffer.from('not-three-parts', 'utf8').toString('base64url');
+      expect(validateState(malformed)).toBe(false);
+    });
+  });
+});

apps/slack-oauth-backend/src/oauth/state.ts

@@ -0,0 +1,77 @@
+import { createHmac, randomBytes, timingSafeEqual } from 'crypto';
+import { config } from '../config';
+
+/**
+ * Stateless, signed OAuth state tokens.
+ *
+ * The token is `nonce.timestamp.signature`, HMAC-SHA256 signed with
+ * `config.sessionSecret`, then base64url-encoded with no padding. This works on
+ * serverless (no shared store needed): /authorize signs the token and /callback
+ * verifies the signature, so a forged or replayed state is rejected without any
+ * cross-instance state. base64url (A-Za-z0-9-_) satisfies the callback
+ * validator's /^[a-zA-Z0-9_-]+$/ length 16..500 check.
+ */
+
+const DEFAULT_MAX_AGE_MS = 10 * 60 * 1000;
+const SEPARATOR = '.';
+
+function sign(payload: string): string {
+  return createHmac('sha256', config.sessionSecret).update(payload).digest('base64url');
+}
+
+/**
+ * Build a signed state token from a CSPRNG nonce and the current timestamp.
+ */
+export function generateState(): string {
+  const nonce = randomBytes(16).toString('base64url');
+  const timestamp = Date.now().toString();
+  const payload = `${nonce}${SEPARATOR}${timestamp}`;
+  const signature = sign(payload);
+  const token = `${payload}${SEPARATOR}${signature}`;
+  return Buffer.from(token, 'utf8').toString('base64url');
+}
+
+/**
+ * Verify a state token's HMAC signature and freshness. Never throws; returns
+ * false on any decode error, signature mismatch, malformed structure, or if the
+ * embedded timestamp is older than maxAgeMs.
+ */
+export function validateState(state: string, maxAgeMs: number = DEFAULT_MAX_AGE_MS): boolean {
+  try {
+    if (typeof state !== 'string' || state.length === 0) {
+      return false;
+    }
+
+    const decoded = Buffer.from(state, 'base64url').toString('utf8');
+    const parts = decoded.split(SEPARATOR);
+    if (parts.length !== 3) {
+      return false;
+    }
+
+    const [nonce, timestamp, signature] = parts;
+    if (!nonce || !timestamp || !signature) {
+      return false;
+    }
+
+    const expectedSignature = sign(`${nonce}${SEPARATOR}${timestamp}`);
+    const provided = Buffer.from(signature, 'utf8');
+    const expected = Buffer.from(expectedSignature, 'utf8');
+    if (provided.length !== expected.length || !timingSafeEqual(provided, expected)) {
+      return false;
+    }
+
+    const issuedAt = Number(timestamp);
+    if (!Number.isInteger(issuedAt) || issuedAt <= 0) {
+      return false;
+    }
+
+    const age = Date.now() - issuedAt;
+    if (age < 0 || age > maxAgeMs) {
+      return false;
+    }
+
+    return true;
+  } catch {
+    return false;
+  }
+}

apps/slack-oauth-backend/src/routes/oauth.ts

@@ -1,6 +1,7 @@
 import type { Request, Response, NextFunction } from 'express';
 import { Router } from 'express';
 import { createOAuthHandler } from '../oauth/handler';
+import { generateState, validateState } from '../oauth/state';
 import { createSlackClient } from '../slack/client';
 import { formatTokenMessage, formatSuccessPage, formatErrorPage } from '../messages/formatter';
 import { logger } from '../utils/logger';
@@ -54,11 +55,9 @@ router.get(
         throw new ValidationError('Missing authorization code', 'code');
       }
 
-      // Create OAuth handler with state validation
+      // Create OAuth handler with signed-state validation (CSRF protection)
       const oauthHandler = createOAuthHandler((state) => {
-        // In production, validate state against session or database
-        // For now, just ensure it exists and has minimum length
-        return typeof state === 'string' && state.length >= 16;
+        return typeof state === 'string' && validateState(state);
       });
 
       // Handle OAuth callback
@@ -73,6 +72,11 @@ router.get(
         // Send error page
         const errorPage = formatErrorPage(result.errorCode || 'unknown', result.error);
 
+        res.set({
+          'Cache-Control': 'no-store, no-cache, must-revalidate, private',
+          Pragma: 'no-cache',
+          Expires: '0',
+        });
         res.status(400).send(errorPage);
         return;
       }
@@ -117,12 +121,20 @@ router.get(
       // Send success page with token displayed if DM failed
       // Generate refresh URL based on request host
       const refreshUrl = `${req.protocol}://${req.get('host')}/slack/refresh`;
+      // Never render the refresh token in HTML; the page can be cached by
+      // intermediaries. The access-token fallback display is the accepted
+      // compromise so the user can still copy a token if the DM failed.
       const successPage = formatSuccessPage(
         result.user?.name,
         dmSent ? undefined : result.accessToken,
-        dmSent ? undefined : result.refreshToken,
+        undefined,
         dmSent ? undefined : refreshUrl
       );
+      res.set({
+        'Cache-Control': 'no-store, no-cache, must-revalidate, private',
+        Pragma: 'no-cache',
+        Expires: '0',
+      });
       res.send(successPage);
     } catch (error) {
       requestLogger.error('OAuth callback error', error);
@@ -134,6 +146,11 @@ router.get(
           error.message
         );
 
+        res.set({
+          'Cache-Control': 'no-store, no-cache, must-revalidate, private',
+          Pragma: 'no-cache',
+          Expires: '0',
+        });
         res.status(400).send(errorPage);
         return;
       }
@@ -167,17 +184,8 @@ router.get(
       redirectUri: parsedUrl.searchParams.get('redirect_uri'),
     });
 
-    // In production, store state in session or database
-    // For now, just redirect
     res.redirect(authUrl);
   }
 );
 
-/**
- * Generate a random state parameter
- */
-function generateState(): string {
-  return `state_${Date.now()}_${Math.random().toString(36).substring(2, 15)}`;
-}
-
 export default router;

apps/slack-oauth-backend/src/routes/refresh.ts

@@ -14,7 +14,7 @@ const router = Router();
  */
 const refreshRateLimiter = rateLimit({
   windowMs: 60 * 1000, // 1 minute
-  max: 10, // limit each IP to 10 requests per minute
+  max: 5, // limit each IP to 5 requests per minute
   message: 'Too many token refresh attempts, please try again later.',
   standardHeaders: true,
   legacyHeaders: false,
@@ -96,8 +96,18 @@ const validateRefreshRequest = (req: Request, res: Response, next: NextFunction)
  * Token refresh endpoint
  * POST /slack/refresh
  *
- * Exchanges a refresh token for a new access token
- * This keeps the client secret server-side for security
+ * Exchanges a refresh token for a new access token, keeping the client secret
+ * server-side.
+ *
+ * Threat model: there is no per-caller auth here, and that is intentional. The
+ * refresh_token IS the bearer credential, so possessing it is the authorization.
+ * This flow is stateless and serverless (no shared session/cookie store across
+ * Vercel instances), so a per-caller session check cannot be added without new
+ * infra. Defense is layered instead: the client_secret never leaves the server,
+ * the endpoint is IP rate-limited, and the main token-leak vector (OAuth tokens
+ * in a cacheable HTML response) is closed by the callback's no-store headers and
+ * by never rendering the refresh token in HTML. The no-store header below keeps
+ * the JSON token response out of intermediary caches as well.
  */
 router.post(
   '/',
@@ -176,6 +186,10 @@ router.post(
       });
 
       // Return the new tokens
+      res.set({
+        'Cache-Control': 'no-store',
+        Pragma: 'no-cache',
+      });
       res.json({
         ok: true,
         access_token: accessToken,

apps/slack-oauth-backend/tests/integration/oauth-flow.spec.ts

@@ -3,6 +3,7 @@ import type { Express } from 'express';
 import express from 'express';
 import { WebClient } from '@slack/web-api';
 import oauthRouter from '../../src/routes/oauth';
+import { generateState } from '../../src/oauth/state';
 import { clearSlackCaches } from '../../src/slack/client';
 import type * as SecurityMiddleware from '../../src/middleware/security';
 
@@ -15,6 +16,7 @@ jest.mock('../../src/config', () => ({
     slackClientId: 'test-client-id',
     slackClientSecret: 'test-client-secret',
     slackRedirectUri: 'https://example.com/callback',
+    sessionSecret: 'test-session-secret-deterministic-32chars',
     notionDocUrl: 'https://notion.so/setup-docs',
     environment: 'test',
   },
@@ -96,7 +98,8 @@ describe('OAuth Flow Integration Tests', () => {
   });
 
   describe('GET /slack/oauth/callback - Success Flow', () => {
-    const validState = '1234567890123456789';
+    // A real HMAC-signed state, the only kind the callback now accepts.
+    const validState = generateState();
     const validCode = 'valid-auth-code';
 
     const mockTokenResponse = {
@@ -270,7 +273,7 @@ describe('OAuth Flow Integration Tests', () => {
 
       const response = await request(app).get('/slack/oauth/callback').query({
         code: 'invalid-code',
-        state: '1234567890123456',
+        state: generateState(),
       });
 
       expect(response.status).toBe(400);
@@ -282,7 +285,7 @@ describe('OAuth Flow Integration Tests', () => {
 
       const response = await request(app).get('/slack/oauth/callback').query({
         code: 'valid-code',
-        state: '1234567890123456',
+        state: generateState(),
       });
 
       expect(response.status).toBe(400);
@@ -309,7 +312,9 @@ describe('OAuth Flow Integration Tests', () => {
       expect(response.headers.location).toContain(
         'redirect_uri=https%3A%2F%2Fexample.com%2Fcallback'
       );
-      expect(response.headers.location).toContain('state=state_');
+      // State is now an HMAC-signed base64url token (A-Za-z0-9-_), not the old
+      // predictable `state_<ts>_<rand>` format.
+      expect(response.headers.location).toMatch(/state=[A-Za-z0-9_-]{16,}/);
     });
   });
 
@@ -366,7 +371,7 @@ describe('OAuth Flow Integration Tests', () => {
 
       const response = await request(app).get('/slack/oauth/callback').query({
         code: 'complete-auth-code',
-        state: 'complete_state_1234567890',
+        state: generateState(),
       });
 
       expect(response.status).toBe(200);
@@ -410,7 +415,7 @@ describe('OAuth Flow Integration Tests', () => {
 
       const response = await request(app).get('/slack/oauth/callback').query({
         code: 'partial-auth-code',
-        state: 'partial_state_1234567890',
+        state: generateState(),
       });
 
       // Should still succeed overall

packages/ai-toolkit-nx-claude/scripts/reset-prerelease-version.sh

@@ -3,6 +3,15 @@
 # Script to reset version for prerelease on next branch
 # This helps fix the issue where next branch has same version as latest
 
+# Source numeric validation helper to guard values that flow into bash arithmetic
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+VALIDATE_NUMERIC_LIB="$SCRIPT_DIR/../../../.github/scripts/validate-numeric.sh"
+if [[ ! -f "$VALIDATE_NUMERIC_LIB" ]]; then
+    echo "❌ Required helper not found: $VALIDATE_NUMERIC_LIB" >&2
+    exit 1
+fi
+source "$VALIDATE_NUMERIC_LIB"
+
 PACKAGE_NAME="@uniswap/ai-toolkit-nx-claude"
 CURRENT_VERSION=$(jq -r '.version' packages/ai-toolkit-nx-claude/package.json)
 
@@ -43,6 +52,7 @@ fi
 # Determine target version for next branch
 if [[ "$MAJOR.$MINOR.$PATCH" == "$LATEST_MAJOR.$LATEST_MINOR.$LATEST_PATCH" ]]; then
     # Same version, need to bump for next
+    MINOR=$(validate_numeric "$MINOR" "minor") || exit 1
     NEW_MINOR=$((MINOR + 1))
     TARGET_VERSION="$MAJOR.$NEW_MINOR.0"
 else
@@ -59,6 +69,7 @@ if [[ -n "$EXISTING_PRERELEASES" ]]; then
     # Extract the prerelease number if it matches our target
     if [[ "$EXISTING_PRERELEASES" == "$TARGET_VERSION-next."* ]]; then
         PRERELEASE_NUM=${EXISTING_PRERELEASES##*-next.}
+        PRERELEASE_NUM=$(validate_numeric "$PRERELEASE_NUM" "prerelease number") || exit 1
         NEXT_PRERELEASE=$((PRERELEASE_NUM + 1))
     else
         NEXT_PRERELEASE=0