Repository navigation
Conversation
✅ Deploy Preview for staging-void-cloud ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
cpojer
pushed a commit
that referenced
this pull request
Sep 11, 2026
…n provider (RFC 0074) (#194) * feat(api): add linkGitHubIdentity DB helper with conflict backstop Foundation for RFC 0074 (decouple GitHub-for-deploys from login provider). Links a GitHub OAuth identity to an already-authenticated Void user. - linkGitHubIdentity(db, userId, providerId): 'linked' | 'already_linked' - idempotent when already owned by userId - throws GitHubIdentityConflictError when owned by another user - GitHubIdentityConflictError (exported) — stable 409 contract for Task 2 - TOCTOU backstop: unique-violation (23505) on insert re-resolves ownership and maps to the same contract; no raw pg error escapes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(api): GitHub-identity link flow with deferred bind (RFC 0074) Add the authenticated GitHub-identity link flow to the platform API: a GitHub user authorizing in their browser ends up linked to the Void account that ran the CLI (the bearer-token holder), never to a user id baked into the OAuth state. Defeats the account-linking CSRF (RFC 0074 "Threat L1") by deferring the bind — the public callback only mints a PKCE-bound link_code and redirects it to the initiator's localhost; the bind happens in an authenticated POST /complete keyed on the token-holder. Routes (all under /github, on githubRoutes): - GET /github/link-url (authMiddleware) — signed state, no userId - GET /github/link-callback (public) — mints link_code, binds nothing - POST /github/link/complete (authMiddleware) — PKCE verify + linkGitHubIdentity Extract the PKCE primitives into auth/pkce.ts (pure move) and export isLocalhostRedirect from routes/auth.ts and exchangeCodeForUserAccountId from github/user-installations.ts so both routers share one implementation. State and link_code use the exact ${btoa(json)}.${sig} + JWT_SECRET scheme the install-url/callback already use. Tests: 14 integration cases (the 12 RFC cases + requires-auth and oauth-exchange-failure), including the Threat-L1 regression that pins the bind to the completing bearer token. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(api): stable github_identity_required code for install-url 403 GET /github/install-url keeps its 403 gate when the caller has no linked GitHub identity, but its body was prose. Return the stable machine code { error: 'github_identity_required' } so the CLI can auto-trigger the link flow without substring-matching prose. Status and gate unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): enforce one GitHub identity per account + backfill installs on link RFC 0074 final-review F1 + F2 (both fail-closed correctness bugs). F1 — one GitHub identity per Void account + deterministic selection: - db.ts: add GitHubIdentityAlreadyLinkedError; linkGitHubIdentity now rejects when the SAME user already has a DIFFERENT github identity (the same-id path stays idempotent 'already_linked'). Residual concurrent same-user double-link noted as deferred (no migration). - github.ts: getCallerGitHubAccountId orders by created_at asc before limit(1) so the signed install state.githubAccountId is deterministic. - /github/link/complete maps GitHubIdentityAlreadyLinkedError to 409 { error: 'github_already_linked' }, distinct from the existing other-user github_identity_conflict. F2 — backfill pre-existing unowned installations on link: - /github/link/complete now runs the same best-effort linkGitHubInstallationToUser the login callback does, so a Google user who links sees/connects their install-before-signin row. Tests: db test (same-user different-id throws, idempotent re-link), route tests (409 github_already_linked; install backfill on link). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(api): GitHub-identity link flow + bump framework (RFC 0074) Decouple GitHub-for-deploys from the login provider: any Void user (Google, GitHub, future SSO) can link a GitHub identity and use Deploy-on-GitHub, without forking the account. Platform (this repo, earlier commits on the branch): - linkGitHubIdentity DB helper + conflict backstop; one identity per account. - 3 link routes: GET /github/link-url (authed), GET /github/link-callback (public, mints a stateless PKCE-bound link_code, never binds), POST /github/link/complete (authed, binds to the bearer-token holder). Deferred bind defeats the account-linking confused-deputy. - exchangeCodeForUserAccountId; auth/pkce.ts extraction; install-url 403 now returns the stable code github_identity_required; backfill installs on link. Bumps framework submodule to include the CLI half (void github link + auto-link on install) — #230. Re-point to the framework merge SHA once #230 lands. Adds RFC meta/design-docs/0074. Deploy prerequisite: register <API>/github/link-callback as a GitHub App callback URL (staging + prod). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(api): re-point framework submodule to merged main (void#230) #230 squash-merged to framework main as cad3c32; move the pointer off the now-deleted feature branch tip (63bd41b) to the merged SHA so platform CI resolves the submodule from main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): link-callback returns 200 HTML redirect, not 302 to localhost `GET /github/link-callback` ended with `context.redirect(...)` (a 302 whose Location is always `http://localhost:<port>/...&link_code=…`). `wrangler dev` rewrites `Location: http://localhost:*` headers (prepends the dev origin port), so the link_code never reaches the browser and local `void github link` stalls. Extract the `/auth/dev-login` 200 + `location.replace` pattern into a shared `html-redirect.ts` `localhostRedirectResponse(target, title?)`, refactor dev-login to use it (behavior-preserving — keeps the "Dev login" title), and use it in link-callback (target is always localhost via isLocalhostRedirect). Update the link-callback test to assert 200 + HTML + the link_code-bearing target embedded in the body, per meta/lessons.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): match redirect_uri on link token exchange + fail-closed HMAC verify #194 review round 2 (both verified against source): - Cursor (High): GET /github/link-url sends redirect_uri on the GitHub authorize URL, but the link-callback token exchange omitted it. GitHub OAuth requires a matching redirect_uri on exchange when present on authorize. Thread an optional redirect_uri through exchangeCodeForUserAccountId/exchangeInstallCodeForUserToken; link-callback derives it via the shared linkCallbackUrl helper (byte-identical to the authorize one). Install flow passes none -> body unchanged. - Codex (P2): a non-base64url state signature (e.g. state=abc.*) made atob() throw inside hmacVerifyState before verifyLinkPayload could return null, surfacing a 500 to unauthenticated probes. Guard the decode -> return false (fixes the link-callback AND the install /callback, which share the primitive). Malformed input now fails closed with 400. Tests: redirect_uri-match + malformed-state-400 route cases, hmacVerifyState unit cases; fixed a non-deterministic forged-signature assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): enforce one-github-per-account on login + harden localhost redirect HTML #194 review round 3 (both verified against source): - Codex (P2): the OAuth login callback's email-match branch linked a new identity via createOAuthAccount directly, bypassing linkGitHubIdentity's one-github-per-user guard. So: link github X via /github/link/complete, then log in with github Y (different account, same verified email) silently created a SECOND provider='github' row. Route the github email-link through linkGitHubIdentity (github-only); on GitHubIdentityAlreadyLinkedError sign the user into the existing account WITHOUT a duplicate row. Matches Vercel's model: one GitHub login identity per account, extra repo access via installations. No migration; the (user_id,'github') partial unique index remains the deferred DB-level backstop. Other providers + new-user signup unchanged. - Cursor (High, not reachable today): localhostRedirectResponse embedded the target in an inline <script> via JSON.stringify, which doesn't escape </script>/&. Both current callers pass URL.toString() (which percent-encodes < >), so it's not exploitable now, but the helper is general. Escape < > & in the JS string literal and HTML-escape the noscript attribute. Tests: login dedup + cross-provider-consolidation cases; html-redirect script-breakout + noscript-escape + plain-URL no-op unit cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): don't escape & in the localhost-redirect script literal Fix wave 5 escaped `&`→`&` in the inline-<script> JS string literal, which broke auth-dev-login (CI integration): the redirect target's `&` query separator became `&`, so a consumer reading the raw HTML (the test, via new URL()) lost the separator. HTML does NOT decode entities inside a <script> element, so a literal `&` is safe there and the browser navigates correctly; only `<` (which forms `</script>`/`<!--`) is a breakout vector. Drop the `&` escape from the JS literal (keep `<`/`>`); the noscript HTML attribute still escapes `&`→`&`, where it IS required. Full suite green (90 files / 1987 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): serialize concurrent github links with a per-user FOR UPDATE lock #194 Cursor (Medium): linkGitHubIdentity was a read-then-insert and oauth_accounts is unique only on (provider, provider_id), so two concurrent same-user links of DIFFERENT github ids could both pass the existing-github check and insert, leaving two github rows (breaking RFC 0074's one-github-per- account invariant). Wrap the check-then-insert in a transaction that first takes SELECT ... FOR UPDATE on the user row (mirrors the deploy/build per-user lock pattern). The second concurrent link blocks until the first commits, then observes its row and is rejected with GitHubIdentityAlreadyLinkedError. A concurrent DIFFERENT-user link of the same github id isn't serialized by the per-user lock; it still collides on the (provider, provider_id) unique index — the insert raises 23505, we roll back and re-resolve ownership OUTSIDE the aborted transaction (the recovery query can't run inside it). No migration. Closes the documented residual; the (user_id,'github') partial unique index is no longer needed. Full suite green (90 files / 1987 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(api): skip install backfill for rejected github identity + bounce link-callback errors to CLI #194 review round 4 (both verified against source): - Codex (P2): after the login email-match branch rejects a SECOND github identity (GitHubIdentityAlreadyLinkedError, wave 5), the post-login github block still ran linkGitHubInstallationToUser(db, providerId, user.id), claiming the rejected id's unowned installations (and preview-repo access) onto the account. Track the rejection with a githubIdentityRejected flag and skip the github-identity-derived side effects when set. - Cursor (Medium): GET /github/link-callback returned JSON on the API host for the missing-code (GitHub denial) and exchange-failure cases, so the browser never reached the CLI's localhost listener and `void github link` hung until its 5-min timeout. After the signed state + localhost redirect are validated, bounce these errors to the CLI localhost via localhostRedirectResponse with an ?error= param (propagating GitHub's error on denial). API-only: the merged CLI's describeError already surfaces ?error=. Pre-state-validation returns stay JSON 400. Tests: rejected-identity install-not-claimed assertion; link-callback exchange-fail and denial now assert the localhost ?error= bounce. Full suite 90 files / 1988 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
amaze amaze amaze 👎