From ed551366757051574bc2cfe60efb0347518c5550 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 30 Jul 2026 16:09:38 +0100 Subject: [PATCH 1/6] Reduce re-review friction for GitHub-verified developer profiles, add Publisher URL verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - upsertOwn no longer clears a profile's approval on every edit when it's currently GitHub org/user-verified, since that's an independently-computed identity signal strong enough to skip re-queuing for manual review. - New github_url_verified signal: cross-checks Publisher URL against GitHub's own on-file website for the org/user, computed at creation and re-checked via the owner's manual "Re-verify" action (POST /developers/me/reverify ?check_url=true) — never by the opportunistic per-login check, which stays GitHub-API-free by design. Claude-Session: https://claude.ai/code/session_014dzTSwUW78Qipx7sYH2dgD --- ...0016_add_developer_github_url_verified.sql | 1 + .../v2/db/migrations/meta/0016_snapshot.json | 946 ++++++++++++++++++ .../v2/db/migrations/meta/_journal.json | 7 + src/services/extensions/v2/db/schema.ts | 11 +- .../extensions/v2/developers-database.ts | 98 +- .../extensions/v2/github-verification.ts | 56 +- src/services/extensions/v2/index.ts | 10 +- src/services/extensions/v2/interfaces.ts | 19 + test/services/extensions/v2/db-fixtures.ts | 8 +- test/services/extensions/v2/index.test.ts | 186 +++- 10 files changed, 1307 insertions(+), 35 deletions(-) create mode 100644 src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql create mode 100644 src/services/extensions/v2/db/migrations/meta/0016_snapshot.json diff --git a/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql b/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql new file mode 100644 index 0000000..9adeb30 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql @@ -0,0 +1 @@ +ALTER TABLE developers ADD COLUMN github_url_verified INTEGER CHECK (github_url_verified IN (0, 1)); diff --git a/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json b/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json new file mode 100644 index 0000000..85e8b12 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json @@ -0,0 +1,946 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "c29f670e-ac1b-4862-8c5e-02e76da09f7c", + "prevId": "c53ca3ad-a28f-48ec-887e-5c0e8096f6c0", + "tables": { + "developer_claims": { + "name": "developer_claims", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "claimant_id": { + "name": "claimant_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "note": { + "name": "note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_claims_developer": { + "name": "idx_developer_claims_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_developer_claims_claimant": { + "name": "idx_developer_claims_claimant", + "columns": [ + "claimant_id" + ], + "isUnique": false + }, + "idx_developer_claims_pending_unique": { + "name": "idx_developer_claims_pending_unique", + "columns": [ + "developer_id", + "claimant_id" + ], + "isUnique": true, + "where": "\"developer_claims\".\"status\" = 'pending'" + }, + "idx_developer_claims_pending_queue": { + "name": "idx_developer_claims_pending_queue", + "columns": [ + "created_at" + ], + "isUnique": false, + "where": "\"developer_claims\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "developer_claims_developer_id_developers_id_fk": { + "name": "developer_claims_developer_id_developers_id_fk", + "tableFrom": "developer_claims", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_claimant_id_users_id_fk": { + "name": "developer_claims_claimant_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "claimant_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_reviewer_id_users_id_fk": { + "name": "developer_claims_reviewer_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developer_claims_status_check": { + "name": "developer_claims_status_check", + "value": "\"developer_claims\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "developer_claims_github_org_verified_check": { + "name": "developer_claims_github_org_verified_check", + "value": "\"developer_claims\".\"github_org_verified\" IN (0, 1)" + } + } + }, + "developer_history": { + "name": "developer_history", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "changed_by": { + "name": "changed_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "changed_at": { + "name": "changed_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + } + }, + "indexes": { + "idx_developer_history_developer_changed_at": { + "name": "idx_developer_history_developer_changed_at", + "columns": [ + "developer_id", + "changed_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developer_history_changed_by_users_id_fk": { + "name": "developer_history_changed_by_users_id_fk", + "tableFrom": "developer_history", + "tableTo": "users", + "columnsFrom": [ + "changed_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_transfers": { + "name": "developer_transfers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_by": { + "name": "created_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "expires_at": { + "name": "expires_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "accepted_by": { + "name": "accepted_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "accepted_at": { + "name": "accepted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_transfers_token": { + "name": "idx_developer_transfers_token", + "columns": [ + "token_hash" + ], + "isUnique": true + }, + "idx_developer_transfers_pending": { + "name": "idx_developer_transfers_pending", + "columns": [ + "developer_id" + ], + "isUnique": true, + "where": "\"developer_transfers\".\"accepted_at\" IS NULL AND \"developer_transfers\".\"revoked_at\" IS NULL" + } + }, + "foreignKeys": { + "developer_transfers_developer_id_developers_id_fk": { + "name": "developer_transfers_developer_id_developers_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_created_by_users_id_fk": { + "name": "developer_transfers_created_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "created_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_accepted_by_users_id_fk": { + "name": "developer_transfers_accepted_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "accepted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developers": { + "name": "developers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_at": { + "name": "approved_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "contact_email": { + "name": "contact_email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "content_revision": { + "name": "content_revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "approved_revision": { + "name": "approved_revision", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_by": { + "name": "approved_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verified_at": { + "name": "github_verified_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_url_verified": { + "name": "github_url_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developers_owner_unique": { + "name": "idx_developers_owner_unique", + "columns": [ + "owner_user_id" + ], + "isUnique": true + }, + "idx_developers_approved": { + "name": "idx_developers_approved", + "columns": [ + "approved_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developers_owner_user_id_users_id_fk": { + "name": "developers_owner_user_id_users_id_fk", + "tableFrom": "developers", + "tableTo": "users", + "columnsFrom": [ + "owner_user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developers_ownership_epoch_check": { + "name": "developers_ownership_epoch_check", + "value": "\"developers\".\"ownership_epoch\" >= 1" + }, + "developers_content_revision_check": { + "name": "developers_content_revision_check", + "value": "\"developers\".\"content_revision\" >= 1" + }, + "developers_github_org_verified_check": { + "name": "developers_github_org_verified_check", + "value": "\"developers\".\"github_org_verified\" IN (0, 1)" + }, + "developers_github_url_verified_check": { + "name": "developers_github_url_verified_check", + "value": "\"developers\".\"github_url_verified\" IN (0, 1)" + } + } + }, + "extension_submissions": { + "name": "extension_submissions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "extension_id": { + "name": "extension_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "submitted_by": { + "name": "submitted_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "payload": { + "name": "payload", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "target_key": { + "name": "target_key", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_submissions_status": { + "name": "idx_submissions_status", + "columns": [ + "status" + ], + "isUnique": false + }, + "idx_submissions_submitted_by": { + "name": "idx_submissions_submitted_by", + "columns": [ + "submitted_by" + ], + "isUnique": false + }, + "idx_submissions_developer": { + "name": "idx_submissions_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_submissions_extension": { + "name": "idx_submissions_extension", + "columns": [ + "extension_id" + ], + "isUnique": false + }, + "idx_extension_submissions_pending_target": { + "name": "idx_extension_submissions_pending_target", + "columns": [ + "target_key" + ], + "isUnique": true, + "where": "\"extension_submissions\".\"status\" = 'pending'" + }, + "idx_extension_submissions_submitter_page": { + "name": "idx_extension_submissions_submitter_page", + "columns": [ + "submitted_by", + "\"created_at\" desc", + "\"id\" desc" + ], + "isUnique": false + }, + "idx_extension_submissions_queue_page": { + "name": "idx_extension_submissions_queue_page", + "columns": [ + "status", + "created_at", + "id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "extension_submissions_extension_id_extensions_id_fk": { + "name": "extension_submissions_extension_id_extensions_id_fk", + "tableFrom": "extension_submissions", + "tableTo": "extensions", + "columnsFrom": [ + "extension_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_submissions_submitted_by_users_id_fk": { + "name": "extension_submissions_submitted_by_users_id_fk", + "tableFrom": "extension_submissions", + "tableTo": "users", + "columnsFrom": [ + "submitted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_submissions_reviewer_id_users_id_fk": { + "name": "extension_submissions_reviewer_id_users_id_fk", + "tableFrom": "extension_submissions", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extension_submissions_status_check": { + "name": "extension_submissions_status_check", + "value": "\"extension_submissions\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "extension_submissions_ownership_epoch_check": { + "name": "extension_submissions_ownership_epoch_check", + "value": "\"extension_submissions\".\"ownership_epoch\" >= 1" + } + } + }, + "extensions": { + "name": "extensions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "author_id": { + "name": "author_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "releases": { + "name": "releases", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "website": { + "name": "website", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "license": { + "name": "license", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "icon_url": { + "name": "icon_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "readme": { + "name": "readme", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "download_url": { + "name": "download_url", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_extensions_type": { + "name": "idx_extensions_type", + "columns": [ + "type" + ], + "isUnique": false + }, + "idx_extensions_author": { + "name": "idx_extensions_author", + "columns": [ + "author_id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "extensions_author_id_developers_id_fk": { + "name": "extensions_author_id_developers_id_fk", + "tableFrom": "extensions", + "tableTo": "developers", + "columnsFrom": [ + "author_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": { + "idx_extension_submissions_submitter_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + } + } + } +} \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/_journal.json b/src/services/extensions/v2/db/migrations/meta/_journal.json index 94f068e..2414352 100644 --- a/src/services/extensions/v2/db/migrations/meta/_journal.json +++ b/src/services/extensions/v2/db/migrations/meta/_journal.json @@ -15,6 +15,13 @@ "when": 1785398492298, "tag": "0015_add_developer_github_verified_at", "breakpoints": true + }, + { + "idx": 16, + "version": "6", + "when": 1785420033761, + "tag": "0016_add_developer_github_url_verified", + "breakpoints": true } ] } \ No newline at end of file diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index 29e2ec8..bad2732 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -85,7 +85,12 @@ export const developers = sqliteTable( // see DevelopersDatabase.reverifyOwn(). Left null/stale on an // inconclusive check (no linked GitHub identity), same as // githubOrgVerified itself. - githubVerifiedAt: text("github_verified_at") + githubVerifiedAt: text("github_verified_at"), + // Whether `url` matches GitHub's own on-file website — see + // github-verification.ts's urlMatchesGithubBlog(). Only ever 1 or null, + // never 0 (see the schema comment on interfaces.ts's + // DeveloperProfileSchema.github_url_verified for why). + githubUrlVerified: integer("github_url_verified") }, (table) => [ uniqueIndex("idx_developers_owner_unique").on(table.ownerUserId), @@ -101,6 +106,10 @@ export const developers = sqliteTable( check( "developers_github_org_verified_check", sql`${table.githubOrgVerified} IN (0, 1)` + ), + check( + "developers_github_url_verified_check", + sql`${table.githubUrlVerified} IN (0, 1)` ) ] ); diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index f7b527f..2db44ad 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -12,7 +12,11 @@ import { import { users as externalUsers } from "./db/external-tables"; import { databaseError, errorMessageChain } from "./errors"; import { toD1Statement } from "./d1-batch"; -import { checkGithubEntityType, matchesClaimant } from "./github-verification"; +import { + checkGithubEntity, + matchesClaimant, + urlMatchesGithubBlog +} from "./github-verification"; import { Developer, DeveloperClaim, @@ -84,7 +88,8 @@ function parseDeveloperRow(row: DeveloperRow): DeveloperProfile { ? undefined : row.githubOrgVerified === 1, github_verification_note: row.githubVerificationNote ?? undefined, - github_verified_at: row.githubVerifiedAt ?? undefined + github_verified_at: row.githubVerifiedAt ?? undefined, + github_url_verified: row.githubUrlVerified === 1 ? true : undefined }; } @@ -150,6 +155,7 @@ export class DevelopersDatabase { .where(eq(developers.id, developer.id)); let githubOrgVerified: number | null = null; + let githubUrlVerified: number | null = null; let githubVerificationNote: string | null = null; let mainStmt; @@ -172,7 +178,8 @@ export class DevelopersDatabase { developer.id, developer.type, userId, - githubToken + githubToken, + developer.URL ); if ("error" in check) { @@ -191,6 +198,7 @@ export class DevelopersDatabase { } githubOrgVerified = check.githubOrgVerified; + githubUrlVerified = check.githubUrlVerified; githubVerificationNote = check.note; mainStmt = this.db.insert(developers).values({ @@ -203,6 +211,7 @@ export class DevelopersDatabase { ownerUserId: userId, approvedAt: null, githubOrgVerified, + githubUrlVerified, githubVerificationNote, githubVerifiedAt: githubOrgVerified !== null ? sql`CURRENT_TIMESTAMP` : null, createdAt: sql`CURRENT_TIMESTAMP`, @@ -219,9 +228,20 @@ export class DevelopersDatabase { }; } - // approved_at is always cleared here, even if nothing meaningful + // 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 values. + // 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) 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. + const keepsApproval = existingOwn.githubOrgVerified === 1; + mainStmt = this.db .update(developers) .set({ @@ -231,9 +251,9 @@ export class DevelopersDatabase { avatarUrl: developer.avatar_url ?? null, contactEmail: developer.contact_email ?? null, contentRevision: sql`content_revision + 1`, - approvedAt: null, - approvedRevision: null, - approvedBy: null, + ...(keepsApproval + ? { approvedRevision: sql`content_revision + 1` } + : { approvedAt: null, approvedRevision: null, approvedBy: null }), updatedAt: sql`CURRENT_TIMESTAMP` }) .where( @@ -955,28 +975,39 @@ export class DevelopersDatabase { // 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 + githubToken?: string, + publisherUrl?: string ): Promise< | { mismatch: true } - | { mismatch: false; githubOrgVerified: number | null; note: string | null } + | { + mismatch: false; + githubOrgVerified: number | null; + githubUrlVerified: number | null; + note: string | null; + } | { error: DatabaseError } > { - const githubEntityType = await checkGithubEntityType( - developerId, - githubToken ?? "" - ); + const githubEntity = await checkGithubEntity(developerId, githubToken ?? ""); - if (githubEntityType === null) { + if (githubEntity === null) { // Also covers a failed lookup (rate limit, network, auth error) — - // checkGithubEntityType can't tell "confirmed absent" from "couldn't + // checkGithubEntity can't tell "confirmed absent" from "couldn't // check", so the note can't claim to know no matching entity exists. return { mismatch: false, githubOrgVerified: null, + githubUrlVerified: null, note: "GitHub entity was not verified automatically — reviewed manually." }; } @@ -986,7 +1017,7 @@ export class DevelopersDatabase { // 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 (githubEntityType !== developerType) { + if (githubEntity.type !== developerType) { return { mismatch: true }; } @@ -1010,6 +1041,7 @@ export class DevelopersDatabase { return { mismatch: false, githubOrgVerified: null, + githubUrlVerified: null, note: "Caller has no linked GitHub identity yet — reviewed manually." }; } @@ -1018,6 +1050,9 @@ export class DevelopersDatabase { return { mismatch: false, githubOrgVerified: 1, + githubUrlVerified: urlMatchesGithubBlog(publisherUrl, githubEntity.blog) + ? 1 + : null, note: "Verified: caller's linked GitHub identity matches." }; } @@ -1034,10 +1069,19 @@ export class DevelopersDatabase { // 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. - async reverifyOwn(userId: string): Promise> { + // 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 }) + .select({ id: developers.id, type: developers.type, url: developers.url }) .from(developers) .where(eq(developers.ownerUserId, userId)); if (!row) { @@ -1076,6 +1120,21 @@ export class DevelopersDatabase { 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. matches:false below clears + // any previously-set githubUrlVerified for the same reason. + let githubUrlVerified: number | null = null; + if (checkUrl && matches) { + const entity = await checkGithubEntity(row.id, githubToken ?? ""); + githubUrlVerified = urlMatchesGithubBlog( + row.url ?? undefined, + entity?.blog ?? null + ) + ? 1 + : null; + } + // 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 @@ -1085,6 +1144,7 @@ export class DevelopersDatabase { .update(developers) .set({ githubOrgVerified: matches ? 1 : 0, + ...(checkUrl ? { githubUrlVerified } : {}), githubVerificationNote: matches ? "Verified: caller's linked GitHub identity matches." : "No longer verified: caller's linked GitHub identity no longer matches.", diff --git a/src/services/extensions/v2/github-verification.ts b/src/services/extensions/v2/github-verification.ts index 81ab359..d94586a 100644 --- a/src/services/extensions/v2/github-verification.ts +++ b/src/services/extensions/v2/github-verification.ts @@ -10,23 +10,37 @@ import { GithubIdentity } from "./users-database"; // on DevelopersDatabase.claim() for the full decision matrix — this module // only answers the two underlying questions, it never decides to block. +export type GithubEntity = { + type: Developer["type"]; + // GitHub's own "website" field for this user/org (the same one shown on + // their profile page), or null if unset. Same endpoint as the type check + // below, so reading it costs nothing extra. + blog: string | null; +}; + // Returns the GitHub account "type" for `id` (translated to this app's -// user/organization vocabulary), or null if no such account exists — or if -// the lookup itself failed (auth error, rate limit, network issue, -// unexpected response shape). A failed lookup is indistinguishable from "no -// such account" here on purpose: both mean claim() can't verify anything and -// must fall back to manual moderator review, never block. -export async function checkGithubEntityType( +// user/organization vocabulary) plus its on-file website, or null if no such +// account exists — or if the lookup itself failed (auth error, rate limit, +// network issue, unexpected response shape). A failed lookup is +// indistinguishable from "no such account" here on purpose: both mean +// claim() can't verify anything and must fall back to manual moderator +// review, never block. +export async function checkGithubEntity( id: string, githubToken: string -): Promise { +): Promise { try { const result = await ghRequest("GET /users/{username}", { username: id, headers: githubToken ? { authorization: `Bearer ${githubToken}` } : {} }); - if (result.data.type === "Organization") return "organization"; - if (result.data.type === "User") return "user"; + const blog = result.data.blog?.trim() || null; + if (result.data.type === "Organization") { + return { type: "organization", blog }; + } + if (result.data.type === "User") { + return { type: "user", blog }; + } return null; } catch (error) { const githubError = classifyGitHubError( @@ -44,6 +58,30 @@ export async function checkGithubEntityType( } } +// Loose match on host + path, ignoring scheme/www/trailing slash — GitHub's +// own `blog` field is freeform text (often missing a scheme, e.g. +// "example.com"), so an exact string compare would miss real matches. +export function urlMatchesGithubBlog( + publisherUrl: string | undefined, + blog: string | null +): boolean { + if (!publisherUrl || !blog) return false; + const normalize = (value: string) => { + try { + const url = new URL(/^https?:\/\//i.test(value) ? value : `https://${value}`); + return `${url.hostname.replace(/^www\./i, "")}${url.pathname}`.replace( + /\/$/, + "" + ); + } catch { + return null; + } + }; + const a = normalize(publisherUrl); + const b = normalize(blog); + return a !== null && a.toLowerCase() === b?.toLowerCase(); +} + // developerId/claimant.githubOrgs are already lowercase (developerId() and // the auth service's org-membership sync both normalize to lowercase). export function matchesClaimant( diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index fe91cee..2b557ff 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -22,6 +22,7 @@ import { PaginationSchema, PublicDeveloperSchema, QueueQuerySchema, + ReverifyQuerySchema, ReviewNoteOptionalSchema, ReviewNoteRequiredSchema, SubmissionPayloadSchema, @@ -707,6 +708,7 @@ const reverifyOwnDeveloperRoute = createRoute({ "Re-check the caller's linked GitHub identity against their own developer profile", security: [{ Bearer: [] }], middleware: [requireAuth()] as const, + request: { query: ReverifyQuerySchema }, responses: { 200: { content: { @@ -737,9 +739,15 @@ const reverifyOwnDeveloperRoute = createRoute({ extensionsV2.openapi(reverifyOwnDeveloperRoute, async (c) => { const auth = getAuth(c); + const { check_url } = c.req.valid("query"); + const platform = getPlatform(c); const db = new DevelopersDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); - const { data, error } = await db.reverifyOwn(auth.userId); + const { data, error } = await db.reverifyOwn( + auth.userId, + check_url, + platform.getEnv("GITHUB_TOKEN") + ); if (error || !data) { const status = error?.code === "NOT_FOUND" diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 045939f..9ad4c33 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -155,6 +155,15 @@ export const DeveloperProfileSchema = DeveloperSchema.extend({ // Set whenever github_org_verified is last (re-)computed to a definitive // true/false — see reverifyOwn(). Absent/stale on an inconclusive check. github_verified_at: z.string().nullable().optional(), + // Whether Publisher URL matches GitHub's own on-file website for this + // org/user — see verifyGithubOwnership()'s urlMatchesGithubBlog() call. + // Only ever true or absent (never false): GitHub's website field is + // optional and often unset, so "doesn't match" isn't itself meaningful — + // there's nothing to flag, unlike github_org_verified's identity check. + // Computed at creation, and re-checked by the owner's own "Re-verify" + // action (never by the opportunistic per-login re-check, which stays + // 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 // producers (getById, create/update/claim/transfer results) don't join @@ -183,6 +192,7 @@ export const PublicDeveloperSchema = DeveloperProfileSchema.omit({ github_org_verified: true, github_verification_note: true, github_verified_at: true, + github_url_verified: true, unclaimed: true, owner_name: true, owner_github_login: true @@ -352,6 +362,15 @@ 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" +// button sets this, never the opportunistic per-login re-check. +export const ReverifyQuerySchema = z.object({ + check_url: z.coerce.boolean().optional().openapi({ + param: { name: "check_url", in: "query" } + }) +}); + export const QueueQuerySchema = z.object({ status: SubmissionStatusSchema.optional().openapi({ param: { name: "status", in: "query" } diff --git a/test/services/extensions/v2/db-fixtures.ts b/test/services/extensions/v2/db-fixtures.ts index 67d9a02..fb2edbc 100644 --- a/test/services/extensions/v2/db-fixtures.ts +++ b/test/services/extensions/v2/db-fixtures.ts @@ -22,6 +22,7 @@ export interface DeveloperRow { github_org_verified: number | null; github_verification_note: string | null; github_verified_at: string | null; + github_url_verified: number | null; } export interface ExtensionRow { @@ -193,8 +194,8 @@ export async function insertDeveloper( (id, type, name, url, owner_user_id, approved_at, created_at, updated_at, avatar_url, contact_email, ownership_epoch, content_revision, approved_revision, approved_by, github_org_verified, github_verification_note, - github_verified_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + github_verified_at, github_url_verified) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` ) .bind( row.id, @@ -213,7 +214,8 @@ export async function insertDeveloper( row.approved_by ?? null, row.github_org_verified ?? null, row.github_verification_note ?? null, - row.github_verified_at ?? null + row.github_verified_at ?? null, + row.github_url_verified ?? null ) .run(); } diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index f47f2a1..b2e5fb6 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -66,9 +66,12 @@ function mockGithubEntityNotFound(): void { }); } -function mockGithubEntity(type: "User" | "Organization"): void { +function mockGithubEntity( + type: "User" | "Organization", + blog?: string +): void { (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation(async () => ({ - data: { type } + data: { type, blog } })); } @@ -978,6 +981,63 @@ describe("Extensions API v2", () => { expect(created.result.github_org_verified).toBe(true); }); + it("verifies the Publisher URL when it matches GitHub's on-file website", async () => { + mockGithubEntity("Organization", "https://www.acme.example/"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["acme-org"]) + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { + id: "acme-org", + type: "organization", + name: "Acme Org", + URL: "https://acme.example" + } + ); + + expect(res.status).toBe(200); + const created = (await res.json()) as { + result: { github_org_verified?: boolean; github_url_verified?: boolean }; + }; + expect(created.result.github_org_verified).toBe(true); + expect(created.result.github_url_verified).toBe(true); + const stored = await getDeveloper(db, "acme-org"); + expect(stored?.github_url_verified).toBe(1); + }); + + it("doesn't claim a URL match when GitHub's website field differs", async () => { + mockGithubEntity("Organization", "https://other.example"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["acme-org"]) + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { + id: "acme-org", + type: "organization", + name: "Acme Org", + URL: "https://acme.example" + } + ); + + expect(res.status).toBe(200); + const created = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(created.result.github_url_verified).toBeUndefined(); + const stored = await getDeveloper(db, "acme-org"); + expect(stored?.github_url_verified).toBeNull(); + }); + it("blocks creating a profile whose id matches a real GitHub org/user the creator doesn't control", async () => { mockGithubEntity("Organization"); await insertUser(db, { @@ -1096,6 +1156,45 @@ describe("Extensions API v2", () => { expect(data.result.approved).toBe(false); }); + it("keeps approval when a GitHub-verified profile is edited", async () => { + mockGithubEntity("User"); + await insertUser(db, { + id: "user-1", + github_login: "dev-developer", + github_orgs: JSON.stringify([]) + }); + const created = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + const createdBody = (await created.json()) as { + result: { github_org_verified?: boolean }; + }; + expect(createdBody.result.github_org_verified).toBe(true); + + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + const approved = await post( + "/extensions/v2/developers/dev-developer/approve", + await authHeaders("mod-1"), + { expected_revision: 1 } + ); + expect(approved.status).toBe(200); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper({ name: "Edited Again" }) + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { approved: boolean; github_org_verified?: boolean }; + }; + expect(data.result.approved).toBe(true); + expect(data.result.github_org_verified).toBe(true); + }); + it("does not update a profile after ownership changes mid-request", async () => { await put( "/extensions/v2/developers/me", @@ -1451,6 +1550,89 @@ describe("Extensions API v2", () => { expect(body.result.github_org_verified).toBe(true); }); + it("doesn't check the Publisher URL without ?check_url=true", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + 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_url_verified?: boolean }; + }; + expect(body.result.github_url_verified).toBeUndefined(); + expect(ghRequest).not.toHaveBeenCalled(); + }); + + it("checks the Publisher URL when re-verified with ?check_url=true", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(body.result.github_url_verified).toBe(true); + }); + + it("clears a previously-verified Publisher URL when identity no longer matches", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1", + github_org_verified: 1, + github_url_verified: 1 + }); + // No longer a member of dev-developer's org. + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify([]) + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_org_verified?: boolean; github_url_verified?: boolean }; + }; + expect(body.result.github_org_verified).toBe(false); + expect(body.result.github_url_verified).toBeUndefined(); + }); + it("404s when the caller doesn't own a developer profile", async () => { const res = await post( "/extensions/v2/developers/me/reverify", From d80a6ed19a753c0cdb7fabd701ceb0d796f051dd Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 30 Jul 2026 16:32:37 +0100 Subject: [PATCH 2/6] Address review feedback: invalidate stale verification signals, fix reverify edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - upsertOwn now invalidates GitHub verification (and approval) when a profile's type changes, and clears github_url_verified when the Publisher URL changes — previously both were left stale after such edits. - reverifyOwn's manual URL re-check now validates the returned GitHub entity's type against the profile's own type, preserves an existing github_url_verified on a transient lookup failure instead of clearing it, and clears github_url_verified whenever identity no longer matches even without ?check_url (cheap — no extra API call). - ReverifyQuerySchema no longer uses z.coerce.boolean() for check_url, which coerced the string "false" to true. - The github_url_verified CHECK constraint now enforces "= 1" (only ever 1 or NULL) rather than "IN (0, 1)", matching its documented contract. - urlMatchesGithubBlog() now preserves port in the host comparison and only lowercases the host, not the path (URL paths are case-sensitive). Claude-Session: https://claude.ai/code/session_014dzTSwUW78Qipx7sYH2dgD --- ...0016_add_developer_github_url_verified.sql | 2 +- .../v2/db/migrations/meta/0016_snapshot.json | 4 +- .../v2/db/migrations/meta/_journal.json | 2 +- src/services/extensions/v2/db/schema.ts | 6 +- .../extensions/v2/developers-database.ts | 62 +++- .../extensions/v2/github-verification.ts | 12 +- src/services/extensions/v2/interfaces.ts | 13 +- test/services/extensions/v2/index.test.ts | 267 ++++++++++++++++++ 8 files changed, 339 insertions(+), 29 deletions(-) diff --git a/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql b/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql index 9adeb30..c207b17 100644 --- a/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql +++ b/src/services/extensions/v2/db/migrations/0016_add_developer_github_url_verified.sql @@ -1 +1 @@ -ALTER TABLE developers ADD COLUMN github_url_verified INTEGER CHECK (github_url_verified IN (0, 1)); +ALTER TABLE developers ADD COLUMN github_url_verified INTEGER CHECK (github_url_verified = 1); diff --git a/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json b/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json index 85e8b12..15364f9 100644 --- a/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json +++ b/src/services/extensions/v2/db/migrations/meta/0016_snapshot.json @@ -1,7 +1,7 @@ { "version": "6", "dialect": "sqlite", - "id": "c29f670e-ac1b-4862-8c5e-02e76da09f7c", + "id": "8586d529-d397-4f43-a85b-512b5cec826a", "prevId": "c53ca3ad-a28f-48ec-887e-5c0e8096f6c0", "tables": { "developer_claims": { @@ -569,7 +569,7 @@ }, "developers_github_url_verified_check": { "name": "developers_github_url_verified_check", - "value": "\"developers\".\"github_url_verified\" IN (0, 1)" + "value": "\"developers\".\"github_url_verified\" = 1" } } }, diff --git a/src/services/extensions/v2/db/migrations/meta/_journal.json b/src/services/extensions/v2/db/migrations/meta/_journal.json index 2414352..687be2a 100644 --- a/src/services/extensions/v2/db/migrations/meta/_journal.json +++ b/src/services/extensions/v2/db/migrations/meta/_journal.json @@ -19,7 +19,7 @@ { "idx": 16, "version": "6", - "when": 1785420033761, + "when": 1785425233853, "tag": "0016_add_developer_github_url_verified", "breakpoints": true } diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index bad2732..d184c8c 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -107,9 +107,13 @@ export const developers = sqliteTable( "developers_github_org_verified_check", sql`${table.githubOrgVerified} IN (0, 1)` ), + // = 1 rather than IN (0, 1): this column is documented to only ever be + // 1 or NULL, never 0 (see interfaces.ts's DeveloperProfileSchema + // comment) — SQLite's CHECK already treats NULL as satisfying `= 1`, so + // this enforces that invariant instead of just validating it's a 0/1. check( "developers_github_url_verified_check", - sql`${table.githubUrlVerified} IN (0, 1)` + sql`${table.githubUrlVerified} = 1` ) ] ); diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index 2db44ad..b85ed87 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -234,13 +234,25 @@ export class DevelopersDatabase { // 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) strong enough on its own - // that re-queuing for manual review on every edit isn't worth the - // moderator load. + // 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. - const keepsApproval = existingOwn.githubOrgVerified === 1; + // + // 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; mainStmt = this.db .update(developers) @@ -254,6 +266,16 @@ export class DevelopersDatabase { ...(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( @@ -1122,17 +1144,29 @@ export class DevelopersDatabase { // 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. matches:false below clears - // any previously-set githubUrlVerified for the same reason. + // 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; - if (checkUrl && matches) { + let writeUrlVerified = false; + if (!matches) { + writeUrlVerified = true; + } else if (checkUrl) { const entity = await checkGithubEntity(row.id, githubToken ?? ""); - githubUrlVerified = urlMatchesGithubBlog( - row.url ?? undefined, - entity?.blog ?? null - ) - ? 1 - : null; + // A failed lookup (rate limit, network, auth error) is inconclusive, + // not a disproof — checkGithubEntity can't tell the two apart (see + // its own docstring) — so it leaves the stored signal untouched + // rather than clearing a real prior verification over a transient + // failure. Only a successful lookup, of the entity type the profile + // itself claims, gets to overwrite it. + if (entity) { + writeUrlVerified = true; + githubUrlVerified = + entity.type === row.type && + urlMatchesGithubBlog(row.url ?? undefined, entity.blog) + ? 1 + : null; + } } // Re-asserts ownership in the write itself (not just the lookup @@ -1144,7 +1178,7 @@ export class DevelopersDatabase { .update(developers) .set({ githubOrgVerified: matches ? 1 : 0, - ...(checkUrl ? { githubUrlVerified } : {}), + ...(writeUrlVerified ? { githubUrlVerified } : {}), githubVerificationNote: matches ? "Verified: caller's linked GitHub identity matches." : "No longer verified: caller's linked GitHub identity no longer matches.", diff --git a/src/services/extensions/v2/github-verification.ts b/src/services/extensions/v2/github-verification.ts index d94586a..9ad14be 100644 --- a/src/services/extensions/v2/github-verification.ts +++ b/src/services/extensions/v2/github-verification.ts @@ -60,7 +60,9 @@ export async function checkGithubEntity( // Loose match on host + path, ignoring scheme/www/trailing slash — GitHub's // own `blog` field is freeform text (often missing a scheme, e.g. -// "example.com"), so an exact string compare would miss real matches. +// "example.com"), so an exact string compare would miss real matches. Host +// is compared case-insensitively (including port, since a non-default port +// is a different site) and path case-sensitively, per URL semantics. export function urlMatchesGithubBlog( publisherUrl: string | undefined, blog: string | null @@ -69,17 +71,15 @@ export function urlMatchesGithubBlog( const normalize = (value: string) => { try { const url = new URL(/^https?:\/\//i.test(value) ? value : `https://${value}`); - return `${url.hostname.replace(/^www\./i, "")}${url.pathname}`.replace( - /\/$/, - "" - ); + const host = url.host.replace(/^www\./i, "").toLowerCase(); + return `${host}${url.pathname}`.replace(/\/$/, ""); } catch { return null; } }; const a = normalize(publisherUrl); const b = normalize(blog); - return a !== null && a.toLowerCase() === b?.toLowerCase(); + return a !== null && a === b; } // developerId/claimant.githubOrgs are already lowercase (developerId() and diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 9ad4c33..21ac41f 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -364,11 +364,16 @@ export const ClaimNoteSchema = z // check_url — opt-in because it costs an extra GitHub API call (see // DevelopersDatabase.reverifyOwn()); only the owner's own manual "Re-verify" -// button sets this, never the opportunistic per-login re-check. +// 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({ - check_url: z.coerce.boolean().optional().openapi({ - param: { name: "check_url", in: "query" } - }) + check_url: z + .enum(["true", "false"]) + .optional() + .transform((value) => value === "true") + .openapi({ + param: { name: "check_url", in: "query" } + }) }); export const QueueQuerySchema = z.object({ diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index b2e5fb6..53e9a33 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -1038,6 +1038,81 @@ describe("Extensions API v2", () => { expect(stored?.github_url_verified).toBeNull(); }); + it("matches the Publisher URL to GitHub's website ignoring scheme/www/trailing slash", async () => { + mockGithubEntity("Organization", "www.acme.example/"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["acme-org"]) + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { + id: "acme-org", + type: "organization", + name: "Acme Org", + URL: "http://acme.example/" + } + ); + + const created = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(created.result.github_url_verified).toBe(true); + }); + + it("doesn't match Publisher URLs that only differ by port", async () => { + mockGithubEntity("Organization", "https://acme.example:8443"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["acme-org"]) + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { + id: "acme-org", + type: "organization", + name: "Acme Org", + URL: "https://acme.example" + } + ); + + const created = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(created.result.github_url_verified).toBeUndefined(); + }); + + it("doesn't match Publisher URLs that only differ by path case", async () => { + mockGithubEntity("Organization", "https://acme.example/Docs"); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["acme-org"]) + }); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { + id: "acme-org", + type: "organization", + name: "Acme Org", + URL: "https://acme.example/docs" + } + ); + + const created = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(created.result.github_url_verified).toBeUndefined(); + }); + it("blocks creating a profile whose id matches a real GitHub org/user the creator doesn't control", async () => { mockGithubEntity("Organization"); await insertUser(db, { @@ -1195,6 +1270,81 @@ describe("Extensions API v2", () => { expect(data.result.github_org_verified).toBe(true); }); + it("clears approval and GitHub verification when the profile type is changed", async () => { + mockGithubEntity("User"); + await insertUser(db, { + id: "user-1", + github_login: "dev-developer", + github_orgs: JSON.stringify([]) + }); + const created = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + const createdBody = (await created.json()) as { + result: { github_org_verified?: boolean }; + }; + expect(createdBody.result.github_org_verified).toBe(true); + + await insertUser(db, { id: "mod-1", is_moderator: 1 }); + await post( + "/extensions/v2/developers/dev-developer/approve", + await authHeaders("mod-1"), + { expected_revision: 1 } + ); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { ...sampleDeveloper(), type: "organization" } + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { + approved: boolean; + github_org_verified?: boolean; + github_url_verified?: boolean; + }; + }; + expect(data.result.approved).toBe(false); + expect(data.result.github_org_verified).toBeUndefined(); + expect(data.result.github_url_verified).toBeUndefined(); + }); + + it("clears github_url_verified when the Publisher URL is edited, but keeps identity verification", async () => { + mockGithubEntity("User", "https://acme.example"); + await insertUser(db, { + id: "user-1", + github_login: "dev-developer", + github_orgs: JSON.stringify([]) + }); + const created = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { ...sampleDeveloper(), URL: "https://acme.example" } + ); + const createdBody = (await created.json()) as { + result: { github_org_verified?: boolean; github_url_verified?: boolean }; + }; + expect(createdBody.result.github_org_verified).toBe(true); + expect(createdBody.result.github_url_verified).toBe(true); + + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { ...sampleDeveloper(), URL: "https://different.example" } + ); + + expect(res.status).toBe(200); + const data = (await res.json()) as { + result: { github_org_verified?: boolean; github_url_verified?: boolean }; + }; + expect(data.result.github_org_verified).toBe(true); + expect(data.result.github_url_verified).toBeUndefined(); + }); + it("does not update a profile after ownership changes mid-request", async () => { await put( "/extensions/v2/developers/me", @@ -1633,6 +1783,123 @@ describe("Extensions API v2", () => { expect(body.result.github_url_verified).toBeUndefined(); }); + it("clears a previously-verified Publisher URL when identity no longer matches, even without ?check_url", async () => { + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1", + github_org_verified: 1, + github_url_verified: 1 + }); + // No longer a member of dev-developer's org. + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify([]) + }); + + 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_url_verified?: boolean }; + }; + expect(body.result.github_org_verified).toBe(false); + expect(body.result.github_url_verified).toBeUndefined(); + // Clearing a stale URL signal on an identity mismatch is a local + // comparison, same as the identity check itself — no GitHub API call. + expect(ghRequest).not.toHaveBeenCalled(); + }); + + it("doesn't verify the Publisher URL against a GitHub entity of the wrong type", async () => { + // The stored profile is a "user", but the GitHub entity currently + // found for this id is an "organization" — matchesClaimant() only + // compares login/org membership, so this discrepancy has to be + // caught separately before trusting the entity's blog field. + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "user", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "dev-developer", + github_orgs: JSON.stringify([]) + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(body.result.github_url_verified).toBeUndefined(); + }); + + it("preserves an existing Publisher URL verification when the GitHub lookup fails", async () => { + mockGithubEntityNotFound(); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1", + github_org_verified: 1, + github_url_verified: 1 + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(body.result.github_url_verified).toBe(true); + }); + + it("treats ?check_url=false the same as omitting it", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=false", + await authHeaders("user-1") + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { github_url_verified?: boolean }; + }; + expect(body.result.github_url_verified).toBeUndefined(); + expect(ghRequest).not.toHaveBeenCalled(); + }); + it("404s when the caller doesn't own a developer profile", async () => { const res = await post( "/extensions/v2/developers/me/reverify", From 2d7fa4151750704766e681d795971cb448661834 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 30 Jul 2026 16:45:47 +0100 Subject: [PATCH 3/6] Rate-limit the manual URL re-verify to protect the shared GitHub API budget POST /developers/me/reverify?check_url=true now returns 429/RATE_LIMITED if called again within 60 seconds by the same caller, using CACHE_KV as a per-user cooldown. The opportunistic per-login reverify (no check_url) is unaffected since it never makes a GitHub API call. Claude-Session: https://claude.ai/code/session_014dzTSwUW78Qipx7sYH2dgD --- src/services/extensions/v2/index.ts | 32 +++++++ test/services/extensions/v2/index.test.ts | 104 ++++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index 2b557ff..fe13b84 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -730,6 +730,10 @@ const reverifyOwnDeveloperRoute = createRoute({ content: { "application/json": { schema: ErrorResponseSchema } }, description: "Developer ownership changed while re-verifying" }, + 429: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "check_url was used again too soon after a previous check" + }, 500: { content: { "application/json": { schema: ErrorResponseSchema } }, description: "Database error" @@ -737,12 +741,40 @@ const reverifyOwnDeveloperRoute = createRoute({ } }); +// How often the owner's own manual "Re-verify" button is allowed to spend +// an extra GitHub API call on the URL check — 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. The opportunistic per-login path never +// hits this since it never sets check_url. +const CHECK_URL_COOLDOWN_SECONDS = 60; + extensionsV2.openapi(reverifyOwnDeveloperRoute, async (c) => { const auth = getAuth(c); const { check_url } = c.req.valid("query"); const platform = getPlatform(c); const db = new DevelopersDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); + if (check_url) { + const cache = platform.getCache("CACHE_KV"); + const rateLimitKey = `reverify-url-check:${auth.userId}`; + if (await cache.get(rateLimitKey)) { + return c.json( + { + error: { + message: + "Please wait a minute before re-checking your Publisher URL again.", + code: "RATE_LIMITED" + } + }, + 429 + ); + } + await cache.put(rateLimitKey, "1", { + expirationTtl: CHECK_URL_COOLDOWN_SECONDS + }); + } + const { data, error } = await db.reverifyOwn( auth.userId, check_url, diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 53e9a33..24bce4c 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -91,6 +91,12 @@ beforeEach(async () => { await resetExtensionsDb(db); vi.clearAllMocks(); mockGithubEntityNotFound(); + + // check_url's rate-limit key (see reverifyOwnDeveloperRoute) would + // otherwise leak across tests that reuse "user-1" — CACHE_KV isn't reset + // by resetExtensionsDb since it isn't part of the D1 database. + const { keys } = await env.CACHE_KV.list(); + await Promise.all(keys.map((key) => env.CACHE_KV.delete(key.name))); }); afterEach(() => { @@ -1753,6 +1759,104 @@ describe("Extensions API v2", () => { expect(body.result.github_url_verified).toBe(true); }); + it("rate-limits repeated ?check_url=true calls from the same caller", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + const first = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(first.status).toBe(200); + vi.clearAllMocks(); + + const second = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(second.status).toBe(429); + const body = (await second.json()) as { error: { code: string } }; + expect(body.error.code).toBe("RATE_LIMITED"); + // The whole point — no GitHub API call for the blocked attempt. + expect(ghRequest).not.toHaveBeenCalled(); + }); + + it("doesn't rate-limit reverify calls that don't use ?check_url", async () => { + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + const first = await post( + "/extensions/v2/developers/me/reverify", + await authHeaders("user-1") + ); + const second = await post( + "/extensions/v2/developers/me/reverify", + await authHeaders("user-1") + ); + expect(first.status).toBe(200); + expect(second.status).toBe(200); + }); + + it("rate-limits ?check_url=true per caller, not globally", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertDeveloper(db, { + id: "other-developer", + type: "organization", + name: "Other", + url: "https://acme.example", + owner_user_id: "user-2" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + await insertUser(db, { + id: "user-2", + github_login: "someone-else", + github_orgs: JSON.stringify(["other-developer"]) + }); + + const first = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + const second = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-2") + ); + expect(first.status).toBe(200); + expect(second.status).toBe(200); + }); + it("clears a previously-verified Publisher URL when identity no longer matches", async () => { mockGithubEntity("Organization", "https://acme.example"); await insertDeveloper(db, { From 120453cc0002db097909c54d6e53361b40e63526 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 30 Jul 2026 17:16:17 +0100 Subject: [PATCH 4/6] Harden reverify's URL re-check: atomic rate limiting, stale-URL and type-mismatch guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rate limiting moved from a KV get-then-put (racy — two concurrent ?check_url=true requests could both pass) to an atomic conditional UPDATE on the developer's own row, gated by a new url_check_cooldown_until column. Reuses the row already being read/written in this function instead of adding a new store. - reverifyOwn's final write now also re-asserts the Publisher URL hasn't changed since it was checked, same pattern as the existing ownership re-assertion — otherwise a concurrent edit could let a stale URL comparison get persisted as if it described the new URL. - When a manual URL re-check finds GitHub's current entity type no longer matches the profile's own type, it now downgrades github_org_verified too, not just github_url_verified — matchesClaimant() alone never queries GitHub's actual current type, so this discrepancy is the only place that catches it post-creation. Claude-Session: https://claude.ai/code/session_014dzTSwUW78Qipx7sYH2dgD --- .../0017_add_developer_url_check_cooldown.sql | 1 + .../v2/db/migrations/meta/0017_snapshot.json | 953 ++++++++++++++++++ .../v2/db/migrations/meta/_journal.json | 7 + src/services/extensions/v2/db/schema.ts | 7 +- .../extensions/v2/developers-database.ts | 94 +- src/services/extensions/v2/index.ts | 32 +- test/services/extensions/v2/index.test.ts | 87 +- 7 files changed, 1132 insertions(+), 49 deletions(-) create mode 100644 src/services/extensions/v2/db/migrations/0017_add_developer_url_check_cooldown.sql create mode 100644 src/services/extensions/v2/db/migrations/meta/0017_snapshot.json diff --git a/src/services/extensions/v2/db/migrations/0017_add_developer_url_check_cooldown.sql b/src/services/extensions/v2/db/migrations/0017_add_developer_url_check_cooldown.sql new file mode 100644 index 0000000..8210e90 --- /dev/null +++ b/src/services/extensions/v2/db/migrations/0017_add_developer_url_check_cooldown.sql @@ -0,0 +1 @@ +ALTER TABLE `developers` ADD `url_check_cooldown_until` text; \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/0017_snapshot.json b/src/services/extensions/v2/db/migrations/meta/0017_snapshot.json new file mode 100644 index 0000000..29bd8ef --- /dev/null +++ b/src/services/extensions/v2/db/migrations/meta/0017_snapshot.json @@ -0,0 +1,953 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "373af293-9e74-4b73-bf23-707f14c9b43c", + "prevId": "8586d529-d397-4f43-a85b-512b5cec826a", + "tables": { + "developer_claims": { + "name": "developer_claims", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "claimant_id": { + "name": "claimant_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "note": { + "name": "note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_claims_developer": { + "name": "idx_developer_claims_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_developer_claims_claimant": { + "name": "idx_developer_claims_claimant", + "columns": [ + "claimant_id" + ], + "isUnique": false + }, + "idx_developer_claims_pending_unique": { + "name": "idx_developer_claims_pending_unique", + "columns": [ + "developer_id", + "claimant_id" + ], + "isUnique": true, + "where": "\"developer_claims\".\"status\" = 'pending'" + }, + "idx_developer_claims_pending_queue": { + "name": "idx_developer_claims_pending_queue", + "columns": [ + "created_at" + ], + "isUnique": false, + "where": "\"developer_claims\".\"status\" = 'pending'" + } + }, + "foreignKeys": { + "developer_claims_developer_id_developers_id_fk": { + "name": "developer_claims_developer_id_developers_id_fk", + "tableFrom": "developer_claims", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_claimant_id_users_id_fk": { + "name": "developer_claims_claimant_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "claimant_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_claims_reviewer_id_users_id_fk": { + "name": "developer_claims_reviewer_id_users_id_fk", + "tableFrom": "developer_claims", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developer_claims_status_check": { + "name": "developer_claims_status_check", + "value": "\"developer_claims\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "developer_claims_github_org_verified_check": { + "name": "developer_claims_github_org_verified_check", + "value": "\"developer_claims\".\"github_org_verified\" IN (0, 1)" + } + } + }, + "developer_history": { + "name": "developer_history", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "changed_by": { + "name": "changed_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "changed_at": { + "name": "changed_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + } + }, + "indexes": { + "idx_developer_history_developer_changed_at": { + "name": "idx_developer_history_developer_changed_at", + "columns": [ + "developer_id", + "changed_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developer_history_changed_by_users_id_fk": { + "name": "developer_history_changed_by_users_id_fk", + "tableFrom": "developer_history", + "tableTo": "users", + "columnsFrom": [ + "changed_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developer_transfers": { + "name": "developer_transfers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_by": { + "name": "created_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "expires_at": { + "name": "expires_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "accepted_by": { + "name": "accepted_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "accepted_at": { + "name": "accepted_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developer_transfers_token": { + "name": "idx_developer_transfers_token", + "columns": [ + "token_hash" + ], + "isUnique": true + }, + "idx_developer_transfers_pending": { + "name": "idx_developer_transfers_pending", + "columns": [ + "developer_id" + ], + "isUnique": true, + "where": "\"developer_transfers\".\"accepted_at\" IS NULL AND \"developer_transfers\".\"revoked_at\" IS NULL" + } + }, + "foreignKeys": { + "developer_transfers_developer_id_developers_id_fk": { + "name": "developer_transfers_developer_id_developers_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "developers", + "columnsFrom": [ + "developer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_created_by_users_id_fk": { + "name": "developer_transfers_created_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "created_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "developer_transfers_accepted_by_users_id_fk": { + "name": "developer_transfers_accepted_by_users_id_fk", + "tableFrom": "developer_transfers", + "tableTo": "users", + "columnsFrom": [ + "accepted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "developers": { + "name": "developers", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_at": { + "name": "approved_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'1970-01-01T00:00:00.000Z'" + }, + "avatar_url": { + "name": "avatar_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "contact_email": { + "name": "contact_email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "content_revision": { + "name": "content_revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "approved_revision": { + "name": "approved_revision", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "approved_by": { + "name": "approved_by", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_org_verified": { + "name": "github_org_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verification_note": { + "name": "github_verification_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_verified_at": { + "name": "github_verified_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "github_url_verified": { + "name": "github_url_verified", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "url_check_cooldown_until": { + "name": "url_check_cooldown_until", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_developers_owner_unique": { + "name": "idx_developers_owner_unique", + "columns": [ + "owner_user_id" + ], + "isUnique": true + }, + "idx_developers_approved": { + "name": "idx_developers_approved", + "columns": [ + "approved_at" + ], + "isUnique": false + } + }, + "foreignKeys": { + "developers_owner_user_id_users_id_fk": { + "name": "developers_owner_user_id_users_id_fk", + "tableFrom": "developers", + "tableTo": "users", + "columnsFrom": [ + "owner_user_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "developers_ownership_epoch_check": { + "name": "developers_ownership_epoch_check", + "value": "\"developers\".\"ownership_epoch\" >= 1" + }, + "developers_content_revision_check": { + "name": "developers_content_revision_check", + "value": "\"developers\".\"content_revision\" >= 1" + }, + "developers_github_org_verified_check": { + "name": "developers_github_org_verified_check", + "value": "\"developers\".\"github_org_verified\" IN (0, 1)" + }, + "developers_github_url_verified_check": { + "name": "developers_github_url_verified_check", + "value": "\"developers\".\"github_url_verified\" = 1" + } + } + }, + "extension_submissions": { + "name": "extension_submissions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "extension_id": { + "name": "extension_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "developer_id": { + "name": "developer_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "submitted_by": { + "name": "submitted_by", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "payload": { + "name": "payload", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "reviewer_id": { + "name": "reviewer_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "review_note": { + "name": "review_note", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "CURRENT_TIMESTAMP" + }, + "reviewed_at": { + "name": "reviewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "ownership_epoch": { + "name": "ownership_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "target_key": { + "name": "target_key", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "idx_submissions_status": { + "name": "idx_submissions_status", + "columns": [ + "status" + ], + "isUnique": false + }, + "idx_submissions_submitted_by": { + "name": "idx_submissions_submitted_by", + "columns": [ + "submitted_by" + ], + "isUnique": false + }, + "idx_submissions_developer": { + "name": "idx_submissions_developer", + "columns": [ + "developer_id" + ], + "isUnique": false + }, + "idx_submissions_extension": { + "name": "idx_submissions_extension", + "columns": [ + "extension_id" + ], + "isUnique": false + }, + "idx_extension_submissions_pending_target": { + "name": "idx_extension_submissions_pending_target", + "columns": [ + "target_key" + ], + "isUnique": true, + "where": "\"extension_submissions\".\"status\" = 'pending'" + }, + "idx_extension_submissions_submitter_page": { + "name": "idx_extension_submissions_submitter_page", + "columns": [ + "submitted_by", + "\"created_at\" desc", + "\"id\" desc" + ], + "isUnique": false + }, + "idx_extension_submissions_queue_page": { + "name": "idx_extension_submissions_queue_page", + "columns": [ + "status", + "created_at", + "id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "extension_submissions_extension_id_extensions_id_fk": { + "name": "extension_submissions_extension_id_extensions_id_fk", + "tableFrom": "extension_submissions", + "tableTo": "extensions", + "columnsFrom": [ + "extension_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_submissions_submitted_by_users_id_fk": { + "name": "extension_submissions_submitted_by_users_id_fk", + "tableFrom": "extension_submissions", + "tableTo": "users", + "columnsFrom": [ + "submitted_by" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + }, + "extension_submissions_reviewer_id_users_id_fk": { + "name": "extension_submissions_reviewer_id_users_id_fk", + "tableFrom": "extension_submissions", + "tableTo": "users", + "columnsFrom": [ + "reviewer_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": { + "extension_submissions_status_check": { + "name": "extension_submissions_status_check", + "value": "\"extension_submissions\".\"status\" IN ('pending', 'approved', 'rejected')" + }, + "extension_submissions_ownership_epoch_check": { + "name": "extension_submissions_ownership_epoch_check", + "value": "\"extension_submissions\".\"ownership_epoch\" >= 1" + } + } + }, + "extensions": { + "name": "extensions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "author_id": { + "name": "author_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "releases": { + "name": "releases", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "website": { + "name": "website", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "license": { + "name": "license", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "icon_url": { + "name": "icon_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "readme": { + "name": "readme", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "version": { + "name": "version", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "download_url": { + "name": "download_url", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_extensions_type": { + "name": "idx_extensions_type", + "columns": [ + "type" + ], + "isUnique": false + }, + "idx_extensions_author": { + "name": "idx_extensions_author", + "columns": [ + "author_id" + ], + "isUnique": false + } + }, + "foreignKeys": { + "extensions_author_id_developers_id_fk": { + "name": "extensions_author_id_developers_id_fk", + "tableFrom": "extensions", + "tableTo": "developers", + "columnsFrom": [ + "author_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": { + "idx_extension_submissions_submitter_page": { + "columns": { + "\"created_at\" desc": { + "isExpression": true + }, + "\"id\" desc": { + "isExpression": true + } + } + } + } + } +} \ No newline at end of file diff --git a/src/services/extensions/v2/db/migrations/meta/_journal.json b/src/services/extensions/v2/db/migrations/meta/_journal.json index 687be2a..6fc3312 100644 --- a/src/services/extensions/v2/db/migrations/meta/_journal.json +++ b/src/services/extensions/v2/db/migrations/meta/_journal.json @@ -22,6 +22,13 @@ "when": 1785425233853, "tag": "0016_add_developer_github_url_verified", "breakpoints": true + }, + { + "idx": 17, + "version": "6", + "when": 1785427859758, + "tag": "0017_add_developer_url_check_cooldown", + "breakpoints": true } ] } \ No newline at end of file diff --git a/src/services/extensions/v2/db/schema.ts b/src/services/extensions/v2/db/schema.ts index d184c8c..7e6410b 100644 --- a/src/services/extensions/v2/db/schema.ts +++ b/src/services/extensions/v2/db/schema.ts @@ -90,7 +90,12 @@ export const developers = sqliteTable( // github-verification.ts's urlMatchesGithubBlog(). Only ever 1 or null, // never 0 (see the schema comment on interfaces.ts's // DeveloperProfileSchema.github_url_verified for why). - githubUrlVerified: integer("github_url_verified") + githubUrlVerified: integer("github_url_verified"), + // Atomic per-owner cooldown gating reverifyOwn()'s check_url path (the + // one that spends a real GitHub API call) — set via a conditional + // UPDATE, not read-then-write, so concurrent requests can't both pass. + // Null until the first check_url reverify. + urlCheckCooldownUntil: text("url_check_cooldown_until") }, (table) => [ uniqueIndex("idx_developers_owner_unique").on(table.ownerUserId), diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index b85ed87..754e414 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -1,4 +1,4 @@ -import { and, asc, desc, eq, isNull, sql } from "drizzle-orm"; +import { and, asc, desc, eq, isNull, or, sql } from "drizzle-orm"; import { DatabaseError, DatabaseResult } from "../../../lib/interfaces"; import { ExtensionsDb } from "../../../lib/db"; import { @@ -27,6 +27,14 @@ import { } 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. @@ -1116,6 +1124,39 @@ export class DevelopersDatabase { }; } + 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), + 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 ); @@ -1149,6 +1190,14 @@ export class DevelopersDatabase { // (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) { @@ -1157,42 +1206,63 @@ export class DevelopersDatabase { // not a disproof — checkGithubEntity can't tell the two apart (see // its own docstring) — so it leaves the stored signal untouched // rather than clearing a real prior verification over a transient - // failure. Only a successful lookup, of the entity type the profile - // itself claims, gets to overwrite it. + // failure. Only a successful lookup gets to overwrite it. if (entity) { writeUrlVerified = true; - githubUrlVerified = - entity.type === row.type && - urlMatchesGithubBlog(row.url ?? undefined, entity.blog) + if (entity.type !== row.type) { + identityTypeContradicted = true; + } else { + githubUrlVerified = urlMatchesGithubBlog( + row.url ?? undefined, + 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. + // 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: matches ? 1 : 0, + githubOrgVerified: verified ? 1 : 0, ...(writeUrlVerified ? { githubUrlVerified } : {}), - githubVerificationNote: matches + githubVerificationNote: verified ? "Verified: caller's linked GitHub identity matches." - : "No longer verified: caller's linked GitHub identity no longer 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)) + and( + eq(developers.id, row.id), + eq(developers.ownerUserId, userId), + ...(writeUrlVerified + ? [ + row.url === null + ? isNull(developers.url) + : eq(developers.url, row.url) + ] + : []) + ) ); if (!result.meta?.changes) { return { data: null, error: { - message: "Developer ownership changed while re-verifying", + message: + "Developer ownership or Publisher URL changed while re-verifying", code: "CONFLICT" } }; diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index fe13b84..591e99b 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -741,40 +741,12 @@ const reverifyOwnDeveloperRoute = createRoute({ } }); -// How often the owner's own manual "Re-verify" button is allowed to spend -// an extra GitHub API call on the URL check — 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. The opportunistic per-login path never -// hits this since it never sets check_url. -const CHECK_URL_COOLDOWN_SECONDS = 60; - extensionsV2.openapi(reverifyOwnDeveloperRoute, async (c) => { const auth = getAuth(c); const { check_url } = c.req.valid("query"); const platform = getPlatform(c); const db = new DevelopersDatabase(getExtensionsDb(c.env.DB_EXTENSIONS)); - if (check_url) { - const cache = platform.getCache("CACHE_KV"); - const rateLimitKey = `reverify-url-check:${auth.userId}`; - if (await cache.get(rateLimitKey)) { - return c.json( - { - error: { - message: - "Please wait a minute before re-checking your Publisher URL again.", - code: "RATE_LIMITED" - } - }, - 429 - ); - } - await cache.put(rateLimitKey, "1", { - expirationTtl: CHECK_URL_COOLDOWN_SECONDS - }); - } - const { data, error } = await db.reverifyOwn( auth.userId, check_url, @@ -786,7 +758,9 @@ extensionsV2.openapi(reverifyOwnDeveloperRoute, async (c) => { ? 404 : error?.code === "CONFLICT" ? 409 - : 500; + : error?.code === "RATE_LIMITED" + ? 429 + : 500; return c.json( { error: { diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index 24bce4c..ce0eaa9 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -91,12 +91,6 @@ beforeEach(async () => { await resetExtensionsDb(db); vi.clearAllMocks(); mockGithubEntityNotFound(); - - // check_url's rate-limit key (see reverifyOwnDeveloperRoute) would - // otherwise leak across tests that reuse "user-1" — CACHE_KV isn't reset - // by resetExtensionsDb since it isn't part of the D1 database. - const { keys } = await env.CACHE_KV.list(); - await Promise.all(keys.map((key) => env.CACHE_KV.delete(key.name))); }); afterEach(() => { @@ -1857,6 +1851,74 @@ describe("Extensions API v2", () => { expect(second.status).toBe(200); }); + it("only lets one of two concurrent ?check_url=true requests through", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + const headers = await authHeaders("user-1"); + const [first, second] = await Promise.all([ + post("/extensions/v2/developers/me/reverify?check_url=true", headers), + post("/extensions/v2/developers/me/reverify?check_url=true", headers) + ]); + + const statuses = [first.status, second.status].sort(); + expect(statuses).toEqual([200, 429]); + }); + + it("does not persist a URL verification computed against a stale URL", async () => { + mockGithubEntity("Organization", "https://acme.example"); + await insertDeveloper(db, { + id: "dev-developer", + type: "organization", + name: "Dev", + url: "https://acme.example", + owner_user_id: "user-1" + }); + await insertUser(db, { + id: "user-1", + github_login: "someone", + github_orgs: JSON.stringify(["dev-developer"]) + }); + + // Fires just before reverifyOwn's final write (identified by + // touching github_verified_at, which only that statement sets) — the + // Publisher URL changes between the check and the write, same shape + // as the existing ownership-race test above. + env.DB_EXTENSIONS = wrapD1WithHook(db, async (sql) => { + if (sql.includes("developers") && sql.includes("github_verified_at")) { + await db + .prepare("UPDATE developers SET url = ? WHERE id = ?") + .bind("https://different.example", "dev-developer") + .run(); + } + }); + + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + env.DB_EXTENSIONS = db; + + expect(res.status).toBe(409); + expect((await getDeveloper(db, "dev-developer"))?.github_url_verified).toBe( + null + ); + expect((await getDeveloper(db, "dev-developer"))?.url).toBe( + "https://different.example" + ); + }); + it("clears a previously-verified Publisher URL when identity no longer matches", async () => { mockGithubEntity("Organization", "https://acme.example"); await insertDeveloper(db, { @@ -1944,9 +2006,20 @@ describe("Extensions API v2", () => { ); expect(res.status).toBe(200); const body = (await res.json()) as { - result: { github_url_verified?: boolean }; + result: { + github_org_verified?: boolean; + github_url_verified?: boolean; + github_verification_note?: string; + }; }; expect(body.result.github_url_verified).toBeUndefined(); + // The same discrepancy that rules out the URL match also undermines + // the identity match itself — matchesClaimant() alone can't catch + // this since it never queries GitHub's actual current entity type. + expect(body.result.github_org_verified).toBe(false); + expect(body.result.github_verification_note).toBe( + "No longer verified: GitHub's on-file entity type no longer matches this profile." + ); }); it("preserves an existing Publisher URL verification when the GitHub lookup fails", async () => { From a30f51543f7c4dc8b66c4beac4579ae26c21e688 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 30 Jul 2026 17:32:04 +0100 Subject: [PATCH 5/6] Reset url_check_cooldown_until on ownership transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit acceptTransfer and approveClaim now both clear it — left unchanged, a new owner would inherit whatever was left of the previous owner's check_url cooldown window, since the column is keyed by whoever currently owns the row. Claude-Session: https://claude.ai/code/session_014dzTSwUW78Qipx7sYH2dgD --- .../extensions/v2/developers-database.ts | 11 ++++++ test/services/extensions/v2/index.test.ts | 35 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index 754e414..354c4c9 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -848,11 +848,16 @@ export class DevelopersDatabase { params: [userId, tokenHash, 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. 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, updated_at = CURRENT_TIMESTAMP WHERE changes() = 1 AND id = ( @@ -1605,6 +1610,12 @@ export class DevelopersDatabase { approvedAt: null, approvedRevision: null, approvedBy: null, + // Not known to be reachable today (claims only ever target + // never-owned profiles, so there's no prior owner's cooldown to + // inherit), but reset for the same reason as acceptTransfer's + // equivalent write — belongs to whichever user currently owns + // this row. + urlCheckCooldownUntil: null, // Carries the claim's own verification result onto the profile it // just transferred ownership to — verifyGithubOwnership() already // ran once, inside claim() itself, so this isn't a fresh check. diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index ce0eaa9..d0869d7 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -2456,6 +2456,41 @@ describe("Extensions API v2", () => { ); }); + it("doesn't inherit the previous owner's check_url cooldown after a transfer", async () => { + await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper() + ); + + const usedCooldown = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-1") + ); + expect(usedCooldown.status).toBe(200); + + 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); + + // user-2 has never called check_url themselves — the previous + // owner's still-active cooldown must not carry over onto them. + const res = await post( + "/extensions/v2/developers/me/reverify?check_url=true", + await authHeaders("user-2") + ); + expect(res.status).toBe(200); + }); + it("does not let replaying an already-used token reassign ownership away from a later owner", async () => { await put( "/extensions/v2/developers/me", From b068e928699cec11077bf6ded734dc0e5cd8a0e9 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 30 Jul 2026 17:34:40 +0100 Subject: [PATCH 6/6] =?UTF-8?q?Clear=20GitHub=20verification=20on=20transf?= =?UTF-8?q?er=20=E2=80=94=20it=20describes=20the=20old=20owner's=20identit?= =?UTF-8?q?y,=20not=20the=20new=20owner's?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit acceptTransfer now resets github_org_verified/github_url_verified/ github_verification_note/github_verified_at to null, same as it already resets approval. Unlike approveClaim (which carries over a claim-time verification computed against the specific claimant), a transfer has no equivalent verification step to carry over — it's a bare ownership handoff, so this can only fall back to unverified until the new owner re-verifies. Claude-Session: https://claude.ai/code/session_014dzTSwUW78Qipx7sYH2dgD --- .../extensions/v2/developers-database.ts | 12 ++++++ test/services/extensions/v2/index.test.ts | 37 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index 354c4c9..a5bd8c1 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -852,12 +852,24 @@ export class DevelopersDatabase { // 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 = ( diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index d0869d7..bec7495 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -2491,6 +2491,43 @@ describe("Extensions API v2", () => { expect(res.status).toBe(200); }); + it("clears GitHub verification on transfer — it described the previous owner's identity, not the new owner's", async () => { + mockGithubEntity("User", "https://acme.example"); + await insertUser(db, { + id: "user-1", + github_login: "dev-developer", + github_orgs: JSON.stringify([]) + }); + const created = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + { ...sampleDeveloper(), URL: "https://acme.example" } + ); + const createdBody = (await created.json()) as { + result: { github_org_verified?: boolean; github_url_verified?: boolean }; + }; + expect(createdBody.result.github_org_verified).toBe(true); + expect(createdBody.result.github_url_verified).toBe(true); + + 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); + const accepted = (await accept.json()) as { + result: { github_org_verified?: boolean; github_url_verified?: boolean }; + }; + expect(accepted.result.github_org_verified).toBeUndefined(); + expect(accepted.result.github_url_verified).toBeUndefined(); + }); + it("does not let replaying an already-used token reassign ownership away from a later owner", async () => { await put( "/extensions/v2/developers/me",