fix: review remediation — timing-safe OAuth, rotation script hardening, redirect encoding

scripts/rotate-db-secret.py

@@ -5,22 +5,27 @@
 2. Resets neondb_owner password on the ChittyRental Neon project via the
    Neon API.
 3. Builds the pooled DATABASE_URL entirely in Python — the credential never
-   touches a shell variable or a command-line argument.
-4. Writes the DATABASE_URL to a temp file (mode 0600, deleted after use),
-   then execs wrangler secret put reading from that file via stdin so the
-   value is never exposed in ps/env output.
+   touches a shell variable or a command-line argument. Password is
+   URL-encoded to handle special characters.
+4. For each target environment, writes the DATABASE_URL to a temp file
+   (mode 0600, deleted after use), then runs wrangler secret put with that
+   file piped to stdin so the value is never exposed in ps/env output.
 
 Run:
-  python3 scripts/rotate-db-secret.py
+  python3 scripts/rotate-db-secret.py                    # default: top-level + production
+  python3 scripts/rotate-db-secret.py --env production    # single env
+  python3 scripts/rotate-db-secret.py --env staging --env production  # multiple envs
 """
 
+import argparse
 import json
 import os
 import stat
 import subprocess
 import sys
 import tempfile
 import urllib.error
+import urllib.parse
 import urllib.request
 
 # ── Config ────────────────────────────────────────────────────────────────────
@@ -38,15 +43,25 @@
 NEON_DB         = "neondb"
 POOLER_HOST     = "ep-delicate-breeze-aj9gmu1i-pooler.c-3.us-east-2.aws.neon.tech"
 
+DEFAULT_ENVS    = [None, "production"]  # top-level (Workers Builds) + production
+
 # ── Helpers ───────────────────────────────────────────────────────────────────
 
 def op_get(path):
     req = urllib.request.Request(
         f"{OP_HOST}{path}",
         headers={"Authorization": f"Bearer {OP_TOKEN}"},
     )
-    with urllib.request.urlopen(req) as r:
-        return json.loads(r.read())
+    try:
+        with urllib.request.urlopen(req, timeout=30) as r:
+            return json.loads(r.read())
+    except urllib.error.HTTPError as e:
+        body = e.read().decode("utf-8", errors="replace")[:200]
+        print(f"ERROR: 1Password Connect returned HTTP {e.code}: {body}", file=sys.stderr)
+        sys.exit(1)
+    except urllib.error.URLError as e:
+        print(f"ERROR: Cannot reach 1Password Connect at {OP_HOST}: {e.reason}", file=sys.stderr)
+        sys.exit(1)
 
 
 def neon_post(path, api_key):
@@ -59,10 +74,75 @@ def neon_post(path, api_key):
         },
         method="POST",
     )
-    with urllib.request.urlopen(req) as r:
-        return json.loads(r.read())
+    try:
+        with urllib.request.urlopen(req, timeout=30) as r:
+            return json.loads(r.read())
+    except urllib.error.HTTPError as e:
+        body = e.read().decode("utf-8", errors="replace")[:200]
+        print(f"ERROR: Neon API returned HTTP {e.code}: {body}", file=sys.stderr)
+        sys.exit(1)
+    except urllib.error.URLError as e:
+        print(f"ERROR: Cannot reach Neon API: {e.reason}", file=sys.stderr)
+        sys.exit(1)
 
 
+def deploy_secret(database_url, wrangler_config, env_name):
+    """Deploy DATABASE_URL to a specific wrangler environment via temp file."""
+    label = env_name or "top-level"
+    fd, tmp_path = tempfile.mkstemp(prefix="chittyfinance_db_", suffix=".tmp")
+    try:
+        os.chmod(tmp_path, stat.S_IRUSR | stat.S_IWUSR)  # 0600
+        with os.fdopen(fd, "w") as fh:
+            fh.write(database_url)
+            fh.flush()
+
+        cmd = ["npx", "wrangler", "secret", "put", "DATABASE_URL", "--config", wrangler_config]
+        if env_name:
+            cmd.extend(["--env", env_name])
+
+        with open(tmp_path, "r") as stdin_fh:
+            try:
+                result = subprocess.run(cmd, stdin=stdin_fh, capture_output=True, text=True, timeout=60)
+            except subprocess.TimeoutExpired:
+                print(f"  [{label}] ERROR: wrangler timed out after 60s", file=sys.stderr)
+                return False
+
+        if result.returncode == 0:
+            print(f"  [{label}] wrangler secret put succeeded", file=sys.stderr)
+        else:
+            print(f"  [{label}] ERROR: wrangler exited {result.returncode}", file=sys.stderr)
+            print(result.stdout, file=sys.stderr)
+            print(result.stderr, file=sys.stderr)
+            return False
+    finally:
+        try:
+            os.unlink(tmp_path)
+        except FileNotFoundError:
+            pass
+        except OSError as e:
+            print(f"  WARNING: Failed to delete temp file {tmp_path}: {e}", file=sys.stderr)
+            print("  SECURITY: Temp file may contain DATABASE_URL in plaintext", file=sys.stderr)
+    return True
+
+
+# ── CLI ───────────────────────────────────────────────────────────────────────
+
+parser = argparse.ArgumentParser(description="Rotate Neon DB password and deploy to Cloudflare Workers")
+parser.add_argument("--env", action="append", dest="envs",
+                    help="Wrangler environment(s) to deploy to (default: top-level + production). "
+                         "Pass multiple times for multiple envs. Use '' for top-level only.")
+args = parser.parse_args()
+target_envs = args.envs if args.envs else DEFAULT_ENVS
+
+# ── Validate required env vars (after argparse so --help works) ──────────────
+
+if not OP_HOST:
+    print("ERROR: OP_CONNECT_HOST environment variable is required", file=sys.stderr)
+    sys.exit(1)
+if not OP_TOKEN:
+    print("ERROR: OP_CONNECT_TOKEN environment variable is required", file=sys.stderr)
+    sys.exit(1)
+
 # ── Step 1: retrieve Neon API key from 1Password ──────────────────────────────
 
 print("[1] Retrieving Neon API key from 1Password Connect...", file=sys.stderr)
@@ -91,52 +171,30 @@ def neon_post(path, api_key):
 
 # ── Step 3: build DATABASE_URL entirely in Python ────────────────────────────
 
+encoded_password = urllib.parse.quote(new_password, safe="")
 database_url = (
-    f"postgresql://{NEON_ROLE}:{new_password}"
+    f"postgresql://{NEON_ROLE}:{encoded_password}"
     f"@{POOLER_HOST}/{NEON_DB}?sslmode=require"
 )
-print("[3] DATABASE_URL constructed", file=sys.stderr)
+print("[3] DATABASE_URL constructed (password URL-encoded)", file=sys.stderr)
 
-# ── Step 4: write DATABASE_URL to a 0600 temp file, pipe into wrangler ────────
-
-print("[4] Writing DATABASE_URL to secure temp file and calling wrangler...", file=sys.stderr)
+# ── Step 4: deploy to wrangler environments ──────────────────────────────────
 
 wrangler_config = os.path.join(
     os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
     "deploy", "system-wrangler.toml",
 )
 
-fd, tmp_path = tempfile.mkstemp(prefix="chittyfinance_db_", suffix=".tmp")
-try:
-    os.chmod(tmp_path, stat.S_IRUSR | stat.S_IWUSR)  # 0600
-    with os.fdopen(fd, "w") as fh:
-        fh.write(database_url)
-        fh.flush()
-
-    with open(tmp_path, "r") as stdin_fh:
-        result = subprocess.run(
-            [
-                "npx", "wrangler", "secret", "put", "DATABASE_URL",
-                "--config", wrangler_config,
-            ],
-            stdin=stdin_fh,
-            capture_output=True,
-            text=True,
-        )
-
-    if result.returncode == 0:
-        print("[4] wrangler secret put succeeded", file=sys.stderr)
-        print(result.stdout, file=sys.stderr)
-    else:
-        print(f"ERROR: wrangler exited {result.returncode}", file=sys.stderr)
-        print(result.stdout, file=sys.stderr)
-        print(result.stderr, file=sys.stderr)
-        sys.exit(1)
-finally:
-    try:
-        os.unlink(tmp_path)
-        print("[4] Temp file deleted", file=sys.stderr)
-    except OSError:
-        pass
+env_labels = [e or "top-level" for e in target_envs]
+print(f"[4] Deploying DATABASE_URL to: {', '.join(env_labels)}", file=sys.stderr)
+
+failed = []
+for env_name in target_envs:
+    if not deploy_secret(database_url, wrangler_config, env_name):
+        failed.append(env_name or "top-level")
+
+if failed:
+    print(f"ERROR: Failed to deploy to: {', '.join(failed)}", file=sys.stderr)
+    sys.exit(1)
 
-print("[5] Done. DATABASE_URL secret updated on chittyfinance Worker.", file=sys.stderr)
+print("[4] Done. DATABASE_URL secret updated on all target environments.", file=sys.stderr)

server/lib/oauth-state-edge.ts

@@ -48,7 +48,10 @@ export async function generateOAuthState(userId: number | string, secret: string
 export async function validateOAuthState(state: string, secret: string): Promise<OAuthStateData | null> {
   try {
     const [payload, signature] = state.split('.');
-    if (!payload || !signature) return null;
+    if (!payload || !signature) {
+      console.error('OAuth state: Invalid format (missing payload or signature)');
+      return null;
+    }
 
     const expected = await hmacSign(payload, secret);
     if (!(await tokenEqual(signature, expected))) {

server/lib/oauth-state.ts

@@ -4,15 +4,15 @@
  * Protects against:
  * - CSRF attacks (random nonce)
  * - Replay attacks (timestamp expiration)
- * - Tampering (HMAC signature)
+ * - Tampering (HMAC signature, timing-safe comparison)
+ *
+ * Legacy Node.js version — used only in standalone dev mode.
+ * Production uses oauth-state-edge.ts (Web Crypto API).
  */
 
-import { createHmac, randomBytes } from 'crypto';
+import { createHmac, randomBytes, timingSafeEqual } from 'crypto';
 
 const STATE_TOKEN_SECRET = process.env.OAUTH_STATE_SECRET;
-if (!STATE_TOKEN_SECRET) {
-  console.warn('OAUTH_STATE_SECRET not set — OAuth flows will fail');
-}
 const STATE_TOKEN_TTL_MS = 10 * 60 * 1000; // 10 minutes
 
 export interface OAuthStateData {
@@ -63,7 +63,9 @@ export function validateOAuthState(state: string): OAuthStateData | null {
       .update(payload)
       .digest('hex');
 
-    if (signature !== expectedSignature) {
+    const sigBuf = Buffer.from(signature);
+    const expectedBuf = Buffer.from(expectedSignature);
+    if (sigBuf.length !== expectedBuf.length || !timingSafeEqual(sigBuf, expectedBuf)) {
       console.error('OAuth state: Invalid signature (possible tampering)');
       return null;
     }

server/routes/google.ts

@@ -54,7 +54,7 @@ googleCallbackRoute.get('/api/integrations/google/callback', async (c) => {
   const error = c.req.query('error');
 
   if (error) {
-    return c.redirect(`${baseUrl}/connections?google=error&reason=${error}`);
+    return c.redirect(`${baseUrl}/connections?google=error&reason=${encodeURIComponent(error)}`);
   }
   if (!code || !state) {
     return c.redirect(`${baseUrl}/connections?google=error&reason=missing_params`);

server/routes/wave.ts

@@ -45,7 +45,7 @@ waveCallbackRoute.get('/api/integrations/wave/callback', async (c) => {
   const error = c.req.query('error');
 
   if (error) {
-    return c.redirect(`${baseUrl}/connections?wave=error&reason=${error}`);
+    return c.redirect(`${baseUrl}/connections?wave=error&reason=${encodeURIComponent(error)}`);
   }
 
   if (!code || !state) {

wrangler.toml

@@ -119,7 +119,7 @@ class_name = "ChittyAgent"
 
 # ─────────────────────────────────────────────────────────────────────
 # env.production — `wrangler deploy --env production`
-# Custom domain routes, production KV/R2, full observability
+# Custom domain routes, production KV/R2
 # ─────────────────────────────────────────────────────────────────────
 [env.production]
 name = "chittyfinance"