Merge pull request #3895 from choksi2212/fix/delete-org-all-sites-cleanup-3863

fix(org): clean up every site when deleting an org
This commit is contained in:
Owen Schwartz
2026-10-13 09:12:58 -04:00
committed by GitHub
3 changed files with 343 additions and 15 deletions
+139
View File
@@ -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 { sql } 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");
const row = (await db.get(
sql`select
(select count(*) from sites) as sites,
(select count(*) from "newt") as newts,
(select count(*) from "newtSession") as sessions,
(select count(*) from clientSitesAssociationsCache where siteId = 3) as assoc`
)) as {
sites: number;
newts: number;
sessions: number;
assoc: number;
};
console.log(
"OBSERVATIONS " +
JSON.stringify({
peerRemovals: peerRemovals.slice().sort(),
terminatedNewtIds: result.deletedNewtIds.slice().sort(),
sitesLeft: Number(row.sites),
newtsLeft: Number(row.newts),
sessionsLeft: Number(row.sessions),
associationRowsLeft: Number(row.assoc)
})
);
server.close();
process.exit(0);
} catch (e: any) {
console.error("CHILD_FAIL", e && e.message ? e.message : String(e));
server.close();
process.exit(1);
}
});
+185
View File
@@ -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();
+19 -15
View File
@@ -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()
@@ -73,20 +73,24 @@ export async function deleteOrgById(
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}`);