Return clear errors instead of crashes on three production failure paths

functions/src/__tests__/experiment-id-validation-emulator.test.js

@@ -0,0 +1,245 @@
+/**
+ * @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.
+ *
+ * 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;
+
+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,
+    body: JSON.stringify(body),
+  });
+  const message = await response.json();
+  return { status: response.status, body: message };
+}
+
+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 (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,
+      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);
+  });
+
+  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" });
+  });
+});

functions/src/__tests__/experiment-id-validation.test.js

@@ -0,0 +1,73 @@
+/**
+ * @jest-environment node
+ *
+ * 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
+ * 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 { isValidDocumentId } = require("../../lib/experiment-id.js");
+
+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(isValidDocumentId(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(isValidDocumentId(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(isValidDocumentId(value)).toBe(false);
+  });
+
+  it("accepts an id exactly at the 1500-byte boundary", () => {
+    expect(isValidDocumentId("a".repeat(1500))).toBe(true);
+  });
+
+  it("accepts a period or double period as part of a longer id, not standing alone", () => {
+    expect(isValidDocumentId("a.b")).toBe(true);
+    expect(isValidDocumentId("a..b")).toBe(true);
+  });
+});