Skip to content

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

Merged
nicknisi merged 5 commits into
mainfrom
fix/installer-security-gate
Jun 8, 2026
Merged

fix: gate install success on doctor's auth-pattern security checks#175
nicknisi merged 5 commits into
mainfrom
fix/installer-security-gate

Conversation

@nicknisi

@nicknisi nicknisi commented Jun 8, 2026

Copy link
Copy Markdown
Member

Problem

workos install 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 success: true install, while workos doctor immediately flagged it. The install --validate step and doctor had drifted apart.

Repro: install into an empty dir scaffolds Next.js + AuthKit, generates export async function GET() { return signOut() } + <form method="GET">, reports "validation passed, 0 issues", then doctor finds SIGNOUT_GET_HANDLER.

Changes

  • New src/lib/validation/security-checks.ts — runs the security subset of doctor's checkAuthPatterns (GET sign-out, client-leaked / in-source API keys, ungitignored .env, mixed env) against the install dir. No network, so it's safe in the install loop. Unit-tested (9 cases).
  • agent-runner.ts wiring — two layers:
    • Self-correction loop now feeds security findings back to the agent alongside build/typecheck errors, so it fixes them before finishing.
    • A final gate throws on error-severity findings that survive retries → success: false, non-zero exit, commit/PR steps skipped (insecure code stays uncommitted).
  • auth-patterns.tsSIGNOUT_GET_HANDLER docsUrl now points at the live https://workos.com/docs/authkit/nextjs#ending-the-session (old /docs/authkit/sign-out 404s).

The blocking set is deliberately tight (SIGNOUT_GET_HANDLER, API_KEY_LEAKED_TO_CLIENT, API_KEY_IN_SOURCE) — high-confidence security errors only. Completeness checks (missing middleware/callback) overlap validateInstallation and are reported but never fail the install.

Related

  • Source-side fix (the sign-out guidance the installer LLM reads): fix(workos): mandate POST server-action sign-out in Next.js skill skills#33 (merged, released as @workos/skills@0.6.1) mandates the POST server-action pattern. This PR bumps the dep to 0.6.1, so that guidance ships here.
  • Follow-up not in this PR: ~15 other workos.com/docs/... deep-links across findings + framework configs are also 404 and need a verified sweep.

Testing

  • pnpm typecheck, pnpm test (2091 passing), pnpm build all pass.
  • New module has unit coverage; the thin agent-runner wiring is not separately tested (no existing agent-runner.spec.ts; logic lives in the tested module).
  • Not run: a live end-to-end workos install (needs credentials + network + LLM).

🤖 Generated with Claude Code

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).
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR closes the gap between workos install and workos doctor by adding a security-check layer (security-checks.ts) that runs the doctor's auth-pattern subset both inside the agent's self-correction retry loop and as a hard final gate — so an insecure GET sign-out or client-leaked API key now fails the install rather than shipping silently. It also bumps @workos/skills to 0.6.1 and fixes a 404 docs URL.

  • New security-checks.ts — extracts the security-relevant subset of checkAuthPatterns (SIGNOUT_GET_HANDLER, API_KEY_LEAKED_TO_CLIENT, API_KEY_IN_SOURCE, and three warning codes), converts them to ValidationIssues for the emitter, and formats them as agent correction prompts; covered by 9 new unit tests.
  • agent-runner.ts wiring — the validateAndFormat callback now appends security findings to any retry prompt, and a final post-retry gate throws (triggering success: false, skipping commit/PR steps) on any error-severity finding that survived retries; the passed_after_retry analytics event is moved after the gate to avoid pairing a "passed" event with a blocked install.
  • auth-patterns.ts — the SIGNOUT_GET_HANDLER docsUrl is corrected from a 404 path to the live anchor.

Confidence Score: 5/5

The change tightens install correctness by adding a security gate — no existing behavior is weakened and the new gate is correctly placed after all retries exhaust.

The new security-check module is pure file inspection with no network calls, the blocking/warning split is clean, and the two integration points in agent-runner (retry callback + final gate) are logically sound. The retry metric is correctly moved after the gate to avoid false positives. No pre-existing validation paths were removed or weakened, and the three blocking codes are well-tested with real file fixtures.

No files require special attention.

Important Files Changed

Filename Overview
src/lib/validation/security-checks.ts New module: runs the security subset of doctor's auth-pattern checks; correctly filters to SECURITY_FINDING_CODES and separates blocking (error) from warning findings. API is clean and well-documented.
src/lib/agent-runner.ts Security checks integrated in two places: retry-loop validateAndFormat callback and final gate. Retry metric correctly moved after gate. analytics.shutdown called before throw on blocking findings, consistent with existing error path.
src/lib/validation/security-checks.spec.ts New test file covering all three blocking codes, warning-only path, clean install, and unknown integration; fixture-based with proper tmp dir cleanup.
src/doctor/checks/auth-patterns.ts Single-line docs URL fix: SIGNOUT_GET_HANDLER docsUrl corrected from 404 path to live anchor.
package.json @workos/skills bumped from 0.6.0 to 0.6.1 to pick up the POST server-action guidance for the installer LLM.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[runAgentInstaller] --> B{noValidate?}
    B -- yes --> C[runAgent - no retries]
    B -- no --> D[Build retryConfig\nwith validateAndFormat]
    D --> E[runAgent with retries]
    E --> F{agentResult.error?}
    F -- yes --> G[analytics.shutdown error\nthrow Error]
    F -- no --> H[validateInstallation\nbuild: false]
    H --> I[runInstallSecurityChecks\nFinal Gate]
    I --> J[Merge issues: validationResult + security]
    J --> K[emit validation:issues\nif any]
    K --> L[emit validation:complete\npassed = passed AND blocking===0]
    L --> M{security.blocking\nlength > 0?}
    M -- yes --> N[analytics.capture\nsecurity gate blocked\nanalytics.shutdown error\nthrow Error]
    M -- no --> O[Track retry metrics\nif retryCount > 0]
    O --> P[Build env vars / Upload / Return summary]
    E -.->|each retry| Q[validateAndFormat callback]
    Q --> R[quickCheckValidateAndFormat]
    Q --> S[runInstallSecurityChecks\nRetry Layer]
    R --> T{quickPrompt===null\nAND blocking===0?}
    S --> T
    T -- yes --> U[return null\nno retry needed]
    T -- no --> V[Join build prompt\n+ security findings\nfor agent correction]
Loading

Reviews (3): Last reviewed commit: "fix: emit retry-summary telemetry only a..." | Re-trigger Greptile

Comment thread src/lib/validation/security-checks.spec.ts Outdated
Comment thread src/lib/agent-runner.ts Outdated

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

nicknisi added 4 commits June 8, 2026 15:59
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.
…omment

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.
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.
@nicknisi nicknisi merged commit 5a50c77 into main Jun 8, 2026
6 checks passed
@nicknisi nicknisi deleted the fix/installer-security-gate branch June 8, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant