From b475b70e2a2c89cca81d7841554a2d21ddd5c1b6 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Wed, 26 Aug 2026 17:39:51 +0100 Subject: [PATCH 1/2] fix(delete-user-data): gate RTDB deletion on a configured instance The extension deletes RTDB data only when `rtdbPaths && databaseURL`, where the URL derives from SELECTED_DATABASE_INSTANCE. The kit gated on `rtdbPaths` alone and then called `ctx.database.ref(path).remove()` against whatever default instance the app resolves -- so RTDB_PATHS with an empty SELECTED_DATABASE_INSTANCE could delete data from an unintended database. Tracked in #2974 as the kit's deletion risk. handleClear now requires getDatabaseUrl(rtdbInstance, rtdbLocation) to resolve before scheduling RTDB deletion, logging rtdbNotConfigured otherwise, matching the extension. Existing rtdb tests gain the instance/location config; a new test asserts the skip. --- kits/delete-user-data/src/handlers.ts | 9 ++++++- kits/delete-user-data/tests/handlers.test.ts | 26 ++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/kits/delete-user-data/src/handlers.ts b/kits/delete-user-data/src/handlers.ts index 87c76102c..bb370d3dc 100644 --- a/kits/delete-user-data/src/handlers.ts +++ b/kits/delete-user-data/src/handlers.ts @@ -19,6 +19,7 @@ import type { DocumentReference } from "firebase-admin/firestore"; import { FieldPath } from "firebase-admin/firestore"; import chunk from "lodash.chunk"; import * as events from "./events"; +import { getDatabaseUrl } from "./export-config"; import { extractUserPaths, hasValidUserPath } from "./helpers"; import * as logs from "./logs"; import { recursiveDelete } from "./recursiveDelete"; @@ -156,7 +157,13 @@ export async function handleClear( } else { logs.firestoreNotConfigured(); } - if (ctx.config.rtdbPaths) { + // Parity with the extension: RTDB deletion runs only when a database URL + // can be derived from the configured instance. Without this, the paths + // would be deleted against whatever default instance the app resolves. + if ( + ctx.config.rtdbPaths && + getDatabaseUrl(ctx.config.rtdbInstance, ctx.config.rtdbLocation) + ) { promises.push(clearDatabaseData(ctx.config.rtdbPaths, uid, ctx)); } else { logs.rtdbNotConfigured(); diff --git a/kits/delete-user-data/tests/handlers.test.ts b/kits/delete-user-data/tests/handlers.test.ts index 4b544d079..2a6486920 100644 --- a/kits/delete-user-data/tests/handlers.test.ts +++ b/kits/delete-user-data/tests/handlers.test.ts @@ -406,7 +406,11 @@ describe("handleClear", () => { test("deletes the configured rtdb paths", async () => { const ctx = makeContext({ - config: { rtdbPaths: "users/{UID},admins/{UID}" }, + config: { + rtdbPaths: "users/{UID},admins/{UID}", + rtdbInstance: "test-rtdb-instance", + rtdbLocation: "us-central1", + }, }); await handleClear(UID, ctx); @@ -418,6 +422,20 @@ describe("handleClear", () => { }); }); + // Parity: the extension gates RTDB deletion on `rtdbPaths && databaseURL`, + // so paths without a configured instance must be skipped, not deleted + // against whatever default RTDB the app resolves. + test("skips rtdb deletion when no database instance is configured", async () => { + const ctx = makeContext({ + config: { rtdbPaths: "users/{UID}" }, + }); + + await handleClear(UID, ctx); + + expect(ctx.rtdbRemovals).toEqual([]); + expect(log.rtdbNotConfigured).toHaveBeenCalled(); + }); + test("deletes the configured storage paths", async () => { const ctx = makeContext({ config: { @@ -463,7 +481,11 @@ describe("handleClear", () => { test("logs rtdb errors without failing", async () => { const error = new Error("boom"); const ctx = makeContext({ - config: { rtdbPaths: "users/{UID}" }, + config: { + rtdbPaths: "users/{UID}", + rtdbInstance: "test-rtdb-instance", + rtdbLocation: "us-central1", + }, rtdbError: error, }); From 741dcc6821dcb0b932ee33fc64e7d91544dc9207 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Thu, 27 Aug 2026 14:12:47 +0100 Subject: [PATCH 2/2] chore(delete-user-data): drop redundant parity comments --- kits/delete-user-data/src/handlers.ts | 3 --- kits/delete-user-data/tests/handlers.test.ts | 3 --- 2 files changed, 6 deletions(-) diff --git a/kits/delete-user-data/src/handlers.ts b/kits/delete-user-data/src/handlers.ts index bb370d3dc..8f240d339 100644 --- a/kits/delete-user-data/src/handlers.ts +++ b/kits/delete-user-data/src/handlers.ts @@ -157,9 +157,6 @@ export async function handleClear( } else { logs.firestoreNotConfigured(); } - // Parity with the extension: RTDB deletion runs only when a database URL - // can be derived from the configured instance. Without this, the paths - // would be deleted against whatever default instance the app resolves. if ( ctx.config.rtdbPaths && getDatabaseUrl(ctx.config.rtdbInstance, ctx.config.rtdbLocation) diff --git a/kits/delete-user-data/tests/handlers.test.ts b/kits/delete-user-data/tests/handlers.test.ts index 2a6486920..e3326f709 100644 --- a/kits/delete-user-data/tests/handlers.test.ts +++ b/kits/delete-user-data/tests/handlers.test.ts @@ -422,9 +422,6 @@ describe("handleClear", () => { }); }); - // Parity: the extension gates RTDB deletion on `rtdbPaths && databaseURL`, - // so paths without a configured instance must be skipped, not deleted - // against whatever default RTDB the app resolves. test("skips rtdb deletion when no database instance is configured", async () => { const ctx = makeContext({ config: { rtdbPaths: "users/{UID}" },