diff --git a/control-plane/src/neon-database-driver.ts b/control-plane/src/neon-database-driver.ts index 83bdd54f93..578614d691 100644 --- a/control-plane/src/neon-database-driver.ts +++ b/control-plane/src/neon-database-driver.ts @@ -17,6 +17,7 @@ // Endpoint paths/response shapes below follow Neon's public v2 API (https://api-docs.neon.tech/reference) as // documented at the time this was written -- verify against a live account before the first real deploy (the // test suite mocks every call; no live Neon credentials are used anywhere in this repo). +import { createHash } from "node:crypto"; import type { DatabaseConnectionDetails, TenantProvisioningRequest } from "./tenant-provisioning-driver.js"; const DEFAULT_API_BASE_URL = "https://console.neon.tech/api/v2"; @@ -49,12 +50,27 @@ type NeonEndpoint = { host: string }; type NeonRole = { name: string; password?: string }; +// #8026: the unconditional .slice(0, 63) below used to have no collision guard -- two distinct tenant names +// sharing the same first ~54 characters (after the "tenant--" prefix and sanitization) would +// truncate to the IDENTICAL Neon branch name. findBranchByName would then find the OTHER tenant's already- +// existing branch and hand back its connection/role/password to the new tenant -- a cross-tenant data- +// isolation bug. Only names that actually need truncating get the suffix, so a short tenant name's branch +// name is completely unchanged (this repo has never deployed against a live Neon project yet -- see this +// file's own header comment -- so there is no pre-existing long-name branch a suffix could orphan). +const NEON_BRANCH_NAME_MAX_LENGTH = 63; +const NEON_BRANCH_NAME_COLLISION_SUFFIX_LENGTH = 8; + /** Neon branch names are case-sensitive but this keeps them predictable and collision-free across products - * sharing a tenant name, and safely truncated well under Neon's own length limit. */ + * sharing a tenant name, and safely truncated well under Neon's own length limit. A name that would + * otherwise be truncated gets a short hash-of-the-untruncated-name suffix instead, so two long, + * prefix-similar tenant names can never collide on the same truncated branch name (#8026). */ function branchNameFor(request: TenantProvisioningRequest): string { const raw = `tenant-${request.product}-${request.tenant.name}`.toLowerCase(); const sanitized = raw.replaceAll(/[^a-z0-9_-]+/g, "-").replaceAll(/-{2,}/g, "-").replace(/^-+|-+$/g, ""); - return sanitized.slice(0, 63); + if (sanitized.length <= NEON_BRANCH_NAME_MAX_LENGTH) return sanitized; + const suffix = createHash("sha256").update(sanitized).digest("hex").slice(0, NEON_BRANCH_NAME_COLLISION_SUFFIX_LENGTH); + const prefixLength = NEON_BRANCH_NAME_MAX_LENGTH - 1 - suffix.length; + return `${sanitized.slice(0, prefixLength)}-${suffix}`; } /** A tenant-scoped role gets the SAME derived name as its branch -- one branch, one role, one database, no diff --git a/control-plane/test/neon-database-driver.test.ts b/control-plane/test/neon-database-driver.test.ts index eafcfac25e..08baeb3544 100644 --- a/control-plane/test/neon-database-driver.test.ts +++ b/control-plane/test/neon-database-driver.test.ts @@ -236,3 +236,51 @@ test("createNeonDatabaseDriver: bundles provision/drop closed over one config", // somewhere else -- the request above only succeeds against the real Neon endpoint shape if `dropDatabase` // routed through the same config-scoped fetch helper `dropNeonDatabase` itself uses. }); + +// #8026: two tenant names sharing a long common prefix (both past Neon's 63-char branch-name limit once the +// "tenant--" prefix is added) used to sanitize+truncate to the IDENTICAL branch name -- provisioning +// the second tenant would find the FIRST tenant's already-existing branch and hand back its connection/role/ +// password. Regression-guards branchNameFor's collision-resistant suffix by reading the actual branch name +// each provision call sends in its create-branch POST body. +test("provisionNeonDatabase: two long, prefix-similar tenant names produce DIFFERENT branch names (#8026)", async () => { + const longPrefix = "a".repeat(60); + const requestA: TenantProvisioningRequest = { tenant: { name: `${longPrefix}-org-alpha` }, product: "orb" }; + const requestB: TenantProvisioningRequest = { tenant: { name: `${longPrefix}-org-beta` }, product: "orb" }; + + const { calls: callsA } = mockFetchSequence([ + { body: { branches: [] } }, + { body: { branch: { id: "br-a", name: "placeholder" }, endpoints: [{ host: "ep-a.neon.tech" }], operations: [] } }, + { body: { role: { name: "placeholder", password: "pw-a" }, operations: [] } }, + { body: { operations: [] } }, + ]); + await provisionNeonDatabase(CONFIG, requestA); + const branchNameA = (bodyOf(callsA[1]!.init) as { branch: { name: string } }).branch.name; + + const { calls: callsB } = mockFetchSequence([ + { body: { branches: [] } }, + { body: { branch: { id: "br-b", name: "placeholder" }, endpoints: [{ host: "ep-b.neon.tech" }], operations: [] } }, + { body: { role: { name: "placeholder", password: "pw-b" }, operations: [] } }, + { body: { operations: [] } }, + ]); + await provisionNeonDatabase(CONFIG, requestB); + const branchNameB = (bodyOf(callsB[1]!.init) as { branch: { name: string } }).branch.name; + + // Both names are long enough that a naive unconditional .slice(0, 63) collapses them to the same 63 + // characters of "a"s well before either "-org-alpha"/"-org-beta" suffix is ever reached. + assert.notEqual(branchNameA, branchNameB); + assert.ok(branchNameA.length <= 63); + assert.ok(branchNameB.length <= 63); +}); + +test("provisionNeonDatabase: a short tenant name's branch name is completely unaffected by the collision-suffix logic", async () => { + const { calls } = mockFetchSequence([ + { body: { branches: [] } }, + { body: { branch: { id: "br-1", name: BRANCH_NAME }, endpoints: [{ host: "ep-1.neon.tech" }], operations: [] } }, + { body: { role: { name: BRANCH_NAME, password: "pw" }, operations: [] } }, + { body: { operations: [] } }, + ]); + + await provisionNeonDatabase(CONFIG, REQUEST); + + assert.equal((bodyOf(calls[1]!.init) as { branch: { name: string } }).branch.name, BRANCH_NAME); +});