fix: enforce folder/file ancestry checks to prevent cross-space document access

User-supplied folderId query parameters were accepted without verifying
they belonged to the target space's Drive folder tree. An authenticated
user in Space A could supply Space B's folder ID to list, upload, or
create folders in unauthorised spaces. Similarly, download and delete
endpoints accepted arbitrary file IDs without ownership verification.

Add verifyFolderAncestry and verifyFileAncestry functions that walk the
Drive API parent chain. Apply format validation and ancestry checks to
all five affected endpoints (list, section list, upload, create folder,
download, delete).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: rorydavidson

apps/bff/src/routes/documents-extra.test.ts

@@ -36,6 +36,8 @@ vi.mock("../services/drive.js", () => ({
   uploadFile: vi.fn(),
   searchFilesInFolders: vi.fn(),
   checkDriveAccess: vi.fn(),
+  verifyFolderAncestry: vi.fn().mockResolvedValue(true),
+  verifyFileAncestry: vi.fn().mockResolvedValue(true),
 }));
 
 import * as db from "../services/db.js";

apps/bff/src/routes/documents.test.ts

@@ -22,6 +22,8 @@ vi.mock('../services/drive.js', () => ({
   uploadFile: vi.fn(),
   searchFilesInFolders: vi.fn(),
   checkDriveAccess: vi.fn(),
+  verifyFolderAncestry: vi.fn().mockResolvedValue(true),
+  verifyFileAncestry: vi.fn().mockResolvedValue(true),
 }));
 
 import * as db from '../services/db.js';

apps/bff/src/routes/documents.ts

@@ -11,7 +11,7 @@ import os from "os";
 import { requireAuth } from "../middleware/requireAuth.js";
 import { asyncHandler } from "../middleware/asyncHandler.js";
 import { getSpaces, getSpaceById, getSectionById, createAuditLog, getCategoryConfigs } from "../services/db.js";
-import { listFiles, downloadFile, uploadFile, deleteFile, createFolder } from "../services/drive.js";
+import { listFiles, downloadFile, uploadFile, deleteFile, createFolder, verifyFolderAncestry, verifyFileAncestry } from "../services/drive.js";
 
 // Allowed MIME types for uploads — documents and common office formats only
 const ALLOWED_MIME_TYPES = new Set([
@@ -87,6 +87,16 @@ export function isAdminUser(groups: string[]): boolean {
   return groups.some((g) => g === "portal_admin" || g === "/portal_admin");
 }
 
+// ---------------------------------------------------------------------------
+// Helper: validate a user-supplied Drive folder ID
+// ---------------------------------------------------------------------------
+
+const DRIVE_ID_PATTERN = /^[a-zA-Z0-9_-]+$/;
+
+function isValidDriveId(id: string): boolean {
+  return DRIVE_ID_PATTERN.test(id) && id.length >= 1 && id.length <= 128;
+}
+
 // ---------------------------------------------------------------------------
 // GET /documents/categories — category sort-order config (auth required, no admin needed)
 // Used by the spaces listing page to order category sections correctly.
@@ -139,7 +149,21 @@ router.get("/:spaceId", async (req: Request, res: Response): Promise<void> => {
 
   try {
     const folderId = req.query.folderId as string | undefined;
-    const targetFolderId = folderId || space.driveFolderId;
+    let targetFolderId = space.driveFolderId;
+
+    if (folderId) {
+      if (!isValidDriveId(folderId)) {
+        res.status(400).json({ error: "Invalid folder ID", code: "INVALID_FOLDER_ID" });
+        return;
+      }
+      const belongs = await verifyFolderAncestry(folderId, space.driveFolderId);
+      if (!belongs) {
+        res.status(403).json({ error: "Folder is not within this space", code: "FOLDER_OUTSIDE_SPACE" });
+        return;
+      }
+      targetFolderId = folderId;
+    }
+
     const files = await listFiles(targetFolderId);
     res.json({ space, files });
   } catch (err) {
@@ -211,7 +235,21 @@ router.get(
 
     try {
       const folderId = req.query.folderId as string | undefined;
-      const targetFolderId = folderId || section.driveFolderId;
+      let targetFolderId = section.driveFolderId;
+
+      if (folderId) {
+        if (!isValidDriveId(folderId)) {
+          res.status(400).json({ error: "Invalid folder ID", code: "INVALID_FOLDER_ID" });
+          return;
+        }
+        const belongs = await verifyFolderAncestry(folderId, space.driveFolderId);
+        if (!belongs) {
+          res.status(403).json({ error: "Folder is not within this space", code: "FOLDER_OUTSIDE_SPACE" });
+          return;
+        }
+        targetFolderId = folderId;
+      }
+
       const files = await listFiles(targetFolderId);
       res.json({ space, section, files });
     } catch (err) {
@@ -253,9 +291,14 @@ router.get(
     }
 
     try {
-      const { stream, mimeType, name } = await downloadFile(
-        String(req.params.fileId),
-      );
+      const fileId = String(req.params.fileId);
+      const fileBelongs = await verifyFileAncestry(fileId, space.driveFolderId);
+      if (!fileBelongs) {
+        res.status(403).json({ error: "File is not within this space", code: "FILE_OUTSIDE_SPACE" });
+        return;
+      }
+
+      const { stream, mimeType, name } = await downloadFile(fileId);
       // ?download=1 → force browser save-as dialog (attachment) regardless of type.
       // Without the flag: PDFs stream inline (so the in-portal PDF viewer can fetch them);
       // all other types are always forced to attachment.
@@ -377,12 +420,22 @@ router.post(
     }
 
     try {
-      // Priority: 1. folderId (subfolder), 2. sectionId (category folder), 3. space.driveFolderId (root)
       const folderId = req.query.folderId as string | undefined;
       const sectionId = req.query.sectionId as string | undefined;
       let targetFolderId = space.driveFolderId;
 
       if (folderId) {
+        if (!isValidDriveId(folderId)) {
+          fs.unlink(file.path, () => { });
+          res.status(400).json({ error: "Invalid folder ID", code: "INVALID_FOLDER_ID" });
+          return;
+        }
+        const belongs = await verifyFolderAncestry(folderId, space.driveFolderId);
+        if (!belongs) {
+          fs.unlink(file.path, () => { });
+          res.status(403).json({ error: "Folder is not within this space", code: "FOLDER_OUTSIDE_SPACE" });
+          return;
+        }
         targetFolderId = folderId;
       } else if (sectionId) {
         const section = await getSectionById(space.id, sectionId);
@@ -482,6 +535,15 @@ router.post(
       let targetFolderId = space.driveFolderId;
 
       if (folderId) {
+        if (!isValidDriveId(folderId)) {
+          res.status(400).json({ error: "Invalid folder ID", code: "INVALID_FOLDER_ID" });
+          return;
+        }
+        const belongs = await verifyFolderAncestry(folderId, space.driveFolderId);
+        if (!belongs) {
+          res.status(403).json({ error: "Folder is not within this space", code: "FOLDER_OUTSIDE_SPACE" });
+          return;
+        }
         targetFolderId = folderId;
       } else if (sectionId) {
         const section = await getSectionById(space.id, sectionId);
@@ -547,6 +609,12 @@ router.delete(
     }
 
     try {
+      const fileBelongs = await verifyFileAncestry(fileId, space.driveFolderId);
+      if (!fileBelongs) {
+        res.status(403).json({ error: "File is not within this space", code: "FILE_OUTSIDE_SPACE" });
+        return;
+      }
+
       await deleteFile(fileId);
 
       res.status(204).end();

apps/bff/src/services/drive.ts

@@ -412,6 +412,61 @@ export async function deleteFile(fileId: string): Promise<void> {
   });
 }
 
+/**
+ * Verify that `folderId` is a descendant of (or equal to) `rootFolderId`
+ * by walking the parents chain. Returns true if the ancestry is confirmed.
+ * Protects against IDOR where a user supplies an arbitrary Drive folder ID
+ * to access files outside their authorised space.
+ */
+export async function verifyFolderAncestry(
+  folderId: string,
+  rootFolderId: string,
+): Promise<boolean> {
+  if (folderId === rootFolderId) return true;
+  if (isMockMode()) return true;
+
+  const MAX_DEPTH = 15;
+  let currentId = folderId;
+
+  for (let i = 0; i < MAX_DEPTH; i++) {
+    const res = await drive().files.get({
+      fileId: currentId,
+      fields: "parents",
+      supportsAllDrives: true,
+    });
+
+    const parents = res.data.parents;
+    if (!parents || parents.length === 0) return false;
+
+    if (parents.includes(rootFolderId)) return true;
+    currentId = parents[0];
+  }
+
+  return false;
+}
+
+/**
+ * Verify that a file belongs to the folder tree rooted at `rootFolderId`.
+ * Gets the file's immediate parent and then walks up the ancestry chain.
+ */
+export async function verifyFileAncestry(
+  fileId: string,
+  rootFolderId: string,
+): Promise<boolean> {
+  if (isMockMode()) return true;
+
+  const res = await drive().files.get({
+    fileId,
+    fields: "parents",
+    supportsAllDrives: true,
+  });
+
+  const parents = res.data.parents;
+  if (!parents || parents.length === 0) return false;
+
+  return verifyFolderAncestry(parents[0], rootFolderId);
+}
+
 /**
  * Check whether the Drive service is reachable (used at startup).
  */

pnpm-workspace.yaml

@@ -1,3 +1,9 @@
 packages:
   - 'apps/*'
   - 'packages/*'
+allowBuilds:
+  better-sqlite3: true
+  canvas: true
+  esbuild: true
+  sharp: true
+  unrs-resolver: true