diff --git a/README.md b/README.md index 9938862..d443faf 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,20 @@ All version responses include a `stale` field that indicates whether the data wa - `GET /central-alerts/v1/list` - Public endpoint for fetching active alerts. +### Extensions v2 ownership verification + +For organization developer IDs, GitHub membership is used for automatic +verification only when the API has a valid, unexpired membership snapshot. A +fresh snapshot that does not contain the organization remains a confirmed +mismatch and is rejected. Missing, malformed, or expired evidence is +inconclusive instead: a new profile remains unapproved and a claim remains +pending for manual moderator review. Moderators must verify ownership through +their normal out-of-band process before approving either workflow. + +`github_org_verified` being absent or `null` is a review signal, not proof of +ownership or an authorization grant. Consumers and moderation tooling must not +treat an inconclusive result as verified. + ## Configuration If you're running this yourself, you'll need a few things set up. diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index 4f4af50..f3dda2d 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -102,7 +102,7 @@ export const developers = sqliteTable( githubOrgVerified: integer("github_org_verified"), githubVerificationNote: text("github_verification_note"), // Set whenever githubOrgVerified is (re-)computed to a definitive 0/1 — - // see DevelopersDatabase.reverifyOwn(). Left null/stale on an + // see DeveloperProfilesDatabase.reverifyOwn(). Left null/stale on an // inconclusive check (no linked GitHub identity), same as // githubOrgVerified itself. githubVerifiedAt: text("github_verified_at"), diff --git a/src/services/extensions/v2/developer-claims-database.ts b/src/services/extensions/v2/developer-claims-database.ts new file mode 100644 index 0000000..1a7340b --- /dev/null +++ b/src/services/extensions/v2/developer-claims-database.ts @@ -0,0 +1,535 @@ +import { and, asc, desc, eq, isNull, sql } from "drizzle-orm"; +import { DatabaseResult } from "../../../lib/interfaces"; +import { ExtensionsDb } from "../../../lib/db"; +import { developerClaims, developers, users } from "./db/schema"; +import { + databaseError, + errorMessageChain, + isDeveloperOwnerConflict +} from "./errors"; +import { toD1Statement } from "./d1-batch"; +import { + Developer, + DeveloperClaim, + DeveloperProfile, + PendingDeveloperClaim +} from "./interfaces"; +import { DeveloperProfilesDatabase } from "./developer-profiles-database"; +import { verifyGithubOwnership } from "./developer-identity-verification"; + +type ClaimRow = typeof developerClaims.$inferSelect; + +function parseClaimRow(row: ClaimRow): DeveloperClaim { + return { + id: row.id, + developer_id: row.developerId, + claimant_id: row.claimantId, + status: row.status as DeveloperClaim["status"], + note: row.note ?? undefined, + review_note: row.reviewNote ?? undefined, + reviewer_id: row.reviewerId ?? undefined, + created_at: row.createdAt, + reviewed_at: row.reviewedAt ?? undefined, + github_org_verified: + row.githubOrgVerified === null || row.githubOrgVerified === undefined + ? undefined + : row.githubOrgVerified === 1, + github_verification_note: row.githubVerificationNote ?? undefined + }; +} + +function isPendingClaimConflict(error: unknown): boolean { + return /UNIQUE constraint failed.*developer_claims/i.test( + errorMessageChain(error) + ); +} + +export class DeveloperClaimsDatabase { + constructor(private db: ExtensionsDb) {} + private async getClaimById( + id: string + ): Promise> { + try { + const [row] = await this.db + .select() + .from(developerClaims) + .where(eq(developerClaims.id, id)); + if (!row) { + return { + data: null, + error: { + message: `Cannot find claim by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + return { data: parseClaimRow(row), error: null }; + } catch (error) { + return databaseError("getClaimById", error); + } + } + + // Used by claim once a developer/eligibility-guarded write + // affects zero rows: distinguishes an inactive claimant, "no such + // developer", and the two possible ownership conflicts for an accurate + // response, without reopening the race the guarded write already closed. + private async claimIneligibilityError( + developerId: string, + claimantId: string + ): Promise<{ + code: "NOT_FOUND" | "CONFLICT" | "ACCOUNT_INACTIVE"; + message: string; + }> { + const [developer] = await this.db + .select({ ownerUserId: developers.ownerUserId }) + .from(developers) + .where(eq(developers.id, developerId)); + if (!developer) { + return { code: "NOT_FOUND", message: "Developer not found" }; + } + + const [claimant] = await this.db + .select({ deletedAt: users.deletedAt }) + .from(users) + .where(eq(users.id, claimantId)); + if (!claimant || claimant.deletedAt !== null) { + return { + code: "ACCOUNT_INACTIVE", + message: "Active account required" + }; + } + + if (developer.ownerUserId !== null) { + return { code: "CONFLICT", message: "This profile is already owned" }; + } + return { + code: "CONFLICT", + message: "You already have a developer profile" + }; + } + + async claim( + developerId: string, + claimantId: string, + note?: string, + githubToken?: string + ): Promise> { + try { + let githubOrgVerified: number | null = null; + let githubVerificationNote: string | null = null; + + const [developer] = await this.db + .select({ type: developers.type }) + .from(developers) + .where( + and(eq(developers.id, developerId), isNull(developers.ownerUserId)) + ); + + if (developer) { + // Cheap short-circuit ahead of the GitHub lookup below: a claimant + // replaying an already-pending claim on this id would otherwise + // trigger a fresh GitHub API call every time, purely to be told the + // INSERT's own guard rejects it as a duplicate — letting one caller + // burn through the shared service-level GitHub quota for free. This + // is safe precisely because it only ever *returns* here when the + // read observes `pending` — it never falls through to verification + // or the INSERT in that case, so it can't itself create an + // unverified claim. Anything else (no claim yet, or one already + // resolved to approved/rejected) always continues through full + // verification below. A pending claim that resolves between this + // read and the response going out can make the message stale + // relative to that instant, but never lets a row get created + // without verification. + const [hasPendingClaim] = await this.db + .select({ one: sql`1` }) + .from(developerClaims) + .where( + and( + eq(developerClaims.developerId, developerId), + eq(developerClaims.claimantId, claimantId), + eq(developerClaims.status, "pending") + ) + ); + + if (hasPendingClaim) { + return { + data: null, + error: { + code: "CONFLICT", + message: "You already have a pending claim on this profile" + } + }; + } + + const check = await verifyGithubOwnership( + this.db, + developerId, + developer.type as Developer["type"], + claimantId, + githubToken + ); + + if ("error" in check) { + return { data: null, error: check.error }; + } + + if (check.mismatch) { + return { + data: null, + error: { + code: "GITHUB_MISMATCH", + message: + "Your linked GitHub account doesn't match this developer's GitHub organization or username, so it can't be claimed automatically. Make sure you're signed in with the right GitHub account, then try again." + } + }; + } + + githubOrgVerified = check.githubOrgVerified; + githubVerificationNote = check.note; + } + + const id = crypto.randomUUID(); + let result; + try { + // Both eligibility checks are folded into the INSERT itself, rather + // than a separate SELECT beforehand — a caller who loses eligibility + // (developer gets claimed/transferred, or the caller picks up a + // different profile) between an up-front check and the write could + // otherwise still slip a stale claim through. (The SELECT above is + // only used to decide the GitHub verification signal, and is always + // re-checked here — it can't itself grant eligibility.) Kept as raw + // sql: an INSERT...SELECT...WHERE EXISTS isn't expressible via + // .insert().values(). + result = await this.db.run(sql` + INSERT INTO ${developerClaims} (id, developer_id, claimant_id, note, github_org_verified, github_verification_note) + SELECT ${id}, ${developerId}, ${claimantId}, ${note ?? null}, ${githubOrgVerified}, ${githubVerificationNote} + WHERE EXISTS (SELECT 1 FROM ${developers} WHERE id = ${developerId} AND owner_user_id IS NULL) + AND NOT EXISTS (SELECT 1 FROM ${developers} WHERE owner_user_id = ${claimantId}) + AND EXISTS (SELECT 1 FROM ${users} WHERE ${users.id} = ${claimantId} AND ${users.deletedAt} IS NULL) + `); + } catch (error) { + if (isPendingClaimConflict(error)) { + return { + data: null, + error: { + code: "CONFLICT", + message: "You already have a pending claim on this profile" + } + }; + } + return databaseError("claim", error); + } + + if (!result.meta?.changes) { + return { + data: null, + error: await this.claimIneligibilityError(developerId, claimantId) + }; + } + + return this.getClaimById(id); + } catch (error) { + return databaseError("claim", error); + } + } + + // Lets a claimant withdraw their own pending claim — scoped to + // claimant_id so this can't be used to cancel someone else's, and to + // status = 'pending' so a moderator's decision can't be undone by it. + async cancelClaim( + claimId: string, + claimantId: string + ): Promise> { + let result; + try { + result = await this.db.delete(developerClaims).where( + and( + eq(developerClaims.id, claimId), + eq(developerClaims.claimantId, claimantId), + eq(developerClaims.status, "pending"), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${claimantId} AND ${users.deletedAt} IS NULL + )` + ) + ); + } catch (error) { + return databaseError("cancelClaim", error); + } + + if (!result.meta?.changes) { + return { + data: null, + error: { + message: `Cannot find pending claim by id: ${claimId}`, + code: "NOT_FOUND" + } + }; + } + + return { data: { id: claimId }, error: null }; + } + + async listMyClaims( + claimantId: string + ): Promise> { + let rows; + try { + rows = await this.db + .select() + .from(developerClaims) + .where(eq(developerClaims.claimantId, claimantId)) + .orderBy(desc(developerClaims.createdAt)); + } catch (error) { + return databaseError("listMyClaims", error); + } + + return { data: rows.map(parseClaimRow), error: null }; + } + + async listPendingClaims(): Promise> { + let rows; + try { + rows = await this.db + .select({ + claim: developerClaims, + developerName: developers.name, + developerType: developers.type, + claimantName: users.name, + claimantGithubLogin: users.githubLogin + }) + .from(developerClaims) + .innerJoin(developers, eq(developers.id, developerClaims.developerId)) + .leftJoin(users, eq(users.id, developerClaims.claimantId)) + .where(eq(developerClaims.status, "pending")) + .orderBy(asc(developerClaims.createdAt)); + } catch (error) { + return databaseError("listPendingClaims", error); + } + + return { + data: rows.map((row) => ({ + ...parseClaimRow(row.claim), + developer_name: row.developerName, + developer_type: + row.developerType as PendingDeveloperClaim["developer_type"], + claimant_name: row.claimantName, + claimant_github_login: row.claimantGithubLogin + })), + error: null + }; + } + + private async explainClaimApprovalNoOp( + claim: DeveloperClaim + ): Promise> { + const latestClaim = await this.getClaimById(claim.id); + if (latestClaim.error || latestClaim.data?.status !== "pending") { + return { + data: null, + error: latestClaim.error ?? { + message: "Claim is not pending", + code: "CONFLICT" + } + }; + } + + try { + const [developer] = await this.db + .select({ ownerUserId: developers.ownerUserId }) + .from(developers) + .where(eq(developers.id, claim.developer_id)); + if (!developer) { + return { + data: null, + error: { message: "Developer not found", code: "NOT_FOUND" } + }; + } + if (developer.ownerUserId !== null) { + return { + data: null, + error: { message: "This profile is already owned", code: "CONFLICT" } + }; + } + return { + data: null, + error: { + message: "The claimant already owns a different developer profile", + code: "CONFLICT" + } + }; + } catch (error) { + return databaseError("approveClaim", error); + } + } + + async approveClaim( + claimId: string, + reviewerId: string + ): Promise> { + const existing = await this.getClaimById(claimId); + if (existing.error || !existing.data) { + return { + data: null, + error: existing.error ?? { + message: `Cannot find claim by id: ${claimId}`, + code: "NOT_FOUND" + } + }; + } + const claim = existing.data; + + if (claim.status !== "pending") { + return { + data: null, + error: { message: "Claim is not pending", code: "CONFLICT" } + }; + } + + // Keep the status transition, ownership handoff, and competing-claim + // rejection in one raw D1 batch. Each write is gated by changes() from + // the immediately preceding statement, so a stale claim cannot transfer + // ownership and a failed transfer cannot reject competing claims. + // + // The assertion statement is intentionally capable of violating the + // ownership_epoch CHECK. D1 rolls the entire batch back when that happens, + // which prevents a zero-row ownership update from leaving the claim + // approved. Its successful no-op update also preserves changes() = 1 for + // the final rejection statement. + let results; + try { + const claimStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developer_claims + SET status = 'approved', reviewer_id = ?, reviewed_at = CURRENT_TIMESTAMP + WHERE id = ? AND status = 'pending' + AND EXISTS ( + SELECT 1 FROM developers d + WHERE d.id = developer_claims.developer_id + AND d.owner_user_id IS NULL + ) + AND NOT EXISTS ( + SELECT 1 FROM developers owned + WHERE owned.owner_user_id = developer_claims.claimant_id + ) + AND EXISTS ( + SELECT 1 FROM users + WHERE users.id = ? AND users.deleted_at IS NULL + )`, + params: [reviewerId, claimId, reviewerId] + }); + const developerStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developers + SET owner_user_id = ?, + ownership_epoch = ownership_epoch + 1, + content_revision = content_revision + 1, + approved_at = NULL, approved_revision = NULL, approved_by = NULL, + url_check_cooldown_until = NULL, + github_org_verified = ?, github_verification_note = ?, + github_verified_at = ?, updated_at = CURRENT_TIMESTAMP + WHERE changes() = 1 AND id = ? AND owner_user_id IS NULL + AND NOT EXISTS (SELECT 1 FROM developers WHERE owner_user_id = ?)`, + params: [ + claim.claimant_id, + claim.github_org_verified === undefined + ? null + : claim.github_org_verified + ? 1 + : 0, + claim.github_verification_note ?? null, + claim.github_org_verified === undefined ? null : claim.created_at, + claim.developer_id, + claim.claimant_id + ] + }); + const assertTransferStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developers + SET ownership_epoch = CASE WHEN changes() = 1 THEN ownership_epoch ELSE 0 END + WHERE id = ?`, + params: [claim.developer_id] + }); + const rejectOthersStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developer_claims + SET status = 'rejected', reviewer_id = ?, reviewed_at = CURRENT_TIMESTAMP, + review_note = 'Another claim on this profile was approved' + WHERE changes() = 1 AND developer_id = ? AND status = 'pending' AND id != ?`, + params: [reviewerId, claim.developer_id, claimId] + }); + + results = await this.db.$client.batch([ + claimStmt, + developerStmt, + assertTransferStmt, + rejectOthersStmt + ]); + } catch (error) { + if ( + /CHECK constraint failed.*ownership_epoch/i.test( + errorMessageChain(error) + ) + ) { + return this.explainClaimApprovalNoOp(claim); + } + if (isDeveloperOwnerConflict(error)) { + return { + data: null, + error: { + message: "The claimant already owns a different developer profile", + code: "CONFLICT" + } + }; + } + return databaseError("approveClaim", error); + } + + const [claimResult] = results; + if (!claimResult.meta?.changes) { + // Diagnose only after the guarded transaction. These reads improve the + // response without participating in (or weakening) its race safety. + return this.explainClaimApprovalNoOp(claim); + } + + return new DeveloperProfilesDatabase(this.db).getById(claim.developer_id); + } + + async rejectClaim( + claimId: string, + reviewerId: string, + reviewNote: string + ): Promise> { + let result; + try { + result = await this.db + .update(developerClaims) + .set({ + status: "rejected", + reviewerId, + reviewNote, + reviewedAt: sql`CURRENT_TIMESTAMP` + }) + .where( + and( + eq(developerClaims.id, claimId), + eq(developerClaims.status, "pending"), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL + )` + ) + ); + } catch (error) { + return databaseError("rejectClaim", error); + } + + if (!result.meta?.changes) { + return { + data: null, + error: { + message: `Cannot find pending claim by id: ${claimId}`, + code: "NOT_FOUND" + } + }; + } + + return this.getClaimById(claimId); + } +} diff --git a/src/services/extensions/v2/developer-identity-verification.ts b/src/services/extensions/v2/developer-identity-verification.ts new file mode 100644 index 0000000..34531cf --- /dev/null +++ b/src/services/extensions/v2/developer-identity-verification.ts @@ -0,0 +1,138 @@ +import { DatabaseError } from "../../../lib/interfaces"; +import { ExtensionsDb } from "../../../lib/db"; +import { + checkGithubEntity, + GithubUnavailableReason, + matchesClaimant, + urlMatchesGithubBlog +} from "./github-verification"; +import { Developer } from "./interfaces"; +import { UsersDatabase } from "./users-database"; + +export type GithubOwnershipVerificationResult = + | { mismatch: true } + | { + mismatch: false; + githubOrgVerified: number | null; + githubUrlVerified: number | null; + note: string | null; + } + | { error: DatabaseError }; + +export function githubUnavailableError( + reason: GithubUnavailableReason +): DatabaseError { + if (reason === "unsupported_entity_type") { + return { + code: "GITHUB_ENTITY_UNSUPPORTED", + message: "This GitHub account type is not supported" + }; + } + return reason === "rate_limited" + ? { + code: "RATE_LIMITED", + message: "GitHub verification is temporarily rate limited" + } + : { + code: "SERVICE_UNAVAILABLE", + message: "GitHub verification is temporarily unavailable" + }; +} + +// `githubToken` authenticates the GitHub entity-existence lookup only (a +// service-level credential, raises the public rate limit) — it is never +// the claimant's own token, which never leaves the auth service. Shared by +// claim() and upsertOwn(): both need the same question answered — does a +// real GitHub org/user exist for this id, and if so, does the caller's own +// linked GitHub identity match it? A positive mismatch is the only automated +// block; no real GitHub entity for this id, or the caller having no linked +// GitHub identity yet, both fall back to an explicitly unverified result for +// manual moderator review. That fallback never grants verified ownership. +// publisherUrl — only ever passed by upsertOwn's create path, which is the +// one place a new Publisher URL is actually being submitted alongside +// identity verification; claim() has no URL of its own to cross-check +// (the developer row it's claiming already exists). Drives +// githubUrlVerified only — a non-matching or unset GitHub "website" field +// never blocks or un-verifies identity, since it's optional and often +// stale, unlike the identity check above. +export async function verifyGithubOwnership( + db: ExtensionsDb, + developerId: string, + developerType: Developer["type"], + callerId: string, + githubToken?: string, + publisherUrl?: string +): Promise { + const githubEntity = await checkGithubEntity(developerId, githubToken ?? ""); + + if (githubEntity.status === "unavailable") { + return { error: githubUnavailableError(githubEntity.reason) }; + } + + if (githubEntity.status === "not_found") { + return { + mismatch: false, + githubOrgVerified: null, + githubUrlVerified: null, + note: "GitHub entity was not verified automatically — reviewed manually." + }; + } + + // A real GitHub entity exists for this id, just under the other type + // (e.g. a real org submitted as a "user") — this is a confirmed + // disagreement with GitHub, not an unknown, so it must block rather than + // fall back to unverified. Otherwise a caller could take a real org/user's + // id unverified simply by submitting the wrong type for it. + if (githubEntity.entity.type !== developerType) { + return { mismatch: true }; + } + + const identity = await new UsersDatabase(db).getGithubIdentity(callerId); + // A real DB/schema failure here is not the same as "caller has no linked + // GitHub identity" — swallowing it would silently let creation/claiming + // proceed unverified during an outage instead of surfacing the error. + if (identity.error || !identity.data) { + return { + error: identity.error ?? { + message: "Failed to load caller's GitHub identity", + code: "DATABASE_ERROR" + } + }; + } + const callerIdentity = identity.data; + + // Organization membership is only a definitive non-match when central auth + // supplied a valid, unexpired membership snapshot. Missing, malformed, or + // expired evidence is inconclusive and must go to manual review instead of + // blocking an otherwise valid claimant/profile owner. This path never + // grants verified ownership; the profile/claim still needs moderation. + if ( + !callerIdentity.githubLogin?.trim() || + (developerType === "organization" && !callerIdentity.githubOrgsAvailable) + ) { + return { + mismatch: false, + githubOrgVerified: null, + githubUrlVerified: null, + note: !callerIdentity.githubLogin?.trim() + ? "Caller has no linked GitHub identity yet — reviewed manually." + : "Caller's GitHub organization memberships could not be confirmed — reviewed manually." + }; + } + + if (matchesClaimant(developerType, developerId, callerIdentity)) { + return { + mismatch: false, + githubOrgVerified: 1, + githubUrlVerified: urlMatchesGithubBlog( + publisherUrl, + githubEntity.entity.blog + ) + ? 1 + : null, + note: "Verified: caller's linked GitHub identity matches." + }; + } + + return { mismatch: true }; +} diff --git a/src/services/extensions/v2/developer-profile-routes.ts b/src/services/extensions/v2/developer-profile-routes.ts index fd1d84b..b77edc7 100644 --- a/src/services/extensions/v2/developer-profile-routes.ts +++ b/src/services/extensions/v2/developer-profile-routes.ts @@ -11,7 +11,7 @@ import { ReverifyQuerySchema, toPublicDeveloper } from "./interfaces"; -import { DevelopersDatabase } from "./developers-database"; +import { DeveloperProfilesDatabase } from "./developer-profiles-database"; import { ExtensionsV2App, RouteDependencies } from "./route-dependencies"; export function registerDeveloperProfileRoutes( @@ -48,7 +48,7 @@ export function registerDeveloperProfileRoutes( app.openapi(getOwnDeveloperRoute, async (c) => { const auth = dependencies.auth(c); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.getOwn(auth.userId); @@ -130,7 +130,7 @@ export function registerDeveloperProfileRoutes( const auth = dependencies.auth(c); const body = c.req.valid("json"); const platform = dependencies.platform(c); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.upsertOwn( @@ -150,7 +150,7 @@ export function registerDeveloperProfileRoutes( ); if (error || !data) { const status = - error?.code === "GITHUB_MISMATCH" + error?.code === "GITHUB_MISMATCH" || error?.code === "ACCOUNT_INACTIVE" ? 403 : error?.code === "PROFILE_CREATION_RATE_LIMITED" ? 429 @@ -215,7 +215,7 @@ export function registerDeveloperProfileRoutes( app.openapi(deleteOwnDeveloperRoute, async (c) => { const auth = dependencies.auth(c); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.deleteOwn(auth.userId); @@ -227,7 +227,9 @@ export function registerDeveloperProfileRoutes( code: error?.code ?? "DATABASE_ERROR" } }, - statusFromErrorCode(error?.code) + error?.code === "ACCOUNT_INACTIVE" + ? 403 + : statusFromErrorCode(error?.code) ); } return c.json({ result: data }, 200); @@ -288,7 +290,7 @@ export function registerDeveloperProfileRoutes( const auth = dependencies.auth(c); const { check_url } = c.req.valid("query"); const platform = dependencies.platform(c); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.reverifyOwn( @@ -297,10 +299,13 @@ export function registerDeveloperProfileRoutes( platform.getEnv("GITHUB_TOKEN") ); if (error || !data) { - const status = statusFromGithubErrorCode( - error?.code, - statusFromErrorCode(error?.code) - ); + const status = + error?.code === "ACCOUNT_INACTIVE" + ? 403 + : statusFromGithubErrorCode( + error?.code, + statusFromErrorCode(error?.code) + ); return c.json( { error: { @@ -348,7 +353,7 @@ export function registerDeveloperProfileRoutes( app.openapi(getDeveloperRoute, async (c) => { const { id } = c.req.valid("param"); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.getById(id); diff --git a/src/services/extensions/v2/developer-profiles-database.ts b/src/services/extensions/v2/developer-profiles-database.ts new file mode 100644 index 0000000..4d02ec0 --- /dev/null +++ b/src/services/extensions/v2/developer-profiles-database.ts @@ -0,0 +1,1023 @@ +import { and, asc, desc, eq, isNull, or, sql } from "drizzle-orm"; +import { DatabaseResult } from "../../../lib/interfaces"; +import { ExtensionsDb } from "../../../lib/db"; +import { + developers, + developerHistory, + developerTransfers, + extensions, + extensionSubmissions, + users +} from "./db/schema"; +import { + databaseError, + isDeveloperIdConflict, + isDeveloperOwnerConflict +} from "./errors"; +import { toD1Statement } from "./d1-batch"; +import { + checkGithubEntity, + matchesClaimant, + urlMatchesGithubBlog +} from "./github-verification"; +import { + Developer, + DeveloperHistoryEntry, + DeveloperProfile +} from "./interfaces"; +import { + githubUnavailableError, + verifyGithubOwnership +} from "./developer-identity-verification"; +import { UsersDatabase } from "./users-database"; + +const URL_CHECK_COOLDOWN_SECONDS = 60; + +type DeveloperRow = typeof developers.$inferSelect; + +function parseDeveloperRow(row: DeveloperRow): DeveloperProfile { + return { + id: row.id, + type: row.type as DeveloperProfile["type"], + name: row.name, + URL: row.url ?? undefined, + avatar_url: row.avatarUrl ?? undefined, + contact_email: row.contactEmail ?? undefined, + approved: + row.approvedAt !== null && + row.approvedAt !== undefined && + (row.approvedRevision == null || + Number(row.approvedRevision) === Number(row.contentRevision ?? 1)), + content_revision: Number(row.contentRevision ?? 1), + github_org_verified: + row.githubOrgVerified === null || row.githubOrgVerified === undefined + ? undefined + : row.githubOrgVerified === 1, + github_verification_note: row.githubVerificationNote ?? undefined, + github_verified_at: row.githubVerifiedAt ?? undefined, + github_url_verified: row.githubUrlVerified === 1 ? true : undefined + }; +} + +function parseDeveloperRowWithOwner(row: { + developer: DeveloperRow; + ownerName: string | null; + ownerGithubLogin: string | null; +}): DeveloperProfile { + return { + ...parseDeveloperRow(row.developer), + unclaimed: row.developer.ownerUserId === null, + owner_name: row.ownerName, + owner_github_login: row.ownerGithubLogin + }; +} + +export class DeveloperProfilesDatabase { + constructor(private db: ExtensionsDb) {} + async getOwn( + userId: string + ): Promise< + | DatabaseResult + | { data: null; error: null } + > { + try { + const [row] = await this.db + .select() + .from(developers) + .where(eq(developers.ownerUserId, userId)); + if (!row) return { data: null, error: null }; + + const [pending] = await this.db + .select({ id: developerTransfers.id }) + .from(developerTransfers) + .where( + and( + eq(developerTransfers.developerId, row.id), + isNull(developerTransfers.acceptedAt), + isNull(developerTransfers.revokedAt), + sql`${developerTransfers.expiresAt} > CURRENT_TIMESTAMP` + ) + ) + .limit(1); + + return { + data: { + ...parseDeveloperRow(row), + unclaimed: false, + has_pending_transfer: pending !== undefined + }, + error: null + }; + } catch (error) { + return databaseError("getOwn", error); + } + } + + // githubToken — see the comment on verifyGithubOwnership(). Only consulted + // when creating a brand-new profile (developer.id is immutable once + // owned, so an update can't need re-verifying); guards against squatting + // on an id that matches a real GitHub org/user the caller doesn't control, + // the one gap claim() alone can't close since it only ever applies to + // rows that already exist unowned. + async upsertOwn( + userId: string, + developer: Developer, + githubToken?: string, + allowCreationAttempt: () => Promise = async () => true + ): Promise> { + try { + const [existingOwn] = await this.db + .select() + .from(developers) + .where(eq(developers.ownerUserId, userId)); + + const [existingById] = await this.db + .select() + .from(developers) + .where(eq(developers.id, developer.id)); + const isCreating = !existingOwn; + + let githubOrgVerified: number | null = null; + let githubUrlVerified: number | null = null; + let githubVerificationNote: string | null = null; + + let mainStmt: D1PreparedStatement; + if (isCreating) { + if (existingById) { + // Distinct from the generic CONFLICT used elsewhere in this file — + // consumers (the extensions repo's create-profile form) need to + // reliably detect this specific case to point the user at the + // claim flow, which a shared, message-string-matched code can't do. + return { + data: null, + error: { + message: "Developer id already exists", + code: "DEVELOPER_ID_TAKEN" + } + }; + } + + // This hook sits after both cheap D1 existence checks and directly + // before the creation-only GitHub lookup. The Worker supplies the + // configured account limiter; keeping it as a callback leaves this + // database/service module runtime-agnostic and ensures updates and + // already-taken ids never spend creation allowance. + if (!(await allowCreationAttempt())) { + return { + data: null, + error: { + message: + "Too many new profile creation attempts; try again in 60 seconds", + code: "PROFILE_CREATION_RATE_LIMITED" + } + }; + } + + const check = await verifyGithubOwnership( + this.db, + developer.id, + developer.type, + userId, + githubToken, + developer.URL + ); + + if ("error" in check) { + return { data: null, error: check.error }; + } + + if (check.mismatch) { + return { + data: null, + error: { + code: "GITHUB_MISMATCH", + message: + "This id matches a real GitHub organization or username that isn't linked to your account, so it can't be used automatically. Make sure you're signed in with the right GitHub account, or choose a different id." + } + }; + } + + githubOrgVerified = check.githubOrgVerified; + githubUrlVerified = check.githubUrlVerified; + githubVerificationNote = check.note; + + // INSERT ... SELECT makes the active-account check part of the + // mutation itself. The middleware check is only an early rejection; + // a deletion can win between that check and this statement. + mainStmt = toD1Statement(this.db.$client, { + sql: `INSERT INTO developers ( + id, type, name, url, avatar_url, contact_email, + owner_user_id, approved_at, created_at, updated_at, + github_org_verified, + github_verification_note, github_verified_at, + github_url_verified + ) + SELECT ?, ?, ?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP, ?, ?, + CASE WHEN ? IS NULL THEN NULL ELSE CURRENT_TIMESTAMP END, + ? + WHERE EXISTS ( + SELECT 1 FROM users + WHERE id = ? AND deleted_at IS NULL + )`, + params: [ + developer.id, + developer.type, + developer.name, + developer.URL ?? null, + developer.avatar_url ?? null, + developer.contact_email ?? null, + userId, + githubOrgVerified, + githubVerificationNote, + githubOrgVerified, + githubUrlVerified, + userId + ] + }); + } else { + if (developer.id !== existingOwn.id) { + return { + data: null, + error: { + message: "Developer id cannot be changed", + code: "CONFLICT" + } + }; + } + + // approved_at is normally cleared here, even if nothing meaningful + // changed — the reviewed content just got overwritten, so the old + // approval no longer applies. Not worth diffing old vs. new field + // values for that. The one exception: a profile that's currently + // GitHub org/user verified keeps its approval across edits — that + // verification is an independently-computed identity signal (this + // write never touches githubOrgVerified, except when the id's type + // changes below) strong enough on its own that re-queuing for + // manual review on every edit isn't worth the moderator load. + // approvedRevision is bumped in lockstep with contentRevision in + // that branch so the existing approval keeps matching (see + // parseDeveloperRow) instead of silently going stale. + // + // A type change invalidates the existing GitHub verification + // outright — matchesClaimant() compares differently per type (org + // membership vs. username), so a signal computed for the old type + // says nothing about the new one. Falls back to approval clearing + // and manual review, same as any other unverified edit. + const typeChanged = developer.type !== existingOwn.type; + // A URL change invalidates only the URL signal, not identity — + // github_url_verified describes whether *this* URL matches GitHub's + // on-file website, so a stale URL can't still be "verified" once + // it's no longer the URL being served. + const urlChanged = (developer.URL ?? null) !== existingOwn.url; + const keepsApproval = + !typeChanged && existingOwn.githubOrgVerified === 1; + + const updateStmt = this.db + .update(developers) + .set({ + type: developer.type, + name: developer.name, + url: developer.URL ?? null, + avatarUrl: developer.avatar_url ?? null, + contactEmail: developer.contact_email ?? null, + contentRevision: sql`content_revision + 1`, + ...(keepsApproval + ? { approvedRevision: sql`content_revision + 1` } + : { approvedAt: null, approvedRevision: null, approvedBy: null }), + ...(typeChanged + ? { + githubOrgVerified: null, + githubVerificationNote: null, + githubVerifiedAt: null, + githubUrlVerified: null + } + : urlChanged + ? { githubUrlVerified: null } + : {}), + updatedAt: sql`CURRENT_TIMESTAMP` + }) + .where( + and( + eq(developers.id, developer.id), + eq(developers.ownerUserId, userId), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL + )` + ) + ); + mainStmt = toD1Statement(this.db.$client, updateStmt.toSQL()); + } + + // Batched via the raw D1 client ($client - see toD1Statement's + // comment): drizzle-orm 0.45.2's D1 batch() throws + // "Cannot read properties of undefined (reading 'bind')" for any + // db.run(sql\`...\`) item that has bound params (confirmed via an + // isolated repro against real D1 - its prepared-query wrapper for + // raw sql lacks the .stmt property batch() unconditionally reads). + // Gated on changes() = 1 (the immediately preceding batch statement) + // rather than a query-builder insert, since there's no FROM table to + // build this against - it's a conditional literal-values insert. + const historyStmt = toD1Statement(this.db.$client, { + sql: `INSERT INTO developer_history (id, developer_id, type, name, url, changed_by, changed_at) + SELECT ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP + WHERE changes() = 1`, + params: [ + crypto.randomUUID(), + developer.id, + developer.type, + developer.name, + developer.URL ?? null, + userId + ] + }); + + let results; + try { + results = await this.db.$client.batch([mainStmt, historyStmt]); + } catch (error) { + if (isDeveloperIdConflict(error)) { + return { + data: null, + error: { + message: "Developer id already exists", + code: "DEVELOPER_ID_TAKEN" + } + }; + } + if (isDeveloperOwnerConflict(error)) { + return { + data: null, + error: { + message: "You already have a developer profile", + code: "CONFLICT" + } + }; + } + return databaseError("upsertOwn", error); + } + + if (!results[0]?.meta?.changes) { + const [activeUser] = await this.db + .select({ id: users.id }) + .from(users) + .where(and(eq(users.id, userId), isNull(users.deletedAt))); + if (!activeUser) { + return { + data: null, + error: { + message: "Active account required", + code: "ACCOUNT_INACTIVE" + } + }; + } + return { + data: null, + error: { + message: "Developer ownership changed while updating the profile", + code: "CONFLICT" + } + }; + } + + const [current] = await this.db + .select() + .from(developers) + .where( + and( + eq(developers.id, developer.id), + eq(developers.ownerUserId, userId) + ) + ); + if (!current) { + return { + data: null, + error: { + message: "Developer ownership changed while updating the profile", + code: "CONFLICT" + } + }; + } + return { data: parseDeveloperRow(current), error: null }; + } catch (error) { + return databaseError("upsertOwn", error); + } + } + + // Diagnoses why the guarded delete in deleteOwn() below affected zero + // rows: distinguishes an inactive caller, no-longer-owned/nonexistent, + // and the two blocking conditions, without reopening the race the guard + // already closed. + private async deletionBlockedError( + developerId: string, + userId: string + ): Promise<{ + code: "NOT_FOUND" | "CONFLICT" | "ACCOUNT_INACTIVE"; + message: string; + }> { + const [developer] = await this.db + .select({ ownerUserId: developers.ownerUserId }) + .from(developers) + .where(eq(developers.id, developerId)); + + if (!developer || developer.ownerUserId !== userId) { + return { code: "NOT_FOUND", message: "Developer not found" }; + } + + const [user] = await this.db + .select({ deletedAt: users.deletedAt }) + .from(users) + .where(eq(users.id, userId)); + if (!user || user.deletedAt !== null) { + return { + code: "ACCOUNT_INACTIVE", + message: "Active account required" + }; + } + + const [extensionCount] = await this.db + .select({ count: sql`COUNT(*)` }) + .from(extensions) + .where(eq(extensions.authorId, developerId)); + const extensionsCount = extensionCount?.count ?? 0; + if (extensionsCount > 0) { + return { + code: "CONFLICT", + message: `You have ${extensionsCount} published extension(s) under this profile. Transfer ownership or remove them before deleting it.` + }; + } + + const [pendingCount] = await this.db + .select({ count: sql`COUNT(*)` }) + .from(extensionSubmissions) + .where( + and( + eq(extensionSubmissions.developerId, developerId), + eq(extensionSubmissions.status, "pending") + ) + ); + if ((pendingCount?.count ?? 0) > 0) { + return { + code: "CONFLICT", + message: + "You have a pending submission under review. Wait for it to be resolved before deleting your profile." + }; + } + + // The guard failed but a fresh look finds nothing wrong — whatever + // blocked it (someone else's transfer/claim landing, a submission + // that has since been resolved) has already cleared. Ask the caller + // to retry rather than guessing at a reason that's no longer true. + return { + code: "CONFLICT", + message: + "Your profile changed while processing this request. Please try again." + }; + } + + // Permanently removes the caller's own developer profile, for a + // privacy-focused account-deletion flow. Refuses while anything would be + // left dangling in a way that isn't just historical record-keeping: + // published extensions (someone still needs to own them) and pending + // submissions (nothing left to approve/reject against once the named + // developer is gone). developer_history is deliberately left alone — + // it's an append-only audit log, moderator-only, never rendered publicly, + // and 0009_drop_developer_history_fk.sql dropped its FK to developers(id) + // specifically so a deleted developer's history rows can outlive it. + async deleteOwn( + userId: string + ): Promise> { + try { + const [developer] = await this.db + .select({ id: developers.id }) + .from(developers) + .where(eq(developers.ownerUserId, userId)); + + if (!developer) { + return { + data: null, + error: { message: "Developer not found", code: "NOT_FOUND" } + }; + } + + // Every statement re-checks eligibility (still owned by this caller, + // no published extensions, no pending submission) at the moment it + // runs, rather than trusting the SELECT above: ownership can move + // (an accepted transfer/claim) and a new extension or pending + // submission can appear between that check and this write, and this + // delete is the caller's only authorization check. The same guard is + // repeated on all three statements — not just the last — so they're + // all-or-nothing: if it fails, nothing here is touched, instead of + // transfers/claims being deleted out from under a profile whose own + // deletion then gets blocked. Kept as raw sql via $client (see + // toD1Statement): the correlated EXISTS subqueries reference the + // outer statement's own table name, which the query builder can't + // express, and this batch needs the raw-D1 escape hatch regardless + // (see upsertOwn's historyStmt comment). + const deleteTransfersStmt = toD1Statement(this.db.$client, { + sql: `DELETE FROM developer_transfers + WHERE developer_id = ? + AND EXISTS ( + SELECT 1 FROM developers + WHERE developers.id = developer_transfers.developer_id + AND developers.owner_user_id = ? + AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = developers.id) + AND NOT EXISTS ( + SELECT 1 FROM extension_submissions + WHERE extension_submissions.developer_id = developers.id + AND extension_submissions.status = 'pending' + ) + AND EXISTS ( + SELECT 1 FROM users active_user + WHERE active_user.id = ? AND active_user.deleted_at IS NULL + ) + )`, + params: [developer.id, userId, userId] + }); + + const deleteClaimsStmt = toD1Statement(this.db.$client, { + sql: `DELETE FROM developer_claims + WHERE developer_id = ? + AND EXISTS ( + SELECT 1 FROM developers + WHERE developers.id = developer_claims.developer_id + AND developers.owner_user_id = ? + AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = developers.id) + AND NOT EXISTS ( + SELECT 1 FROM extension_submissions + WHERE extension_submissions.developer_id = developers.id + AND extension_submissions.status = 'pending' + ) + AND EXISTS ( + SELECT 1 FROM users active_user + WHERE active_user.id = ? AND active_user.deleted_at IS NULL + ) + )`, + params: [developer.id, userId, userId] + }); + + const deleteDeveloperStmt = toD1Statement(this.db.$client, { + sql: `DELETE FROM developers + WHERE id = ? + AND owner_user_id = ? + AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = developers.id) + AND NOT EXISTS ( + SELECT 1 FROM extension_submissions + WHERE extension_submissions.developer_id = developers.id + AND extension_submissions.status = 'pending' + ) + AND EXISTS ( + SELECT 1 FROM users active_user + WHERE active_user.id = ? AND active_user.deleted_at IS NULL + )`, + params: [developer.id, userId, userId] + }); + + let results; + try { + results = await this.db.$client.batch([ + deleteTransfersStmt, + deleteClaimsStmt, + deleteDeveloperStmt + ]); + } catch (error) { + return databaseError("deleteOwn", error); + } + + const [, , developerResult] = results; + if (!developerResult.meta?.changes) { + return { + data: null, + error: await this.deletionBlockedError(developer.id, userId) + }; + } + + return { data: { id: developer.id, deleted: true }, error: null }; + } catch (error) { + return databaseError("deleteOwn", error); + } + } + + async getById( + id: string + ): Promise> { + try { + const [row] = await this.db + .select() + .from(developers) + .where(eq(developers.id, id)); + if (!row) { + return { + data: null, + error: { + message: `Cannot find developer by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + return { + data: { + ...parseDeveloperRow(row), + unclaimed: row.ownerUserId === null + }, + error: null + }; + } catch (error) { + return databaseError("getById", error); + } + } + + async listAll(): Promise> { + let rows; + try { + rows = await this.db + .select({ + developer: developers, + ownerName: users.name, + ownerGithubLogin: users.githubLogin + }) + .from(developers) + .leftJoin(users, eq(users.id, developers.ownerUserId)) + .orderBy(asc(developers.name)); + } catch (error) { + return databaseError("listAll", error); + } + + return { data: rows.map(parseDeveloperRowWithOwner), error: null }; + } + + async listUnapproved(): Promise> { + let rows; + try { + rows = await this.db + .select({ + developer: developers, + ownerName: users.name, + ownerGithubLogin: users.githubLogin + }) + .from(developers) + .leftJoin(users, eq(users.id, developers.ownerUserId)) + .where(isNull(developers.approvedAt)) + .orderBy(asc(developers.createdAt)); + } catch (error) { + return databaseError("listUnapproved", error); + } + + return { data: rows.map(parseDeveloperRowWithOwner), error: null }; + } + + async approve( + id: string, + expectedRevision: number, + reviewerId: string + ): Promise> { + let result; + try { + result = await this.db + .update(developers) + .set({ + approvedAt: sql`CURRENT_TIMESTAMP`, + approvedRevision: sql`content_revision`, + approvedBy: reviewerId + }) + .where( + and( + eq(developers.id, id), + eq(developers.contentRevision, expectedRevision), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL + )` + ) + ); + } catch (error) { + return databaseError("approve", error); + } + + if (!result.meta?.changes) { + let reviewer: { deletedAt: string | null } | undefined; + try { + [reviewer] = await this.db + .select({ deletedAt: users.deletedAt }) + .from(users) + .where(eq(users.id, reviewerId)); + } catch (error) { + return databaseError("approve", error); + } + if (!reviewer || reviewer.deletedAt !== null) { + return { + data: null, + error: { + code: "ACCOUNT_INACTIVE", + message: "Active account required" + } + }; + } + + const existing = await this.getById(id); + if (existing.error) { + // A failed diagnostic lookup is not evidence that the developer is + // missing. Preserve database errors (and any other lookup error) so + // transient failures are not misreported as HTTP 404. + return { data: null, error: existing.error }; + } + + return { + data: null, + error: { + message: + "Developer profile changed after it was reviewed; reload it and approve the current revision", + code: "CONFLICT" + } + }; + } + + return { data: { id, approved: true }, error: null }; + } + + async listHistory( + developerId: string + ): Promise> { + let rows; + try { + rows = await this.db + .select({ + developerId: developerHistory.developerId, + type: developerHistory.type, + name: developerHistory.name, + url: developerHistory.url, + changedBy: developerHistory.changedBy, + changedByName: users.name, + changedAt: developerHistory.changedAt + }) + .from(developerHistory) + .leftJoin(users, eq(users.id, developerHistory.changedBy)) + .where(eq(developerHistory.developerId, developerId)) + // CURRENT_TIMESTAMP has only second resolution, so two writes in + // the same second tie on changed_at; rowid (insertion order, + // implicit - not a declared schema column) breaks the tie so + // "newest first" is never ambiguous. + .orderBy( + desc(developerHistory.changedAt), + sql`"developer_history".rowid DESC` + ); + } catch (error) { + return databaseError("listHistory", error); + } + + return { + data: rows.map((row) => ({ + developer_id: row.developerId, + type: row.type as DeveloperHistoryEntry["type"], + name: row.name, + URL: row.url ?? undefined, + changed_by: row.changedBy, + changed_by_name: row.changedByName, + changed_at: row.changedAt + })), + error: null + }; + } + + // Re-runs the same identity match verifyGithubOwnership() does for a + // brand-new claim/creation, but for a profile the caller already owns — + // no GitHub API call needed. checkGithubEntityType() (the GitHub call + // verifyGithubOwnership() makes) only exists to confirm a *new* id isn't + // squatting on a real GitHub org/user; that doesn't apply once ownership + // already exists, so this only re-derives the match from the caller's + // own already-synced github_login/github_orgs. Called opportunistically + // on every login for a developer-owning user, and by the owner's own + // "Re-verify" action — both share this one method. + // checkUrl/githubToken — only set by the owner's own manual "Re-verify" + // button, never by the opportunistic per-login call in extensions' + // auth/callback.ts. Re-checking Publisher URL against GitHub's on-file + // website needs a fresh GitHub API call (unlike the identity match below), + // so it stays opt-in to keep the automatic login path GitHub-API-free. + async reverifyOwn( + userId: string, + checkUrl?: boolean, + githubToken?: string + ): Promise> { + try { + const [row] = await this.db + .select({ + id: developers.id, + type: developers.type, + url: developers.url + }) + .from(developers) + .where(eq(developers.ownerUserId, userId)); + if (!row) { + return { + data: null, + error: { + message: "You don't own a developer profile", + code: "NOT_FOUND" + } + }; + } + + if (checkUrl) { + // Atomic conditional UPDATE, not a read-then-write — the WHERE + // clause only matches (and thus only "wins") when the cooldown is + // absent or already expired, so two concurrent check_url requests + // can't both pass. This is the only reason check_url spends a real + // GitHub API call, so it's the only path that needs this. + const cooldown = await this.db + .update(developers) + .set({ + urlCheckCooldownUntil: sql`datetime('now', ${`+${URL_CHECK_COOLDOWN_SECONDS} seconds`})` + }) + .where( + and( + eq(developers.id, row.id), + eq(developers.ownerUserId, userId), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL + )`, + or( + isNull(developers.urlCheckCooldownUntil), + sql`${developers.urlCheckCooldownUntil} < CURRENT_TIMESTAMP` + ) + ) + ); + if (!cooldown.meta?.changes) { + const [activeUser] = await this.db + .select({ id: users.id }) + .from(users) + .where(and(eq(users.id, userId), isNull(users.deletedAt))); + if (!activeUser) { + return { + data: null, + error: { + message: "Active account required", + code: "ACCOUNT_INACTIVE" + } + }; + } + return { + data: null, + error: { + message: + "Please wait a minute before re-checking your Publisher URL again.", + code: "RATE_LIMITED" + } + }; + } + } + + const identity = await new UsersDatabase(this.db).getGithubIdentity( + userId + ); + if (identity.error || !identity.data) { + return { + data: null, + error: identity.error ?? { + message: "Failed to load caller's GitHub identity", + code: "DATABASE_ERROR" + } + }; + } + + // No linked GitHub identity at all shouldn't be reachable in practice + // (GitHub is this system's sole login provider), but stays a no-op + // rather than writing a misleading "unverified" result over it. + if (!identity.data.githubLogin?.trim()) { + return this.getById(row.id); + } + + // An expired or malformed organization-membership snapshot is + // inconclusive, not proof that the owner left the organization. Keep + // the stored verification signal and timestamp until central auth + // supplies a fresh snapshot. The cooldown (when check_url was used) + // was already reserved above, so this remains bounded even on retries. + if (row.type === "organization" && !identity.data.githubOrgsAvailable) { + return this.getById(row.id); + } + + const matches = matchesClaimant( + row.type as Developer["type"], + row.id, + identity.data + ); + + // Only bothers with the extra GitHub API call when the identity match + // above still holds — a URL "verified" against an entity the caller no + // longer controls wouldn't mean anything. When identity no longer + // matches, any previously-set githubUrlVerified is cleared below + // (cheap — no API call needed, same as githubOrgVerified itself). + let githubUrlVerified: number | null = null; + let writeUrlVerified = false; + // Set when a fresh lookup (only possible when checkUrl actually ran) + // finds GitHub's *current* entity type no longer matches the + // profile's own type — matchesClaimant() above only compares + // login/org membership, it never confirms the entity is still the + // type the profile claims, unlike creation-time verification. This + // downgrades the identity signal too, not just the URL one, since the + // same discrepancy undermines both. + let identityTypeContradicted = false; + if (!matches) { + writeUrlVerified = true; + } else if (checkUrl) { + const entity = await checkGithubEntity(row.id, githubToken ?? ""); + // An unavailable lookup is explicitly inconclusive, not a disproof, + // so it leaves the stored URL verification signal untouched rather + // than clearing a real prior verification over a transient failure. + // A confirmed absence likewise provides no website to compare. Only + // a successful lookup gets to overwrite the stored URL signal. + if (entity.status === "unavailable") { + // Keep the cooldown reservation even though no signal was changed. + // Otherwise a caller could repeatedly hit GitHub while the shared + // service token is throttled or the upstream service is failing. + return { data: null, error: githubUnavailableError(entity.reason) }; + } + if (entity.status === "found") { + writeUrlVerified = true; + if (entity.entity.type !== row.type) { + identityTypeContradicted = true; + } else { + githubUrlVerified = urlMatchesGithubBlog( + row.url ?? undefined, + entity.entity.blog + ) + ? 1 + : null; + } + } + } + const verified = matches && !identityTypeContradicted; + + // Re-asserts ownership in the write itself (not just the lookup + // above) — otherwise a transfer/claim landing in between would let + // this write a result computed from the *former* owner's GitHub + // identity onto the profile after it's changed hands. Same guard as + // upsertOwn's update branch. Also re-asserts the URL is still the one + // just checked — otherwise a concurrent Publisher URL edit landing in + // between would let a stale URL comparison get written as if it + // described the new URL. Finally, the users predicates below re-check + // the exact GitHub identity snapshot used for this result, so a newer + // central-auth sync cannot be overwritten by this in-flight request. + const sameGithubIdentity = [ + identity.data.githubLogin === null + ? isNull(users.githubLogin) + : eq(users.githubLogin, identity.data.githubLogin), + identity.data.githubOrgsSnapshot === null + ? isNull(users.githubOrgs) + : eq(users.githubOrgs, identity.data.githubOrgsSnapshot), + identity.data.githubOrgsExpiresAt === null + ? isNull(users.githubOrgsExpiresAt) + : eq(users.githubOrgsExpiresAt, identity.data.githubOrgsExpiresAt) + ]; + const result = await this.db + .update(developers) + .set({ + githubOrgVerified: verified ? 1 : 0, + ...(writeUrlVerified ? { githubUrlVerified } : {}), + githubVerificationNote: verified + ? "Verified: caller's linked GitHub identity matches." + : identityTypeContradicted + ? "No longer verified: GitHub's on-file entity type no longer matches this profile." + : "No longer verified: caller's linked GitHub identity no longer matches.", + githubVerifiedAt: sql`CURRENT_TIMESTAMP` + }) + .where( + and( + eq(developers.id, row.id), + eq(developers.type, row.type), + eq(developers.ownerUserId, userId), + sql`EXISTS ( + SELECT 1 FROM ${users} + WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL + AND ${sameGithubIdentity[0]} + AND ${sameGithubIdentity[1]} + AND ${sameGithubIdentity[2]} + )`, + ...(writeUrlVerified + ? [ + row.url === null + ? isNull(developers.url) + : eq(developers.url, row.url) + ] + : []) + ) + ); + + if (!result.meta?.changes) { + return { + data: null, + error: { + message: + "Developer ownership or Publisher URL changed while re-verifying", + code: "CONFLICT" + } + }; + } + + return this.getById(row.id); + } catch (error) { + return databaseError("reverifyOwn", error); + } + } +} diff --git a/src/services/extensions/v2/developer-transfers-database.ts b/src/services/extensions/v2/developer-transfers-database.ts new file mode 100644 index 0000000..14b50e5 --- /dev/null +++ b/src/services/extensions/v2/developer-transfers-database.ts @@ -0,0 +1,414 @@ +import { and, eq, isNull, sql } from "drizzle-orm"; +import { DatabaseResult } from "../../../lib/interfaces"; +import { ExtensionsDb } from "../../../lib/db"; +import { developers, developerTransfers, users } from "./db/schema"; +import { + databaseError, + errorMessageChain, + isDeveloperOwnerConflict +} from "./errors"; +import { toD1Statement } from "./d1-batch"; +import { DeveloperProfile, DeveloperTransfer } from "./interfaces"; +import { DeveloperProfilesDatabase } from "./developer-profiles-database"; + +async function sha256Hex(input: string): Promise { + const digest = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(input) + ); + return [...new Uint8Array(digest)] + .map((b) => b.toString(16).padStart(2, "0")) + .join(""); +} + +function toSqliteDatetime(date: Date): string { + return date.toISOString().slice(0, 19).replace("T", " "); +} + +export class DeveloperTransfersDatabase { + constructor(private db: ExtensionsDb) {} + // Shared by initiateTransfer/revokeTransfer: both are owner-only actions on + // an existing developer, so both need the same NOT_FOUND/FORBIDDEN/ + // ACCOUNT_INACTIVE check. + private async checkOwnership( + developerId: string, + userId: string + ): Promise<{ + code: "NOT_FOUND" | "FORBIDDEN" | "ACCOUNT_INACTIVE"; + message: string; + } | null> { + const [owner] = await this.db + .select({ + ownerUserId: developers.ownerUserId, + ownerDeletedAt: users.deletedAt + }) + .from(developers) + .leftJoin(users, eq(users.id, developers.ownerUserId)) + .where(eq(developers.id, developerId)); + + if (!owner) { + return { code: "NOT_FOUND", message: "Developer not found" }; + } + if (owner.ownerUserId !== userId) { + return { code: "FORBIDDEN", message: "You don't own this profile" }; + } + if (owner.ownerDeletedAt !== null) { + return { code: "ACCOUNT_INACTIVE", message: "Active account required" }; + } + return null; + } + + async initiateTransfer( + developerId: string, + userId: string + ): Promise> { + try { + const token = + crypto.randomUUID().replace(/-/g, "") + + crypto.randomUUID().replace(/-/g, ""); + const tokenHash = await sha256Hex(token); + const expiresAt = toSqliteDatetime( + new Date(Date.now() + 24 * 60 * 60 * 1000) + ); + + // Both writes are conditioned on current ownership in the same + // statement, rather than a separate SELECT beforehand — a caller who + // loses ownership between an up-front check and the write could + // otherwise still slip the write through. Superseding any existing + // pending transfer (rather than stacking up) keeps + // idx_developer_transfers_pending satisfied without a separate cleanup + // pass. Kept as raw sql via $client (see toD1Statement): the EXISTS + // subqueries are correlated against the outer table's own name, and + // this batch needs the raw-D1 escape hatch regardless (see + // upsertOwn's historyStmt comment). + const revokeStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developer_transfers SET revoked_at = CURRENT_TIMESTAMP + WHERE developer_id = ? AND accepted_at IS NULL AND revoked_at IS NULL + AND EXISTS ( + SELECT 1 FROM developers + WHERE developers.id = developer_transfers.developer_id + AND developers.owner_user_id = ? + ) + AND EXISTS ( + SELECT 1 FROM users + WHERE users.id = ? AND users.deleted_at IS NULL + )`, + params: [developerId, userId, userId] + }); + const insertStmt = toD1Statement(this.db.$client, { + sql: `INSERT INTO developer_transfers (id, developer_id, token_hash, created_by, expires_at) + SELECT ?, ?, ?, ?, ? + WHERE EXISTS ( + SELECT 1 FROM developers WHERE id = ? AND owner_user_id = ? + ) + AND EXISTS ( + SELECT 1 FROM users + WHERE users.id = ? AND users.deleted_at IS NULL + )`, + params: [ + crypto.randomUUID(), + developerId, + tokenHash, + userId, + expiresAt, + developerId, + userId, + userId + ] + }); + + const results = await this.db.$client.batch([revokeStmt, insertStmt]); + + // The INSERT only writes a row when the ownership guard above passes, + // so zero rows written means the caller doesn't currently own this + // developer — a follow-up read distinguishes NOT_FOUND, ownership, and + // inactive-account errors without reopening the race the guard closes. + if (!results[1]?.meta?.changes) { + const ownershipError = await this.checkOwnership(developerId, userId); + return { + data: null, + error: ownershipError ?? { + code: "FORBIDDEN", + message: "You don't own this profile" + } + }; + } + + return { data: { token, expires_at: expiresAt }, error: null }; + } catch (error) { + return databaseError("initiateTransfer", error); + } + } + + async revokeTransfer( + developerId: string, + userId: string + ): Promise> { + try { + const result = await this.db.run(sql` + UPDATE ${developerTransfers} SET revoked_at = CURRENT_TIMESTAMP + WHERE developer_id = ${developerId} AND accepted_at IS NULL AND revoked_at IS NULL + AND EXISTS (SELECT 1 FROM ${developers} WHERE developers.id = developer_transfers.developer_id AND developers.owner_user_id = ${userId}) + AND EXISTS (SELECT 1 FROM ${users} WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL) + `); + + // Zero rows changed is ambiguous by itself (no pending transfer vs. + // not the owner vs. no such developer), since the ownership guard is + // folded into the write above rather than checked beforehand. A + // follow-up read-only check distinguishes them for the response + // without reopening the race that guard closes. + if (!result.meta?.changes) { + const ownershipError = await this.checkOwnership(developerId, userId); + if (ownershipError) { + return { data: null, error: ownershipError }; + } + } + + return { data: { id: developerId, revoked: true }, error: null }; + } catch (error) { + return databaseError("revokeTransfer", error); + } + } + + async acceptTransfer( + token: string, + userId: string + ): Promise> { + try { + const tokenHash = await sha256Hex(token); + + // Keep the developer id separate from the transfer row that the batch + // will claim. The accepting user can delete the newly transferred + // profile immediately after the batch commits; deleteOwn() removes the + // associated transfer row as part of that same operation. Looking up + // the transfer after the commit would then lose the id of the profile + // whose ownership was already moved and turn a successful handoff into + // a spurious DATABASE_ERROR. This read is only an identity snapshot — + // the claim and ownership guards below remain the authorization source + // of truth. + const [transferBeforeCommit] = await this.db + .select({ developerId: developerTransfers.developerId }) + .from(developerTransfers) + .where(eq(developerTransfers.tokenHash, tokenHash)); + const transferredDeveloperId = transferBeforeCommit?.developerId; + + // Claim the transfer and move ownership in the same atomic batch, + // rather than as two separate writes. Splitting them would leave a + // window, after the claim commits but before ownership actually + // moves, where the *former* owner's initiateTransfer call would still + // see itself as the current owner (per the developers row) and could + // mint a fresh, valid link for a profile that's already mid-handoff. + // It would also mean a failure on the ownership write alone (e.g. the + // recipient racing to create another profile) permanently burns the + // token without ever transferring ownership, with no way to retry. + // Batching both as one D1 transaction makes them succeed or fail as a + // unit. The `changes() = 1` guard on the second statement is load- + // bearing, not redundant with the subquery: accepted_by/accepted_at + // are a permanent historical record once a token is claimed, so the + // subquery alone would match a *previously* accepted token forever, + // letting a replay of an old, already-used link silently reassign + // ownership again (even to a profile since handed off to someone + // else) despite the claim itself changing zero rows. + // `changes()` reports the row count from the immediately preceding + // statement on this same connection, so it's only 1 when *this* + // batch's claim just fired — proving the update below is reacting to + // a fresh claim, not replaying an old one. + // + // The claim's NOT EXISTS guard folds the self-accept case (accepting + // user already owns *this* developer) and the already-owns-a- + // different-profile case into the same atomic decision, so the token + // is never consumed unless the accepting user is actually eligible. A + // plain check-then-act (SELECT the row, decide, then write) would let + // two concurrent accepts both read it as valid before either one + // wrote to it, making the token usable more than once. + // Kept as raw sql via $client (see toD1Statement): correlated + // subqueries plus the changes()=1 gates need the raw-D1 escape hatch + // (see upsertOwn's historyStmt comment). + const claimStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developer_transfers SET accepted_at = CURRENT_TIMESTAMP, accepted_by = ? + WHERE token_hash = ? AND accepted_at IS NULL AND revoked_at IS NULL AND expires_at > CURRENT_TIMESTAMP + AND NOT EXISTS (SELECT 1 FROM developers WHERE owner_user_id = ?) + AND EXISTS (SELECT 1 FROM users WHERE id = ? AND deleted_at IS NULL)`, + params: [userId, tokenHash, userId, userId] + }); + const updateDeveloperStmt = toD1Statement(this.db.$client, { + // url_check_cooldown_until is reset here too — it's keyed by + // whichever user currently owns this row, so left unchanged it + // would rate-limit the *new* owner's first check_url reverify for + // whatever's left of the *previous* owner's cooldown window. + // + // github_org_verified/github_url_verified/github_verification_note/ + // github_verified_at are cleared for the same underlying reason: + // they describe whether the *previous* owner's linked GitHub + // identity matched this profile — a fact that says nothing about + // the new owner, who was never checked. Unlike approveClaim() (the + // other ownership-transfer path), there's no claim-time + // verification to carry over here — a transfer is a bare handoff, + // not a claim — so this can only ever fall back to null/unverified, + // same as a brand-new profile with no GitHub identity yet. + sql: `UPDATE developers + SET owner_user_id = ?, + ownership_epoch = ownership_epoch + 1, + content_revision = content_revision + 1, + approved_at = NULL, approved_revision = NULL, approved_by = NULL, + url_check_cooldown_until = NULL, + github_org_verified = NULL, github_url_verified = NULL, + github_verification_note = NULL, github_verified_at = NULL, + updated_at = CURRENT_TIMESTAMP + WHERE changes() = 1 + AND id = ( + SELECT developer_id FROM developer_transfers + WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL + )`, + params: [userId, tokenHash, userId] + }); + + // Re-assert that the ownership update was caused by this batch's fresh + // claim before cleaning up the two kinds of pending work attached to + // the developer. If claimStmt or updateDeveloperStmt matched zero + // rows, this deliberately violates the ownership_epoch CHECK and D1 + // rolls the whole batch back. The assertion lets both cleanup UPDATEs + // run without their own changes() gates, so a zero-row cleanup of one + // table cannot suppress cleanup of the other table. + const assertTransferStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developers + SET ownership_epoch = CASE WHEN changes() = 1 THEN ownership_epoch ELSE 0 END + WHERE id = ( + SELECT developer_id FROM developer_transfers + WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL + )`, + params: [tokenHash, userId] + }); + + const rejectPendingSubmissionsStmt = toD1Statement(this.db.$client, { + sql: `UPDATE extension_submissions + SET status = 'rejected', + review_note = 'Ownership changed before review', + reviewed_at = CURRENT_TIMESTAMP + WHERE developer_id = ( + SELECT developer_id FROM developer_transfers + WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL + ) + AND status = 'pending'`, + params: [tokenHash, userId] + }); + + const rejectPendingClaimsStmt = toD1Statement(this.db.$client, { + sql: `UPDATE developer_claims + SET status = 'rejected', + review_note = 'Ownership changed before review', + reviewed_at = CURRENT_TIMESTAMP + WHERE developer_id = ( + SELECT developer_id FROM developer_transfers + WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL + ) + AND status = 'pending'`, + params: [tokenHash, userId] + }); + + let results; + try { + results = await this.db.$client.batch([ + claimStmt, + updateDeveloperStmt, + assertTransferStmt, + rejectPendingSubmissionsStmt, + rejectPendingClaimsStmt + ]); + } catch (error) { + // A replayed token reaches the assertion with changes() = 0. The + // deliberate CHECK failure rolls back the batch, and is handled like + // any other unsuccessful claim below so callers still receive the + // documented invalid/used/expired-link response. + if ( + /CHECK constraint failed.*ownership_epoch/i.test( + errorMessageChain(error) + ) + ) { + results = [{ meta: { changes: 0 } }]; + } else { + if (isDeveloperOwnerConflict(error)) { + return { + data: null, + error: { + code: "CONFLICT", + message: + "You already have a developer profile — remove or transfer it before accepting a new one" + } + }; + } + return databaseError("acceptTransfer", error); + } + } + + const [claim] = results; + if (!claim.meta?.changes) { + // The claim can fail either because the token itself is bad (used, + // revoked, expired, unknown) or because it's still valid but the + // ownership guard rejected it — check which, for an accurate error. + const [recipient] = await this.db + .select({ deletedAt: users.deletedAt }) + .from(users) + .where(eq(users.id, userId)); + + // The active-account middleware runs before this transaction, so a + // recipient can be deactivated in the small window before the + // guarded claim. Preserve the documented inactive-account response + // rather than misclassifying a still-pending token as an ownership + // conflict. + if (!recipient || recipient.deletedAt !== null) { + return { + data: null, + error: { + code: "ACCOUNT_INACTIVE", + message: "Active account required" + } + }; + } + + const [stillPending] = await this.db + .select({ one: sql`1` }) + .from(developerTransfers) + .where( + and( + eq(developerTransfers.tokenHash, tokenHash), + isNull(developerTransfers.acceptedAt), + isNull(developerTransfers.revokedAt), + sql`${developerTransfers.expiresAt} > CURRENT_TIMESTAMP` + ) + ); + + if (stillPending) { + return { + data: null, + error: { + code: "CONFLICT", + message: + "You already have a developer profile — remove or transfer it before accepting a new one" + } + }; + } + return { + data: null, + error: { + code: "NOT_FOUND", + message: "This transfer link is invalid, used, or expired" + } + }; + } + + if (!transferredDeveloperId) { + return databaseError( + "acceptTransfer", + new Error("Claimed transfer row not found") + ); + } + + return new DeveloperProfilesDatabase(this.db).getById( + transferredDeveloperId + ); + } catch (error) { + return databaseError("acceptTransfer", error); + } + } +} diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts deleted file mode 100644 index e5fe8f4..0000000 --- a/src/services/extensions/v2/developers-database.ts +++ /dev/null @@ -1,1883 +0,0 @@ -import { and, asc, desc, eq, isNull, or, sql } from "drizzle-orm"; -import { DatabaseError, DatabaseResult } from "../../../lib/interfaces"; -import { ExtensionsDb } from "../../../lib/db"; -import { - developers, - developerHistory, - developerTransfers, - developerClaims, - extensions, - extensionSubmissions, - users -} from "./db/schema"; -import { databaseError, errorMessageChain } from "./errors"; -import { toD1Statement } from "./d1-batch"; -import { - checkGithubEntity, - GithubUnavailableReason, - matchesClaimant, - urlMatchesGithubBlog -} from "./github-verification"; -import { - Developer, - DeveloperClaim, - DeveloperHistoryEntry, - DeveloperProfile, - DeveloperTransfer, - PendingDeveloperClaim -} from "./interfaces"; -import { UsersDatabase } from "./users-database"; - -// How often reverifyOwn's check_url path is allowed to spend a real GitHub -// API call per caller — that call uses the shared service-level -// GITHUB_TOKEN (see verifyGithubOwnership's comment), so an unbounded -// number of clicks from one caller could crowd out everyone else's -// GitHub-dependent requests. See reverifyOwn's own comment for how this is -// enforced atomically. -const URL_CHECK_COOLDOWN_SECONDS = 60; - -// Matches the SQLite/D1 message for the idx_developers_owner_unique -// violation, which is how a lost race between two concurrent first-time PUT -// /developers/me requests (same caller, different ids) surfaces. -function isOwnerConflict(error: unknown): boolean { - return /UNIQUE constraint failed.*owner_user_id/i.test( - errorMessageChain(error) - ); -} - -// Matches the SQLite/D1 message for the idx_developer_claims_pending_unique -// violation, which is how a duplicate claim() call while one is already -// pending surfaces. -function isPendingClaimConflict(error: unknown): boolean { - return /UNIQUE constraint failed.*developer_claims/i.test( - errorMessageChain(error) - ); -} - -function githubUnavailableError( - reason: GithubUnavailableReason -): DatabaseError { - if (reason === "unsupported_entity_type") { - return { - code: "GITHUB_ENTITY_UNSUPPORTED", - message: "This GitHub account type is not supported" - }; - } - return reason === "rate_limited" - ? { - code: "RATE_LIMITED", - message: "GitHub verification is temporarily rate limited" - } - : { - code: "SERVICE_UNAVAILABLE", - message: "GitHub verification is temporarily unavailable" - }; -} - -async function sha256Hex(input: string): Promise { - const digest = await crypto.subtle.digest( - "SHA-256", - new TextEncoder().encode(input) - ); - return [...new Uint8Array(digest)] - .map((b) => b.toString(16).padStart(2, "0")) - .join(""); -} - -// SQLite's CURRENT_TIMESTAMP renders as "YYYY-MM-DD HH:MM:SS" (space -// separator, no milliseconds, no "Z"). expires_at is compared against it -// directly in SQL (`expires_at > CURRENT_TIMESTAMP`), which is a plain -// string comparison — a JS `Date#toISOString()` value ("...THH:MM:SS.sssZ") -// would sort wrong once the two share the same calendar day, since 'T' -// (0x54) always outranks ' ' (0x20) at that position regardless of the -// actual time that follows. Matching the format keeps the comparison correct. -function toSqliteDatetime(date: Date): string { - return date.toISOString().slice(0, 19).replace("T", " "); -} - -type DeveloperRow = typeof developers.$inferSelect; -type ClaimRow = typeof developerClaims.$inferSelect; - -function parseDeveloperRow(row: DeveloperRow): DeveloperProfile { - return { - id: row.id, - type: row.type as DeveloperProfile["type"], - name: row.name, - URL: row.url ?? undefined, - avatar_url: row.avatarUrl ?? undefined, - contact_email: row.contactEmail ?? undefined, - approved: - row.approvedAt !== null && - row.approvedAt !== undefined && - (row.approvedRevision == null || - Number(row.approvedRevision) === Number(row.contentRevision ?? 1)), - content_revision: Number(row.contentRevision ?? 1), - github_org_verified: - row.githubOrgVerified === null || row.githubOrgVerified === undefined - ? undefined - : row.githubOrgVerified === 1, - github_verification_note: row.githubVerificationNote ?? undefined, - github_verified_at: row.githubVerifiedAt ?? undefined, - github_url_verified: row.githubUrlVerified === 1 ? true : undefined - }; -} - -// Used by listAll/listUnapproved, whose queries left-join users on -// developers.owner_user_id to save the moderator a lookup per row (see -// PendingDeveloperClaim's claimant_name/claimant_github_login for the same -// pattern on the claims queue). -function parseDeveloperRowWithOwner(row: { - developer: DeveloperRow; - ownerName: string | null; - ownerGithubLogin: string | null; -}): DeveloperProfile { - return { - ...parseDeveloperRow(row.developer), - unclaimed: row.developer.ownerUserId === null, - owner_name: row.ownerName, - owner_github_login: row.ownerGithubLogin - }; -} - -function parseClaimRow(row: ClaimRow): DeveloperClaim { - return { - id: row.id, - developer_id: row.developerId, - claimant_id: row.claimantId, - status: row.status as DeveloperClaim["status"], - note: row.note ?? undefined, - review_note: row.reviewNote ?? undefined, - reviewer_id: row.reviewerId ?? undefined, - created_at: row.createdAt, - reviewed_at: row.reviewedAt ?? undefined, - github_org_verified: - row.githubOrgVerified === null || row.githubOrgVerified === undefined - ? undefined - : row.githubOrgVerified === 1, - github_verification_note: row.githubVerificationNote ?? undefined - }; -} - -export class DevelopersDatabase { - constructor(private db: ExtensionsDb) {} - - async getOwn( - userId: string - ): Promise< - | DatabaseResult - | { data: null; error: null } - > { - try { - const [row] = await this.db - .select() - .from(developers) - .where(eq(developers.ownerUserId, userId)); - if (!row) return { data: null, error: null }; - - const [pending] = await this.db - .select({ id: developerTransfers.id }) - .from(developerTransfers) - .where( - and( - eq(developerTransfers.developerId, row.id), - isNull(developerTransfers.acceptedAt), - isNull(developerTransfers.revokedAt), - sql`${developerTransfers.expiresAt} > CURRENT_TIMESTAMP` - ) - ) - .limit(1); - - return { - data: { - ...parseDeveloperRow(row), - unclaimed: false, - has_pending_transfer: pending !== undefined - }, - error: null - }; - } catch (error) { - return databaseError("getOwn", error); - } - } - - // githubToken — see the comment on verifyGithubOwnership(). Only consulted - // when creating a brand-new profile (developer.id is immutable once - // owned, so an update can't need re-verifying); guards against squatting - // on an id that matches a real GitHub org/user the caller doesn't control, - // the one gap claim() alone can't close since it only ever applies to - // rows that already exist unowned. - async upsertOwn( - userId: string, - developer: Developer, - githubToken?: string, - allowCreationAttempt: () => Promise = async () => true - ): Promise> { - try { - const [existingOwn] = await this.db - .select() - .from(developers) - .where(eq(developers.ownerUserId, userId)); - - const [existingById] = await this.db - .select() - .from(developers) - .where(eq(developers.id, developer.id)); - - let githubOrgVerified: number | null = null; - let githubUrlVerified: number | null = null; - let githubVerificationNote: string | null = null; - - let mainStmt: D1PreparedStatement; - if (!existingOwn) { - if (existingById) { - // Distinct from the generic CONFLICT used elsewhere in this file — - // consumers (the extensions repo's create-profile form) need to - // reliably detect this specific case to point the user at the - // claim flow, which a shared, message-string-matched code can't do. - return { - data: null, - error: { - message: "Developer id already exists", - code: "DEVELOPER_ID_TAKEN" - } - }; - } - - // This hook sits after both cheap D1 existence checks and directly - // before the creation-only GitHub lookup. The Worker supplies the - // configured account limiter; keeping it as a callback leaves this - // database/service module runtime-agnostic and ensures updates and - // already-taken ids never spend creation allowance. - if (!(await allowCreationAttempt())) { - return { - data: null, - error: { - message: - "Too many new profile creation attempts; try again in 60 seconds", - code: "PROFILE_CREATION_RATE_LIMITED" - } - }; - } - - const check = await this.verifyGithubOwnership( - developer.id, - developer.type, - userId, - githubToken, - developer.URL - ); - - if ("error" in check) { - return { data: null, error: check.error }; - } - - if (check.mismatch) { - return { - data: null, - error: { - code: "GITHUB_MISMATCH", - message: - "This id matches a real GitHub organization or username that isn't linked to your account, so it can't be used automatically. Make sure you're signed in with the right GitHub account, or choose a different id." - } - }; - } - - githubOrgVerified = check.githubOrgVerified; - githubUrlVerified = check.githubUrlVerified; - githubVerificationNote = check.note; - - // INSERT ... SELECT makes the active-account check part of the - // mutation itself. The middleware check is only an early rejection; - // a deletion can win between that check and this statement. - mainStmt = toD1Statement(this.db.$client, { - sql: `INSERT INTO developers ( - id, type, name, url, avatar_url, contact_email, - owner_user_id, approved_at, created_at, updated_at, - github_org_verified, - github_verification_note, github_verified_at, - github_url_verified - ) - SELECT ?, ?, ?, ?, ?, ?, ?, NULL, CURRENT_TIMESTAMP, - CURRENT_TIMESTAMP, ?, ?, - CASE WHEN ? IS NULL THEN NULL ELSE CURRENT_TIMESTAMP END, - ? - WHERE EXISTS ( - SELECT 1 FROM users - WHERE id = ? AND deleted_at IS NULL - )`, - params: [ - developer.id, - developer.type, - developer.name, - developer.URL ?? null, - developer.avatar_url ?? null, - developer.contact_email ?? null, - userId, - githubOrgVerified, - githubVerificationNote, - githubOrgVerified, - githubUrlVerified, - userId - ] - }); - } else { - if (developer.id !== existingOwn.id) { - return { - data: null, - error: { - message: "Developer id cannot be changed", - code: "CONFLICT" - } - }; - } - - // approved_at is normally cleared here, even if nothing meaningful - // changed — the reviewed content just got overwritten, so the old - // approval no longer applies. Not worth diffing old vs. new field - // values for that. The one exception: a profile that's currently - // GitHub org/user verified keeps its approval across edits — that - // verification is an independently-computed identity signal (this - // write never touches githubOrgVerified, except when the id's type - // changes below) strong enough on its own that re-queuing for - // manual review on every edit isn't worth the moderator load. - // approvedRevision is bumped in lockstep with contentRevision in - // that branch so the existing approval keeps matching (see - // parseDeveloperRow) instead of silently going stale. - // - // A type change invalidates the existing GitHub verification - // outright — matchesClaimant() compares differently per type (org - // membership vs. username), so a signal computed for the old type - // says nothing about the new one. Falls back to approval clearing - // and manual review, same as any other unverified edit. - const typeChanged = developer.type !== existingOwn.type; - // A URL change invalidates only the URL signal, not identity — - // github_url_verified describes whether *this* URL matches GitHub's - // on-file website, so a stale URL can't still be "verified" once - // it's no longer the URL being served. - const urlChanged = (developer.URL ?? null) !== existingOwn.url; - const keepsApproval = - !typeChanged && existingOwn.githubOrgVerified === 1; - - const updateStmt = this.db - .update(developers) - .set({ - type: developer.type, - name: developer.name, - url: developer.URL ?? null, - avatarUrl: developer.avatar_url ?? null, - contactEmail: developer.contact_email ?? null, - contentRevision: sql`content_revision + 1`, - ...(keepsApproval - ? { approvedRevision: sql`content_revision + 1` } - : { approvedAt: null, approvedRevision: null, approvedBy: null }), - ...(typeChanged - ? { - githubOrgVerified: null, - githubVerificationNote: null, - githubVerifiedAt: null, - githubUrlVerified: null - } - : urlChanged - ? { githubUrlVerified: null } - : {}), - updatedAt: sql`CURRENT_TIMESTAMP` - }) - .where( - and( - eq(developers.id, developer.id), - eq(developers.ownerUserId, userId), - sql`EXISTS ( - SELECT 1 FROM ${users} - WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL - )` - ) - ); - mainStmt = toD1Statement(this.db.$client, updateStmt.toSQL()); - } - - // Batched via the raw D1 client ($client - see toD1Statement's - // comment): drizzle-orm 0.45.2's D1 batch() throws - // "Cannot read properties of undefined (reading 'bind')" for any - // db.run(sql\`...\`) item that has bound params (confirmed via an - // isolated repro against real D1 - its prepared-query wrapper for - // raw sql lacks the .stmt property batch() unconditionally reads). - // Gated on changes() = 1 (the immediately preceding batch statement) - // rather than a query-builder insert, since there's no FROM table to - // build this against - it's a conditional literal-values insert. - const historyStmt = toD1Statement(this.db.$client, { - sql: `INSERT INTO developer_history (id, developer_id, type, name, url, changed_by, changed_at) - SELECT ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP - WHERE changes() = 1`, - params: [ - crypto.randomUUID(), - developer.id, - developer.type, - developer.name, - developer.URL ?? null, - userId - ] - }); - - let results; - try { - results = await this.db.$client.batch([mainStmt, historyStmt]); - } catch (error) { - if (isOwnerConflict(error)) { - return { - data: null, - error: { - message: "You already have a developer profile", - code: "CONFLICT" - } - }; - } - return databaseError("upsertOwn", error); - } - - if (!results[0]?.meta?.changes) { - return { - data: null, - error: { - message: "Developer ownership changed while updating the profile", - code: "CONFLICT" - } - }; - } - - const [current] = await this.db - .select() - .from(developers) - .where( - and( - eq(developers.id, developer.id), - eq(developers.ownerUserId, userId) - ) - ); - if (!current) { - return { - data: null, - error: { - message: "Developer ownership changed while updating the profile", - code: "CONFLICT" - } - }; - } - return { data: parseDeveloperRow(current), error: null }; - } catch (error) { - return databaseError("upsertOwn", error); - } - } - - // Diagnoses why the guarded delete in deleteOwn() below affected zero - // rows: distinguishes no-longer-owned/nonexistent from the two blocking - // conditions, without reopening the race the guard already closed. - private async deletionBlockedError( - developerId: string, - userId: string - ): Promise<{ code: "NOT_FOUND" | "CONFLICT"; message: string }> { - const [developer] = await this.db - .select({ ownerUserId: developers.ownerUserId }) - .from(developers) - .where(eq(developers.id, developerId)); - - if (!developer || developer.ownerUserId !== userId) { - return { code: "NOT_FOUND", message: "Developer not found" }; - } - - const [extensionCount] = await this.db - .select({ count: sql`COUNT(*)` }) - .from(extensions) - .where(eq(extensions.authorId, developerId)); - const extensionsCount = extensionCount?.count ?? 0; - if (extensionsCount > 0) { - return { - code: "CONFLICT", - message: `You have ${extensionsCount} published extension(s) under this profile. Transfer ownership or remove them before deleting it.` - }; - } - - const [pendingCount] = await this.db - .select({ count: sql`COUNT(*)` }) - .from(extensionSubmissions) - .where( - and( - eq(extensionSubmissions.developerId, developerId), - eq(extensionSubmissions.status, "pending") - ) - ); - if ((pendingCount?.count ?? 0) > 0) { - return { - code: "CONFLICT", - message: - "You have a pending submission under review. Wait for it to be resolved before deleting your profile." - }; - } - - // The guard failed but a fresh look finds nothing wrong — whatever - // blocked it (someone else's transfer/claim landing, a submission - // that has since been resolved) has already cleared. Ask the caller - // to retry rather than guessing at a reason that's no longer true. - return { - code: "CONFLICT", - message: - "Your profile changed while processing this request. Please try again." - }; - } - - // Permanently removes the caller's own developer profile, for a - // privacy-focused account-deletion flow. Refuses while anything would be - // left dangling in a way that isn't just historical record-keeping: - // published extensions (someone still needs to own them) and pending - // submissions (nothing left to approve/reject against once the named - // developer is gone). developer_history is deliberately left alone — - // it's an append-only audit log, moderator-only, never rendered publicly, - // and 0009_drop_developer_history_fk.sql dropped its FK to developers(id) - // specifically so a deleted developer's history rows can outlive it. - async deleteOwn( - userId: string - ): Promise> { - try { - const [developer] = await this.db - .select({ id: developers.id }) - .from(developers) - .where(eq(developers.ownerUserId, userId)); - - if (!developer) { - return { - data: null, - error: { message: "Developer not found", code: "NOT_FOUND" } - }; - } - - // Every statement re-checks eligibility (still owned by this caller, - // no published extensions, no pending submission) at the moment it - // runs, rather than trusting the SELECT above: ownership can move - // (an accepted transfer/claim) and a new extension or pending - // submission can appear between that check and this write, and this - // delete is the caller's only authorization check. The same guard is - // repeated on all three statements — not just the last — so they're - // all-or-nothing: if it fails, nothing here is touched, instead of - // transfers/claims being deleted out from under a profile whose own - // deletion then gets blocked. Kept as raw sql via $client (see - // toD1Statement): the correlated EXISTS subqueries reference the - // outer statement's own table name, which the query builder can't - // express, and this batch needs the raw-D1 escape hatch regardless - // (see upsertOwn's historyStmt comment). - const deleteTransfersStmt = toD1Statement(this.db.$client, { - sql: `DELETE FROM developer_transfers - WHERE developer_id = ? - AND EXISTS ( - SELECT 1 FROM developers - WHERE developers.id = developer_transfers.developer_id - AND developers.owner_user_id = ? - AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = developers.id) - AND NOT EXISTS ( - SELECT 1 FROM extension_submissions - WHERE extension_submissions.developer_id = developers.id - AND extension_submissions.status = 'pending' - ) - AND EXISTS ( - SELECT 1 FROM users active_user - WHERE active_user.id = ? AND active_user.deleted_at IS NULL - ) - )`, - params: [developer.id, userId, userId] - }); - - const deleteClaimsStmt = toD1Statement(this.db.$client, { - sql: `DELETE FROM developer_claims - WHERE developer_id = ? - AND EXISTS ( - SELECT 1 FROM developers - WHERE developers.id = developer_claims.developer_id - AND developers.owner_user_id = ? - AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = developers.id) - AND NOT EXISTS ( - SELECT 1 FROM extension_submissions - WHERE extension_submissions.developer_id = developers.id - AND extension_submissions.status = 'pending' - ) - AND EXISTS ( - SELECT 1 FROM users active_user - WHERE active_user.id = ? AND active_user.deleted_at IS NULL - ) - )`, - params: [developer.id, userId, userId] - }); - - const deleteDeveloperStmt = toD1Statement(this.db.$client, { - sql: `DELETE FROM developers - WHERE id = ? - AND owner_user_id = ? - AND NOT EXISTS (SELECT 1 FROM extensions WHERE extensions.author_id = developers.id) - AND NOT EXISTS ( - SELECT 1 FROM extension_submissions - WHERE extension_submissions.developer_id = developers.id - AND extension_submissions.status = 'pending' - ) - AND EXISTS ( - SELECT 1 FROM users active_user - WHERE active_user.id = ? AND active_user.deleted_at IS NULL - )`, - params: [developer.id, userId, userId] - }); - - let results; - try { - results = await this.db.$client.batch([ - deleteTransfersStmt, - deleteClaimsStmt, - deleteDeveloperStmt - ]); - } catch (error) { - return databaseError("deleteOwn", error); - } - - const [, , developerResult] = results; - if (!developerResult.meta?.changes) { - return { - data: null, - error: await this.deletionBlockedError(developer.id, userId) - }; - } - - return { data: { id: developer.id, deleted: true }, error: null }; - } catch (error) { - return databaseError("deleteOwn", error); - } - } - - async getById( - id: string - ): Promise> { - try { - const [row] = await this.db - .select() - .from(developers) - .where(eq(developers.id, id)); - if (!row) { - return { - data: null, - error: { - message: `Cannot find developer by id: ${id}`, - code: "NOT_FOUND" - } - }; - } - return { - data: { - ...parseDeveloperRow(row), - unclaimed: row.ownerUserId === null - }, - error: null - }; - } catch (error) { - return databaseError("getById", error); - } - } - - async listAll(): Promise> { - let rows; - try { - rows = await this.db - .select({ - developer: developers, - ownerName: users.name, - ownerGithubLogin: users.githubLogin - }) - .from(developers) - .leftJoin(users, eq(users.id, developers.ownerUserId)) - .orderBy(asc(developers.name)); - } catch (error) { - return databaseError("listAll", error); - } - - return { data: rows.map(parseDeveloperRowWithOwner), error: null }; - } - - async listUnapproved(): Promise> { - let rows; - try { - rows = await this.db - .select({ - developer: developers, - ownerName: users.name, - ownerGithubLogin: users.githubLogin - }) - .from(developers) - .leftJoin(users, eq(users.id, developers.ownerUserId)) - .where(isNull(developers.approvedAt)) - .orderBy(asc(developers.createdAt)); - } catch (error) { - return databaseError("listUnapproved", error); - } - - return { data: rows.map(parseDeveloperRowWithOwner), error: null }; - } - - async approve( - id: string, - expectedRevision: number, - reviewerId: string - ): Promise> { - let result; - try { - result = await this.db - .update(developers) - .set({ - approvedAt: sql`CURRENT_TIMESTAMP`, - approvedRevision: sql`content_revision`, - approvedBy: reviewerId - }) - .where( - and( - eq(developers.id, id), - eq(developers.contentRevision, expectedRevision), - sql`EXISTS ( - SELECT 1 FROM ${users} - WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL - )` - ) - ); - } catch (error) { - return databaseError("approve", error); - } - - if (!result.meta?.changes) { - const existing = await this.getById(id); - return existing.error - ? { - data: null, - error: { - message: `Cannot find developer by id: ${id}`, - code: "NOT_FOUND" - } - } - : { - data: null, - error: { - message: - "Developer profile changed after it was reviewed; reload it and approve the current revision", - code: "CONFLICT" - } - }; - } - - return { data: { id, approved: true }, error: null }; - } - - async listHistory( - developerId: string - ): Promise> { - let rows; - try { - rows = await this.db - .select({ - developerId: developerHistory.developerId, - type: developerHistory.type, - name: developerHistory.name, - url: developerHistory.url, - changedBy: developerHistory.changedBy, - changedByName: users.name, - changedAt: developerHistory.changedAt - }) - .from(developerHistory) - .leftJoin(users, eq(users.id, developerHistory.changedBy)) - .where(eq(developerHistory.developerId, developerId)) - // CURRENT_TIMESTAMP has only second resolution, so two writes in - // the same second tie on changed_at; rowid (insertion order, - // implicit - not a declared schema column) breaks the tie so - // "newest first" is never ambiguous. - .orderBy( - desc(developerHistory.changedAt), - sql`"developer_history".rowid DESC` - ); - } catch (error) { - return databaseError("listHistory", error); - } - - return { - data: rows.map((row) => ({ - developer_id: row.developerId, - type: row.type as DeveloperHistoryEntry["type"], - name: row.name, - URL: row.url ?? undefined, - changed_by: row.changedBy, - changed_by_name: row.changedByName, - changed_at: row.changedAt - })), - error: null - }; - } - - // Shared by initiateTransfer/revokeTransfer: both are owner-only actions on - // an existing developer, so both need the same NOT_FOUND/FORBIDDEN check. - private async checkOwnership( - developerId: string, - userId: string - ): Promise<{ code: "NOT_FOUND" | "FORBIDDEN"; message: string } | null> { - const [owner] = await this.db - .select({ ownerUserId: developers.ownerUserId }) - .from(developers) - .where(eq(developers.id, developerId)); - - if (!owner) { - return { code: "NOT_FOUND", message: "Developer not found" }; - } - if (owner.ownerUserId !== userId) { - return { code: "FORBIDDEN", message: "You don't own this profile" }; - } - return null; - } - - async initiateTransfer( - developerId: string, - userId: string - ): Promise> { - try { - const token = - crypto.randomUUID().replace(/-/g, "") + - crypto.randomUUID().replace(/-/g, ""); - const tokenHash = await sha256Hex(token); - const expiresAt = toSqliteDatetime( - new Date(Date.now() + 24 * 60 * 60 * 1000) - ); - - // Both writes are conditioned on current ownership in the same - // statement, rather than a separate SELECT beforehand — a caller who - // loses ownership between an up-front check and the write could - // otherwise still slip the write through. Superseding any existing - // pending transfer (rather than stacking up) keeps - // idx_developer_transfers_pending satisfied without a separate cleanup - // pass. Kept as raw sql via $client (see toD1Statement): the EXISTS - // subqueries are correlated against the outer table's own name, and - // this batch needs the raw-D1 escape hatch regardless (see - // upsertOwn's historyStmt comment). - const revokeStmt = toD1Statement(this.db.$client, { - sql: `UPDATE developer_transfers SET revoked_at = CURRENT_TIMESTAMP - WHERE developer_id = ? AND accepted_at IS NULL AND revoked_at IS NULL - AND EXISTS ( - SELECT 1 FROM developers - WHERE developers.id = developer_transfers.developer_id - AND developers.owner_user_id = ? - ) - AND EXISTS ( - SELECT 1 FROM users - WHERE users.id = ? AND users.deleted_at IS NULL - )`, - params: [developerId, userId, userId] - }); - const insertStmt = toD1Statement(this.db.$client, { - sql: `INSERT INTO developer_transfers (id, developer_id, token_hash, created_by, expires_at) - SELECT ?, ?, ?, ?, ? - WHERE EXISTS ( - SELECT 1 FROM developers WHERE id = ? AND owner_user_id = ? - ) - AND EXISTS ( - SELECT 1 FROM users - WHERE users.id = ? AND users.deleted_at IS NULL - )`, - params: [ - crypto.randomUUID(), - developerId, - tokenHash, - userId, - expiresAt, - developerId, - userId, - userId - ] - }); - - const results = await this.db.$client.batch([revokeStmt, insertStmt]); - - // The INSERT only writes a row when the ownership guard above passes, - // so zero rows written means the caller doesn't currently own this - // developer — a follow-up read distinguishes NOT_FOUND from FORBIDDEN - // for the response without reopening the race the guard closes. - if (!results[1]?.meta?.changes) { - const ownershipError = await this.checkOwnership(developerId, userId); - return { - data: null, - error: ownershipError ?? { - code: "FORBIDDEN", - message: "You don't own this profile" - } - }; - } - - return { data: { token, expires_at: expiresAt }, error: null }; - } catch (error) { - return databaseError("initiateTransfer", error); - } - } - - async revokeTransfer( - developerId: string, - userId: string - ): Promise> { - try { - const result = await this.db.run(sql` - UPDATE ${developerTransfers} SET revoked_at = CURRENT_TIMESTAMP - WHERE developer_id = ${developerId} AND accepted_at IS NULL AND revoked_at IS NULL - AND EXISTS (SELECT 1 FROM ${developers} WHERE developers.id = developer_transfers.developer_id AND developers.owner_user_id = ${userId}) - AND EXISTS (SELECT 1 FROM ${users} WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL) - `); - - // Zero rows changed is ambiguous by itself (no pending transfer vs. - // not the owner vs. no such developer), since the ownership guard is - // folded into the write above rather than checked beforehand. A - // follow-up read-only check distinguishes them for the response - // without reopening the race that guard closes. - if (!result.meta?.changes) { - const ownershipError = await this.checkOwnership(developerId, userId); - if (ownershipError) { - return { data: null, error: ownershipError }; - } - } - - return { data: { id: developerId, revoked: true }, error: null }; - } catch (error) { - return databaseError("revokeTransfer", error); - } - } - - async acceptTransfer( - token: string, - userId: string - ): Promise> { - try { - const tokenHash = await sha256Hex(token); - - // Claim the transfer and move ownership in the same atomic batch, - // rather than as two separate writes. Splitting them would leave a - // window, after the claim commits but before ownership actually - // moves, where the *former* owner's initiateTransfer call would still - // see itself as the current owner (per the developers row) and could - // mint a fresh, valid link for a profile that's already mid-handoff. - // It would also mean a failure on the ownership write alone (e.g. the - // recipient racing to create another profile) permanently burns the - // token without ever transferring ownership, with no way to retry. - // Batching both as one D1 transaction makes them succeed or fail as a - // unit. The `changes() = 1` guard on the second statement is load- - // bearing, not redundant with the subquery: accepted_by/accepted_at - // are a permanent historical record once a token is claimed, so the - // subquery alone would match a *previously* accepted token forever, - // letting a replay of an old, already-used link silently reassign - // ownership again (even to a profile since handed off to someone - // else) despite the claim itself changing zero rows. - // `changes()` reports the row count from the immediately preceding - // statement on this same connection, so it's only 1 when *this* - // batch's claim just fired — proving the update below is reacting to - // a fresh claim, not replaying an old one. - // - // The claim's NOT EXISTS guard folds the self-accept case (accepting - // user already owns *this* developer) and the already-owns-a- - // different-profile case into the same atomic decision, so the token - // is never consumed unless the accepting user is actually eligible. A - // plain check-then-act (SELECT the row, decide, then write) would let - // two concurrent accepts both read it as valid before either one - // wrote to it, making the token usable more than once. - // Kept as raw sql via $client (see toD1Statement): correlated - // subqueries plus the changes()=1 gates need the raw-D1 escape hatch - // (see upsertOwn's historyStmt comment). - const claimStmt = toD1Statement(this.db.$client, { - sql: `UPDATE developer_transfers SET accepted_at = CURRENT_TIMESTAMP, accepted_by = ? - WHERE token_hash = ? AND accepted_at IS NULL AND revoked_at IS NULL AND expires_at > CURRENT_TIMESTAMP - AND NOT EXISTS (SELECT 1 FROM developers WHERE owner_user_id = ?) - AND EXISTS (SELECT 1 FROM users WHERE id = ? AND deleted_at IS NULL)`, - params: [userId, tokenHash, userId, userId] - }); - const updateDeveloperStmt = toD1Statement(this.db.$client, { - // url_check_cooldown_until is reset here too — it's keyed by - // whichever user currently owns this row, so left unchanged it - // would rate-limit the *new* owner's first check_url reverify for - // whatever's left of the *previous* owner's cooldown window. - // - // github_org_verified/github_url_verified/github_verification_note/ - // github_verified_at are cleared for the same underlying reason: - // they describe whether the *previous* owner's linked GitHub - // identity matched this profile — a fact that says nothing about - // the new owner, who was never checked. Unlike approveClaim() (the - // other ownership-transfer path), there's no claim-time - // verification to carry over here — a transfer is a bare handoff, - // not a claim — so this can only ever fall back to null/unverified, - // same as a brand-new profile with no GitHub identity yet. - sql: `UPDATE developers - SET owner_user_id = ?, - ownership_epoch = ownership_epoch + 1, - content_revision = content_revision + 1, - approved_at = NULL, approved_revision = NULL, approved_by = NULL, - url_check_cooldown_until = NULL, - github_org_verified = NULL, github_url_verified = NULL, - github_verification_note = NULL, github_verified_at = NULL, - updated_at = CURRENT_TIMESTAMP - WHERE changes() = 1 - AND id = ( - SELECT developer_id FROM developer_transfers - WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL - )`, - params: [userId, tokenHash, userId] - }); - - const rejectPendingStmt = toD1Statement(this.db.$client, { - sql: `UPDATE extension_submissions - SET status = 'rejected', - review_note = 'Ownership changed before review', - reviewed_at = CURRENT_TIMESTAMP - WHERE changes() = 1 - AND developer_id = ( - SELECT developer_id FROM developer_transfers - WHERE token_hash = ? AND accepted_by = ? AND accepted_at IS NOT NULL - ) - AND status = 'pending'`, - params: [tokenHash, userId] - }); - - let results; - try { - results = await this.db.$client.batch([ - claimStmt, - updateDeveloperStmt, - rejectPendingStmt - ]); - } catch (error) { - if (isOwnerConflict(error)) { - return { - data: null, - error: { - code: "CONFLICT", - message: - "You already have a developer profile — remove or transfer it before accepting a new one" - } - }; - } - return databaseError("acceptTransfer", error); - } - - const [claim] = results; - if (!claim.meta?.changes) { - // The claim can fail either because the token itself is bad (used, - // revoked, expired, unknown) or because it's still valid but the - // ownership guard rejected it — check which, for an accurate error. - const [stillPending] = await this.db - .select({ one: sql`1` }) - .from(developerTransfers) - .where( - and( - eq(developerTransfers.tokenHash, tokenHash), - isNull(developerTransfers.acceptedAt), - isNull(developerTransfers.revokedAt), - sql`${developerTransfers.expiresAt} > CURRENT_TIMESTAMP` - ) - ); - - if (stillPending) { - return { - data: null, - error: { - code: "CONFLICT", - message: - "You already have a developer profile — remove or transfer it before accepting a new one" - } - }; - } - return { - data: null, - error: { - code: "NOT_FOUND", - message: "This transfer link is invalid, used, or expired" - } - }; - } - - const [transfer] = await this.db - .select({ developerId: developerTransfers.developerId }) - .from(developerTransfers) - .where(eq(developerTransfers.tokenHash, tokenHash)); - if (!transfer) { - return databaseError( - "acceptTransfer", - new Error("Claimed transfer row not found") - ); - } - - return this.getById(transfer.developerId); - } catch (error) { - return databaseError("acceptTransfer", error); - } - } - - private async getClaimById( - id: string - ): Promise> { - try { - const [row] = await this.db - .select() - .from(developerClaims) - .where(eq(developerClaims.id, id)); - if (!row) { - return { - data: null, - error: { - message: `Cannot find claim by id: ${id}`, - code: "NOT_FOUND" - } - }; - } - return { data: parseClaimRow(row), error: null }; - } catch (error) { - return databaseError("getClaimById", error); - } - } - - // Shared by claim/approveClaim once a developer/eligibility-guarded write - // affects zero rows: distinguishes "no such developer" from the two - // possible ownership conflicts for an accurate response, without - // reopening the race the guarded write already closed. - private async claimIneligibilityError( - developerId: string - ): Promise<{ code: "NOT_FOUND" | "CONFLICT"; message: string }> { - const [developer] = await this.db - .select({ ownerUserId: developers.ownerUserId }) - .from(developers) - .where(eq(developers.id, developerId)); - if (!developer) { - return { code: "NOT_FOUND", message: "Developer not found" }; - } - if (developer.ownerUserId !== null) { - return { code: "CONFLICT", message: "This profile is already owned" }; - } - return { - code: "CONFLICT", - message: "You already have a developer profile" - }; - } - - // githubToken authenticates the GitHub entity-existence lookup only (a - // service-level credential, raises the public rate limit) — it is never - // the claimant's own token, which never leaves the auth service. Shared by - // claim() and upsertOwn(): both need the same question answered — does a - // real GitHub org/user exist for this id, and if so, does the caller's own - // linked GitHub identity match it? A positive mismatch is the only outcome - // that ever blocks; no real GitHub entity for this id, or the caller - // having no linked GitHub identity yet, both fall back to unverified - // (manual moderator review), never to a block. - // publisherUrl — only ever passed by upsertOwn's create path, which is the - // one place a new Publisher URL is actually being submitted alongside - // identity verification; claim() has no URL of its own to cross-check - // (the developer row it's claiming already exists). Drives - // githubUrlVerified only — a non-matching or unset GitHub "website" field - // never blocks or un-verifies identity, since it's optional and often - // stale, unlike the identity check above. - private async verifyGithubOwnership( - developerId: string, - developerType: Developer["type"], - callerId: string, - githubToken?: string, - publisherUrl?: string - ): Promise< - | { mismatch: true } - | { - mismatch: false; - githubOrgVerified: number | null; - githubUrlVerified: number | null; - note: string | null; - } - | { error: DatabaseError } - > { - const githubEntity = await checkGithubEntity( - developerId, - githubToken ?? "" - ); - - if (githubEntity.status === "unavailable") { - return { error: githubUnavailableError(githubEntity.reason) }; - } - - if (githubEntity.status === "not_found") { - return { - mismatch: false, - githubOrgVerified: null, - githubUrlVerified: null, - note: "GitHub entity was not verified automatically — reviewed manually." - }; - } - - // A real GitHub entity exists for this id, just under the other type - // (e.g. a real org submitted as a "user") — this is a confirmed - // disagreement with GitHub, not an unknown, so it must block rather than - // fall back to unverified. Otherwise a caller could take a real org/user's - // id unverified simply by submitting the wrong type for it. - if (githubEntity.entity.type !== developerType) { - return { mismatch: true }; - } - - const identity = await new UsersDatabase(this.db).getGithubIdentity( - callerId - ); - // A real DB/schema failure here is not the same as "caller has no linked - // GitHub identity" — swallowing it would silently let creation/claiming - // proceed unverified during an outage instead of surfacing the error. - if (identity.error || !identity.data) { - return { - error: identity.error ?? { - message: "Failed to load caller's GitHub identity", - code: "DATABASE_ERROR" - } - }; - } - const callerIdentity = identity.data; - - if (!callerIdentity.githubLogin) { - return { - mismatch: false, - githubOrgVerified: null, - githubUrlVerified: null, - note: "Caller has no linked GitHub identity yet — reviewed manually." - }; - } - - if (matchesClaimant(developerType, developerId, callerIdentity)) { - return { - mismatch: false, - githubOrgVerified: 1, - githubUrlVerified: urlMatchesGithubBlog( - publisherUrl, - githubEntity.entity.blog - ) - ? 1 - : null, - note: "Verified: caller's linked GitHub identity matches." - }; - } - - return { mismatch: true }; - } - - // Re-runs the same identity match verifyGithubOwnership() does for a - // brand-new claim/creation, but for a profile the caller already owns — - // no GitHub API call needed. checkGithubEntityType() (the GitHub call - // verifyGithubOwnership() makes) only exists to confirm a *new* id isn't - // squatting on a real GitHub org/user; that doesn't apply once ownership - // already exists, so this only re-derives the match from the caller's - // own already-synced github_login/github_orgs. Called opportunistically - // on every login for a developer-owning user, and by the owner's own - // "Re-verify" action — both share this one method. - // checkUrl/githubToken — only set by the owner's own manual "Re-verify" - // button, never by the opportunistic per-login call in extensions' - // auth/callback.ts. Re-checking Publisher URL against GitHub's on-file - // website needs a fresh GitHub API call (unlike the identity match below), - // so it stays opt-in to keep the automatic login path GitHub-API-free. - async reverifyOwn( - userId: string, - checkUrl?: boolean, - githubToken?: string - ): Promise> { - try { - const [row] = await this.db - .select({ - id: developers.id, - type: developers.type, - url: developers.url - }) - .from(developers) - .where(eq(developers.ownerUserId, userId)); - if (!row) { - return { - data: null, - error: { - message: "You don't own a developer profile", - code: "NOT_FOUND" - } - }; - } - - if (checkUrl) { - // Atomic conditional UPDATE, not a read-then-write — the WHERE - // clause only matches (and thus only "wins") when the cooldown is - // absent or already expired, so two concurrent check_url requests - // can't both pass. This is the only reason check_url spends a real - // GitHub API call, so it's the only path that needs this. - const cooldown = await this.db - .update(developers) - .set({ - urlCheckCooldownUntil: sql`datetime('now', ${`+${URL_CHECK_COOLDOWN_SECONDS} seconds`})` - }) - .where( - and( - eq(developers.id, row.id), - eq(developers.ownerUserId, userId), - sql`EXISTS ( - SELECT 1 FROM ${users} - WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL - )`, - or( - isNull(developers.urlCheckCooldownUntil), - sql`${developers.urlCheckCooldownUntil} < CURRENT_TIMESTAMP` - ) - ) - ); - if (!cooldown.meta?.changes) { - return { - data: null, - error: { - message: - "Please wait a minute before re-checking your Publisher URL again.", - code: "RATE_LIMITED" - } - }; - } - } - - const identity = await new UsersDatabase(this.db).getGithubIdentity( - userId - ); - if (identity.error || !identity.data) { - return { - data: null, - error: identity.error ?? { - message: "Failed to load caller's GitHub identity", - code: "DATABASE_ERROR" - } - }; - } - - // No linked GitHub identity at all shouldn't be reachable in practice - // (GitHub is this system's sole login provider), but stays a no-op - // rather than writing a misleading "unverified" result over it. - if (!identity.data.githubLogin) { - return this.getById(row.id); - } - - const matches = matchesClaimant( - row.type as Developer["type"], - row.id, - identity.data - ); - - // Only bothers with the extra GitHub API call when the identity match - // above still holds — a URL "verified" against an entity the caller no - // longer controls wouldn't mean anything. When identity no longer - // matches, any previously-set githubUrlVerified is cleared below - // (cheap — no API call needed, same as githubOrgVerified itself). - let githubUrlVerified: number | null = null; - let writeUrlVerified = false; - // Set when a fresh lookup (only possible when checkUrl actually ran) - // finds GitHub's *current* entity type no longer matches the - // profile's own type — matchesClaimant() above only compares - // login/org membership, it never confirms the entity is still the - // type the profile claims, unlike creation-time verification. This - // downgrades the identity signal too, not just the URL one, since the - // same discrepancy undermines both. - let identityTypeContradicted = false; - if (!matches) { - writeUrlVerified = true; - } else if (checkUrl) { - const entity = await checkGithubEntity(row.id, githubToken ?? ""); - // An unavailable lookup is explicitly inconclusive, not a disproof, - // so it leaves the stored URL verification signal untouched rather - // than clearing a real prior verification over a transient failure. - // A confirmed absence likewise provides no website to compare. Only - // a successful lookup gets to overwrite the stored URL signal. - if (entity.status === "unavailable") { - // Keep the cooldown reservation even though no signal was changed. - // Otherwise a caller could repeatedly hit GitHub while the shared - // service token is throttled or the upstream service is failing. - return { data: null, error: githubUnavailableError(entity.reason) }; - } - if (entity.status === "found") { - writeUrlVerified = true; - if (entity.entity.type !== row.type) { - identityTypeContradicted = true; - } else { - githubUrlVerified = urlMatchesGithubBlog( - row.url ?? undefined, - entity.entity.blog - ) - ? 1 - : null; - } - } - } - const verified = matches && !identityTypeContradicted; - - // Re-asserts ownership in the write itself (not just the lookup - // above) — otherwise a transfer/claim landing in between would let - // this write a result computed from the *former* owner's GitHub - // identity onto the profile after it's changed hands. Same guard as - // upsertOwn's update branch. Also re-asserts the URL is still the one - // just checked — otherwise a concurrent Publisher URL edit landing in - // between would let a stale URL comparison get written as if it - // described the new URL. - const result = await this.db - .update(developers) - .set({ - githubOrgVerified: verified ? 1 : 0, - ...(writeUrlVerified ? { githubUrlVerified } : {}), - githubVerificationNote: verified - ? "Verified: caller's linked GitHub identity matches." - : identityTypeContradicted - ? "No longer verified: GitHub's on-file entity type no longer matches this profile." - : "No longer verified: caller's linked GitHub identity no longer matches.", - githubVerifiedAt: sql`CURRENT_TIMESTAMP` - }) - .where( - and( - eq(developers.id, row.id), - eq(developers.ownerUserId, userId), - sql`EXISTS ( - SELECT 1 FROM ${users} - WHERE ${users.id} = ${userId} AND ${users.deletedAt} IS NULL - )`, - ...(writeUrlVerified - ? [ - row.url === null - ? isNull(developers.url) - : eq(developers.url, row.url) - ] - : []) - ) - ); - - if (!result.meta?.changes) { - return { - data: null, - error: { - message: - "Developer ownership or Publisher URL changed while re-verifying", - code: "CONFLICT" - } - }; - } - - return this.getById(row.id); - } catch (error) { - return databaseError("reverifyOwn", error); - } - } - - async claim( - developerId: string, - claimantId: string, - note?: string, - githubToken?: string - ): Promise> { - try { - let githubOrgVerified: number | null = null; - let githubVerificationNote: string | null = null; - - const [developer] = await this.db - .select({ type: developers.type }) - .from(developers) - .where( - and(eq(developers.id, developerId), isNull(developers.ownerUserId)) - ); - - if (developer) { - // Cheap short-circuit ahead of the GitHub lookup below: a claimant - // replaying an already-pending claim on this id would otherwise - // trigger a fresh GitHub API call every time, purely to be told the - // INSERT's own guard rejects it as a duplicate — letting one caller - // burn through the shared service-level GitHub quota for free. This - // is safe precisely because it only ever *returns* here when the - // read observes `pending` — it never falls through to verification - // or the INSERT in that case, so it can't itself create an - // unverified claim. Anything else (no claim yet, or one already - // resolved to approved/rejected) always continues through full - // verification below. A pending claim that resolves between this - // read and the response going out can make the message stale - // relative to that instant, but never lets a row get created - // without verification. - const [hasPendingClaim] = await this.db - .select({ one: sql`1` }) - .from(developerClaims) - .where( - and( - eq(developerClaims.developerId, developerId), - eq(developerClaims.claimantId, claimantId), - eq(developerClaims.status, "pending") - ) - ); - - if (hasPendingClaim) { - return { - data: null, - error: { - code: "CONFLICT", - message: "You already have a pending claim on this profile" - } - }; - } - - const check = await this.verifyGithubOwnership( - developerId, - developer.type as Developer["type"], - claimantId, - githubToken - ); - - if ("error" in check) { - return { data: null, error: check.error }; - } - - if (check.mismatch) { - return { - data: null, - error: { - code: "GITHUB_MISMATCH", - message: - "Your linked GitHub account doesn't match this developer's GitHub organization or username, so it can't be claimed automatically. Make sure you're signed in with the right GitHub account, then try again." - } - }; - } - - githubOrgVerified = check.githubOrgVerified; - githubVerificationNote = check.note; - } - - const id = crypto.randomUUID(); - let result; - try { - // Both eligibility checks are folded into the INSERT itself, rather - // than a separate SELECT beforehand — a caller who loses eligibility - // (developer gets claimed/transferred, or the caller picks up a - // different profile) between an up-front check and the write could - // otherwise still slip a stale claim through. (The SELECT above is - // only used to decide the GitHub verification signal, and is always - // re-checked here — it can't itself grant eligibility.) Kept as raw - // sql: an INSERT...SELECT...WHERE EXISTS isn't expressible via - // .insert().values(). - result = await this.db.run(sql` - INSERT INTO ${developerClaims} (id, developer_id, claimant_id, note, github_org_verified, github_verification_note) - SELECT ${id}, ${developerId}, ${claimantId}, ${note ?? null}, ${githubOrgVerified}, ${githubVerificationNote} - WHERE EXISTS (SELECT 1 FROM ${developers} WHERE id = ${developerId} AND owner_user_id IS NULL) - AND NOT EXISTS (SELECT 1 FROM ${developers} WHERE owner_user_id = ${claimantId}) - AND EXISTS (SELECT 1 FROM ${users} WHERE ${users.id} = ${claimantId} AND ${users.deletedAt} IS NULL) - `); - } catch (error) { - if (isPendingClaimConflict(error)) { - return { - data: null, - error: { - code: "CONFLICT", - message: "You already have a pending claim on this profile" - } - }; - } - return databaseError("claim", error); - } - - if (!result.meta?.changes) { - return { - data: null, - error: await this.claimIneligibilityError(developerId) - }; - } - - return this.getClaimById(id); - } catch (error) { - return databaseError("claim", error); - } - } - - // Lets a claimant withdraw their own pending claim — scoped to - // claimant_id so this can't be used to cancel someone else's, and to - // status = 'pending' so a moderator's decision can't be undone by it. - async cancelClaim( - claimId: string, - claimantId: string - ): Promise> { - let result; - try { - result = await this.db.delete(developerClaims).where( - and( - eq(developerClaims.id, claimId), - eq(developerClaims.claimantId, claimantId), - eq(developerClaims.status, "pending"), - sql`EXISTS ( - SELECT 1 FROM ${users} - WHERE ${users.id} = ${claimantId} AND ${users.deletedAt} IS NULL - )` - ) - ); - } catch (error) { - return databaseError("cancelClaim", error); - } - - if (!result.meta?.changes) { - return { - data: null, - error: { - message: `Cannot find pending claim by id: ${claimId}`, - code: "NOT_FOUND" - } - }; - } - - return { data: { id: claimId }, error: null }; - } - - async listMyClaims( - claimantId: string - ): Promise> { - let rows; - try { - rows = await this.db - .select() - .from(developerClaims) - .where(eq(developerClaims.claimantId, claimantId)) - .orderBy(desc(developerClaims.createdAt)); - } catch (error) { - return databaseError("listMyClaims", error); - } - - return { data: rows.map(parseClaimRow), error: null }; - } - - async listPendingClaims(): Promise> { - let rows; - try { - rows = await this.db - .select({ - claim: developerClaims, - developerName: developers.name, - developerType: developers.type, - claimantName: users.name, - claimantGithubLogin: users.githubLogin - }) - .from(developerClaims) - .innerJoin(developers, eq(developers.id, developerClaims.developerId)) - .leftJoin(users, eq(users.id, developerClaims.claimantId)) - .where(eq(developerClaims.status, "pending")) - .orderBy(asc(developerClaims.createdAt)); - } catch (error) { - return databaseError("listPendingClaims", error); - } - - return { - data: rows.map((row) => ({ - ...parseClaimRow(row.claim), - developer_name: row.developerName, - developer_type: - row.developerType as PendingDeveloperClaim["developer_type"], - claimant_name: row.claimantName, - claimant_github_login: row.claimantGithubLogin - })), - error: null - }; - } - - private async explainClaimApprovalNoOp( - claim: DeveloperClaim - ): Promise> { - const latestClaim = await this.getClaimById(claim.id); - if (latestClaim.error || latestClaim.data?.status !== "pending") { - return { - data: null, - error: latestClaim.error ?? { - message: "Claim is not pending", - code: "CONFLICT" - } - }; - } - - try { - const [developer] = await this.db - .select({ ownerUserId: developers.ownerUserId }) - .from(developers) - .where(eq(developers.id, claim.developer_id)); - if (!developer) { - return { - data: null, - error: { message: "Developer not found", code: "NOT_FOUND" } - }; - } - if (developer.ownerUserId !== null) { - return { - data: null, - error: { message: "This profile is already owned", code: "CONFLICT" } - }; - } - return { - data: null, - error: { - message: "The claimant already owns a different developer profile", - code: "CONFLICT" - } - }; - } catch (error) { - return databaseError("approveClaim", error); - } - } - - async approveClaim( - claimId: string, - reviewerId: string - ): Promise> { - const existing = await this.getClaimById(claimId); - if (existing.error || !existing.data) { - return { - data: null, - error: existing.error ?? { - message: `Cannot find claim by id: ${claimId}`, - code: "NOT_FOUND" - } - }; - } - const claim = existing.data; - - if (claim.status !== "pending") { - return { - data: null, - error: { message: "Claim is not pending", code: "CONFLICT" } - }; - } - - // Keep the status transition, ownership handoff, and competing-claim - // rejection in one raw D1 batch. Each write is gated by changes() from - // the immediately preceding statement, so a stale claim cannot transfer - // ownership and a failed transfer cannot reject competing claims. - // - // The assertion statement is intentionally capable of violating the - // ownership_epoch CHECK. D1 rolls the entire batch back when that happens, - // which prevents a zero-row ownership update from leaving the claim - // approved. Its successful no-op update also preserves changes() = 1 for - // the final rejection statement. - let results; - try { - const claimStmt = toD1Statement(this.db.$client, { - sql: `UPDATE developer_claims - SET status = 'approved', reviewer_id = ?, reviewed_at = CURRENT_TIMESTAMP - WHERE id = ? AND status = 'pending' - AND EXISTS ( - SELECT 1 FROM developers d - WHERE d.id = developer_claims.developer_id - AND d.owner_user_id IS NULL - ) - AND NOT EXISTS ( - SELECT 1 FROM developers owned - WHERE owned.owner_user_id = developer_claims.claimant_id - ) - AND EXISTS ( - SELECT 1 FROM users - WHERE users.id = ? AND users.deleted_at IS NULL - )`, - params: [reviewerId, claimId, reviewerId] - }); - const developerStmt = toD1Statement(this.db.$client, { - sql: `UPDATE developers - SET owner_user_id = ?, - ownership_epoch = ownership_epoch + 1, - content_revision = content_revision + 1, - approved_at = NULL, approved_revision = NULL, approved_by = NULL, - url_check_cooldown_until = NULL, - github_org_verified = ?, github_verification_note = ?, - github_verified_at = ?, updated_at = CURRENT_TIMESTAMP - WHERE changes() = 1 AND id = ? AND owner_user_id IS NULL - AND NOT EXISTS (SELECT 1 FROM developers WHERE owner_user_id = ?)`, - params: [ - claim.claimant_id, - claim.github_org_verified === undefined - ? null - : claim.github_org_verified - ? 1 - : 0, - claim.github_verification_note ?? null, - claim.github_org_verified === undefined ? null : claim.created_at, - claim.developer_id, - claim.claimant_id - ] - }); - const assertTransferStmt = toD1Statement(this.db.$client, { - sql: `UPDATE developers - SET ownership_epoch = CASE WHEN changes() = 1 THEN ownership_epoch ELSE 0 END - WHERE id = ?`, - params: [claim.developer_id] - }); - const rejectOthersStmt = toD1Statement(this.db.$client, { - sql: `UPDATE developer_claims - SET status = 'rejected', reviewer_id = ?, reviewed_at = CURRENT_TIMESTAMP, - review_note = 'Another claim on this profile was approved' - WHERE changes() = 1 AND developer_id = ? AND status = 'pending' AND id != ?`, - params: [reviewerId, claim.developer_id, claimId] - }); - - results = await this.db.$client.batch([ - claimStmt, - developerStmt, - assertTransferStmt, - rejectOthersStmt - ]); - } catch (error) { - if ( - /CHECK constraint failed.*ownership_epoch/i.test( - errorMessageChain(error) - ) - ) { - return this.explainClaimApprovalNoOp(claim); - } - if (isOwnerConflict(error)) { - return { - data: null, - error: { - message: "The claimant already owns a different developer profile", - code: "CONFLICT" - } - }; - } - return databaseError("approveClaim", error); - } - - const [claimResult] = results; - if (!claimResult.meta?.changes) { - // Diagnose only after the guarded transaction. These reads improve the - // response without participating in (or weakening) its race safety. - return this.explainClaimApprovalNoOp(claim); - } - - return this.getById(claim.developer_id); - } - - async rejectClaim( - claimId: string, - reviewerId: string, - reviewNote: string - ): Promise> { - let result; - try { - result = await this.db - .update(developerClaims) - .set({ - status: "rejected", - reviewerId, - reviewNote, - reviewedAt: sql`CURRENT_TIMESTAMP` - }) - .where( - and( - eq(developerClaims.id, claimId), - eq(developerClaims.status, "pending"), - sql`EXISTS ( - SELECT 1 FROM ${users} - WHERE ${users.id} = ${reviewerId} AND ${users.deletedAt} IS NULL - )` - ) - ); - } catch (error) { - return databaseError("rejectClaim", error); - } - - if (!result.meta?.changes) { - return { - data: null, - error: { - message: `Cannot find pending claim by id: ${claimId}`, - code: "NOT_FOUND" - } - }; - } - - return this.getClaimById(claimId); - } -} diff --git a/src/services/extensions/v2/errors.ts b/src/services/extensions/v2/errors.ts index b1e8814..8a54e18 100644 --- a/src/services/extensions/v2/errors.ts +++ b/src/services/extensions/v2/errors.ts @@ -5,8 +5,8 @@ import { logError } from "../../../lib/logger"; // .message is a generic "Failed to run the query ''" - the actual // SQLite/D1 message (e.g. "UNIQUE constraint failed: ...") lives in // .cause, not .message. Regex-matching driver error text (see -// isOwnerConflict/isPendingClaimConflict/isPendingTargetConflict) needs the -// whole chain, not just the outermost message. +// the ownership/id conflict classifiers need the whole chain, not just the +// outermost message. export function errorMessageChain(error: unknown): string { const parts: string[] = []; let current: unknown = error; @@ -17,6 +17,25 @@ export function errorMessageChain(error: unknown): string { return parts.join(" "); } +// Matches the SQLite/D1 message for the unique owner index. Several +// ownership workflows need to translate this race into the same conflict +// response, so keep the classifier beside the shared database error helpers. +export function isDeveloperOwnerConflict(error: unknown): boolean { + return /UNIQUE constraint failed.*owner_user_id/i.test( + errorMessageChain(error) + ); +} + +// A concurrent first-time profile creation can lose the developers primary-key +// race after both requests pass the cheap existence check. Translate that +// SQLite/D1 constraint failure into the same conflict returned by the +// pre-flight check instead of exposing it as a generic database error. +export function isDeveloperIdConflict(error: unknown): boolean { + return /UNIQUE constraint failed.*developers\.id/i.test( + errorMessageChain(error) + ); +} + // Logs the real error server-side and returns a generic message to the // caller — DB exception text can leak schema/backend details otherwise. export function databaseError( diff --git a/src/services/extensions/v2/github-verification.ts b/src/services/extensions/v2/github-verification.ts index dce3c66..7d8e21e 100644 --- a/src/services/extensions/v2/github-verification.ts +++ b/src/services/extensions/v2/github-verification.ts @@ -4,10 +4,10 @@ import { logWarn } from "../../../lib/logger"; import { Developer } from "./interfaces"; import { GithubIdentity } from "./users-database"; -// Used by DevelopersDatabase.claim() to gate self-service claims on an +// Used by DeveloperClaimsDatabase.claim() to gate self-service claims on an // unowned developer id: does a real GitHub org/user exist for this id, and // does the claimant's own linked GitHub identity match it? See the comment -// on DevelopersDatabase.claim() for the full decision matrix — this module +// on DeveloperClaimsDatabase.claim() for the full decision matrix — this module // only answers the two underlying questions, it never decides to block. export type GithubEntity = { diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 1a9a3fd..4437110 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -164,7 +164,7 @@ export type SubmissionPayload = z.infer; export const DeveloperProfileSchema = DeveloperSchema.extend({ approved: z.boolean(), content_revision: z.int().positive(), - // Server-computed — see DevelopersDatabase.verifyGithubOwnership() (at + // Server-computed — see verifyGithubOwnership() (at // claim/creation time) and reverifyOwn() (opportunistic re-check on // login, or the owner's own "Re-verify" action). Never part of the // client-supplied DeveloperSchema. @@ -183,7 +183,7 @@ export const DeveloperProfileSchema = DeveloperSchema.extend({ // GitHub-API-free by design). github_url_verified: z.boolean().optional(), // Only populated by the moderator listAll/listUnapproved queries (see - // DevelopersDatabase.listAll/listUnapproved) — other DeveloperProfile + // DeveloperProfilesDatabase.listAll/listUnapproved) — other DeveloperProfile // producers (getById, create/update/claim/transfer results) don't join // for it, so it's absent rather than null there. `unclaimed` is the // authoritative "has an owner" signal (owner_user_id IS NULL) — don't @@ -458,8 +458,8 @@ export const DeveloperClaimSchema = z // Server-computed at claim() time only — never accepted from the // client (see ClaimNoteSchema below). Undefined when there was no // verifiable GitHub org/user for this id, or the claimant had no linked - // GitHub identity yet; both fall back to manual moderator review rather - // than gating anything. + // GitHub identity yet; both fall back to manual moderator review. An + // absent value is not proof of ownership and must not bypass approval. github_org_verified: z.boolean().optional(), github_verification_note: z.string().optional() }) @@ -486,7 +486,7 @@ export const ClaimNoteSchema = z .openapi("ClaimNote"); // check_url — opt-in because it costs an extra GitHub API call (see -// DevelopersDatabase.reverifyOwn()); only the owner's own manual "Re-verify" +// DeveloperProfilesDatabase.reverifyOwn()); only the owner's own manual "Re-verify" // button sets this, never the opportunistic per-login re-check. Not // z.coerce.boolean(): that coerces the non-empty string "false" to true. export const ReverifyQuerySchema = z.object({ diff --git a/src/services/extensions/v2/moderation-routes.ts b/src/services/extensions/v2/moderation-routes.ts index 43f81ea..c07fcc0 100644 --- a/src/services/extensions/v2/moderation-routes.ts +++ b/src/services/extensions/v2/moderation-routes.ts @@ -13,7 +13,7 @@ import { ReviewNoteRequiredSchema, SubmissionSchema } from "./interfaces"; -import { DevelopersDatabase } from "./developers-database"; +import { DeveloperProfilesDatabase } from "./developer-profiles-database"; import { SubmissionsDatabase } from "./submissions-database"; import { ExtensionsV2App, RouteDependencies } from "./route-dependencies"; @@ -294,7 +294,7 @@ export function registerModerationRoutes( }); app.openapi(allDevelopersRoute, async (c) => { - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.listAll(); @@ -347,7 +347,7 @@ export function registerModerationRoutes( }); app.openapi(unapprovedDevelopersRoute, async (c) => { - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.listUnapproved(); @@ -422,7 +422,7 @@ export function registerModerationRoutes( const auth = dependencies.auth(c); const { id } = c.req.valid("param"); const { expected_revision } = c.req.valid("json"); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.approve( @@ -431,7 +431,10 @@ export function registerModerationRoutes( auth.userId ); if (error || !data) { - const status = statusFromErrorCode(error?.code); + const status = + error?.code === "ACCOUNT_INACTIVE" + ? 403 + : statusFromErrorCode(error?.code); return c.json( { error: { @@ -486,7 +489,7 @@ export function registerModerationRoutes( app.openapi(developerHistoryRoute, async (c) => { const { id } = c.req.valid("param"); - const db = new DevelopersDatabase( + const db = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.listHistory(id); diff --git a/src/services/extensions/v2/owner-extensions-routes.ts b/src/services/extensions/v2/owner-extensions-routes.ts index 046dad8..1f20c3f 100644 --- a/src/services/extensions/v2/owner-extensions-routes.ts +++ b/src/services/extensions/v2/owner-extensions-routes.ts @@ -5,7 +5,7 @@ import { ExtensionListResponseSchema, ExtensionMineListQuerySchema } from "./interfaces"; -import { DevelopersDatabase } from "./developers-database"; +import { DeveloperProfilesDatabase } from "./developer-profiles-database"; import { ExtensionsDatabase, isValidExtensionCursor @@ -66,7 +66,7 @@ export function registerOwnerExtensionsRoutes( ); } - const ownerDb = new DevelopersDatabase( + const ownerDb = new DeveloperProfilesDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const owner = await ownerDb.getOwn(auth.userId); diff --git a/src/services/extensions/v2/ownership-routes.ts b/src/services/extensions/v2/ownership-routes.ts index a0ff85e..87bf67b 100644 --- a/src/services/extensions/v2/ownership-routes.ts +++ b/src/services/extensions/v2/ownership-routes.ts @@ -16,7 +16,8 @@ import { ReviewNoteRequiredSchema, TransferAcceptanceSchema } from "./interfaces"; -import { DevelopersDatabase } from "./developers-database"; +import { DeveloperClaimsDatabase } from "./developer-claims-database"; +import { DeveloperTransfersDatabase } from "./developer-transfers-database"; import { ExtensionsV2App, RouteDependencies } from "./route-dependencies"; export function registerOwnershipRoutes( @@ -88,7 +89,7 @@ export function registerOwnershipRoutes( const { id } = c.req.valid("param"); const { note } = c.req.valid("json"); const platform = dependencies.platform(c); - const db = new DevelopersDatabase( + const db = new DeveloperClaimsDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.claim( @@ -105,7 +106,7 @@ export function registerOwnershipRoutes( code: error?.code ?? "DATABASE_ERROR" } }, - error?.code === "GITHUB_MISMATCH" + error?.code === "GITHUB_MISMATCH" || error?.code === "ACCOUNT_INACTIVE" ? 403 : statusFromGithubErrorCode( error?.code, @@ -158,7 +159,7 @@ export function registerOwnershipRoutes( app.openapi(cancelClaimRoute, async (c) => { const auth = dependencies.auth(c); const { id } = c.req.valid("param"); - const db = new DevelopersDatabase( + const db = new DeveloperClaimsDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.cancelClaim(id, auth.userId); @@ -207,7 +208,7 @@ export function registerOwnershipRoutes( app.openapi(myClaimsRoute, async (c) => { const auth = dependencies.auth(c); - const db = new DevelopersDatabase( + const db = new DeveloperClaimsDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.listMyClaims(auth.userId); @@ -260,7 +261,7 @@ export function registerOwnershipRoutes( }); app.openapi(pendingClaimsRoute, async (c) => { - const db = new DevelopersDatabase( + const db = new DeveloperClaimsDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.listPendingClaims(); @@ -330,7 +331,7 @@ export function registerOwnershipRoutes( app.openapi(approveClaimRoute, async (c) => { const auth = dependencies.auth(c); const { id } = c.req.valid("param"); - const db = new DevelopersDatabase( + const db = new DeveloperClaimsDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.approveClaim(id, auth.userId); @@ -400,7 +401,7 @@ export function registerOwnershipRoutes( const auth = dependencies.auth(c); const { id } = c.req.valid("param"); const { review_note } = c.req.valid("json"); - const db = new DevelopersDatabase( + const db = new DeveloperClaimsDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.rejectClaim(id, auth.userId, review_note); @@ -464,7 +465,7 @@ export function registerOwnershipRoutes( app.openapi(initiateTransferRoute, async (c) => { const auth = dependencies.auth(c); const { id } = c.req.valid("param"); - const db = new DevelopersDatabase( + const db = new DeveloperTransfersDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.initiateTransfer(id, auth.userId); @@ -528,7 +529,7 @@ export function registerOwnershipRoutes( app.openapi(revokeTransferRoute, async (c) => { const auth = dependencies.auth(c); const { id } = c.req.valid("param"); - const db = new DevelopersDatabase( + const db = new DeveloperTransfersDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.revokeTransfer(id, auth.userId); @@ -594,12 +595,15 @@ export function registerOwnershipRoutes( app.openapi(acceptTransferRoute, async (c) => { const auth = dependencies.auth(c); const { token } = c.req.valid("json"); - const db = new DevelopersDatabase( + const db = new DeveloperTransfersDatabase( dependencies.database(c.env.DB_EXTENSIONS) ); const { data, error } = await db.acceptTransfer(token, auth.userId); if (error || !data) { - const status = statusFromErrorCode(error?.code); + const status = + error?.code === "ACCOUNT_INACTIVE" + ? 403 + : statusFromErrorCode(error?.code); return c.json( { error: { diff --git a/src/services/extensions/v2/route-errors.ts b/src/services/extensions/v2/route-errors.ts index 41bddb1..c379b01 100644 --- a/src/services/extensions/v2/route-errors.ts +++ b/src/services/extensions/v2/route-errors.ts @@ -29,6 +29,6 @@ export function statusFromGithubErrorCode( export function statusFromOwnershipErrorCode(code?: string): 403 | 404 | 500 { if (code === "NOT_FOUND") return 404; - if (code === "FORBIDDEN") return 403; + if (code === "FORBIDDEN" || code === "ACCOUNT_INACTIVE") return 403; return 500; } diff --git a/src/services/extensions/v2/users-database.ts b/src/services/extensions/v2/users-database.ts index 4118cd0..1d6a482 100644 --- a/src/services/extensions/v2/users-database.ts +++ b/src/services/extensions/v2/users-database.ts @@ -8,6 +8,16 @@ import { toD1Statement } from "./d1-batch"; export type GithubIdentity = { githubLogin: string | null; githubOrgs: string[]; + // Distinguishes a freshly synchronized empty membership list (a confirmed + // non-member) from absent, malformed, or expired evidence. The latter must + // fall back to moderator review rather than being treated as a mismatch. + githubOrgsAvailable: boolean; + // Raw values used to make a re-verification write conditional on the same + // identity snapshot that was read. The parsed fields above are sufficient + // for matching, but cannot distinguish a concurrent sync from an unchanged + // empty/missing membership list on their own. + githubOrgsSnapshot: string | null; + githubOrgsExpiresAt: string | null; }; export type UserIdentityInput = { @@ -33,13 +43,52 @@ export type UserRecord = { }; const RFC3339_TIMESTAMP = - /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})$/; + /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.\d+)?(?:Z|[+-](\d{2}):(\d{2}))$/; function isFutureGithubOrgsExpiry( value: string | null | undefined, now = Date.now() ): boolean { - if (!value || !RFC3339_TIMESTAMP.test(value)) return false; + if (!value) return false; + + const match = RFC3339_TIMESTAMP.exec(value); + if (!match) return false; + + // Date.parse normalizes out-of-range calendar days (for example, + // 2025-02-30 becomes 2025-03-02) instead of rejecting them. Validate the + // date portion before parsing so malformed central-auth evidence cannot be + // treated as a usable, future membership snapshot. + const year = Number(match[1]); + const month = Number(match[2]); + const day = Number(match[3]); + if (month < 1 || month > 12 || day < 1) return false; + + const isLeapYear = year % 4 === 0 && (year % 100 !== 0 || year % 400 === 0); + const daysInMonth = [ + 31, + isLeapYear ? 29 : 28, + 31, + 30, + 31, + 30, + 31, + 31, + 30, + 31, + 30, + 31 + ][month - 1]; + if (day > daysInMonth) return false; + + const hour = Number(match[4]); + const minute = Number(match[5]); + const second = Number(match[6]); + if (hour > 23 || minute > 59 || second > 59) return false; + + const offsetHour = match[7] === undefined ? 0 : Number(match[7]); + const offsetMinute = match[8] === undefined ? 0 : Number(match[8]); + if (offsetHour > 23 || offsetMinute > 59) return false; + const expiresAt = Date.parse(value); return Number.isFinite(expiresAt) && expiresAt > now; } @@ -336,10 +385,11 @@ export class UsersDatabase { } // Used to verify developer-profile claims against the claimant's own - // linked GitHub identity — see DevelopersDatabase.claim(). github_orgs is + // linked GitHub identity — see DeveloperClaimsDatabase.claim(). github_orgs is // only usable while its central-auth expiry is in the future; absent, // malformed, or expired organization evidence resolves to no memberships - // rather than throwing. + // rather than throwing. githubOrgsAvailable preserves whether that empty + // result is a confirmed snapshot or merely unavailable evidence. async getGithubIdentity( userId: string ): Promise> { @@ -355,10 +405,20 @@ export class UsersDatabase { .where(eq(users.id, userId)); if (row?.deletedAt !== null && row?.deletedAt !== undefined) { - return { data: { githubLogin: null, githubOrgs: [] }, error: null }; + return { + data: { + githubLogin: null, + githubOrgs: [], + githubOrgsAvailable: false, + githubOrgsSnapshot: null, + githubOrgsExpiresAt: null + }, + error: null + }; } let githubOrgs: string[] = []; + let githubOrgsAvailable = false; if ( row?.githubOrgs && isFutureGithubOrgsExpiry(row.githubOrgsExpiresAt) @@ -370,6 +430,7 @@ export class UsersDatabase { parsed.every((org) => typeof org === "string") ) { githubOrgs = parsed; + githubOrgsAvailable = true; } } catch { // Malformed JSON is treated the same as "no orgs recorded" — @@ -378,7 +439,13 @@ export class UsersDatabase { } return { - data: { githubLogin: row?.githubLogin ?? null, githubOrgs }, + data: { + githubLogin: row?.githubLogin ?? null, + githubOrgs, + githubOrgsAvailable, + githubOrgsSnapshot: row?.githubOrgs ?? null, + githubOrgsExpiresAt: row?.githubOrgsExpiresAt ?? null + }, error: null }; } catch (error) { diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index f6c55de..e19f342 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -13,7 +13,7 @@ import { } from "cloudflare:test"; import { env } from "cloudflare:workers"; -// Mocked so DevelopersDatabase.claim()'s GitHub entity-existence check never +// Mocked so DeveloperClaimsDatabase.claim()'s GitHub entity-existence check never // makes a real network call. Defaults to "not found" (matching classifyGitHubError's // NotFoundError check in github-verification.ts), which makes claim() fall // back to today's unverified/manual-review path — the same behavior these @@ -1115,6 +1115,38 @@ describe("Extensions API v2", () => { expect(body.error.code).toBe("DEVELOPER_ID_TAKEN"); }); + it("classifies a concurrent id collision as DEVELOPER_ID_TAKEN", async () => { + const headers = await authHeaders("user-1"); + let raced = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!raced && sql.includes("INSERT INTO developers")) { + raced = true; + await insertDeveloper(db, { + id: "raced-developer", + type: "user", + name: "Concurrent Creator", + owner_user_id: "user-2" + }); + } + }); + + const res = await put( + "/extensions/v2/developers/me", + headers, + sampleDeveloper({ id: "raced-developer" }) + ); + env.DB_EXTENSIONS = db; + + expect(raced).toBe(true); + expect(res.status).toBe(409); + expect(await res.json()).toMatchObject({ + error: { code: "DEVELOPER_ID_TAKEN" } + }); + expect((await getDeveloper(db, "raced-developer"))?.owner_user_id).toBe( + "user-2" + ); + }); + it("rejects changing the id on an existing profile", async () => { await put( "/extensions/v2/developers/me", @@ -1179,7 +1211,7 @@ describe("Extensions API v2", () => { ["missing", null], ["malformed", "2099"] ])( - "does not verify an organization from %s GitHub membership evidence", + "falls back to manual review when %s GitHub membership evidence is unavailable", async (_state, github_orgs_expires_at) => { mockGithubEntity("Organization"); await insertUser(db, { @@ -1195,12 +1227,36 @@ describe("Extensions API v2", () => { { id: "acme-org", type: "organization", name: "Acme Org" } ); - expect(res.status).toBe(403); - const body = (await res.json()) as { error: { code: string } }; - expect(body.error.code).toBe("GITHUB_MISMATCH"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_org_verified?: boolean }; + }; + expect(body.result.github_org_verified).toBeUndefined(); } ); + it("falls back to manual review when the linked GitHub login is whitespace-only", async () => { + mockGithubEntity("Organization"); + await insertUser(db, { + id: "user-1", + github_login: " ", + github_orgs: JSON.stringify(["acme-org"]), + github_orgs_expires_at: "2099-01-01T00:00:00.000Z" + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { id: "acme-org", type: "organization", name: "Acme Org" } + ); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_org_verified?: boolean }; + }; + expect(body.result.github_org_verified).toBeUndefined(); + }); + it("does not verify an organization from a fresh confirmed empty list", async () => { mockGithubEntity("Organization"); await insertUser(db, { @@ -1730,6 +1786,46 @@ describe("Extensions API v2", () => { ).toHaveLength(1); }); + it("reports an inactive account when deactivated during an existing profile update", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + + let deactivated = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + const normalizedSql = sql.toLowerCase(); + if ( + !deactivated && + normalizedSql.includes('update "developers"') && + normalizedSql.includes("content_revision") + ) { + deactivated = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "user-1") + .run(); + } + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper({ name: "Inactive owner write" }) + ); + env.DB_EXTENSIONS = db; + + expect(deactivated).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + expect((await getDeveloper(db, "dev-developer"))?.name).toBe( + "Dev Developer" + ); + }); + it("does not create a profile after the account is tombstoned mid-request", async () => { const headers = await authHeaders("deleted-during-write"); let tombstoned = false; @@ -1751,7 +1847,10 @@ describe("Extensions API v2", () => { env.DB_EXTENSIONS = db; expect(tombstoned).toBe(true); - expect(res.status).toBe(409); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); expect(await hasDeveloper(db, "deleted-during-write-profile")).toBe( false ); @@ -1978,9 +2077,80 @@ describe("Extensions API v2", () => { const body = (await stillThere.json()) as { result: { id: string } }; expect(body.result.id).toBe("dev-developer"); }); + + it("reports an inactive owner when the account is deactivated during deletion", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + + let deactivated = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!deactivated && sql.includes("DELETE FROM developer_transfers")) { + deactivated = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "user-1") + .run(); + } + }); + + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("user-1") + ); + env.DB_EXTENSIONS = db; + + expect(deactivated).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + expect(await hasDeveloper(db, "dev-developer")).toBe(true); + }); }); describe("POST /developers/me/reverify", () => { + it("reports an inactive account when deactivated during a URL cooldown reservation", async () => { + await insertUser(db, { id: "user-1" }); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + + let deactivated = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + const normalizedSql = sql.toLowerCase(); + if ( + !deactivated && + normalizedSql.includes('update "developers"') && + normalizedSql.includes("url_check_cooldown_until") + ) { + deactivated = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "user-1") + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + env.DB_EXTENSIONS = db; + + expect(deactivated).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + }); + it("re-verifies and refreshes the timestamp when the owner's GitHub org still matches", async () => { await insertDeveloper(db, { id: "dev-developer", @@ -2013,6 +2183,81 @@ describe("Extensions API v2", () => { ); }); + it.each([ + [ + "expired", + JSON.stringify(["dev-developer"]), + "2000-01-01T00:00:00.000Z" + ], + ["malformed", "not-json", "2099-01-01T00:00:00.000Z"] + ])( + "preserves verification when the owner's organization evidence is %s", + async (_state, github_orgs, github_orgs_expires_at) => { + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: null, + owner_user_id: "user-1", + github_org_verified: 1, + github_verification_note: + "Verified: caller's linked GitHub identity matches.", + github_verified_at: "2020-01-01T00:00:00.000Z" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs, + github_orgs_expires_at + }); + + const res = await post( + "/extensions/v2/developers/me/reverify", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { + github_org_verified?: boolean; + github_verified_at?: string; + }; + }; + expect(body.result.github_org_verified).toBe(true); + expect(body.result.github_verified_at).toBe("2020-01-01T00:00:00.000Z"); + } + ); + + it("preserves verification when the owner's GitHub login is whitespace-only", async () => { + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: null, + owner_user_id: "user-1", + github_org_verified: 1, + github_verification_note: + "Verified: caller's linked GitHub identity matches.", + github_verified_at: "2020-01-01T00:00:00.000Z" + }); + await insertUser(db, { + id: "user-1", + github_login: " ", + github_orgs: JSON.stringify(["dev-developer"]), + github_orgs_expires_at: "2099-01-01T00:00:00.000Z" + }); + + const res = await post( + "/extensions/v2/developers/me/reverify", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_org_verified?: boolean; github_verified_at?: string }; + }; + expect(body.result.github_org_verified).toBe(true); + expect(body.result.github_verified_at).toBe("2020-01-01T00:00:00.000Z"); + }); + it("flips to unverified when the owner's GitHub org membership no longer matches", async () => { await insertDeveloper(db, { id: "dev-developer", @@ -2551,6 +2796,103 @@ describe("Extensions API v2", () => { expect(developerRow?.owner_user_id).toBe("user-2"); expect(developerRow?.github_org_verified).toBeNull(); }); + + it("refuses to overwrite verification if the profile type changes during the check", async () => { + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: null, + owner_user_id: "user-1", + github_org_verified: 1, + github_verification_note: + "Verified: caller's linked GitHub identity matches.", + github_verified_at: "2020-01-01T00:00:00.000Z" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + let changed = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!changed && sql.includes("github_verified_at")) { + changed = true; + await db + .prepare("UPDATE developers SET type = ? WHERE id = ?") + .bind("user", "dev-developer") + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/me/reverify", + await authHeaders("user-1") + ); + env.DB_EXTENSIONS = db; + + expect(changed).toBe(true); + expect(res.status).toBe(409); + const developerRow = await getDeveloper(db, "dev-developer"); + expect(developerRow?.type).toBe("user"); + expect(developerRow?.github_org_verified).toBe(1); + expect(developerRow?.github_verified_at).toBe("2020-01-01T00:00:00.000Z"); + }); + + it("refuses to overwrite verification if GitHub identity sync wins the race", async () => { + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: null, + owner_user_id: "user-1", + github_org_verified: 1, + github_verification_note: + "Verified: caller's linked GitHub identity matches.", + github_verified_at: "2020-01-01T00:00:00.000Z" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]), + github_orgs_expires_at: "2099-01-01T00:00:00.000Z" + }); + + let synced = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!synced && sql.includes("github_verified_at")) { + synced = true; + await db + .prepare( + `UPDATE users + SET github_login = ?, github_orgs = ?, github_orgs_expires_at = ?, + updated_at = ? + WHERE id = ?` + ) + .bind( + "different-user", + JSON.stringify(["different-org"]), + "2099-01-01T00:00:00.000Z", + new Date().toISOString(), + "user-1" + ) + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/me/reverify", + await authHeaders("user-1") + ); + env.DB_EXTENSIONS = db; + + expect(synced).toBe(true); + expect(res.status).toBe(409); + const developerRow = await getDeveloper(db, "dev-developer"); + expect(developerRow?.github_org_verified).toBe(1); + expect(developerRow?.github_verified_at).toBe("2020-01-01T00:00:00.000Z"); + }); }); describe("developer moderation", () => { @@ -2574,13 +2916,79 @@ describe("Extensions API v2", () => { ); expect(stale.status).toBe(409); - const current = await post( + const current = await post( + "/extensions/v2/developers/dev-developer/approve", + await authHeaders("mod-1"), + { expected_revision: 2 } + ); + expect(current.status).toBe(200); + }); + + it("does not turn an approval diagnosis database failure into not found", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + + env.DB_EXTENSIONS = wrapD1WithHook(db, (sql) => { + if (/^\s*select/i.test(sql) && /from\s+"developers"/i.test(sql)) { + throw new Error("simulated approval diagnosis failure"); + } + }); + + const res = await post( + "/extensions/v2/developers/dev-developer/approve", + await authHeaders("mod-1"), + { expected_revision: 999 } + ); + env.DB_EXTENSIONS = db; + + expect(res.status).toBe(500); + expect((await res.json()) as { error: { code: string } }).toMatchObject({ + error: { code: "DATABASE_ERROR" } + }); + }); + + it("reports an inactive moderator when the account is deactivated during approval", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + + let deactivated = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if ( + !deactivated && + /update/i.test(sql) && + sql.includes("approved_at") + ) { + deactivated = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "mod-1") + .run(); + } + }); + + const res = await post( "/extensions/v2/developers/dev-developer/approve", await authHeaders("mod-1"), - { expected_revision: 2 } + { expected_revision: 1 } ); - expect(current.status).toBe(200); + env.DB_EXTENSIONS = db; + + expect(deactivated).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + expect((await getDeveloper(db, "dev-developer"))?.approved_at).toBeNull(); }); + it("approves a developer and removes it from the unapproved list", async () => { await put( "/extensions/v2/developers/me", @@ -2887,6 +3295,171 @@ describe("Extensions API v2", () => { ); }); + it("does not turn a committed transfer into a database error if the profile is deleted before the response lookup", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + const initiate = await post( + "/extensions/v2/developers/dev-developer/transfer", + await authHeaders("user-1") + ); + const token = ((await initiate.json()) as { result: { token: string } }) + .result.token; + + let deleted = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if ( + !deleted && + /^\s*select/i.test(sql) && + /from\s+"developers"/i.test(sql) + ) { + deleted = true; + await db + .prepare("DELETE FROM developer_transfers WHERE developer_id = ?") + .bind("dev-developer") + .run(); + await db + .prepare("DELETE FROM developers WHERE id = ?") + .bind("dev-developer") + .run(); + } + }); + + const accept = await post( + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } + ); + env.DB_EXTENSIONS = db; + + expect(deleted).toBe(true); + expect(accept.status).toBe(404); + expect(await accept.json()).toMatchObject({ + error: { code: "NOT_FOUND" } + }); + }); + + it("rejects pending submissions and claims when ownership changes", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + await insertDeveloperClaim(db, { + id: "transfer-pending-claim", + developer_id: "dev-developer", + claimant_id: "user-3" + }); + await insertSubmission(db, { + id: "transfer-pending-submission", + developer_id: "dev-developer", + submitted_by: "user-3", + payload: JSON.stringify(samplePayload({ developerId: "dev-developer" })) + }); + + const initiate = await post( + "/extensions/v2/developers/dev-developer/transfer", + await authHeaders("user-1") + ); + const token = ((await initiate.json()) as { result: { token: string } }) + .result.token; + + const accept = await post( + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } + ); + expect(accept.status).toBe(200); + + expect( + await getDeveloperClaim(db, "transfer-pending-claim") + ).toMatchObject({ + status: "rejected", + review_note: "Ownership changed before review" + }); + expect( + await getSubmission(db, "transfer-pending-submission") + ).toMatchObject({ + status: "rejected", + review_note: "Ownership changed before review" + }); + }); + + it("reports an inactive owner when the account is deactivated during initiation", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + + let tombstoned = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!tombstoned && sql.includes("INSERT INTO developer_transfers")) { + tombstoned = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "user-1") + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/dev-developer/transfer", + await authHeaders("user-1") + ); + env.DB_EXTENSIONS = db; + + expect(tombstoned).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + }); + + it("reports an inactive recipient when the account is deactivated during acceptance", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + + const initiate = await post( + "/extensions/v2/developers/dev-developer/transfer", + await authHeaders("user-1") + ); + const token = ((await initiate.json()) as { result: { token: string } }) + .result.token; + + let deactivated = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (!deactivated && sql.includes("UPDATE developer_transfers")) { + deactivated = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "user-2") + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/transfers/accept", + await authHeaders("user-2"), + { token } + ); + env.DB_EXTENSIONS = db; + + expect(deactivated).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + expect((await getDeveloper(db, "dev-developer"))?.owner_user_id).toBe( + "user-1" + ); + }); + it("doesn't inherit the previous owner's check_url cooldown after a transfer", async () => { await put( "/extensions/v2/developers/me", @@ -3210,6 +3783,59 @@ describe("Extensions API v2", () => { expect(res.status).toBe(409); }); + it.each([ + [ + "expired", + JSON.stringify(["some-other-org"]), + "2000-01-01T00:00:00.000Z" + ], + ["malformed", "not-json", "2099-01-01T00:00:00.000Z"] + ])( + "keeps a claim pending for manual review when %s GitHub membership evidence is unavailable", + async (_state, github_orgs, github_orgs_expires_at) => { + mockGithubEntity("Organization"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs, + github_orgs_expires_at + }); + await insertDeveloper(db, { + id: "acme-org", + type: "organization", + name: "Acme Org", + url: null, + owner_user_id: null + }); + + const res = await post( + "/extensions/v2/developers/acme-org/claim", + await authHeaders("user-1"), + {} + ); + + expect(res.status).toBe(201); + const body = (await res.json()) as { + result: { + id: string; + status: string; + github_org_verified?: boolean; + github_verification_note?: string; + }; + }; + expect(body.result.status).toBe("pending"); + expect(body.result.github_org_verified).toBeUndefined(); + expect(body.result.github_verification_note).toContain( + "could not be confirmed" + ); + expect((await getDeveloper(db, "acme-org"))?.owner_user_id).toBeNull(); + + const stored = await getDeveloperClaim(db, body.result.id); + expect(stored?.status).toBe("pending"); + expect(stored?.github_org_verified).toBeNull(); + } + ); + it("does not create a duplicate row for a second claim while one is already pending", async () => { await seedUnownedDeveloper("legacy-developer"); @@ -3306,6 +3932,39 @@ describe("Extensions API v2", () => { expect(res.status).toBe(409); }); + it("reports an account deactivated during claim creation", async () => { + await seedUnownedDeveloper("legacy-developer"); + const headers = await authHeaders("user-1"); + let deactivated = false; + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if ( + !deactivated && + sql.includes("developer_claims") && + sql.includes("INSERT") + ) { + deactivated = true; + await db + .prepare("UPDATE users SET deleted_at = ? WHERE id = ?") + .bind(new Date().toISOString(), "user-1") + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/legacy-developer/claim", + headers, + {} + ); + env.DB_EXTENSIONS = db; + + expect(deactivated).toBe(true); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ + error: { code: "ACCOUNT_INACTIVE", message: "Active account required" } + }); + expect(await countDeveloperClaims(db)).toBe(0); + }); + it("rolls back claim approval when a later ownership statement fails", async () => { await seedUnownedDeveloper("legacy-developer"); await insertUser(db, { id: "mod-1", is_moderator: 1 }); @@ -3595,6 +4254,62 @@ describe("Extensions API v2", () => { expect(created.result.github_org_verified).toBeUndefined(); }); + it("falls back to unverified manual review when organization membership evidence is stale", async () => { + await insertDeveloper(db, { + id: "legacy-developer", + type: "organization", + name: "Legacy Developer", + url: null, + owner_user_id: null + }); + mockGithubEntity("Organization"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["legacy-developer"]), + github_orgs_expires_at: "2000-01-01T00:00:00.000Z" + }); + + const res = await post( + "/extensions/v2/developers/legacy-developer/claim", + await authHeaders("user-1"), + {} + ); + expect(res.status).toBe(201); + const created = (await res.json()) as { + result: { github_org_verified?: boolean }; + }; + expect(created.result.github_org_verified).toBeUndefined(); + }); + + it("does not verify an organization claim for a whitespace-only GitHub login", async () => { + await insertDeveloper(db, { + id: "legacy-developer", + type: "organization", + name: "Legacy Developer", + url: null, + owner_user_id: null + }); + mockGithubEntity("Organization"); + await insertUser(db, { + id: "user-1", + github_login: " ", + github_orgs: JSON.stringify(["legacy-developer"]), + github_orgs_expires_at: "2099-01-01T00:00:00.000Z" + }); + + const res = await post( + "/extensions/v2/developers/legacy-developer/claim", + await authHeaders("user-1"), + {} + ); + expect(res.status).toBe(201); + const created = (await res.json()) as { + result: { github_org_verified?: boolean }; + }; + expect(created.result.github_org_verified).toBeUndefined(); + }); + it("falls back to unverified manual review when no matching GitHub org/user exists for the id", async () => { await seedUnownedDeveloper("legacy-developer"); mockGithubEntityNotFound(); @@ -4122,6 +4837,47 @@ describe("Extensions API v2", () => { }); }); + it.each([ + ["an impossible calendar day", "2099-02-30T00:00:00.000Z"], + ["an out-of-range hour", "2099-01-01T24:00:00.000Z"], + ["an out-of-range offset", "2099-01-01T00:00:00.000+24:00"] + ])( + "does not treat %s as usable organization evidence", + async (_description, github_orgs_expires_at) => { + const res = await put( + "/extensions/v2/users/me/identity", + await authHeaders("impossible-org-date"), + { + name: "Impossible Date", + email: "impossible-date@example.com", + email_verified: true, + picture: null, + github_login: "someone", + github_orgs: ["fossbilling"], + github_orgs_expires_at + } + ); + + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ + result: { github_linked: false } + }); + const row = await db + .prepare( + "SELECT github_orgs, github_orgs_expires_at FROM users WHERE id = ?" + ) + .bind("impossible-org-date") + .first<{ + github_orgs: string | null; + github_orgs_expires_at: string | null; + }>(); + expect(row).toEqual({ + github_orgs: null, + github_orgs_expires_at: null + }); + } + ); + it("tombstones and later reactivates an account", async () => { const headers = await authHeaders("delete-me"); const deleted = await del("/extensions/v2/users/me", headers);