fix(security): authorization & input hardening (#5 review)

[DevHDI]

Summary

Addresses 3 security findings surfaced by the Raven's Verdict review on bmad-method-ui#5.

Each commit is atomic and references its finding.

Findings addressed

#	Severity	Issue	Fix
#7	🔴 Critical	listRepoBranches fell through to GitHub when repoConfig was null — caller could proxy GitHub API for repos not registered in their dashboard	Return NOT_FOUND when the repo is not registered for the authenticated user
#14	🟡 Moderate	detectBmadRepos accepted an unbounded array, allowing ceil(N/30) sequential GraphQL requests	Cap at MAX_DETECT_REPOS=300 with LIMIT_EXCEEDED early return
#2	🔴 Critical	Sign-up form distinguished HTTP 422 vs other errors → email enumeration	Collapse all non-403 sign-up failures into a single ambiguous message

Commits

• 9e3b61d fix(actions): guard listRepoBranches against non-owned repos
• 0973c93 fix(actions): cap detectBmadRepos input array (max 300)
• 7b2c60b fix(login): use generic error for non-403 signup failures

Verification

• pnpm tsc --noEmit — clean
• pnpm lint — 0 errors (2 pre-existing warnings unrelated)
• pnpm test — 151/151 passing

Test plan

• Unit test listRepoBranches with a repo not registered for the caller → NOT_FOUND
• Manual: POST detectBmadRepos with 500 repos → LIMIT_EXCEEDED
• Manual: sign up with an already-registered email → generic message, no 422-specific text

Context

This is PR 1 of 4 addressing the bmad-method-ui#5 review. Follow-ups:

• PR 2: cache revalidation hardening (feat(bmad): support alphanumeric IDs and introduce agile timeline dashboard #6, feat(epics): Kanban board view with side-panel drill-down #8, Migrate User.role from String to Prisma enum #13)
• PR 3: docker & env hygiene (fix(local): scan only BMAD dirs to avoid file count limit #3, Test #10, Test #11)
• PR 4: observability (fix(cache): revalidate route hardening & ordering (#5 review) #15, fix(infra): docker fail-fast & env hygiene (#5 review) #16, chore(observability): admin metrics & error logging (#5 review) #17)

CSP (#4) tracked in #12. Role enum migration (#12 from review) tracked in #13.

[DevHDI]

Follow-up review fixes

Addressed 2 issues raised in second-pass review of this PR.

Finding 1 (P1) — email enumeration was only client-side

Original commit 7b2c60b normalized the form message but the Better Auth endpoint /api/auth/sign-up/email was still directly reachable and returned distinguishable HTTP 422 responses. Valid critique — the misleading commit message claimed enumeration was prevented when it wasn't.

Fix in c37c119: added an after hook in src/lib/auth/auth.ts that intercepts every non-FORBIDDEN APIError on /sign-up/email and rewrites it to a generic BAD_REQUEST. The FORBIDDEN branch (registration disabled) is preserved because it reflects a server-wide flag, not a per-email signal. The client-side change in 7b2c60b is kept as defense-in-depth.

Finding 2 (P2) — MAX_DETECT_REPOS=300 broke detection for large accounts

Original commit 0973c93 capped at 300 which was too low; the add-repo dialog also silently swallowed the LIMIT_EXCEEDED error, leaving all repos with hasBmad=false and no UI signal. Valid regression.

Fix in 451bae7:

• Raised MAX_DETECT_REPOS from 300 to 2000 (covers virtually every realistic account, including orgs)
• Added a non-blocking amber warning in the dialog when detection fails (LIMIT_EXCEEDED or transient errors). The repo list stays usable, the user just sees "BMAD badges may be missing".

Verification

• pnpm tsc --noEmit — clean
• pnpm lint — 0 errors (2 pre-existing warnings)
• pnpm test — 151/151

[DevHDI]

Third-pass fix: HTTP status normalization

Reviewer's third-pass finding was correct and verified against Better Auth's source.

What I confirmed in better-auth@1.4.18

Inspecting node_modules/better-auth/dist/api/to-auth-endpoints.mjs:

1. Endpoint throws APIError("UNPROCESSABLE_ENTITY") (the 422 for email-exists). The .catch captures {response, status: 422, headers}.
2. runAfterHooks runs my after hook, which re-throws APIError("BAD_REQUEST"). The hook runner's .catch only captures {response, headers} — not status.
3. The outer code applies result.response = after.response but result.status stays 422.
4. toResponse(result.response, { headers, status: result.status }) → HTTP 422 leaks.

So the body was generic but the status code still leaked email existence. Reviewer was right.

Fix in 2e7272d

Moved normalization one layer up to a Next.js route wrapper at src/app/api/auth/sign-up/email/route.ts. Being more specific than /api/auth/[...all], it takes precedence in App Router routing. The wrapper:

• Calls auth.handler(request) (unchanged Better Auth flow)
• Lets 2xx responses pass through unchanged (cookies/headers preserved)
• Preserves 403 (registration-disabled flag, not a per-email signal)
• Rewrites anything else to a uniform HTTP 400 + { message: "Sign-up failed" }

The after hook in auth.ts was removed since the route wrapper is now the single point of normalization for both status and body.

Verification

• pnpm tsc --noEmit — clean
• pnpm lint — 0 errors (2 pre-existing warnings)
• pnpm test — 151/151

Manual test plan for whoever runs the env

• curl -X POST /api/auth/sign-up/email -d '{"email":"existing@test.com","password":"...","name":"..."}' → expect HTTP 400 with {message: "Sign-up failed"}, identical to curl -X POST with a new email + invalid password.
• curl -X POST /api/auth/sign-up/email -d '{...}' with ALLOW_REGISTRATION=false → expect HTTP 403 with the registration-disabled message.