From f2f5f2f0f38f82d42a81c5efe5db5548f45ca683 Mon Sep 17 00:00:00 2001 From: Josh de Leeuw Date: Fri, 25 Sep 2026 12:06:56 -0400 Subject: [PATCH 1/2] Return clear errors instead of crashes on three production failure paths - Reject experiment IDs Firestore can't store (e.g. an unfilled "__DATAPIPE_STUDY1_ID__" placeholder) with the normal not-found response. Firestore throws on these instead of missing, so /api/data answered with a 500. - Log the provider error when createexperiment returns a 502, so the failure is visible in Cloud Logging and not only in the browser. - Check the HTTP status in OSF listFiles. An OSF error response crashed with "Cannot read properties of undefined (reading 'filter')", which hid OSF's real status and failed uploads permanently through the collision cache. Co-Authored-By: Claude Opus 5.5 --- .../experiment-id-validation-emulator.test.js | 74 +++++++++++++++++++ .../experiment-id-validation.test.js | 68 +++++++++++++++++ functions/src/__tests__/providers-osf.test.js | 44 +++++++++++ functions/src/api-base64.ts | 8 ++ functions/src/api-condition.ts | 8 ++ functions/src/api-data.ts | 8 ++ functions/src/api-finalize.ts | 7 ++ functions/src/api-queue-status.ts | 7 ++ functions/src/api-session-start.ts | 8 ++ functions/src/create-experiment.ts | 3 + functions/src/experiment-id.ts | 42 +++++++++++ functions/src/providers/osf.ts | 15 +++- 12 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 functions/src/__tests__/experiment-id-validation-emulator.test.js create mode 100644 functions/src/__tests__/experiment-id-validation.test.js create mode 100644 functions/src/experiment-id.ts diff --git a/functions/src/__tests__/experiment-id-validation-emulator.test.js b/functions/src/__tests__/experiment-id-validation-emulator.test.js new file mode 100644 index 0000000..6fbd0df --- /dev/null +++ b/functions/src/__tests__/experiment-id-validation-emulator.test.js @@ -0,0 +1,74 @@ +/** + * @jest-environment node + * + * Regression coverage for the reserved/invalid-experimentID 500 bug. + * + * Production logs showed a participant site POSTing + * experimentID: "__DATAPIPE_STUDY1_ID__" -- an unfilled template placeholder + * -- to /api/data. db.collection("experiments").doc(experimentID).get() + * throws "3 INVALID_ARGUMENT: Resource id ... is invalid because it is + * reserved" for that shape, and the throw escaped as an unhandled 500 instead + * of the ordinary 400 EXPERIMENT_NOT_FOUND a nonexistent-but-valid id already + * gets. + * + * isValidExperimentId() (experiment-id.ts) now gates every public, + * unauthenticated endpoint that looks an experiment up by a client-supplied + * id before the Firestore call that would otherwise throw. This suite proves + * the three data-submission endpoints that dispatch straight from a request + * body (api/data, api/base64, api/condition) answer with the normal + * not-found response instead of a 500. + */ + +import MESSAGES from "../api-messages"; +import { fnUrl } from "./helpers/fn-url.js"; + +process.env.FIRESTORE_EMULATOR_HOST = "localhost:8080"; + +// Exactly the placeholder observed in production logs -- also exactly +// Firestore's reserved /^__.*__$/ shape. +const RESERVED_ID = "__DATAPIPE_STUDY1_ID__"; + +async function postJSON(url, body) { + const response = await fetch(url, { + method: "POST", + headers: { "Content-Type": "application/json", Accept: "*/*" }, + body: JSON.stringify(body), + }); + const message = await response.json(); + return { status: response.status, body: message }; +} + +jest.setTimeout(30000); + +describe("reserved/invalid experimentID does not 500", () => { + it("POST /api/data returns 400 EXPERIMENT_NOT_FOUND, not a 500", async () => { + const { status, body } = await postJSON(fnUrl("/api/data"), { + experimentID: RESERVED_ID, + filename: "data.csv", + data: "trial_type\nhtml-keyboard-response\n", + }); + + expect(status).toBe(400); + expect(body).toEqual(MESSAGES.EXPERIMENT_NOT_FOUND); + }); + + it("POST /api/base64 returns 400 EXPERIMENT_NOT_FOUND, not a 500", async () => { + const { status, body } = await postJSON(fnUrl("/api/base64"), { + experimentID: RESERVED_ID, + filename: "image.png", + data: "data:image/png;base64,aGVsbG8=", + }); + + expect(status).toBe(400); + expect(body).toEqual(MESSAGES.EXPERIMENT_NOT_FOUND); + }); + + it("POST /api/condition returns 400 EXPERIMENT_NOT_FOUND, not a 500", async () => { + const { status, body } = await postJSON(fnUrl("/api/condition"), { + experimentID: RESERVED_ID, + }); + + expect(status).toBe(400); + expect(body).toEqual(MESSAGES.EXPERIMENT_NOT_FOUND); + }); +}); diff --git a/functions/src/__tests__/experiment-id-validation.test.js b/functions/src/__tests__/experiment-id-validation.test.js new file mode 100644 index 0000000..60c30c6 --- /dev/null +++ b/functions/src/__tests__/experiment-id-validation.test.js @@ -0,0 +1,68 @@ +/** + * @jest-environment node + * + * isValidExperimentId (experiment-id.ts) -- the gate between a client- + * supplied experimentID and db.collection("experiments").doc(experimentID). + * + * The bug this guards against: Firestore does not treat an invalid document + * id as a lookup miss, it THROWS synchronously out of doc()/get() for a + * handful of specific shapes -- most commonly a participant site that never + * filled in a template placeholder, e.g. "__DATAPIPE_STUDY1_ID__", which + * matches Firestore's own reserved __...__ pattern. That throw was escaping + * every public endpoint that looked an experiment up by id as an unhandled + * 500, instead of the ordinary 400 EXPERIMENT_NOT_FOUND a nonexistent id + * already gets. + * + * Unlike isValidSessionId (staging.ts), this is deliberately NOT pinned to + * the nanoid alphabet create-experiment.ts mints new ids from -- older + * experiment ids may use other formats, so this only has to reject what + * Firestore itself would reject. + */ + +const { isValidExperimentId } = require("../../lib/experiment-id.js"); + +describe("isValidExperimentId", () => { + it.each([ + ["a 12-char nanoid-style id, as create-experiment.ts mints", "aB3xY9kLm2Qz"], + ["an id with mixed alphanumeric, hyphen and underscore characters", "abc-DEF_123"], + ])("accepts %s", (_label, value) => { + expect(isValidExperimentId(value)).toBe(true); + }); + + it.each([ + ["the empty string", ""], + ["a reserved __...__ id (the unfilled-template-placeholder case)", "__DATAPIPE_STUDY1_ID__"], + ["a reserved __...__ id with nothing in between", "____"], + ["an id containing a forward slash", "abc/def"], + ["a leading slash", "/abc"], + ["exactly a single period", "."], + ["exactly a double period", ".."], + ["longer than 1500 bytes in UTF-8", "a".repeat(1501)], + // A multi-byte character pushes this over 1500 BYTES despite being under + // 1500 JS string characters -- proves the check is byte-length, not + // string-length. + ["over 1500 bytes via multi-byte characters, under 1500 JS characters", "é".repeat(751)], + ])("rejects %s", (_label, value) => { + expect(isValidExperimentId(value)).toBe(false); + }); + + it.each([ + ["null", null], + ["undefined", undefined], + ["a number", 123], + ["a plain object", {}], + ["an array", ["a"]], + ["a boolean", true], + ])("rejects %s (not a string)", (_label, value) => { + expect(isValidExperimentId(value)).toBe(false); + }); + + it("accepts an id exactly at the 1500-byte boundary", () => { + expect(isValidExperimentId("a".repeat(1500))).toBe(true); + }); + + it("accepts a period or double period as part of a longer id, not standing alone", () => { + expect(isValidExperimentId("a.b")).toBe(true); + expect(isValidExperimentId("a..b")).toBe(true); + }); +}); diff --git a/functions/src/__tests__/providers-osf.test.js b/functions/src/__tests__/providers-osf.test.js index 0f21296..2928f0f 100644 --- a/functions/src/__tests__/providers-osf.test.js +++ b/functions/src/__tests__/providers-osf.test.js @@ -152,6 +152,8 @@ describe("osfProvider.listFiles", () => { it("returns name/id pairs and filters out folder entries", async () => { mockFetch.mockResolvedValueOnce({ + status: 200, + statusText: "OK", json: () => Promise.resolve({ data: [ @@ -174,6 +176,46 @@ describe("osfProvider.listFiles", () => { expect.objectContaining({ method: "GET" }) ); }); + + // Regression coverage: a non-OK response (403/404/410/429/...) has no + // `data` field -- OSF returns a JSON:API error object instead -- so this + // must be checked before the body is parsed. Left unchecked, + // folder["data"].filter throws an opaque "Cannot read properties of + // undefined (reading 'filter')" instead of naming OSF's real status, which + // is exactly what reached production through the collision cache as + // "Collision cache rehydration failed: ... Cannot read properties of + // undefined (reading 'filter')" with no clue it was really an OSF 403. + it.each([ + [403, "Forbidden"], + [404, "Not Found"], + [410, "Gone"], + [429, "Too Many Requests"], + ])("throws with the status and statusText on a %i response", async (status, statusText) => { + mockFetch.mockResolvedValueOnce({ + status, + statusText, + json: () => Promise.resolve({ errors: [{ detail: "nope" }] }), + }); + + await expect(osfProvider.listFiles(auth, container)).rejects.toThrow( + `OSF listing failed: ${status} ${statusText}` + ); + }); + + // Same failure class as above, but with no status code to blame: a 200 + // whose body doesn't have the expected `data` array. Must not crash inside + // the filter/map with an unhelpful TypeError. + it("throws a clear error on a 200 response with no `data` array", async () => { + mockFetch.mockResolvedValueOnce({ + status: 200, + statusText: "OK", + json: () => Promise.resolve({ unexpected: "shape" }), + }); + + await expect(osfProvider.listFiles(auth, container)).rejects.toThrow( + "OSF listing failed: response body had no file list" + ); + }); }); // Real experiment documents store osfFilesLink as the osfstorage root's @@ -197,6 +239,8 @@ describe("osfProvider file URLs against a real osfstorage upload link", () => { it("updateFile addresses a listFiles id without doubling the provider segment", async () => { mockFetch.mockResolvedValueOnce({ + status: 200, + statusText: "OK", json: () => Promise.resolve({ data: [ diff --git a/functions/src/api-base64.ts b/functions/src/api-base64.ts index 9b35c11..4319f3f 100644 --- a/functions/src/api-base64.ts +++ b/functions/src/api-base64.ts @@ -13,6 +13,7 @@ import { getProviderForExperiment, claimNameFor } from "./providers/index.js"; import { WriteResult, ResolvedAuth } from "./providers/types.js"; import { claimFilename, claimFilenameWithoutCredentials, confirmClaim, CollisionCacheUnavailableError } from "./collision-cache.js"; import { isCompactionInFlight, COMPACTION_HOLD_REASON } from "./compaction-gate.js"; +import { isValidExperimentId } from "./experiment-id.js"; import { ExperimentData, UserData } from './interfaces'; // NO onRequest OPTIONS HERE ANY MORE. apiBase64Handler used to also back a @@ -33,6 +34,13 @@ export async function apiBase64Handler(req: Request, res: Response): Promise = db.collection("experiments").doc(experimentID); const exp_doc: DocumentSnapshot = await exp_doc_ref.get(); diff --git a/functions/src/api-condition.ts b/functions/src/api-condition.ts index 1a80bcf..d541b23 100644 --- a/functions/src/api-condition.ts +++ b/functions/src/api-condition.ts @@ -4,6 +4,7 @@ import { DocumentReference, DocumentData, DocumentSnapshot } from "firebase-admi import { db } from "./app.js"; import writeLog from "./write-log.js"; import MESSAGES from "./api-messages.js"; +import { isValidExperimentId } from "./experiment-id.js"; import { ExperimentData } from './interfaces'; // Plain handler, dispatched from participant-api.ts alongside @@ -18,6 +19,13 @@ export async function apiConditionHandler(req: Request, res: Response): Promise< return; } + // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. + // No writeLog: logs/{experimentID} would throw the same way. + if (!isValidExperimentId(experimentID)) { + res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); + return; + } + const exp_doc_ref: DocumentReference = db.collection("experiments").doc(experimentID); const exp_doc: DocumentSnapshot = await exp_doc_ref.get(); diff --git a/functions/src/api-data.ts b/functions/src/api-data.ts index 23dfddb..b2a51e9 100644 --- a/functions/src/api-data.ts +++ b/functions/src/api-data.ts @@ -21,6 +21,7 @@ import { WriteResult, ResolvedAuth } from "./providers/types.js"; import { claimFilename, claimFilenameWithoutCredentials, confirmClaim, CollisionCacheUnavailableError } from "./collision-cache.js"; import { isCompactionInFlight, COMPACTION_HOLD_REASON } from "./compaction-gate.js"; import { discardSession, isValidSessionId } from "./staging.js"; +import { isValidExperimentId } from "./experiment-id.js"; import { ExperimentData, UserData, RequestBody } from './interfaces'; import { apiBase64Handler } from "./api-base64.js"; @@ -185,6 +186,13 @@ export async function apiDataHandler(req: Request, res: Response): Promise if (isValidSessionId(sessionId)) await discardSession(sessionId); }; + // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. + // No writeLog: logs/{experimentID} would throw the same way. + if (!isValidExperimentId(experimentID)) { + res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); + return; + } + const exp_doc_ref: DocumentReference = db.collection("experiments").doc(experimentID); const exp_doc: DocumentSnapshot = await exp_doc_ref.get(); diff --git a/functions/src/api-finalize.ts b/functions/src/api-finalize.ts index 7d8c5cf..ec03320 100644 --- a/functions/src/api-finalize.ts +++ b/functions/src/api-finalize.ts @@ -48,6 +48,7 @@ import { db, functions } from "./app.js"; import { finalizeExperiment } from "./finalization.js"; import { ExperimentData, FinalizationState } from "./interfaces.js"; import { requireUser } from "./require-user.js"; +import { isValidExperimentId } from "./experiment-id.js"; // Task-queue functions cap at 1800s (see module header). finalizeExperiment // streams rather than buffers (docs/finalization-spec.md's "why streaming, @@ -87,6 +88,12 @@ export async function apiFinalizeHandler(req: Request, res: Response): Promise { if (req.method !== "GET") { @@ -32,6 +33,12 @@ export const apiQueueStatus = onRequest({ cors: true }, async (req, res) => { return; } + // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. + if (!isValidExperimentId(experimentID)) { + res.status(403).json({ error: "Access denied" }); + return; + } + // Verify the user owns this experiment const expDoc = await db.doc(`experiments/${experimentID}`).get(); if (!expDoc.exists || expDoc.data()?.owner !== uid) { diff --git a/functions/src/api-session-start.ts b/functions/src/api-session-start.ts index 1e9562c..8476b38 100644 --- a/functions/src/api-session-start.ts +++ b/functions/src/api-session-start.ts @@ -49,6 +49,7 @@ import { DocumentSnapshot } from "firebase-admin/firestore"; import { db } from "./app.js"; import writeLog from "./write-log.js"; import MESSAGES from "./api-messages.js"; +import { isValidExperimentId } from "./experiment-id.js"; import { ExperimentData } from "./interfaces.js"; import { openSession, @@ -100,6 +101,13 @@ export async function apiSessionStartHandler(req: Request, res: Response): Promi return; } + // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. + // No writeLog: logs/{experimentID} would throw the same way. + if (!isValidExperimentId(experimentID)) { + res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); + return; + } + const exp_doc: DocumentSnapshot = await db .collection("experiments") .doc(experimentID) diff --git a/functions/src/create-experiment.ts b/functions/src/create-experiment.ts index 18e6ca7..642a86c 100644 --- a/functions/src/create-experiment.ts +++ b/functions/src/create-experiment.ts @@ -177,6 +177,9 @@ export async function createExperimentHandler(req: Request, res: Response): Prom providerContainer = await storageProvider.createDataContainer(auth, containerInput); } catch (e) { const detail = e instanceof Error ? e.message : "Unknown error"; + // Otherwise this failure leaves no server-side trace: the 502 below is + // the only record, and it goes to the browser, not Cloud Logging. + console.error(`Error creating storage container for provider ${provider}:`, detail); res.status(502).json({ error: "Failed to create storage container", detail }); return; } diff --git a/functions/src/experiment-id.ts b/functions/src/experiment-id.ts new file mode 100644 index 0000000..d76e31a --- /dev/null +++ b/functions/src/experiment-id.ts @@ -0,0 +1,42 @@ +// Whether a client-supplied experimentID is even a value Firestore will +// accept as a document id -- checked BEFORE it is ever spliced into +// db.collection("experiments").doc(experimentID). Firestore does not treat an +// invalid id as a lookup miss: it THROWS synchronously for a handful of +// specific shapes ("3 INVALID_ARGUMENT: Resource id ... is invalid because it +// is reserved"), and that throw was reaching participant-facing endpoints +// unhandled as a 500. The commonest real case is a participant site that +// never filled in a template placeholder -- e.g. "__DATAPIPE_STUDY1_ID__" -- +// which happens to be exactly the reserved __...__ shape below. +// +// Kept in its own module rather than folded into staging.ts: it has nothing +// to do with the RTDB staging tier that file owns, it just happens to be the +// same kind of gate as isValidSessionId there -- the one check a +// client-controlled string has to clear before it is trusted as a Firestore +// document id. +// +// Deliberately NOT restricted to the nanoid alphabet create-experiment.ts +// mints new ids from: older experiment ids may use other formats, and the +// only thing this function has to guarantee is that whatever it accepts, a +// Firestore doc() call will accept too. (Contrast isValidSessionId in +// staging.ts, which DOES pin the exact format the server mints, because a +// sessionId is never anything but server-generated.) +// +// Firestore's own restrictions on a document id (see +// https://firebase.google.com/docs/firestore/quotas#collections_documents): +// - must be a non-empty string +// - may not contain "/" +// - may not be exactly "." or ".." +// - may not match /^__.*__$/ (reserved for Firestore's own use) +// - may not exceed 1500 bytes in UTF-8 +const RESERVED_ID_PATTERN = /^__.*__$/; +const MAX_ID_BYTES = 1500; + +/** Whether `value` is a string Firestore will accept as a document id. */ +export function isValidExperimentId(value: unknown): value is string { + if (typeof value !== "string" || value.length === 0) return false; + if (value.includes("/")) return false; + if (value === "." || value === "..") return false; + if (RESERVED_ID_PATTERN.test(value)) return false; + if (Buffer.byteLength(value, "utf-8") > MAX_ID_BYTES) return false; + return true; +} diff --git a/functions/src/providers/osf.ts b/functions/src/providers/osf.ts index 7ef03dc..9e00fba 100644 --- a/functions/src/providers/osf.ts +++ b/functions/src/providers/osf.ts @@ -244,8 +244,19 @@ export const osfProvider: StorageProvider = { }, }); - const folder = (await osfResult.json()) as { data: OSFFile[] }; - const listOfFiles: OSFFile[] = folder["data"]; + // An error response has no `data`, so the filter below would throw an opaque + // TypeError. Throw OSF's status instead (same shape as zenodo/gdrive listFiles); + // collision-cache's rehydrate wraps it in CollisionCacheUnavailableError. + if (osfResult.status !== 200) { + throw new Error(`OSF listing failed: ${osfResult.status} ${osfResult.statusText}`); + } + + const folder = (await osfResult.json()) as { data?: OSFFile[] }; + const listOfFiles = folder["data"]; + + if (!Array.isArray(listOfFiles)) { + throw new Error("OSF listing failed: response body had no file list"); + } return listOfFiles .filter((file) => file.attributes.kind === "file") From 8018c38cf299ddda25fac5705440362cf41b068b Mon Sep 17 00:00:00 2001 From: Josh de Leeuw Date: Fri, 25 Sep 2026 13:45:46 -0400 Subject: [PATCH 2/2] Route experiment lookups through one helper and tighten error logs getExperiment() returns null for a missing experiment and for an id Firestore would reject outright, so every endpoint that looks an experiment up by a client-supplied id keeps a single not-found branch and none can reach the doc() call that throws. This replaces the guard copied into six handlers and extends the fix to clear-errors and ensure-derived-paths, which still returned 500s. - api-queue-status checks its `download` queue-entry id the same way (404 instead of a 500 on "__x__" or "a/b"). - writeLog skips ids Firestore would reject, rather than every caller avoiding it on a comment that said it would throw (it never did). - create-experiment logs the Error object and the uid on a 502. - OSF listFiles errors carry OSF's own errors[0].detail, falling back to statusText. - Emulator coverage now includes session start, finalize, clear-errors, ensure-derived-paths and queue-status (experimentID and download). Co-Authored-By: Claude Opus 5.5 --- .../experiment-id-validation-emulator.test.js | 191 +++++++++++++++++- .../experiment-id-validation.test.js | 25 ++- functions/src/__tests__/providers-osf.test.js | 39 +++- functions/src/api-base64.ts | 18 +- functions/src/api-condition.ts | 18 +- functions/src/api-data.ts | 17 +- functions/src/api-finalize.ts | 17 +- functions/src/api-queue-status.ts | 23 ++- functions/src/api-session-start.ts | 20 +- functions/src/clear-errors.ts | 12 +- functions/src/create-experiment.ts | 8 +- functions/src/ensure-derived-paths.ts | 11 +- functions/src/experiment-id.ts | 24 ++- functions/src/providers/osf.ts | 22 +- functions/src/write-log.ts | 6 + 15 files changed, 336 insertions(+), 115 deletions(-) diff --git a/functions/src/__tests__/experiment-id-validation-emulator.test.js b/functions/src/__tests__/experiment-id-validation-emulator.test.js index 6fbd0df..2b1ce03 100644 --- a/functions/src/__tests__/experiment-id-validation-emulator.test.js +++ b/functions/src/__tests__/experiment-id-validation-emulator.test.js @@ -11,36 +11,98 @@ * of the ordinary 400 EXPERIMENT_NOT_FOUND a nonexistent-but-valid id already * gets. * - * isValidExperimentId() (experiment-id.ts) now gates every public, - * unauthenticated endpoint that looks an experiment up by a client-supplied - * id before the Firestore call that would otherwise throw. This suite proves - * the three data-submission endpoints that dispatch straight from a request - * body (api/data, api/base64, api/condition) answer with the normal - * not-found response instead of a 500. + * getExperiment() (experiment-id.ts) -- built on isValidDocumentId() -- now + * gates every endpoint that looks an experiment up by a client-supplied id + * before the Firestore call that would otherwise throw, so each endpoint + * folds the reserved-id case into the not-found branch it already had. This + * suite covers all of them: + * - the unauthenticated, participant-facing endpoints that answer 400 + * EXPERIMENT_NOT_FOUND for both a reserved id and a nonexistent one: + * /api/data, /api/base64, /api/condition, /api/session. + * - the authenticated, researcher-facing dashboard endpoints that answer + * 403 Access denied for a reserved id, a nonexistent id, and someone + * else's experiment alike (the same convention each of them already used + * for "doesn't exist" vs. "not yours"): /api/finalize, /api/clearerrors, + * /api/ensurederivedpaths, and /api/queuestatus's own experimentID + * parameter. + * - /api/queuestatus's separate `download` query parameter, which is + * checked with the same isValidDocumentId() gate but answers its own + * shape (404 "Queue entry not found") since a queue entry, not the + * experiment, is what a reserved or slash-containing id there would + * otherwise 500 trying to look up. */ +import { initializeApp, getApp } from "firebase-admin/app"; +import { getFirestore } from "firebase-admin/firestore"; +import { randomUUID } from "crypto"; import MESSAGES from "../api-messages"; import { fnUrl } from "./helpers/fn-url.js"; process.env.FIRESTORE_EMULATOR_HOST = "localhost:8080"; +const config = { projectId: "datapipe-test" }; +const AUTH_EMULATOR_SIGNUP_URL = + "http://localhost:9099/identitytoolkit.googleapis.com/v1/accounts:signUp?key=fake"; + // Exactly the placeholder observed in production logs -- also exactly // Firestore's reserved /^__.*__$/ shape. const RESERVED_ID = "__DATAPIPE_STUDY1_ID__"; +// A shorter reserved id, used for api-queue-status's `download` param below +// -- any /^__.*__$/ shape triggers the same Firestore throw, so this only +// needs to be reserved, not the exact production placeholder. +const RESERVED_DOWNLOAD_ID = "__x__"; + +jest.setTimeout(30000); + +let db; -async function postJSON(url, body) { +beforeAll(() => { + let app; + try { + app = getApp("experiment-id-validation-test"); + } catch { + app = initializeApp(config, "experiment-id-validation-test"); + } + db = getFirestore(app); +}); + +async function signUpEmulatorUser() { + const email = `experiment-id-validation-${randomUUID()}@example.test`; + const res = await fetch(AUTH_EMULATOR_SIGNUP_URL, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ email, password: "Password123!", returnSecureToken: true }), + }); + const body = await res.json(); + if (!res.ok) { + throw new Error(`Auth emulator signUp failed (${res.status}): ${JSON.stringify(body)}`); + } + return { uid: body.localId, idToken: body.idToken }; +} + +async function postJSON(url, body, idToken) { + const headers = { "Content-Type": "application/json", Accept: "*/*" }; + if (idToken !== undefined) { + headers.Authorization = `Bearer ${idToken}`; + } const response = await fetch(url, { method: "POST", - headers: { "Content-Type": "application/json", Accept: "*/*" }, + headers, body: JSON.stringify(body), }); const message = await response.json(); return { status: response.status, body: message }; } -jest.setTimeout(30000); +async function getJSON(url, idToken) { + const response = await fetch(url, { + headers: idToken !== undefined ? { Authorization: `Bearer ${idToken}` } : {}, + }); + const message = await response.json(); + return { status: response.status, body: message }; +} -describe("reserved/invalid experimentID does not 500", () => { +describe("reserved/invalid experimentID does not 500 (unauthenticated participant endpoints)", () => { it("POST /api/data returns 400 EXPERIMENT_NOT_FOUND, not a 500", async () => { const { status, body } = await postJSON(fnUrl("/api/data"), { experimentID: RESERVED_ID, @@ -71,4 +133,113 @@ describe("reserved/invalid experimentID does not 500", () => { expect(status).toBe(400); expect(body).toEqual(MESSAGES.EXPERIMENT_NOT_FOUND); }); + + it("POST /api/session returns 400 EXPERIMENT_NOT_FOUND, not a 500", async () => { + const { status, body } = await postJSON(fnUrl("/api/session"), { + experimentID: RESERVED_ID, + }); + + expect(status).toBe(400); + expect(body).toEqual(MESSAGES.EXPERIMENT_NOT_FOUND); + }); +}); + +describe("reserved/invalid experimentID does not 500 (authenticated dashboard endpoints)", () => { + it("POST /api/finalize returns 403 Access denied, not a 500", async () => { + const { idToken } = await signUpEmulatorUser(); + const { status, body } = await postJSON( + fnUrl("/api/finalize"), + { experimentID: RESERVED_ID }, + idToken + ); + + expect(status).toBe(403); + expect(body).toEqual({ error: "Access denied" }); + }); + + it("POST /api/clearerrors returns 403 Access denied, not a 500", async () => { + const { idToken } = await signUpEmulatorUser(); + const { status, body } = await postJSON( + fnUrl("/api/clearerrors"), + { experimentID: RESERVED_ID }, + idToken + ); + + expect(status).toBe(403); + expect(body).toEqual({ error: "Access denied" }); + }); + + it("POST /api/ensurederivedpaths returns 403 Access denied, not a 500", async () => { + const { idToken } = await signUpEmulatorUser(); + const { status, body } = await postJSON( + fnUrl("/api/ensurederivedpaths"), + { experimentID: RESERVED_ID }, + idToken + ); + + expect(status).toBe(403); + expect(body).toEqual({ error: "Access denied" }); + }); + + it("GET /api/queuestatus?experimentID=__X__ returns 403 Access denied, not a 500", async () => { + const { idToken } = await signUpEmulatorUser(); + const { status, body } = await getJSON( + `${fnUrl("/api/queuestatus")}?experimentID=${RESERVED_ID}`, + idToken + ); + + expect(status).toBe(403); + expect(body).toEqual({ error: "Access denied" }); + }); +}); + +describe("api-queue-status: reserved/slash-containing `download` id does not 500", () => { + const createdExperimentIds = []; + + afterEach(async () => { + if (createdExperimentIds.length === 0) return; + const batch = db.batch(); + for (const experimentID of createdExperimentIds) { + batch.delete(db.collection("experiments").doc(experimentID)); + } + await batch.commit(); + createdExperimentIds.length = 0; + }); + + async function seedOwnedExperiment(uid) { + const experimentID = `queue-status-reserved-download-${randomUUID()}`; + createdExperimentIds.push(experimentID); + await db.collection("experiments").doc(experimentID).set({ + owner: uid, + active: true, + storageProvider: "gdrive", + }); + return experimentID; + } + + it("returns 404 Queue entry not found for a reserved __x__ download id, on an experiment the caller owns", async () => { + const { uid, idToken } = await signUpEmulatorUser(); + const experimentID = await seedOwnedExperiment(uid); + + const { status, body } = await getJSON( + `${fnUrl("/api/queuestatus")}?experimentID=${experimentID}&download=${RESERVED_DOWNLOAD_ID}`, + idToken + ); + + expect(status).toBe(404); + expect(body).toEqual({ error: "Queue entry not found" }); + }); + + it("returns 404 Queue entry not found for a slash-containing download id (a%2Fb), on an experiment the caller owns", async () => { + const { uid, idToken } = await signUpEmulatorUser(); + const experimentID = await seedOwnedExperiment(uid); + + const { status, body } = await getJSON( + `${fnUrl("/api/queuestatus")}?experimentID=${experimentID}&download=${encodeURIComponent("a/b")}`, + idToken + ); + + expect(status).toBe(404); + expect(body).toEqual({ error: "Queue entry not found" }); + }); }); diff --git a/functions/src/__tests__/experiment-id-validation.test.js b/functions/src/__tests__/experiment-id-validation.test.js index 60c30c6..740bcd8 100644 --- a/functions/src/__tests__/experiment-id-validation.test.js +++ b/functions/src/__tests__/experiment-id-validation.test.js @@ -1,8 +1,13 @@ /** * @jest-environment node * - * isValidExperimentId (experiment-id.ts) -- the gate between a client- - * supplied experimentID and db.collection("experiments").doc(experimentID). + * isValidDocumentId (experiment-id.ts) -- the gate between a client-supplied + * id and db.collection(...).doc(id). Named for what it actually checks (any + * Firestore document id), not just the experiments collection: api-queue- + * status.ts runs its `download` queue-entry id through the same check, and + * write-log.ts its logs/{experimentID} id. It was renamed from + * isValidExperimentId to isValidDocumentId to reflect that; the logic itself + * is unchanged. * * The bug this guards against: Firestore does not treat an invalid document * id as a lookup miss, it THROWS synchronously out of doc()/get() for a @@ -19,14 +24,14 @@ * Firestore itself would reject. */ -const { isValidExperimentId } = require("../../lib/experiment-id.js"); +const { isValidDocumentId } = require("../../lib/experiment-id.js"); -describe("isValidExperimentId", () => { +describe("isValidDocumentId", () => { it.each([ ["a 12-char nanoid-style id, as create-experiment.ts mints", "aB3xY9kLm2Qz"], ["an id with mixed alphanumeric, hyphen and underscore characters", "abc-DEF_123"], ])("accepts %s", (_label, value) => { - expect(isValidExperimentId(value)).toBe(true); + expect(isValidDocumentId(value)).toBe(true); }); it.each([ @@ -43,7 +48,7 @@ describe("isValidExperimentId", () => { // string-length. ["over 1500 bytes via multi-byte characters, under 1500 JS characters", "é".repeat(751)], ])("rejects %s", (_label, value) => { - expect(isValidExperimentId(value)).toBe(false); + expect(isValidDocumentId(value)).toBe(false); }); it.each([ @@ -54,15 +59,15 @@ describe("isValidExperimentId", () => { ["an array", ["a"]], ["a boolean", true], ])("rejects %s (not a string)", (_label, value) => { - expect(isValidExperimentId(value)).toBe(false); + expect(isValidDocumentId(value)).toBe(false); }); it("accepts an id exactly at the 1500-byte boundary", () => { - expect(isValidExperimentId("a".repeat(1500))).toBe(true); + expect(isValidDocumentId("a".repeat(1500))).toBe(true); }); it("accepts a period or double period as part of a longer id, not standing alone", () => { - expect(isValidExperimentId("a.b")).toBe(true); - expect(isValidExperimentId("a..b")).toBe(true); + expect(isValidDocumentId("a.b")).toBe(true); + expect(isValidDocumentId("a..b")).toBe(true); }); }); diff --git a/functions/src/__tests__/providers-osf.test.js b/functions/src/__tests__/providers-osf.test.js index 2928f0f..edc2fb5 100644 --- a/functions/src/__tests__/providers-osf.test.js +++ b/functions/src/__tests__/providers-osf.test.js @@ -190,11 +190,11 @@ describe("osfProvider.listFiles", () => { [404, "Not Found"], [410, "Gone"], [429, "Too Many Requests"], - ])("throws with the status and statusText on a %i response", async (status, statusText) => { + ])("throws with the status and statusText on a %i response with no detail in the body", async (status, statusText) => { mockFetch.mockResolvedValueOnce({ status, statusText, - json: () => Promise.resolve({ errors: [{ detail: "nope" }] }), + json: () => Promise.resolve({ errors: [{}] }), }); await expect(osfProvider.listFiles(auth, container)).rejects.toThrow( @@ -202,6 +202,41 @@ describe("osfProvider.listFiles", () => { ); }); + // OSF's JSON:API error body carries its own explanation in errors[0].detail + // -- surfaced in preference to the generic statusText whenever it's present, + // since "403 Forbidden" alone gives a researcher nothing actionable while + // OSF's own detail says exactly what went wrong. + it("throws with OSF's own errors[0].detail on a 403 response, not just the statusText", async () => { + mockFetch.mockResolvedValueOnce({ + status: 403, + statusText: "Forbidden", + json: () => + Promise.resolve({ + errors: [{ detail: "You do not have permission to perform this action." }], + }), + }); + + await expect(osfProvider.listFiles(auth, container)).rejects.toThrow( + "OSF listing failed: 403 You do not have permission to perform this action." + ); + }); + + // Some error responses (a proxy timeout page, an HTML 5xx) are not JSON at + // all -- .json() rejects instead of resolving with a body that merely lacks + // `errors`. Must fall back to statusText rather than let that rejection + // propagate as an unrelated, confusing failure. + it("falls back to statusText when the error body is not JSON", async () => { + mockFetch.mockResolvedValueOnce({ + status: 502, + statusText: "Bad Gateway", + json: () => Promise.reject(new SyntaxError("Unexpected token < in JSON at position 0")), + }); + + await expect(osfProvider.listFiles(auth, container)).rejects.toThrow( + "OSF listing failed: 502 Bad Gateway" + ); + }); + // Same failure class as above, but with no status code to blame: a 200 // whose body doesn't have the expected `data` array. Must not crash inside // the filter/map with an unhelpful TypeError. diff --git a/functions/src/api-base64.ts b/functions/src/api-base64.ts index 4319f3f..c8ca477 100644 --- a/functions/src/api-base64.ts +++ b/functions/src/api-base64.ts @@ -1,7 +1,6 @@ import type { Request } from "firebase-functions/v2/https"; import type { Response } from "express"; import { randomUUID } from "crypto"; -import { DocumentReference, DocumentData, DocumentSnapshot } from "firebase-admin/firestore"; import { db } from "./app.js"; import writeLog from "./write-log.js"; import isBase64 from "is-base64"; @@ -13,7 +12,7 @@ import { getProviderForExperiment, claimNameFor } from "./providers/index.js"; import { WriteResult, ResolvedAuth } from "./providers/types.js"; import { claimFilename, claimFilenameWithoutCredentials, confirmClaim, CollisionCacheUnavailableError } from "./collision-cache.js"; import { isCompactionInFlight, COMPACTION_HOLD_REASON } from "./compaction-gate.js"; -import { isValidExperimentId } from "./experiment-id.js"; +import { getExperiment } from "./experiment-id.js"; import { ExperimentData, UserData } from './interfaces'; // NO onRequest OPTIONS HERE ANY MORE. apiBase64Handler used to also back a @@ -34,22 +33,17 @@ export async function apiBase64Handler(req: Request, res: Response): Promise = db.collection("experiments").doc(experimentID); - const exp_doc: DocumentSnapshot = await exp_doc_ref.get(); + // null for a missing experiment AND for an id Firestore would reject + // outright (e.g. an unfilled "__X__" placeholder); see experiment-id.ts. + const exp_doc = await getExperiment(experimentID); - if (!exp_doc.exists) { + if (!exp_doc) { res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); await writeLog(experimentID, "logError", MESSAGES.EXPERIMENT_NOT_FOUND); return; } + const exp_doc_ref = exp_doc.ref; const exp_data: ExperimentData = exp_doc.data() as ExperimentData; if (!exp_data) { diff --git a/functions/src/api-condition.ts b/functions/src/api-condition.ts index d541b23..85122b5 100644 --- a/functions/src/api-condition.ts +++ b/functions/src/api-condition.ts @@ -1,10 +1,9 @@ import type { Request } from "firebase-functions/v2/https"; import type { Response } from "express"; -import { DocumentReference, DocumentData, DocumentSnapshot } from "firebase-admin/firestore"; import { db } from "./app.js"; import writeLog from "./write-log.js"; import MESSAGES from "./api-messages.js"; -import { isValidExperimentId } from "./experiment-id.js"; +import { getExperiment } from "./experiment-id.js"; import { ExperimentData } from './interfaces'; // Plain handler, dispatched from participant-api.ts alongside @@ -19,22 +18,17 @@ export async function apiConditionHandler(req: Request, res: Response): Promise< return; } - // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. - // No writeLog: logs/{experimentID} would throw the same way. - if (!isValidExperimentId(experimentID)) { - res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); - return; - } - - const exp_doc_ref: DocumentReference = db.collection("experiments").doc(experimentID); - const exp_doc: DocumentSnapshot = await exp_doc_ref.get(); + // null for a missing experiment AND for an id Firestore would reject + // outright (e.g. an unfilled "__X__" placeholder); see experiment-id.ts. + const exp_doc = await getExperiment(experimentID); - if (!exp_doc.exists) { + if (!exp_doc) { res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); await writeLog(experimentID, "logError", MESSAGES.EXPERIMENT_NOT_FOUND); return; } + const exp_doc_ref = exp_doc.ref; const exp_data: ExperimentData = exp_doc.data() as ExperimentData; if (!exp_data) { diff --git a/functions/src/api-data.ts b/functions/src/api-data.ts index b2a51e9..1bebe0b 100644 --- a/functions/src/api-data.ts +++ b/functions/src/api-data.ts @@ -21,7 +21,7 @@ import { WriteResult, ResolvedAuth } from "./providers/types.js"; import { claimFilename, claimFilenameWithoutCredentials, confirmClaim, CollisionCacheUnavailableError } from "./collision-cache.js"; import { isCompactionInFlight, COMPACTION_HOLD_REASON } from "./compaction-gate.js"; import { discardSession, isValidSessionId } from "./staging.js"; -import { isValidExperimentId } from "./experiment-id.js"; +import { getExperiment } from "./experiment-id.js"; import { ExperimentData, UserData, RequestBody } from './interfaces'; import { apiBase64Handler } from "./api-base64.js"; @@ -186,23 +186,18 @@ export async function apiDataHandler(req: Request, res: Response): Promise if (isValidSessionId(sessionId)) await discardSession(sessionId); }; - // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. - // No writeLog: logs/{experimentID} would throw the same way. - if (!isValidExperimentId(experimentID)) { - res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); - return; - } - - const exp_doc_ref: DocumentReference = db.collection("experiments").doc(experimentID); - const exp_doc: DocumentSnapshot = await exp_doc_ref.get(); + // null for a missing experiment AND for an id Firestore would reject + // outright (e.g. an unfilled "__X__" placeholder); see experiment-id.ts. + const exp_doc = await getExperiment(experimentID); - if (!exp_doc.exists) { + if (!exp_doc) { res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); await writeLog(experimentID, "logError", MESSAGES.EXPERIMENT_NOT_FOUND); return; } + const exp_doc_ref = exp_doc.ref; const exp_data: ExperimentData = exp_doc.data() as ExperimentData; if (!exp_data) { diff --git a/functions/src/api-finalize.ts b/functions/src/api-finalize.ts index ec03320..9bab8cc 100644 --- a/functions/src/api-finalize.ts +++ b/functions/src/api-finalize.ts @@ -48,7 +48,7 @@ import { db, functions } from "./app.js"; import { finalizeExperiment } from "./finalization.js"; import { ExperimentData, FinalizationState } from "./interfaces.js"; import { requireUser } from "./require-user.js"; -import { isValidExperimentId } from "./experiment-id.js"; +import { getExperiment } from "./experiment-id.js"; // Task-queue functions cap at 1800s (see module header). finalizeExperiment // streams rather than buffers (docs/finalization-spec.md's "why streaming, @@ -88,22 +88,17 @@ export async function apiFinalizeHandler(req: Request, res: Response): Promise { if (req.method !== "GET") { @@ -33,15 +33,10 @@ export const apiQueueStatus = onRequest({ cors: true }, async (req, res) => { return; } - // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. - if (!isValidExperimentId(experimentID)) { - res.status(403).json({ error: "Access denied" }); - return; - } - - // Verify the user owns this experiment - const expDoc = await db.doc(`experiments/${experimentID}`).get(); - if (!expDoc.exists || expDoc.data()?.owner !== uid) { + // Verify the user owns this experiment. getExperiment's null also covers an + // id Firestore would reject outright, which would otherwise throw as a 500. + const expDoc = await getExperiment(experimentID); + if (!expDoc || expDoc.data()?.owner !== uid) { res.status(403).json({ error: "Access denied" }); return; } @@ -49,7 +44,13 @@ export const apiQueueStatus = onRequest({ cors: true }, async (req, res) => { const download = req.query.download as string | undefined; if (download) { - // Return a signed download URL for a specific queue entry + // Return a signed download URL for a specific queue entry. The id is + // checked first for the same reason as experimentID: doc() throws on a + // reserved "__X__" id, and a "/" would make this a collection path. + if (!isValidDocumentId(download)) { + res.status(404).json({ error: "Queue entry not found" }); + return; + } const queueDoc = await db.doc(`uploadQueue/${download}`).get(); if (!queueDoc.exists || queueDoc.data()?.experimentID !== experimentID) { res.status(404).json({ error: "Queue entry not found" }); diff --git a/functions/src/api-session-start.ts b/functions/src/api-session-start.ts index 8476b38..2635b4e 100644 --- a/functions/src/api-session-start.ts +++ b/functions/src/api-session-start.ts @@ -45,11 +45,9 @@ import type { Request } from "firebase-functions/v2/https"; import type { Response } from "express"; -import { DocumentSnapshot } from "firebase-admin/firestore"; -import { db } from "./app.js"; import writeLog from "./write-log.js"; import MESSAGES from "./api-messages.js"; -import { isValidExperimentId } from "./experiment-id.js"; +import { getExperiment } from "./experiment-id.js"; import { ExperimentData } from "./interfaces.js"; import { openSession, @@ -101,19 +99,11 @@ export async function apiSessionStartHandler(req: Request, res: Response): Promi return; } - // Firestore throws (not misses) on reserved ids like "__X__"; see experiment-id.ts. - // No writeLog: logs/{experimentID} would throw the same way. - if (!isValidExperimentId(experimentID)) { - res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); - return; - } - - const exp_doc: DocumentSnapshot = await db - .collection("experiments") - .doc(experimentID) - .get(); + // null for a missing experiment AND for an id Firestore would reject + // outright (e.g. an unfilled "__X__" placeholder); see experiment-id.ts. + const exp_doc = await getExperiment(experimentID); - if (!exp_doc.exists) { + if (!exp_doc) { res.status(400).json(MESSAGES.EXPERIMENT_NOT_FOUND); await writeLog(experimentID, "logError", MESSAGES.EXPERIMENT_NOT_FOUND); return; diff --git a/functions/src/clear-errors.ts b/functions/src/clear-errors.ts index dc61462..e7caec8 100644 --- a/functions/src/clear-errors.ts +++ b/functions/src/clear-errors.ts @@ -29,10 +29,7 @@ import { FieldValue } from "firebase-admin/firestore"; import { db } from "./app.js"; import MESSAGES from "./api-messages.js"; import { requireUser } from "./require-user.js"; - -function experimentRef(experimentID: string) { - return db.collection("experiments").doc(experimentID); -} +import { getExperiment } from "./experiment-id.js"; function logsRef(experimentID: string) { return db.collection("logs").doc(experimentID); @@ -57,12 +54,13 @@ export async function clearErrorsHandler(req: Request, res: Response): Promise MAX_ID_BYTES) return false; return true; } + +/** + * experiments/{experimentID}, or null when there is no such experiment -- + * including when `experimentID` is not an id Firestore would accept at all. + * Every endpoint that looks an experiment up by a client-supplied id goes + * through this, so none of them can reach the doc() call that throws, and + * each keeps the single not-found branch it already had. + */ +export async function getExperiment(experimentID: unknown): Promise { + if (!isValidDocumentId(experimentID)) return null; + const snap = await db.collection("experiments").doc(experimentID).get(); + return snap.exists ? snap : null; +} diff --git a/functions/src/providers/osf.ts b/functions/src/providers/osf.ts index 9e00fba..c629bad 100644 --- a/functions/src/providers/osf.ts +++ b/functions/src/providers/osf.ts @@ -57,6 +57,20 @@ function mapStatus(errorCode: number | null): ProviderErrorCode { } } +// OSF's own explanation of an error response: the JSON:API body's +// errors[0].detail, e.g. "You do not have permission to perform this action." +// Empty when the body is not JSON or has no detail -- the caller falls back to +// statusText, which is itself empty behind some proxies and over HTTP/2. +async function osfErrorDetail(response: { json: () => Promise }): Promise { + try { + const body = (await response.json()) as { errors?: { detail?: unknown }[] }; + const detail = body?.errors?.[0]?.detail; + return typeof detail === "string" ? detail : ""; + } catch { + return ""; + } +} + export const osfProvider: StorageProvider = { id: "osf", authMethod: "oauth2", @@ -245,10 +259,12 @@ export const osfProvider: StorageProvider = { }); // An error response has no `data`, so the filter below would throw an opaque - // TypeError. Throw OSF's status instead (same shape as zenodo/gdrive listFiles); - // collision-cache's rehydrate wraps it in CollisionCacheUnavailableError. + // TypeError. Throw OSF's status and its own explanation instead (same shape + // as zenodo/gdrive listFiles); collision-cache's rehydrate wraps it in + // CollisionCacheUnavailableError. if (osfResult.status !== 200) { - throw new Error(`OSF listing failed: ${osfResult.status} ${osfResult.statusText}`); + const reason = (await osfErrorDetail(osfResult)) || osfResult.statusText; + throw new Error(`OSF listing failed: ${osfResult.status} ${reason}`.trim()); } const folder = (await osfResult.json()) as { data?: OSFFile[] }; diff --git a/functions/src/write-log.ts b/functions/src/write-log.ts index 5ee8cf5..be68092 100644 --- a/functions/src/write-log.ts +++ b/functions/src/write-log.ts @@ -1,6 +1,7 @@ import { db } from "./app.js"; import { FieldValue, Timestamp } from "firebase-admin/firestore"; import { StorageProviderId } from "./providers/types.js"; +import { isValidDocumentId } from "./experiment-id.js"; /** * logs/{experimentID} — the per-experiment activity record. @@ -144,6 +145,11 @@ export default async function writeLog( error?: object, context?: LogContext ): Promise { + // A client-supplied id Firestore would reject (an unfilled "__X__" + // placeholder, say) can never have a log document. doc() would throw into + // the catch below and print an error per request; skip it quietly instead. + if (!isValidDocumentId(experimentID)) return false; + try { const log_doc_ref = db.collection("logs").doc(experimentID);