From 57e96b9552c70d8ca98d1d947914c25941f1be51 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 3 Aug 2026 23:07:38 -0700 Subject: [PATCH] fix(data-connections): stop listing other accounts' connections on product create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listUsableDataConnections filtered only on UseDataConnection and GetDataConnection, neither of which looks at `owner`. Every account-owned connection in the table therefore reached /products/new and was serialized into the page — bucket names, regions and prefix templates included — for any logged-in user. ProductCreationForm's `!dc.owner || dc.owner === forAccountId` filter runs in the browser, so it only ever hid them from view. Take the accounts the user may create products under and apply the same rule server-side, in the helper rather than at the call site so a future caller can't reintroduce the leak. Co-Authored-By: Claude Opus 5 --- src/app/(app)/products/new/page.tsx | 17 ++++++++++------ src/lib/data-connections.test.ts | 30 ++++++++++++++++++++++++++--- src/lib/data-connections.ts | 19 ++++++++++++++---- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/app/(app)/products/new/page.tsx b/src/app/(app)/products/new/page.tsx index 6f7a5b25..66ba59fe 100644 --- a/src/app/(app)/products/new/page.tsx +++ b/src/app/(app)/products/new/page.tsx @@ -51,12 +51,17 @@ export default async function NewProductPage({ )), ]; - // Strip credentials before handing connections to the client component. - const dataConnections = (await listUsableDataConnections(session)).map( - (connection) => - DataConnectionObjectSchema.omit({ authentication: true }).parse( - connection - ) + // Only connections usable by an account this user can create products under — + // an owned connection exposes its account's bucket names and prefixes, so the + // form's owner filter must not be the only one. Strip credentials before + // handing what remains to the client component. + const dataConnections = ( + await listUsableDataConnections( + session, + potentialOwnerAccounts.map((account) => account.account_id) + ) + ).map((connection) => + DataConnectionObjectSchema.omit({ authentication: true }).parse(connection) ); return ( diff --git a/src/lib/data-connections.test.ts b/src/lib/data-connections.test.ts index 7763f0ea..37e36ef0 100644 --- a/src/lib/data-connections.test.ts +++ b/src/lib/data-connections.test.ts @@ -137,7 +137,9 @@ describe("listUsableDataConnections (issue #461)", () => { test("offers an org's own connection to an org owner", async () => { listing([orgConnection]); expect( - ids(await listUsableDataConnections(sessions["organization-owner-user"])) + ids(await listUsableDataConnections(sessions["organization-owner-user"], [ + "organization", + ])) ).toEqual(["organization--byob"]); }); @@ -150,16 +152,37 @@ describe("listUsableDataConnections (issue #461)", () => { listing([orgConnection]); expect( - ids(await listUsableDataConnections(sessions["organization-owner-user"])) + ids(await listUsableDataConnections(sessions["organization-owner-user"], [ + "organization", + ])) ).toEqual(["organization--byob"]); }); test("keeps a read-only connection out of the list", async () => { listing([{ ...orgConnection, read_only: true } as DataConnection]); expect( - ids(await listUsableDataConnections(sessions["organization-owner-user"])) + ids(await listUsableDataConnections(sessions["organization-owner-user"], [ + "organization", + ])) ).toEqual([]); }); + + // The owner filter has to happen here, not in ProductCreationForm: an owned + // connection carries its account's bucket name and prefixes, so a client-side + // filter would still have shipped them to every user's browser. + test("withholds another account's connection from the listing", async () => { + listing([orgConnection]); + expect( + ids(await listUsableDataConnections(sessions["regular-user"], ["regular"])) + ).toEqual([]); + }); + + test("still offers system-level connections to any account", async () => { + listing([{ ...orgConnection, owner: undefined } as DataConnection]); + expect( + ids(await listUsableDataConnections(sessions["regular-user"], ["regular"])) + ).toEqual(["organization--byob"]); + }); }); // Which connections may back a product owned by a given account: system-level @@ -205,3 +228,4 @@ describe("canUseDataConnectionFor", () => { ).toBe(false); }); }); + diff --git a/src/lib/data-connections.ts b/src/lib/data-connections.ts index 1f8c454d..49cc4993 100644 --- a/src/lib/data-connections.ts +++ b/src/lib/data-connections.ts @@ -63,20 +63,31 @@ export function canUseDataConnectionFor( } /** - * List the data connections a user is permitted to use when creating a product. + * List the data connections a user is permitted to use when creating a product + * under one of `ownerAccountIds` (the accounts they may create products for). + * + * A connection is usable when it is available to one of those accounts — + * system-level (unowned) or owned by it — and the session is authorized both to + * read it (`GetDataConnection`) and to create products against it + * (`UseDataConnection`). + * + * The owner filter belongs here rather than at the call site: an owned + * connection carries its account's bucket names and prefixes, so filtering it + * out in the browser would still have shipped it there. * - * A connection is usable when the session is authorized both to read it - * (`GetDataConnection`) and to create products against it (`UseDataConnection`). * The returned objects are unsanitized (credentials intact); callers that hand * these to the client must strip `authentication` first. */ export async function listUsableDataConnections( - session: UserSession | null + session: UserSession | null, + ownerAccountIds: string[] ): Promise { + const usableBy = new Set(ownerAccountIds); const dataConnections = await dataConnectionsTable.listAll(); return dataConnections.filter( (dataConnection) => + (!dataConnection.owner || usableBy.has(dataConnection.owner)) && isAuthorized(session, dataConnection, Actions.UseDataConnection) && isAuthorized(session, dataConnection, Actions.GetDataConnection) );