Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 5 additions & 5 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/doctor/checks/auth-patterns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
}
}
Expand Down
80 changes: 63 additions & 17 deletions src/lib/agent-runner.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
148 changes: 148 additions & 0 deletions src/lib/validation/security-checks.spec.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
Loading
Loading