diff --git a/src/services/extensions/v2/db/migrations/0009_drop_developer_history_fk.sql b/src/services/extensions/v2/db/migrations/0009_drop_developer_history_fk.sql new file mode 100644 index 0000000..f2e8e1b --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0009_drop_developer_history_fk.sql @@ -0,0 +1,47 @@ +-- v2: DELETE /developers/me needs to hard-delete a developers row while +-- deliberately keeping its developer_history rows (the audit trail must +-- survive profile deletion). developer_history.developer_id currently has a +-- hard FK to developers(id) with no ON DELETE action, so as soon as D1 +-- enforces foreign keys (it does, by default) that delete fails outright — +-- confirmed empirically against local D1 before writing this migration. +-- +-- SQLite can't ALTER a FK constraint away, so the table is rebuilt without +-- it. developer_id keeps its NOT NULL and its historical value once a +-- developer is deleted; it's just no longer a live, enforced reference. +-- Everything else (columns, the append-only triggers, the index) is +-- unchanged. Existing rows are preserved, re-inserted in original rowid +-- order so the "ORDER BY changed_at DESC, rowid DESC" tie-break in +-- listHistory keeps meaning what it always meant. + +CREATE TABLE developer_history_new ( + id TEXT PRIMARY KEY NOT NULL, + developer_id TEXT NOT NULL, + type TEXT NOT NULL, + name TEXT NOT NULL, + url TEXT, + changed_by TEXT NOT NULL REFERENCES users(id), + changed_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +INSERT INTO developer_history_new (id, developer_id, type, name, url, changed_by, changed_at) +SELECT id, developer_id, type, name, url, changed_by, changed_at +FROM developer_history +ORDER BY rowid; + +DROP TABLE developer_history; +ALTER TABLE developer_history_new RENAME TO developer_history; + +CREATE INDEX IF NOT EXISTS idx_developer_history_developer_changed_at + ON developer_history(developer_id, changed_at); + +CREATE TRIGGER IF NOT EXISTS trg_developer_history_no_update +BEFORE UPDATE ON developer_history +BEGIN + SELECT RAISE(ABORT, 'developer_history is append-only: rows cannot be updated'); +END; + +CREATE TRIGGER IF NOT EXISTS trg_developer_history_no_delete +BEFORE DELETE ON developer_history +BEGIN + SELECT RAISE(ABORT, 'developer_history is append-only: rows cannot be deleted'); +END; diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index 6c6c575..d858f8f 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -212,6 +212,188 @@ export class DevelopersDatabase { } } + // 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 + .prepare("SELECT owner_user_id FROM developers WHERE id = ?") + .bind(developerId) + .first<{ owner_user_id: string | null }>(); + + if (!developer || developer.owner_user_id !== userId) { + return { code: "NOT_FOUND", message: "Developer not found" }; + } + + const extensionCount = await this.db + .prepare("SELECT COUNT(*) AS count FROM extensions WHERE author_id = ?") + .bind(developerId) + .first<{ count: number }>(); + 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 + .prepare( + "SELECT COUNT(*) AS count FROM extension_submissions WHERE developer_id = ? AND status = 'pending'" + ) + .bind(developerId) + .first<{ count: number }>(); + 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 + .prepare("SELECT id FROM developers WHERE owner_user_id = ?") + .bind(userId) + .first<{ id: string }>(); + + if (!developer) { + return { + data: null, + error: { message: "Developer not found", code: "NOT_FOUND" } + }; + } + + if (!this.db.batch) { + return databaseError( + "deleteOwn", + new Error("Database adapter does not support batch operations") + ); + } + + // 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. + const deleteTransfersStmt = this.db + .prepare( + `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' + ) + )` + ) + .bind(developer.id, userId); + + const deleteClaimsStmt = this.db + .prepare( + `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' + ) + )` + ) + .bind(developer.id, userId); + + const deleteDeveloperStmt = this.db + .prepare( + `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' + )` + ) + .bind(developer.id, userId); + + let results; + try { + results = (await this.db.batch([ + deleteTransfersStmt, + deleteClaimsStmt, + deleteDeveloperStmt + ])) as Array<{ + success: boolean; + error?: string; + meta?: { changes?: number }; + }>; + } catch (error) { + return databaseError("deleteOwn", error); + } + + const failed = results.find((r) => !r.success); + if (failed) { + return databaseError( + "deleteOwn", + new Error(failed.error || "Database write failed") + ); + } + + 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 diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index 5cc5c45..4452656 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -591,6 +591,65 @@ extensionsV2.openapi(upsertOwnDeveloperRoute, async (c) => { return c.json({ result: data }, 200); }); +const deleteOwnDeveloperRoute = createRoute({ + method: "delete", + path: "/developers/me", + tags: ["Developers"], + summary: "Permanently delete the caller's own developer profile", + security: [{ Bearer: [] }], + middleware: [requireAuth()] as const, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ + result: z.object({ id: z.string(), deleted: z.literal(true) }) + }) + } + }, + description: "Profile deleted" + }, + 401: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Missing or invalid bearer token" + }, + 404: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Caller has no developer profile" + }, + 409: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: + "Profile still has published extensions, or has a pending submission awaiting review" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(deleteOwnDeveloperRoute, async (c) => { + const auth = getAuth(c); + const platform = getPlatform(c); + const db = new DevelopersDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.deleteOwn(auth.userId); + if (error || !data) { + return c.json( + { + error: { + message: error?.message ?? "Unable to delete developer profile", + code: error?.code ?? "DATABASE_ERROR" + } + }, + statusFromErrorCode(error?.code) + ); + } + + return c.json({ result: data }, 200); +}); + function statusFromOwnershipErrorCode(code?: string): 403 | 404 | 500 { if (code === "NOT_FOUND") return 404; if (code === "FORBIDDEN") return 403; diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index ee2f07d..dc18887 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -137,6 +137,13 @@ async function get(path: string, headers: Record) { return res; } +async function del(path: string, headers: Record) { + const ctx = createExecutionContext(); + const res = await app.request(path, { method: "DELETE", headers }, env, ctx); + await waitOnExecutionContext(ctx); + return res; +} + async function put( path: string, headers: Record, @@ -769,6 +776,154 @@ describe("Extensions API v2", () => { }); }); + describe("DELETE /developers/me", () => { + it("deletes a profile with no extensions or pending submissions", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { id: string; deleted: boolean }; + }; + expect(body.result).toEqual({ id: "dev-developer", deleted: true }); + + const getRes = await get("/extensions/v2/developers/dev-developer", {}); + expect(getRes.status).toBe(404); + }); + + it("404s for a caller with no developer profile", async () => { + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("no-profile-user") + ); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: { code: string } }; + expect(body.error.code).toBe("NOT_FOUND"); + }); + + it("409s when the profile still has published extensions", async () => { + seedOwnedExtension(); + + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("owner-1") + ); + expect(res.status).toBe(409); + const body = (await res.json()) as { + error: { code: string; message: string }; + }; + expect(body.error.code).toBe("CONFLICT"); + expect(body.error.message).toContain("1 published extension(s)"); + }); + + it("409s when a submission is pending", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + tables.extension_submissions.set("sub-1", { + id: "sub-1", + extension_id: null, + developer_id: "dev-developer", + submitted_by: "user-1", + status: "pending", + payload: JSON.stringify(samplePayload()), + reviewer_id: null, + review_note: null, + created_at: new Date().toISOString(), + reviewed_at: null + }); + + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("user-1") + ); + expect(res.status).toBe(409); + const body = (await res.json()) as { error: { code: string } }; + expect(body.error.code).toBe("CONFLICT"); + }); + + it("removes transfer tokens and claims but keeps history", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + await post( + "/extensions/v2/developers/dev-developer/transfer", + await authHeaders("user-1") + ); + tables.developer_claims.set("claim-1", { + id: "claim-1", + developer_id: "dev-developer", + claimant_id: "user-2", + status: "rejected", + note: null, + review_note: "no", + reviewer_id: "mod-1", + created_at: new Date().toISOString(), + reviewed_at: new Date().toISOString() + }); + + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + + expect( + [...tables.developer_transfers.values()].filter( + (r) => r.developer_id === "dev-developer" + ) + ).toHaveLength(0); + expect( + [...tables.developer_claims.values()].filter( + (r) => r.developer_id === "dev-developer" + ) + ).toHaveLength(0); + expect( + [...tables.developer_history.values()].filter( + (r) => r.developer_id === "dev-developer" + ).length + ).toBeGreaterThan(0); + }); + + it("refuses to delete if ownership moves away between the lookup and the delete", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + tables.users.set("user-2", { id: "user-2" }); + // Simulates a transfer/claim landing in the window between deleteOwn's + // initial "find my profile" lookup and its guarded delete — the + // delete must re-check ownership at that point, not trust the lookup. + tables.raceOwnerChangeTo = "user-2"; + + const res = await del( + "/extensions/v2/developers/me", + await authHeaders("user-1") + ); + expect(res.status).toBe(404); + + const stillThere = await get( + "/extensions/v2/developers/dev-developer", + {} + ); + expect(stillThere.status).toBe(200); + const body = (await stillThere.json()) as { result: { id: string } }; + expect(body.result.id).toBe("dev-developer"); + }); + }); + describe("developer moderation", () => { it("approves a developer and removes it from the unapproved list", async () => { await put( diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index a5e371e..b6ee40d 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -10,6 +10,11 @@ export interface MockTables { users: Map; // Test-only seam for simulating a write-through failure during approve(). forceExtensionWriteFailure?: boolean; + // Test-only seam for simulating a profile changing ownership in the + // window between deleteOwn()'s initial lookup and its guarded delete — + // fires (once) the first time that lookup runs, mutating the row so the + // guard sees a different owner than the caller who's mid-request. + raceOwnerChangeTo?: string; } export function createTables(): MockTables { @@ -66,6 +71,22 @@ class MockStatement implements D1PreparedStatement { return this.query.replace(/\s+/g, " ").trim(); } + private isEligibleForDeveloperDeletion( + developerId: string, + ownerUserId: unknown + ): boolean { + const developer = this.tables.developers.get(developerId); + if (!developer || developer.owner_user_id !== ownerUserId) return false; + const hasExtensions = [...this.tables.extensions.values()].some( + (r) => r.author_id === developerId + ); + if (hasExtensions) return false; + const hasPendingSubmission = [ + ...this.tables.extension_submissions.values() + ].some((r) => r.developer_id === developerId && r.status === "pending"); + return !hasPendingSubmission; + } + private execute(): Row[] { const q = this.normalizedQuery; const p = this.params; @@ -161,11 +182,44 @@ class MockStatement implements D1PreparedStatement { return row ? [row] : []; } + if ( + q.startsWith( + "SELECT COUNT(*) AS count FROM extensions WHERE author_id = ?" + ) + ) { + const count = [...this.tables.extensions.values()].filter( + (r) => r.author_id === p[0] + ).length; + return [{ count }]; + } + + if ( + q.startsWith( + "SELECT COUNT(*) AS count FROM extension_submissions WHERE developer_id = ? AND status = 'pending'" + ) + ) { + const count = [...this.tables.extension_submissions.values()].filter( + (r) => r.developer_id === p[0] && r.status === "pending" + ).length; + return [{ count }]; + } + if (q.startsWith("SELECT owner_user_id FROM developers WHERE id = ?")) { const row = this.tables.developers.get(String(p[0])); return row ? [row] : []; } + if (q.startsWith("SELECT id FROM developers WHERE owner_user_id = ?")) { + const row = [...this.tables.developers.values()].find( + (r) => r.owner_user_id === p[0] + ); + if (row && this.tables.raceOwnerChangeTo !== undefined) { + row.owner_user_id = this.tables.raceOwnerChangeTo; + this.tables.raceOwnerChangeTo = undefined; + } + return row ? [{ id: row.id }] : []; + } + if (q.startsWith("SELECT * FROM developers WHERE owner_user_id = ?")) { const row = [...this.tables.developers.values()].find( (r) => r.owner_user_id === p[0] @@ -772,6 +826,57 @@ class MockStatement implements D1PreparedStatement { return []; } + // These three mirror deleteOwn()'s guarded deletes: eligibility (still + // owned by the given user, no extensions, no pending submission) is + // re-checked per statement, same as the real correlated-subquery SQL, + // so a stale/raced caller sees all three affect zero rows here too. + if ( + q.startsWith("DELETE FROM developer_transfers WHERE developer_id = ?") + ) { + const [developer_id, owner_user_id] = p; + let changes = 0; + if ( + this.isEligibleForDeveloperDeletion(String(developer_id), owner_user_id) + ) { + for (const [key, row] of this.tables.developer_transfers) { + if (row.developer_id === developer_id) { + this.tables.developer_transfers.delete(key); + changes++; + } + } + } + this.changes = changes; + return []; + } + + if (q.startsWith("DELETE FROM developer_claims WHERE developer_id = ?")) { + const [developer_id, owner_user_id] = p; + let changes = 0; + if ( + this.isEligibleForDeveloperDeletion(String(developer_id), owner_user_id) + ) { + for (const [key, row] of this.tables.developer_claims) { + if (row.developer_id === developer_id) { + this.tables.developer_claims.delete(key); + changes++; + } + } + } + this.changes = changes; + return []; + } + + if (q.startsWith("DELETE FROM developers WHERE id = ?")) { + const [id, owner_user_id] = p; + const eligible = this.isEligibleForDeveloperDeletion( + String(id), + owner_user_id + ); + this.changes = + eligible && this.tables.developers.delete(String(id)) ? 1 : 0; + return []; + } + throw new Error(`MockStatement: unhandled query: ${q}`); }