Merge branch 'main' into dev

server/lib/deleteOrg.childTest.ts

@@ -0,0 +1,139 @@
+// Test child for deleteOrg.test.ts. Boots the real sqlite driver against a
+// fresh database in the temp workdir (cwd), seeds an org with two wireguard
+// sites and two newt sites, runs deleteOrgById against a local HTTP stand-in
+// for the exit node, and prints one JSON line of observations.
+// The app boots with `import "./extendZod"` before anything else; zod
+// schemas in the import chain use .openapi(), so do the same here.
+import "@server/extendZod";
+import http from "http";
+import { deleteOrgById } from "@server/lib/deleteOrg";
+import {
+    db,
+    exitNodes,
+    orgs,
+    sites,
+    newts,
+    newtSessions,
+    clientSitesAssociationsCache
+} from "@server/db";
+import { count, eq } from "drizzle-orm";
+
+const peerRemovals: string[] = [];
+const server = http.createServer((req, res) => {
+    let body = "";
+    req.on("data", (c) => (body += c));
+    req.on("end", () => {
+        if (req.method === "DELETE") {
+            if (!req.url) {
+                res.writeHead(200, { "Content-Type": "application/json" });
+                res.end(JSON.stringify({ status: "ok" }));
+                return;
+            }
+            const url = new URL(req.url, "http://localhost");
+            const pubKey = url.searchParams.get("public_key");
+            if (pubKey !== null) {
+                peerRemovals.push(pubKey);
+            }
+        }
+        res.writeHead(200, { "Content-Type": "application/json" });
+        res.end(JSON.stringify({ status: "ok" }));
+    });
+});
+
+server.listen(0, "127.0.0.1", async () => {
+    const addr = server.address();
+    if (addr === null || typeof addr === "string") {
+        throw new Error("failed to bind the stub exit node");
+    }
+    const port = addr.port;
+    try {
+        await db.insert(exitNodes).values({
+            exitNodeId: 1,
+            name: "en",
+            address: "10.0.0.1",
+            endpoint: "10.0.0.1:51820",
+            publicKey: "enKey",
+            listenPort: 51820,
+            reachableAt: "http://127.0.0.1:" + port,
+            type: "wireguard"
+        });
+        await db.insert(orgs).values({
+            orgId: "org1",
+            name: "Org",
+            settingsJitModeLimit: 1
+        });
+        for (let i = 1; i <= 2; i++) {
+            await db.insert(sites).values({
+                siteId: i,
+                orgId: "org1",
+                niceId: String(i),
+                name: "wg" + i,
+                pubKey: "wgKey" + i,
+                exitNodeId: 1,
+                type: "wireguard"
+            });
+        }
+        for (let i = 3; i <= 4; i++) {
+            await db.insert(sites).values({
+                siteId: i,
+                orgId: "org1",
+                niceId: String(i),
+                name: "newt" + i,
+                pubKey: "newtPub" + i,
+                publicKey: "newtSession" + i,
+                type: "newt"
+            });
+            await db.insert(newts).values({
+                newtId: "newtId" + i,
+                secretHash: "h" + i,
+                dateCreated: "2026-01-01",
+                siteId: i
+            });
+            await db.insert(newtSessions).values({
+                sessionId: "sess" + i,
+                newtId: "newtId" + i,
+                expiresAt: 4102444800
+            });
+        }
+        // A stale association-cache row pinned to one of the newt sites, the
+        // kind cleanupSiteAssociations exists to clear.
+        await db.insert(clientSitesAssociationsCache).values({
+            clientId: 99,
+            siteId: 3,
+            isRelayed: false
+        });
+
+        const result = await deleteOrgById("org1");
+
+        // Read the leftovers through the query builder, not db.get(): the
+        // pg build type-checks this file against the pg driver too, and
+        // .get() exists only on the sqlite driver.
+        const [sitesRow] = await db.select({ count: count() }).from(sites);
+        const [newtsRow] = await db.select({ count: count() }).from(newts);
+        const [sessionsRow] = await db
+            .select({ count: count() })
+            .from(newtSessions);
+        const [assocRow] = await db
+            .select({ count: count() })
+            .from(clientSitesAssociationsCache)
+            .where(eq(clientSitesAssociationsCache.siteId, 3));
+
+        console.log(
+            "OBSERVATIONS " +
+                JSON.stringify({
+                    peerRemovals: peerRemovals.slice().sort(),
+                    terminatedNewtIds: result.deletedNewtIds.slice().sort(),
+                    sitesLeft: sitesRow.count,
+                    newtsLeft: newtsRow.count,
+                    sessionsLeft: sessionsRow.count,
+                    associationRowsLeft: assocRow.count
+                })
+        );
+        server.close();
+        process.exit(0);
+    } catch (e: any) {
+        console.error("CHILD_FAIL", e && e.message ? e.message : String(e));
+        server.close();
+        process.exit(1);
+    }
+});

server/lib/deleteOrg.test.ts

@@ -0,0 +1,185 @@
+import { execFileSync } from "child_process";
+import fs from "fs";
+import os from "os";
+import path from "path";
+import { fileURLToPath } from "url";
+import { assertEquals } from "@test/assert";
+
+const here = path.dirname(fileURLToPath(import.meta.url));
+const repoRoot = path.resolve(here, "..", "..");
+
+const CHILD_SCRIPT_PATH = path.join(
+    repoRoot,
+    "server",
+    "lib",
+    "deleteOrg.childTest.ts"
+);
+
+function runChild(workdir: string): {
+    exitCode: number;
+    observations: any;
+    output: string;
+} {
+    const tsconfig = ["tsconfig.json", "tsconfig.oss.json"]
+        .map((file) => path.join(repoRoot, file))
+        .find((file) => fs.existsSync(file));
+    if (!tsconfig) {
+        throw new Error("No tsconfig found for @server path aliases");
+    }
+    const tsxCli = path.join(
+        repoRoot,
+        "node_modules",
+        "tsx",
+        "dist",
+        "cli.mjs"
+    );
+    if (!fs.existsSync(tsxCli)) {
+        throw new Error("tsx is not installed; run npm ci first");
+    }
+    try {
+        const output = execFileSync(
+            process.execPath,
+            [tsxCli, "--tsconfig", tsconfig, CHILD_SCRIPT_PATH],
+            {
+                cwd: workdir,
+                timeout: 120000,
+                encoding: "utf8",
+                env: { ...process.env, NODE_ENV: "test" }
+            }
+        );
+        const m = output.match(/OBSERVATIONS (.*)/);
+        if (!m) {
+            throw new Error("child did not print observations: " + output);
+        }
+        return { exitCode: 0, observations: JSON.parse(m[1]), output };
+    } catch (error: any) {
+        const output = String(error?.stdout ?? "");
+        return { exitCode: 1, observations: null, output };
+    }
+}
+
+function createTestEnvironment(): string {
+    for (const generated of ["server/build.ts", "server/db/index.ts"]) {
+        if (!fs.existsSync(path.join(repoRoot, generated))) {
+            throw new Error(
+                `Missing ${generated}; run npm run set:oss && npm run set:sqlite first`
+            );
+        }
+    }
+    const workdir = fs.mkdtempSync(
+        path.join(os.tmpdir(), "pangolin-org-cleanup-")
+    );
+    fs.mkdirSync(path.join(workdir, "config", "db"), { recursive: true });
+    fs.copyFileSync(
+        path.join(repoRoot, "config", "config.example.yml"),
+        path.join(workdir, "config", "config.yml")
+    );
+    fs.symlinkSync(
+        path.join(repoRoot, "server"),
+        path.join(workdir, "server"),
+        process.platform === "win32" ? "junction" : "dir"
+    );
+    fs.symlinkSync(
+        path.join(repoRoot, "test"),
+        path.join(workdir, "test"),
+        process.platform === "win32" ? "junction" : "dir"
+    );
+    fs.symlinkSync(
+        path.join(repoRoot, "node_modules"),
+        path.join(workdir, "node_modules"),
+        process.platform === "win32" ? "junction" : "dir"
+    );
+    return workdir;
+}
+
+function applyMigrations(workdir: string) {
+    const tsconfig = ["tsconfig.json", "tsconfig.oss.json"]
+        .map((file) => path.join(repoRoot, file))
+        .find((file) => fs.existsSync(file));
+    if (!tsconfig) {
+        throw new Error("No tsconfig found for @server path aliases");
+    }
+    const tsxCli = path.join(
+        repoRoot,
+        "node_modules",
+        "tsx",
+        "dist",
+        "cli.mjs"
+    );
+    // CI applies migrations with `npm run db:push` before booting the app;
+    // server/db/migrate.ts is the drizzle migrator that script invokes.
+    const migrationsScript = path.join(repoRoot, "server", "db", "migrate.ts");
+    execFileSync(
+        process.execPath,
+        [tsxCli, "--tsconfig", tsconfig, migrationsScript],
+        {
+            cwd: workdir,
+            timeout: 120000,
+            encoding: "utf8",
+            env: { ...process.env, NODE_ENV: "test" }
+        }
+    );
+}
+
+function testOrgCleanupCoversEverySite() {
+    console.log("Running deleteOrgById per-site cleanup test...");
+    const workdir = createTestEnvironment();
+    applyMigrations(workdir);
+
+    const { exitCode, observations, output } = runChild(workdir);
+    if (exitCode !== 0) {
+        throw new Error("deleteOrgById child run failed: " + output);
+    }
+
+    // The exact bug from the issue: with a single-row sites fetch, only one
+    // of the four sites gets its side effects. All wireguard peers must be
+    // removed from the exit node...
+    assertEquals(
+        JSON.stringify(observations.peerRemovals),
+        JSON.stringify(["wgKey1", "wgKey2"]),
+        "deleteOrgById must remove every wireguard site's peer from its exit node"
+    );
+    // ...and every newt connector must be collected for the terminate message.
+    assertEquals(
+        JSON.stringify(observations.terminatedNewtIds),
+        JSON.stringify(["newtId3", "newtId4"]),
+        "deleteOrgById must collect every newt id so all connectors get the terminate message"
+    );
+    // DB rows must still cascade to nothing left.
+    assertEquals(observations.sitesLeft, 0, "all site rows must be gone");
+    assertEquals(observations.newtsLeft, 0, "all newt rows must be gone");
+    assertEquals(
+        observations.sessionsLeft,
+        0,
+        "all newt session rows must be gone"
+    );
+
+    // The gap the issue calls out separately: cleanupSiteAssociations must
+    // run for newt sites (deleteSite.ts already does this), so the stale
+    // association cache rows for these sites must be cleared.
+    assertEquals(
+        observations.associationRowsLeft,
+        0,
+        "newt site cleanup must clear clientSitesAssociationsCache rows"
+    );
+
+    console.log("deleteOrgById per-site cleanup test passed");
+}
+
+function main() {
+    let failed = 0;
+    for (const test of [testOrgCleanupCoversEverySite]) {
+        try {
+            test();
+        } catch (error) {
+            failed++;
+            console.error(`FAIL: ${(error as Error).message}`);
+        }
+    }
+    if (failed > 0) {
+        process.exit(1);
+    }
+    console.log("deleteOrg tests complete");
+}
+
+main();

server/lib/deleteOrg.ts

@@ -23,6 +23,7 @@ import { sendToClient } from "#dynamic/routers/ws";
 import { deletePeer } from "@server/routers/gerbil/peers";
 import { OlmErrorCodes } from "@server/routers/olm/error";
 import { sendTerminateClient } from "@server/routers/client/terminate";
+import { cleanupSiteAssociations } from "@server/lib/rebuildClientAssociations";
 import { usageService } from "./billing/usageService";
 import { LimitId } from "./billing";
 
@@ -55,8 +56,7 @@ export async function deleteOrgById(
     const orgSites = await db
         .select()
         .from(sites)
-        .where(eq(sites.orgId, orgId))
-        .limit(1);
+        .where(eq(sites.orgId, orgId));
 
     const orgClients = await db
         .select()
@@ -70,23 +70,31 @@ export async function deleteOrgById(
     let siteCount: number | null = null;
     let userCount: number | null = null;
     let remoteExitNodeCount: number | null = null;
+    // billingOrgId has no FK, so the billing org may already be gone (e.g.
+    // a partially failed account deletion). Writing usage for it would
+    // violate the usage FK and abort the transaction.
+    let billingOrgExists = false;
 
     await db.transaction(async (trx) => {
         for (const site of orgSites) {
-            if (site.pubKey) {
-                if (site.type == "wireguard") {
+            if (site.type == "wireguard") {
+                if (site.pubKey) {
                     await deletePeer(site.exitNodeId!, site.pubKey);
-                } else if (site.type == "newt") {
-                    const [deletedNewt] = await trx
-                        .delete(newts)
-                        .where(eq(newts.siteId, site.siteId))
-                        .returning();
-                    if (deletedNewt) {
-                        deletedNewtIds.push(deletedNewt.newtId);
-                        await trx
-                            .delete(newtSessions)
-                            .where(eq(newtSessions.newtId, deletedNewt.newtId));
-                    }
+                }
+            } else if (site.type == "newt") {
+                // Same per-site cleanup deleteSite.ts performs, so that
+                // removing an org does not leave other orgs' clients
+                // holding stale associations for these sites.
+                await cleanupSiteAssociations(site, trx);
+                const [deletedNewt] = await trx
+                    .delete(newts)
+                    .where(eq(newts.siteId, site.siteId))
+                    .returning();
+                if (deletedNewt) {
+                    deletedNewtIds.push(deletedNewt.newtId);
+                    await trx
+                        .delete(newtSessions)
+                        .where(eq(newtSessions.newtId, deletedNewt.newtId));
                 }
             }
             logger.info(`Deleting site ${site.siteId}`);
@@ -157,7 +165,17 @@ export async function deleteOrgById(
                 .where(inArray(domains.domainId, domainIdsToDelete));
         }
 
-        await usageService.add(orgId, LimitId.ORGANIZATIONS, -1, trx); // here we are decreasing the org count BEFORE deleting the org because we need to still be able to get the org to get the billing org inside of here
+        const [billingOrg] = org.billingOrgId
+            ? await trx
+                  .select({ orgId: orgs.orgId })
+                  .from(orgs)
+                  .where(eq(orgs.orgId, org.billingOrgId))
+                  .limit(1)
+            : [org];
+        billingOrgExists = !!billingOrg;
+        if (billingOrgExists) {
+            await usageService.add(orgId, LimitId.ORGANIZATIONS, -1, trx); // here we are decreasing the org count BEFORE deleting the org because we need to still be able to get the org to get the billing org inside of here
+        }
 
         await trx.delete(orgs).where(eq(orgs.orgId, orgId));
 
@@ -201,7 +219,11 @@ export async function deleteOrgById(
         }
     });
 
-    if (org.billingOrgId) {
+    if (
+        org.billingOrgId &&
+        org.billingOrgId !== orgId &&
+        billingOrgExists
+    ) {
         usageService.updateCount(
             org.billingOrgId,
             LimitId.DOMAINS,

server/lib/exitNodes/getUniqueSubnetForExitNode.ts

@@ -1,41 +1,62 @@
-import { db, ExitNode, Transaction, sites, clients } from "@server/db";
+import { ExitNode, primaryDb, sites, clients } from "@server/db";
 import { eq } from "drizzle-orm";
 import config from "@server/lib/config";
 import { findNextAvailableCidr } from "@server/lib/ip";
 import { lockManager } from "#dynamic/lib/lock";
 
+/**
+ * Picks the next free site/client subnet on the given exit node. This acquires
+ * a per-exit-node lock that the caller MUST release (via the returned
+ * `release`) only after the chosen subnet has been persisted to the database
+ * (or the operation has failed) — otherwise a concurrent caller on another
+ * node could pick the same subnet. If no subnet is available the lock is
+ * released before returning and `value` is null.
+ */
 export async function getUniqueSubnetForExitNode(
-    exitNode: ExitNode,
-    trx: Transaction | typeof db = db
-): Promise<string | null> {
+    exitNode: ExitNode
+): Promise<{ value: string | null; release: () => Promise<void> }> {
     const lockKey = `subnet-allocation:${exitNode.exitNodeId}`;
+    const acquired = await lockManager.acquireLockWithRetry(lockKey, 6000);
+    if (!acquired) {
+        throw new Error(`Failed to acquire lock: ${lockKey}`);
+    }
+    const release = () => lockManager.releaseLock(lockKey, acquired);
 
-    return await lockManager.withLock(
-        lockKey,
-        async () => {
-            const [sitesQuery, clientsQuery] = await Promise.all([
-                trx
-                    .select({ subnet: sites.exitNodeSubnet })
-                    .from(sites)
-                    .where(eq(sites.exitNodeId, exitNode.exitNodeId)),
-                trx
-                    .select({ subnet: clients.exitNodeSubnet })
-                    .from(clients)
-                    .where(eq(clients.exitNodeId, exitNode.exitNodeId))
-            ]);
+    try {
+        // read from the primary so we see subnets committed by the previous lock holder
+        const [sitesQuery, clientsQuery] = await Promise.all([
+            primaryDb
+                .select({ subnet: sites.exitNodeSubnet })
+                .from(sites)
+                .where(eq(sites.exitNodeId, exitNode.exitNodeId)),
+            primaryDb
+                .select({ subnet: clients.exitNodeSubnet })
+                .from(clients)
+                .where(eq(clients.exitNodeId, exitNode.exitNodeId))
+        ]);
 
-            const blockSize = config.getRawConfig().gerbil.site_block_size;
-            const subnets = [...sitesQuery, ...clientsQuery]
-                .map((row) => row.subnet)
-                .filter(
-                    (subnet): subnet is string =>
-                        !!subnet &&
-                        /^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$/.test(subnet)
-                );
-            subnets.push(exitNode.address.replace(/\/\d+$/, `/${blockSize}`));
+        const blockSize = config.getRawConfig().gerbil.site_block_size;
+        const subnets = [...sitesQuery, ...clientsQuery]
+            .map((row) => row.subnet)
+            .filter(
+                (subnet): subnet is string =>
+                    !!subnet && /^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$/.test(subnet)
+            );
+        subnets.push(exitNode.address.replace(/\/\d+$/, `/${blockSize}`));
 
-            return findNextAvailableCidr(subnets, blockSize, exitNode.address);
-        },
-        5000 // 5 second lock TTL - subnet allocation should be quick
-    );
+        const subnet = findNextAvailableCidr(
+            subnets,
+            blockSize,
+            exitNode.address
+        );
+        if (!subnet) {
+            await release();
+            return { value: null, release: async () => {} };
+        }
+
+        return { value: subnet, release };
+    } catch (e) {
+        await release();
+        throw e;
+    }
 }