fix: gate install success on doctor's auth-pattern security checks (#175)

* fix: gate install success on doctor's auth-pattern security checks

The installer ran build/typecheck self-correction but never ran doctor's
authPatterns checks, so an insecure GET sign-out (SIGNOUT_GET_HANDLER) could
pass the build and ship as a "successful" install while `workos doctor`
immediately flagged it.

- Add src/lib/validation/security-checks.ts: runs doctor's security subset
  (GET sign-out, client-leaked/in-source API keys, ungitignored .env, mixed
  env) against the install dir with no network.
- Wire into agent-runner: the self-correction loop now feeds security findings
  back to the agent, and a final gate throws on error-severity findings that
  survive retries (success: false, non-zero exit, commit step skipped).
- Point the SIGNOUT_GET_HANDLER finding at a live docs URL (old
  /docs/authkit/sign-out 404s).

* chore(deps): bump @workos/skills to 0.6.1

Picks up the Next.js sign-out POST server-action guidance (workos/skills#33),
so the installer agent no longer scaffolds an unsafe GET sign-out route.

* chore: formatting

* test: cover API_KEY_LEAKED_TO_CLIENT blocking path; clarify warning comment

Addresses Greptile review on #175:
- Add a unit test for the API_KEY_LEAKED_TO_CLIENT blocking code (a
  NEXT_PUBLIC_-prefixed secret in .env.local), mirroring the API_KEY_IN_SOURCE
  test, so a regression in the prefix/key detection is caught.
- Correct the self-correction comment: warning findings ride along only when a
  retry is already triggered by an error or build failure; they are otherwise
  surfaced in the final validation report.

* fix: emit retry-summary telemetry only after the security gate passes

Addresses Greptile review on #175:
- Move the 'agent retry summary' (passed_after_retry: true) capture below the
  security gate so a blocked install no longer emits a contradictory
  pass-after-retry event alongside 'security gate blocked install'.
- Use obviously-fake fixture key strings in the spec (still satisfy the
  detection regex) so secrets scanners and the no-hardcoded-secrets rule stay
  quiet.

Author: nicknisi

package.json

@@ -54,7 +54,7 @@
     "@workos-inc/node": "^8.7.0",
     "@workos/migrations": "^2.0.0",
     "@workos/openapi-spec": "^0.1.0",
-    "@workos/skills": "0.6.0",
+    "@workos/skills": "0.6.1",
     "chalk": "^5.6.2",
     "diff": "^8.0.3",
     "fast-glob": "^3.3.3",

pnpm-lock.yaml

@@ -136,7 +136,7 @@ function checkSignoutGetHandler(ctx: CheckContext): AuthPatternFinding[] {
         filePath: relative(ctx.installDir, route),
         remediation:
           'Convert to a POST server action. GET routes with side effects are vulnerable to CSRF and will be triggered by Next.js link prefetching.',
-        docsUrl: 'https://workos.com/docs/authkit/sign-out',
+        docsUrl: 'https://workos.com/docs/authkit/nextjs#ending-the-session',
       });
     }
   }

src/doctor/checks/auth-patterns.ts

@@ -1,6 +1,12 @@
 import { getReference } from '@workos/skills';
 import { SPINNER_MESSAGE, type FrameworkConfig } from './framework-config.js';
 import { validateInstallation, quickCheckValidateAndFormat } from './validation/index.js';
+import {
+  runInstallSecurityChecks,
+  securityFindingsToIssues,
+  formatSecurityFindingsForAgent,
+  formatBlockingSecurityError,
+} from './validation/security-checks.js';
 import type { InstallerOptions } from '../utils/types.js';
 import {
   ensurePackageIsInstalled,
@@ -114,11 +120,27 @@ export async function runAgentInstaller(config: FrameworkConfig, options: Instal
     options,
   );
 
+  const integration = config.metadata.integration;
+
   const retryConfig: RetryConfig | undefined = options.noValidate
     ? undefined
     : {
         maxRetries: options.maxRetries ?? 2,
-        validateAndFormat: quickCheckValidateAndFormat,
+        // Self-correction combines two layers: build/typecheck (existing) AND the
+        // security subset of doctor's auth-pattern checks. The latter is what was
+        // missing — it's why an insecure GET sign-out could pass the build and
+        // ship as a "successful" install. Only error-severity security findings
+        // force a retry; warning findings ride along in the prompt only when a
+        // retry is already triggered by an error or a build failure (warnings are
+        // still surfaced in the final validation report regardless).
+        validateAndFormat: async (workingDirectory: string) => {
+          const quickPrompt = await quickCheckValidateAndFormat(workingDirectory);
+          const security = await runInstallSecurityChecks(integration, workingDirectory);
+          if (quickPrompt === null && security.blocking.length === 0) return null;
+          return [quickPrompt, formatSecurityFindingsForAgent(security.findings)]
+            .filter((p): p is string => Boolean(p))
+            .join('\n\n');
+        },
       };
 
   // Run agent with retry support — agent gets correction prompts on validation failure
@@ -144,34 +166,58 @@ export async function runAgentInstaller(config: FrameworkConfig, options: Instal
     throw new Error(message);
   }
 
-  // Track retry metrics
-  if (agentResult.retryCount !== undefined && agentResult.retryCount > 0) {
-    analytics.capture(INSTALLER_INTERACTION_EVENT_NAME, {
-      action: 'agent retry summary',
-      retry_count: agentResult.retryCount,
-      max_retries: options.maxRetries ?? 2,
-      passed_after_retry: true,
-    });
-  }
-
   // Run full validation after agent (with retries) completes
   // Quick checks already ran inside the retry loop — skip build
   if (!options.noValidate) {
-    options.emitter?.emit('validation:start', { framework: config.metadata.integration });
+    options.emitter?.emit('validation:start', { framework: integration });
 
-    const validationResult = await validateInstallation(config.metadata.integration, options.installDir, {
+    const validationResult = await validateInstallation(integration, options.installDir, {
       runBuild: false,
     });
 
-    if (validationResult.issues.length > 0) {
-      options.emitter?.emit('validation:issues', { issues: validationResult.issues });
+    // Run doctor's security subset as the final gate. Its absence here is the
+    // install-validate ↔ doctor gap: install reported success while `workos
+    // doctor` immediately found a SIGNOUT_GET_HANDLER hole.
+    const security = await runInstallSecurityChecks(integration, options.installDir);
+    const allIssues = [...validationResult.issues, ...securityFindingsToIssues(security.findings)];
+
+    if (allIssues.length > 0) {
+      options.emitter?.emit('validation:issues', { issues: allIssues });
     }
 
     options.emitter?.emit('validation:complete', {
-      passed: validationResult.passed,
-      issueCount: validationResult.issues.length,
+      passed: validationResult.passed && security.blocking.length === 0,
+      issueCount: allIssues.length,
       durationMs: validationResult.durationMs,
     });
+
+    // Block success: an error-severity security finding that survived the
+    // self-correction retries fails the install rather than shipping silently.
+    // Throwing routes through the state machine's error state (success: false,
+    // non-zero exit) and skips the commit/PR steps, leaving the insecure code
+    // uncommitted for the user to inspect.
+    if (security.blocking.length > 0) {
+      analytics.capture(INSTALLER_INTERACTION_EVENT_NAME, {
+        action: 'security gate blocked install',
+        integration,
+        codes: security.blocking.map((f) => f.code).join(','),
+      });
+      await analytics.shutdown('error');
+      throw new Error(formatBlockingSecurityError(security.blocking));
+    }
+  }
+
+  // Track retry metrics AFTER the security gate. `passed_after_retry` must
+  // reflect a genuinely successful install, not just an exhausted retry loop —
+  // emitting it before the gate could pair a "passed after retry" event with a
+  // "security gate blocked install" failure for the same run.
+  if (agentResult.retryCount !== undefined && agentResult.retryCount > 0) {
+    analytics.capture(INSTALLER_INTERACTION_EVENT_NAME, {
+      action: 'agent retry summary',
+      retry_count: agentResult.retryCount,
+      max_retries: options.maxRetries ?? 2,
+      passed_after_retry: true,
+    });
   }
 
   // Build environment variables from WorkOS credentials

src/lib/agent-runner.ts

@@ -0,0 +1,148 @@
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
+import { join } from 'node:path';
+import { tmpdir } from 'node:os';
+import {
+  runInstallSecurityChecks,
+  securityFindingsToIssues,
+  formatSecurityFindingsForAgent,
+  formatBlockingSecurityError,
+} from './security-checks.js';
+import type { AuthPatternFinding } from '../../doctor/types.js';
+
+function createTempDir(): string {
+  return mkdtempSync(join(tmpdir(), 'install-security-'));
+}
+
+function writeFixtureFile(dir: string, relativePath: string, content: string) {
+  const fullPath = join(dir, relativePath);
+  mkdirSync(join(fullPath, '..'), { recursive: true });
+  writeFileSync(fullPath, content);
+}
+
+describe('runInstallSecurityChecks', () => {
+  let testDir: string;
+
+  beforeEach(() => {
+    testDir = createTempDir();
+  });
+
+  afterEach(() => {
+    rmSync(testDir, { recursive: true, force: true });
+  });
+
+  it('flags a GET sign-out route for Next.js as a blocking finding', async () => {
+    writeFixtureFile(testDir, 'app/auth/signout/route.ts', 'export async function GET() { return signOut(); }');
+
+    const { findings, blocking } = await runInstallSecurityChecks('nextjs', testDir);
+
+    const signout = findings.find((f) => f.code === 'SIGNOUT_GET_HANDLER');
+    expect(signout).toBeDefined();
+    expect(blocking.map((f) => f.code)).toContain('SIGNOUT_GET_HANDLER');
+  });
+
+  it('returns no blocking findings for a clean POST sign-out install', async () => {
+    writeFixtureFile(
+      testDir,
+      'app/auth/actions.ts',
+      "'use server';\nexport async function signOutAction() { await signOut(); }",
+    );
+
+    const { blocking } = await runInstallSecurityChecks('nextjs', testDir);
+
+    expect(blocking).toEqual([]);
+  });
+
+  it('treats a hardcoded API key in source as blocking (framework-agnostic)', async () => {
+    writeFixtureFile(testDir, 'app/page.tsx', 'const key = "sk_test_FIXTUREKEYFORTESTING1";');
+
+    const { blocking } = await runInstallSecurityChecks('nextjs', testDir);
+
+    expect(blocking.map((f) => f.code)).toContain('API_KEY_IN_SOURCE');
+  });
+
+  it('treats a client-exposed secret API key as blocking', async () => {
+    // NEXT_PUBLIC_ prefix ships the secret to the browser bundle.
+    writeFixtureFile(testDir, '.env.local', 'NEXT_PUBLIC_WORKOS_API_KEY=sk_live_FIXTUREKEYFORTESTING1\n');
+
+    const { blocking } = await runInstallSecurityChecks('nextjs', testDir);
+
+    expect(blocking.map((f) => f.code)).toContain('API_KEY_LEAKED_TO_CLIENT');
+  });
+
+  it('reports warning-severity findings without blocking', async () => {
+    // .env.local present but not gitignored -> ENV_FILE_NOT_GITIGNORED (warning)
+    writeFixtureFile(testDir, '.env.local', 'WORKOS_CLIENT_ID=client_test\n');
+
+    const { findings, blocking } = await runInstallSecurityChecks('nextjs', testDir);
+
+    expect(findings.map((f) => f.code)).toContain('ENV_FILE_NOT_GITIGNORED');
+    expect(blocking).toEqual([]);
+  });
+
+  it('still runs cross-framework checks for an unknown integration', async () => {
+    writeFixtureFile(testDir, 'src/app.ts', 'const key = "sk_live_FIXTUREKEYFORTESTING1";');
+
+    const { blocking } = await runInstallSecurityChecks('some-backend', testDir);
+
+    expect(blocking.map((f) => f.code)).toContain('API_KEY_IN_SOURCE');
+  });
+});
+
+describe('securityFindingsToIssues', () => {
+  it('maps findings to pattern issues, folding filePath into the message', () => {
+    const findings: AuthPatternFinding[] = [
+      {
+        code: 'SIGNOUT_GET_HANDLER',
+        severity: 'error',
+        message: 'Signout uses GET',
+        filePath: 'app/auth/signout/route.ts',
+        remediation: 'Use a POST server action.',
+      },
+    ];
+
+    const issues = securityFindingsToIssues(findings);
+
+    expect(issues).toEqual([
+      {
+        type: 'pattern',
+        severity: 'error',
+        message: 'Signout uses GET (app/auth/signout/route.ts)',
+        hint: 'Use a POST server action.',
+      },
+    ]);
+  });
+});
+
+describe('formatSecurityFindingsForAgent', () => {
+  it('returns an empty string when there are no findings', () => {
+    expect(formatSecurityFindingsForAgent([])).toBe('');
+  });
+
+  it('includes the message, location, and remediation for each finding', () => {
+    const prompt = formatSecurityFindingsForAgent([
+      {
+        code: 'SIGNOUT_GET_HANDLER',
+        severity: 'error',
+        message: 'Signout uses GET',
+        filePath: 'app/auth/signout/route.ts',
+        remediation: 'Use a POST server action.',
+      },
+    ]);
+
+    expect(prompt).toContain('Signout uses GET');
+    expect(prompt).toContain('app/auth/signout/route.ts');
+    expect(prompt).toContain('Use a POST server action.');
+  });
+});
+
+describe('formatBlockingSecurityError', () => {
+  it('lists each blocking finding by code', () => {
+    const message = formatBlockingSecurityError([
+      { code: 'SIGNOUT_GET_HANDLER', severity: 'error', message: 'Signout uses GET' },
+    ]);
+
+    expect(message).toContain('SIGNOUT_GET_HANDLER');
+    expect(message).toContain('workos doctor');
+  });
+});