fix(proxy): harden AGY risk persistence and confirm UX

- add lock-held mutateUnifiedConfig read-modify-write path for config.yaml - use atomic mutation in /api/settings/auth/antigravity-risk to prevent stale overwrites - replace browser confirm with in-app 2-step banner + typed phrase validation - verify saved AGY power-user value with post-save read before showing success Refs #509
2026-07-16 12:15:57 +00:00 · 2026-02-24 18:45:41 +07:00
parent c0eb786127
commit 36d5cb723e
3 changed files with 257 additions and 78 deletions
@@ -719,14 +719,24 @@ function generateYamlWithComments(config: UnifiedConfig): string {
 }

 /**
- * Save unified config to YAML file.
- * Uses atomic write (temp file + rename) to prevent corruption.
- * Uses lockfile to prevent concurrent writes.
+ * Sync sleep helper for lock retry loops.
+ * Uses Atomics.wait when available to avoid CPU-intensive busy-wait.
 */
-export function saveUnifiedConfig(config: UnifiedConfig): void {
-  const yamlPath = getConfigYamlPath();
-  const dir = path.dirname(yamlPath);
+function sleepSync(ms: number): void {
+  try {
+    Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
+  } catch {
+    const end = Date.now() + ms;
+    while (Date.now() < end) {
+      /* busy-wait */
+    }
+  }
+}

+/**
+ * Execute a callback while holding the config lock.
+ */
+function withConfigWriteLock<T>(callback: () => T): T {
  // Acquire lock (retry for up to 1 second)
  const maxRetries = 10;
  const retryDelayMs = 100;
@@ -736,18 +746,7 @@ export function saveUnifiedConfig(config: UnifiedConfig): void {
      lockAcquired = true;
      break;
    }
-    // Synchronous sleep without CPU-intensive busy-wait
-    // Uses Atomics.wait which properly sleeps the thread
-    // Note: saveUnifiedConfig is sync API with 19+ callers, converting to async not feasible
-    try {
-      Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, retryDelayMs);
-    } catch {
-      // Fallback for environments without SharedArrayBuffer/Atomics support
-      const end = Date.now() + retryDelayMs;
-      while (Date.now() < end) {
-        /* busy-wait */
-      }
-    }
+    sleepSync(retryDelayMs);
  }

  if (!lockAcquired) {
@@ -755,57 +754,112 @@ export function saveUnifiedConfig(config: UnifiedConfig): void {
  }

  try {
-    // Ensure directory exists
-    if (!fs.existsSync(dir)) {
-      fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
-    }
-
-    // Ensure version is set
-    config.version = UNIFIED_CONFIG_VERSION;
-
-    // Generate YAML with section comments
-    const yamlContent = generateYamlWithComments(config);
-    const content = generateYamlHeader() + yamlContent;
-
-    // Atomic write: write to temp file, then rename
-    const tempPath = `${yamlPath}.tmp.${process.pid}`;
-
-    try {
-      fs.writeFileSync(tempPath, content, { mode: 0o600 });
-      fs.renameSync(tempPath, yamlPath);
-    } catch (error) {
-      // Clean up temp file on error
-      if (fs.existsSync(tempPath)) {
-        try {
-          fs.unlinkSync(tempPath);
-        } catch {
-          // Ignore cleanup errors
-        }
-      }
-      // Classify filesystem errors
-      const err = error as NodeJS.ErrnoException;
-      if (err.code === 'ENOSPC') {
-        throw new Error('Disk full - cannot save config. Free up space and try again.');
-      } else if (err.code === 'EROFS' || err.code === 'EACCES') {
-        throw new Error(`Cannot write config - check file permissions: ${err.message}`);
-      }
-      throw error;
-    }
+    return callback();
  } finally {
    // Always release lock
    releaseLock();
  }
 }

+/**
+ * Load unified config directly from disk while lock is already held.
+ * Falls back to empty config when file doesn't exist.
+ */
+function loadUnifiedConfigWithLockHeld(): UnifiedConfig {
+  const yamlPath = getConfigYamlPath();
+  if (!fs.existsSync(yamlPath)) {
+    return createEmptyUnifiedConfig();
+  }
+
+  const content = fs.readFileSync(yamlPath, 'utf8');
+  const parsed = yaml.load(content);
+
+  if (!isUnifiedConfig(parsed)) {
+    throw new Error(`Invalid config format in ${yamlPath}`);
+  }
+
+  const merged = mergeWithDefaults(parsed);
+  validateCompositeVariants(merged);
+  return merged;
+}
+
+/**
+ * Write unified config to disk while lock is already held.
+ */
+function writeUnifiedConfigWithLockHeld(config: UnifiedConfig): void {
+  const yamlPath = getConfigYamlPath();
+  const dir = path.dirname(yamlPath);
+
+  // Ensure directory exists
+  if (!fs.existsSync(dir)) {
+    fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
+  }
+
+  // Ensure version is set
+  config.version = UNIFIED_CONFIG_VERSION;
+
+  // Generate YAML with section comments
+  const yamlContent = generateYamlWithComments(config);
+  const content = generateYamlHeader() + yamlContent;
+
+  // Atomic write: write to temp file, then rename
+  const tempPath = `${yamlPath}.tmp.${process.pid}`;
+
+  try {
+    fs.writeFileSync(tempPath, content, { mode: 0o600 });
+    fs.renameSync(tempPath, yamlPath);
+  } catch (error) {
+    // Clean up temp file on error
+    if (fs.existsSync(tempPath)) {
+      try {
+        fs.unlinkSync(tempPath);
+      } catch {
+        // Ignore cleanup errors
+      }
+    }
+    // Classify filesystem errors
+    const err = error as NodeJS.ErrnoException;
+    if (err.code === 'ENOSPC') {
+      throw new Error('Disk full - cannot save config. Free up space and try again.');
+    } else if (err.code === 'EROFS' || err.code === 'EACCES') {
+      throw new Error(`Cannot write config - check file permissions: ${err.message}`);
+    }
+    throw error;
+  }
+}
+
+/**
+ * Save unified config to YAML file.
+ * Uses atomic write (temp file + rename) to prevent corruption.
+ * Uses lockfile to prevent concurrent writes.
+ */
+export function saveUnifiedConfig(config: UnifiedConfig): void {
+  withConfigWriteLock(() => {
+    writeUnifiedConfigWithLockHeld(config);
+  });
+}
+
+/**
+ * Atomically mutate unified config with lock held across read-modify-write.
+ * Prevents stale writes from overwriting concurrent updates.
+ */
+export function mutateUnifiedConfig(mutator: (config: UnifiedConfig) => void): UnifiedConfig {
+  return withConfigWriteLock(() => {
+    const current = loadUnifiedConfigWithLockHeld();
+    mutator(current);
+    writeUnifiedConfigWithLockHeld(current);
+    return current;
+  });
+}
+
 /**
 * Update unified config with partial data.
 * Loads existing config, merges changes, and saves.
 */
 export function updateUnifiedConfig(updates: Partial<UnifiedConfig>): UnifiedConfig {
-  const config = loadOrCreateUnifiedConfig();
-  const updated = { ...config, ...updates };
-  saveUnifiedConfig(updated);
-  return updated;
+  return mutateUnifiedConfig((config) => {
+    Object.assign(config, updates);
+  });
 }

 /**
@@ -22,7 +22,7 @@ import { deduplicateCcsHooks } from '../../utils/websearch/hook-utils';
 import {
  getDashboardAuthConfig,
  loadOrCreateUnifiedConfig,
-  saveUnifiedConfig,
+  mutateUnifiedConfig,
 } from '../../config/unified-config-loader';
 import type { Settings } from '../../types/config';

@@ -487,16 +487,16 @@ router.put('/auth/antigravity-risk', (req: Request, res: Response): void => {
      return;
    }

-    const config = loadOrCreateUnifiedConfig();
-    config.cliproxy.safety = {
-      ...(config.cliproxy.safety ?? {}),
-      antigravity_ack_bypass: antigravityAckBypass,
-    };
-    saveUnifiedConfig(config);
+    const updatedConfig = mutateUnifiedConfig((config) => {
+      config.cliproxy.safety = {
+        ...(config.cliproxy.safety ?? {}),
+        antigravity_ack_bypass: antigravityAckBypass,
+      };
+    });

    res.json({
      success: true,
-      antigravityAckBypass,
+      antigravityAckBypass: updatedConfig.cliproxy?.safety?.antigravity_ack_bypass === true,
    });
  } catch (error) {
    const classified = classifyConfigSaveFailure(error);
@@ -6,6 +6,7 @@
 import { useEffect, useState, useCallback, useRef } from 'react';
 import { Button } from '@/components/ui/button';
 import { Alert, AlertDescription } from '@/components/ui/alert';
+import { Input } from '@/components/ui/input';
 import { ScrollArea } from '@/components/ui/scroll-area';
 import { Switch } from '@/components/ui/switch';
 import {
@@ -18,6 +19,7 @@ import {
  Box,
  AlertTriangle,
  ShieldAlert,
+  ExternalLink,
 } from 'lucide-react';
 import { useProxyConfig, useRawConfig } from '../../hooks';
 import { useUpdateBackend, useProxyStatus } from '@/hooks/use-cliproxy';
@@ -26,6 +28,7 @@ import { RemoteProxyCard } from './remote-proxy-card';
 import { ProxyStatusWidget } from '@/components/monitoring/proxy-status-widget';
 import { api } from '@/lib/api-client';
 import { CLIPROXY_DEFAULT_PORT } from '@/lib/preset-utils';
+import { RISK_ACK_PHRASE } from '@/components/account/antigravity-responsibility-constants';
 import { toast } from 'sonner';

 /** LocalStorage key for debug mode preference */
@@ -34,6 +37,10 @@ const DEBUG_MODE_KEY = 'ccs_debug_mode';
 /** Providers only available on CLIProxyAPIPlus */
 const PLUS_ONLY_PROVIDERS = ['kiro', 'ghcp'];

+function normalizeRiskAckPhrase(value: string): string {
+  return value.trim().replace(/\s+/g, ' ').toUpperCase();
+}
+
 export default function ProxySection() {
  const {
    config,
@@ -71,7 +78,11 @@ export default function ProxySection() {
  const [agyAckBypass, setAgyAckBypass] = useState(false);
  const [agyAckBypassLoading, setAgyAckBypassLoading] = useState(true);
  const [agyAckBypassSaving, setAgyAckBypassSaving] = useState(false);
+  const [showAgyEnableConfirm, setShowAgyEnableConfirm] = useState(false);
+  const [agyEnableConfirmPhrase, setAgyEnableConfirmPhrase] = useState('');
  const agyAckBypassSavingRef = useRef(false);
+  const isAgyConfirmPhraseValid =
+    normalizeRiskAckPhrase(agyEnableConfirmPhrase) === RISK_ACK_PHRASE;

  const handleDebugModeChange = (enabled: boolean) => {
    setDebugMode(enabled);
@@ -99,17 +110,10 @@ export default function ProxySection() {
    }
  }, []);

-  const saveAgyAckBypass = useCallback(
+  const persistAgyAckBypass = useCallback(
    async (nextValue: boolean) => {
      if (agyAckBypassSavingRef.current || agyAckBypassSaving || saving) return;

-      if (nextValue) {
-        const confirmed = window.confirm(
-          'Enable AGY power user mode?\n\nThis skips AGY safety prompts. You accept the OAuth risk.'
-        );
-        if (!confirmed) return;
-      }
-
      try {
        agyAckBypassSavingRef.current = true;
        setAgyAckBypassSaving(true);
@@ -120,12 +124,34 @@ export default function ProxySection() {
          body: JSON.stringify({ antigravityAckBypass: nextValue }),
        });

+        const payload = (await response.json()) as {
+          antigravityAckBypass?: boolean;
+          error?: string;
+        };
+
        if (!response.ok) {
-          const data = (await response.json()) as { error?: string };
-          throw new Error(data.error || 'Failed to update AGY power user mode');
+          throw new Error(payload.error || 'Failed to update AGY power user mode');
        }

-        setAgyAckBypass(nextValue);
+        const persistedValue = payload.antigravityAckBypass === true;
+
+        const verifyResponse = await fetch('/api/settings/auth/antigravity-risk', {
+          cache: 'no-store',
+        });
+        if (!verifyResponse.ok) {
+          throw new Error('Failed to verify AGY power user mode persistence');
+        }
+        const verifyData = (await verifyResponse.json()) as { antigravityAckBypass?: boolean };
+        const verifiedValue = verifyData.antigravityAckBypass === true;
+        if (verifiedValue !== nextValue) {
+          throw new Error(
+            'AGY power user mode was not persisted. Config may have been modified by another process.'
+          );
+        }
+
+        setAgyAckBypass(verifiedValue && persistedValue);
+        setShowAgyEnableConfirm(false);
+        setAgyEnableConfirmPhrase('');
        toast.success(nextValue ? 'AGY power user mode enabled.' : 'AGY power user mode disabled.');
        await fetchRawConfig();
      } catch (err) {
@@ -138,6 +164,30 @@ export default function ProxySection() {
    [agyAckBypassSaving, fetchRawConfig, saving]
  );

+  const handleAgyAckBypassChange = useCallback(
+    (nextValue: boolean) => {
+      if (agyAckBypassSavingRef.current || agyAckBypassSaving || saving) return;
+
+      if (nextValue) {
+        setShowAgyEnableConfirm(true);
+        return;
+      }
+
+      setShowAgyEnableConfirm(false);
+      setAgyEnableConfirmPhrase('');
+      void persistAgyAckBypass(false);
+    },
+    [agyAckBypassSaving, persistAgyAckBypass, saving]
+  );
+
+  const confirmAgyEnable = useCallback(() => {
+    if (!isAgyConfirmPhraseValid) {
+      toast.error(`Type "${RISK_ACK_PHRASE}" to continue.`);
+      return;
+    }
+    void persistAgyAckBypass(true);
+  }, [isAgyConfirmPhraseValid, persistAgyAckBypass]);
+
  // Backend state (loaded from API) + mutation hook for proper query invalidation
  const [backend, setBackend] = useState<'original' | 'plus'>('plus');
  const [hasKiroGhcpVariants, setHasKiroGhcpVariants] = useState(false);
@@ -484,7 +534,7 @@ export default function ProxySection() {
                  aria-describedby="agy-power-user-mode-description"
                  checked={agyAckBypass}
                  disabled={agyAckBypassLoading || agyAckBypassSaving || saving}
-                  onCheckedChange={saveAgyAckBypass}
+                  onCheckedChange={handleAgyAckBypassChange}
                />
              </div>
              <p
@@ -494,6 +544,81 @@ export default function ProxySection() {
                Use only if you fully understand the OAuth suspension/ban risk pattern (#509). CCS
                cannot assume responsibility for account loss.
              </p>
+              {showAgyEnableConfirm && (
+                <div className="space-y-3 rounded-lg border border-rose-500/40 bg-rose-500/[0.08] p-3.5">
+                  <div className="space-y-1.5">
+                    <p className="text-xs font-semibold tracking-wide text-rose-900 dark:text-rose-200">
+                      Final confirmation required
+                    </p>
+                    <p className="text-xs leading-relaxed text-rose-800/95 dark:text-rose-200/90">
+                      Enabling this will skip AGY safety checkpoints in both dashboard and CLI.
+                      Review issue #509 and type the exact phrase to proceed.
+                    </p>
+                  </div>
+                  <div className="grid gap-2 md:grid-cols-2">
+                    <div className="rounded-md border border-rose-400/30 bg-rose-500/10 p-2.5">
+                      <p className="text-[11px] font-semibold uppercase tracking-wide text-rose-900 dark:text-rose-200">
+                        Step 1
+                      </p>
+                      <a
+                        href="https://github.com/kaitranntt/ccs/issues/509"
+                        target="_blank"
+                        rel="noreferrer"
+                        className="mt-1 inline-flex items-center gap-1.5 text-xs font-medium text-rose-800 underline decoration-rose-500/60 underline-offset-2 transition-colors hover:text-rose-700 dark:text-rose-200"
+                      >
+                        Read issue #509
+                        <ExternalLink className="h-3.5 w-3.5" />
+                      </a>
+                    </div>
+                    <div className="rounded-md border border-rose-400/30 bg-rose-500/10 p-2.5">
+                      <p className="text-[11px] font-semibold uppercase tracking-wide text-rose-900 dark:text-rose-200">
+                        Step 2
+                      </p>
+                      <p className="mt-1 text-xs text-rose-800/95 dark:text-rose-200/90">
+                        Type{' '}
+                        <code className="rounded bg-background/80 px-1 py-0.5 font-mono">
+                          {RISK_ACK_PHRASE}
+                        </code>{' '}
+                        to enable.
+                      </p>
+                    </div>
+                  </div>
+                  <div className="space-y-2">
+                    <Input
+                      value={agyEnableConfirmPhrase}
+                      onChange={(e) => setAgyEnableConfirmPhrase(e.target.value)}
+                      placeholder={RISK_ACK_PHRASE}
+                      disabled={agyAckBypassSaving || saving}
+                      className="font-mono text-xs"
+                      aria-label="Type I ACCEPT RISK to enable Antigravity power user mode"
+                    />
+                    <p className="text-[11px] text-rose-800/90 dark:text-rose-200/80">
+                      Exact phrase required.
+                    </p>
+                  </div>
+                  <div className="flex items-center justify-end gap-2">
+                    <Button
+                      variant="outline"
+                      size="sm"
+                      onClick={() => {
+                        setShowAgyEnableConfirm(false);
+                        setAgyEnableConfirmPhrase('');
+                      }}
+                      disabled={agyAckBypassSaving || saving}
+                    >
+                      Cancel
+                    </Button>
+                    <Button
+                      size="sm"
+                      variant="destructive"
+                      onClick={confirmAgyEnable}
+                      disabled={!isAgyConfirmPhraseValid || agyAckBypassSaving || saving}
+                    >
+                      Enable Power User Mode
+                    </Button>
+                  </div>
+                </div>
+              )}
              <span id="agy-power-user-mode-label" className="sr-only">
                Toggle AGY power user mode
              </span>