feat(mirrors): reject prefixes that overlap another product

src/lib/actions/product-mirrors.test.ts

@@ -14,6 +14,7 @@ jest.mock("../clients", () => ({
   productsTable: {
     fetchById: jest.fn(),
     update: jest.fn(),
+    listProductsByConnectionId: jest.fn(),
   },
   dataConnectionsTable: {
     fetchById: jest.fn(),
@@ -104,9 +105,26 @@ beforeEach(() => {
   mockIsAuthorized.mockReturnValue(true);
   mockCanManageDataConnection.mockResolvedValue(true);
   mockProductsTable.update.mockImplementation(async (p) => p);
+  mockProductsTable.listProductsByConnectionId.mockResolvedValue([]);
   mockDataConnectionsTable.fetchById.mockResolvedValue(s3Connection);
 });
 
+// A different product mirroring to `connectionId` at `prefix`, for overlap tests.
+function siblingProduct(
+  productId: string,
+  connectionId: string,
+  prefix: string
+): Product {
+  return {
+    account_id: "acct",
+    product_id: productId,
+    metadata: {
+      mirrors: { [connectionId]: mirror({ connection_id: connectionId, prefix }) },
+      primary_mirror: connectionId,
+    },
+  } as Product;
+}
+
 describe("addProductMirror", () => {
   test("rejects non-admins before any write", async () => {
     mockIsAdmin.mockReturnValue(false);
@@ -147,6 +165,27 @@ describe("addProductMirror", () => {
     });
   });
 
+  test("rejects when the resolved prefix overlaps another product on the connection", async () => {
+    mockProductsTable.fetchById.mockResolvedValue(productWith({}, ""));
+    // s3Connection resolves to "acct/prod/"; a sibling nested under it overlaps.
+    mockProductsTable.listProductsByConnectionId.mockResolvedValue([
+      siblingProduct("other-prod", "conn-a", "acct/prod/sub/"),
+    ]);
+
+    const result = await addProductMirror(
+      FORM_STATE,
+      formDataFor({
+        account_id: "acct",
+        product_id: "prod",
+        connection_id: "conn-a",
+      })
+    );
+
+    expect(result.success).toBe(false);
+    expect(result.message).toMatch(/overlap/i);
+    expect(mockProductsTable.update).not.toHaveBeenCalled();
+  });
+
   test("passes the read updated_at as an optimistic lock and reports conflicts", async () => {
     mockProductsTable.fetchById.mockResolvedValue({
       ...productWith({}, ""),
@@ -192,6 +231,29 @@ describe("addProductMirror", () => {
     expect(updated.metadata.mirrors["conn-a"].prefix).toBe("");
   });
 
+  test("slash-terminates a prefix from a template missing its trailing slash", async () => {
+    // Without this, the stored "acct/prod" literally matches keys under a
+    // sibling "acct/prod2/", overlapping despite the normalized overlap check.
+    mockProductsTable.fetchById.mockResolvedValue(productWith({}, ""));
+    mockDataConnectionsTable.fetchById.mockResolvedValue({
+      data_connection_id: "conn-a",
+      prefix_template: "{{repository.account_id}}/{{repository.repository_id}}",
+      details: { provider: "s3" },
+    } as DataConnection);
+
+    await addProductMirror(
+      FORM_STATE,
+      formDataFor({
+        account_id: "acct",
+        product_id: "prod",
+        connection_id: "conn-a",
+      })
+    );
+
+    const updated = mockProductsTable.update.mock.calls[0][0];
+    expect(updated.metadata.mirrors["conn-a"].prefix).toBe("acct/prod/");
+  });
+
   test("maps a GCP connection to the gcs storage type", async () => {
     mockProductsTable.fetchById.mockResolvedValue(productWith({}, ""));
     mockDataConnectionsTable.fetchById.mockResolvedValue({
@@ -510,4 +572,78 @@ describe("updateMirrorPrefix", () => {
       expect(mockProductsTable.update).not.toHaveBeenCalled();
     }
   );
+
+  test.each([
+    ["nested under a sibling", "alice/foo/bar", "alice/foo/"],
+    ["containing a sibling", "alice/foo", "alice/foo/bar/"],
+  ])(
+    "rejects a prefix %s on the same connection",
+    async (_desc, prefix, siblingPrefix) => {
+      mockProductsTable.fetchById.mockResolvedValue(
+        productWith({ "conn-a": mirror({ connection_id: "conn-a" }) }, "conn-a")
+      );
+      mockProductsTable.listProductsByConnectionId.mockResolvedValue([
+        siblingProduct("other-prod", "conn-a", siblingPrefix),
+      ]);
+
+      const result = await updateMirrorPrefix(
+        FORM_STATE,
+        formDataFor({
+          account_id: "acct",
+          product_id: "prod",
+          mirror_key: "conn-a",
+          prefix,
+        })
+      );
+
+      expect(result.success).toBe(false);
+      expect(result.message).toMatch(/overlaps/i);
+      expect(mockProductsTable.update).not.toHaveBeenCalled();
+    }
+  );
+
+  test("ignores the product being edited when checking overlap", async () => {
+    mockProductsTable.fetchById.mockResolvedValue(
+      productWith({ "conn-a": mirror({ connection_id: "conn-a" }) }, "conn-a")
+    );
+    // The scan returns this very product; its own prefix must not block itself.
+    mockProductsTable.listProductsByConnectionId.mockResolvedValue([
+      siblingProduct("prod", "conn-a", "alice/foo/"),
+    ]);
+
+    const result = await updateMirrorPrefix(
+      FORM_STATE,
+      formDataFor({
+        account_id: "acct",
+        product_id: "prod",
+        mirror_key: "conn-a",
+        prefix: "alice/foo/bar/",
+      })
+    );
+
+    expect(result.success).toBe(true);
+    expect(mockProductsTable.update).toHaveBeenCalled();
+  });
+
+  test("allows a non-overlapping sibling prefix", async () => {
+    mockProductsTable.fetchById.mockResolvedValue(
+      productWith({ "conn-a": mirror({ connection_id: "conn-a" }) }, "conn-a")
+    );
+    mockProductsTable.listProductsByConnectionId.mockResolvedValue([
+      siblingProduct("other-prod", "conn-a", "bob/other/"),
+    ]);
+
+    const result = await updateMirrorPrefix(
+      FORM_STATE,
+      formDataFor({
+        account_id: "acct",
+        product_id: "prod",
+        mirror_key: "conn-a",
+        prefix: "alice/foo/",
+      })
+    );
+
+    expect(result.success).toBe(true);
+    expect(mockProductsTable.update).toHaveBeenCalled();
+  });
 });

src/lib/actions/product-mirrors.ts

@@ -7,6 +7,7 @@ import { productsTable, dataConnectionsTable } from "../clients";
 import {
   Actions,
   DataProvider,
+  Product,
   ProductMirror,
   resolveMirrorPrefix,
 } from "@/types";
@@ -29,6 +30,44 @@ function isConcurrentEdit(error: unknown): boolean {
   );
 }
 
+// Slash-terminate so prefixes compare on directory boundaries; root ("") stays
+// "". Keys are built by literal concatenation (`${prefix}${key}`), so without a
+// trailing slash "acct/prod" would also match "acct/prod2/".
+const normalizePrefix = (p: string) => (p === "" || p.endsWith("/") ? p : `${p}/`);
+
+// Prefixes on the same connection (= same bucket) overlap when one is nested in
+// the other (either direction). `"".startsWith(x)` semantics make a root prefix
+// overlap everything, which is correct — a root prefix owns the whole bucket.
+const prefixesOverlap = (a: string, b: string) =>
+  a.startsWith(b) || b.startsWith(a);
+
+// The first *other* product mirroring to this connection whose prefix overlaps
+// `prefix`, or null. Excludes the product being edited (self).
+//
+// ponytail: listProductsByConnectionId is a full table scan (no GSI on the
+// mirror map's connection_id). Fine for this low-traffic admin path; if product
+// volume explodes, add a denormalized connection index.
+async function findPrefixConflict(
+  connectionId: string,
+  prefix: string,
+  self: { accountId: string; productId: string }
+): Promise<Product | null> {
+  const normalized = normalizePrefix(prefix);
+  const siblings = await productsTable.listProductsByConnectionId(connectionId);
+  for (const p of siblings) {
+    if (p.account_id === self.accountId && p.product_id === self.productId) {
+      continue;
+    }
+    const m = Object.values(p.metadata.mirrors).find(
+      (mm) => mm.connection_id === connectionId
+    );
+    if (m && prefixesOverlap(normalized, normalizePrefix(m.prefix))) {
+      return p;
+    }
+  }
+  return null;
+}
+
 export async function addProductMirror(
   _prevState: FormState<unknown>,
   formData: FormData
@@ -96,12 +135,30 @@ export async function addProductMirror(
       };
     }
 
-    const prefix = resolveMirrorPrefix(
-      connection.prefix_template,
-      accountId,
-      productId
+    // Normalize before storing (not just comparing): a prefix_template may omit
+    // the trailing slash (it's optional in the schema), and the raw value is
+    // what's later concatenated into keys — so "acct/prod" would match keys
+    // under "acct/prod2/" and slip past the overlap check below.
+    const prefix = normalizePrefix(
+      resolveMirrorPrefix(connection.prefix_template, accountId, productId)
     );
 
+    // Guards against a prefix_template that omits {{repository.repository_id}}
+    // (two products resolve to the same prefix) or attaching a second product at
+    // a shared root prefix.
+    const conflict = await findPrefixConflict(connectionId, prefix, {
+      accountId,
+      productId,
+    });
+    if (conflict) {
+      return {
+        fieldErrors: {},
+        data: formData,
+        message: `This connection's prefix would overlap product ${conflict.account_id}/${conflict.product_id}. Adjust the connection's prefix template.`,
+        success: false,
+      };
+    }
+
     const isFirst = Object.keys(product.metadata.mirrors).length === 0;
 
     const storageTypeByProvider: Record<
@@ -328,8 +385,6 @@ export async function updateMirrorPrefix(
   // concatenation (`${prefix}${key}`), so reject traversal/leading-slash and
   // force a trailing separator — without it "acct/prod" also matches keys under
   // "acct/prod2/" on a shared connection.
-  // ponytail: no cross-product collision scan; managing the connection already
-  // grants broad access. Add a scan if prefix overlap becomes a real problem.
   if (rawPrefix.includes("..") || rawPrefix.startsWith("/")) {
     return {
       fieldErrors: {},
@@ -384,6 +439,19 @@ export async function updateMirrorPrefix(
       };
     }
 
+    const conflict = await findPrefixConflict(existing.connection_id, prefix, {
+      accountId,
+      productId,
+    });
+    if (conflict) {
+      return {
+        fieldErrors: {},
+        data: formData,
+        message: `This prefix overlaps product ${conflict.account_id}/${conflict.product_id} on the same data connection. Choose a non-overlapping prefix.`,
+        success: false,
+      };
+    }
+
     const updatedProduct = {
       ...product,
       metadata: {