Skip to content

Commit 4660a0a

Browse files
committed
fix(webapp): schema-qualify waitpoint connected-run queries
The raw queries reading a waitpoint's connected runs relied on search_path, so they broke on deployments using a non-public database schema. Qualify the tables with the configured schema so they resolve correctly.
1 parent bfb9ddf commit 4660a0a

8 files changed

Lines changed: 270 additions & 16 deletions

apps/webapp/app/presenters/v3/WaitpointPresenter.server.ts

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { isWaitpointOutputTimeout, prettyPrintPacket } from "@trigger.dev/core/v3";
2-
import { type PrismaClientOrTransaction, type PrismaReplicaClient } from "~/db.server";
2+
import {
3+
DATABASE_SCHEMA,
4+
type PrismaClientOrTransaction,
5+
type PrismaReplicaClient,
6+
} from "~/db.server";
37
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
48
import { generateHttpCallbackUrl } from "~/services/httpCallback.server";
59
import { logger } from "~/services/logger.server";
@@ -119,30 +123,38 @@ export class WaitpointPresenter extends BasePresenter {
119123
// deleted), the control-plane full schema the implicit `_WaitpointRunConnections` M2M
120124
// (A = TaskRun.id, B = Waitpoint.id). Both branches existence-filter AT THE QUERY via a JOIN to
121125
// TaskRun, so a dangling connection row can never occupy a LIMIT slot ahead of a real one.
122-
// CONNECTED_RUNS_DISPLAY_LIMIT is a compile-time constant int, not an interpolated value.
126+
// Tables are schema-qualified with DATABASE_SCHEMA (trusted boot constant) so a non-`public`
127+
// schema= deployment resolves the right tables instead of leaning on search_path. $queryRawUnsafe,
128+
// not a `sqlDatabaseSchema` Prisma.Sql fragment: `client` may be the dedicated run-ops client (a
129+
// different Prisma runtime) which would bind a foreign runtime's Sql fragment as a param instead
130+
// of inlining it. waitpointId and the constant limit stay bound params ($1/$2).
123131
async #connectedRunIdsOn(client: PrismaReplicaClient, waitpointId: string): Promise<string[]> {
124132
const isDedicated = Boolean(
125133
(client as unknown as { waitpointRunConnection?: unknown }).waitpointRunConnection
126134
);
127135

128136
if (isDedicated) {
129-
const rows = await client.$queryRaw<{ taskRunId: string }[]>`
130-
SELECT c."taskRunId" AS "taskRunId"
131-
FROM "WaitpointRunConnection" c
132-
JOIN "TaskRun" t ON t."id" = c."taskRunId"
133-
WHERE c."waitpointId" = ${waitpointId}
134-
LIMIT ${CONNECTED_RUNS_DISPLAY_LIMIT}
135-
`;
137+
const rows = await client.$queryRawUnsafe<{ taskRunId: string }[]>(
138+
`SELECT c."taskRunId" AS "taskRunId"
139+
FROM ${DATABASE_SCHEMA}."WaitpointRunConnection" c
140+
JOIN ${DATABASE_SCHEMA}."TaskRun" t ON t."id" = c."taskRunId"
141+
WHERE c."waitpointId" = $1
142+
LIMIT $2`,
143+
waitpointId,
144+
CONNECTED_RUNS_DISPLAY_LIMIT
145+
);
136146
return rows.map((row) => row.taskRunId);
137147
}
138148

139-
const rows = await client.$queryRaw<{ A: string }[]>`
140-
SELECT c."A" AS "A"
141-
FROM "_WaitpointRunConnections" c
142-
JOIN "TaskRun" t ON t."id" = c."A"
143-
WHERE c."B" = ${waitpointId}
144-
LIMIT ${CONNECTED_RUNS_DISPLAY_LIMIT}
145-
`;
149+
const rows = await client.$queryRawUnsafe<{ A: string }[]>(
150+
`SELECT c."A" AS "A"
151+
FROM ${DATABASE_SCHEMA}."_WaitpointRunConnections" c
152+
JOIN ${DATABASE_SCHEMA}."TaskRun" t ON t."id" = c."A"
153+
WHERE c."B" = $1
154+
LIMIT $2`,
155+
waitpointId,
156+
CONNECTED_RUNS_DISPLAY_LIMIT
157+
);
146158
return rows.map((row) => row.A);
147159
}
148160

apps/webapp/test/waitpointPresenter.connectedRunsBounded.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ vi.mock("~/db.server", async () => {
3535
runOpsLegacyPrisma: replicaProxy,
3636
runOpsLegacyReplica: replicaProxy,
3737
sqlDatabaseSchema: Prisma.sql([`public`]),
38+
DATABASE_SCHEMA: "public",
3839
};
3940
});
4041

apps/webapp/test/waitpointPresenter.controlPlane.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ vi.mock("~/db.server", async () => {
3232
$replica: proxy,
3333
runOpsNewPrisma: proxy,
3434
sqlDatabaseSchema: Prisma.sql([`public`]),
35+
DATABASE_SCHEMA: "public",
3536
};
3637
});
3738

apps/webapp/test/waitpointPresenter.danglingConnectedRuns.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ vi.mock("~/db.server", async () => {
3131
runOpsLegacyPrisma: replicaProxy,
3232
runOpsLegacyReplica: replicaProxy,
3333
sqlDatabaseSchema: Prisma.sql([`public`]),
34+
DATABASE_SCHEMA: "public",
3435
};
3536
});
3637

apps/webapp/test/waitpointPresenter.dedicatedConnectedRuns.readthrough.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ vi.mock("~/db.server", async () => {
3131
runOpsLegacyPrisma: replicaProxy,
3232
runOpsLegacyReplica: replicaProxy,
3333
sqlDatabaseSchema: Prisma.sql([`public`]),
34+
DATABASE_SCHEMA: "public",
3435
};
3536
});
3637

apps/webapp/test/waitpointPresenter.readthrough.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ vi.mock("~/db.server", async () => {
3939
runOpsLegacyPrisma: replicaProxy,
4040
runOpsLegacyReplica: replicaProxy,
4141
sqlDatabaseSchema: Prisma.sql([`public`]),
42+
DATABASE_SCHEMA: "public",
4243
};
4344
});
4445

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
// RED->GREEN guard: #connectedRunIdsOn's raw SQL must schema-qualify every table reference with
2+
// sqlDatabaseSchema (the repo-wide convention every other raw-SQL call site follows —
3+
// task.server.ts, TestPresenter, DeploymentListPresenter). Unqualified names ("WaitpointRunConnection",
4+
// "TaskRun", "_WaitpointRunConnections") resolve against search_path, so a non-`public` schema=
5+
// deployment would hit the wrong schema or miss the tables entirely.
6+
//
7+
// This intercepts the REAL $queryRawUnsafe on BOTH physical clients (pure instrumentation over a
8+
// real testcontainer client — the DB still executes the query and returns real rows) and captures
9+
// the SQL text, then asserts every table reference is schema-qualified. Uses the split topology so
10+
// BOTH raw-SQL branches run: dedicated `WaitpointRunConnection` (prisma17) and legacy implicit M2M
11+
// `_WaitpointRunConnections` (prisma14). $queryRawUnsafe (not a Prisma.Sql fragment) is exactly what
12+
// the fix uses: the two clients are different Prisma runtimes, so a foreign runtime's Sql fragment
13+
// would be bound as a param rather than inlined.
14+
import { describe, expect, vi } from "vitest";
15+
16+
const legacyReplicaHolder = vi.hoisted(() => ({ client: undefined as any }));
17+
const newClientHolder = vi.hoisted(() => ({ client: undefined as any }));
18+
19+
vi.mock("~/db.server", async () => {
20+
const { Prisma } = await import("@trigger.dev/database");
21+
const lazyProxy = (holder: { client: any }, label: string) =>
22+
new Proxy(
23+
{},
24+
{
25+
get(_t, prop) {
26+
if (!holder.client) {
27+
throw new Error(`${label} not set for this test`);
28+
}
29+
return holder.client[prop];
30+
},
31+
}
32+
);
33+
const replicaProxy = lazyProxy(legacyReplicaHolder, "legacyReplicaHolder.client");
34+
return {
35+
prisma: replicaProxy,
36+
$replica: replicaProxy,
37+
runOpsNewPrisma: lazyProxy(newClientHolder, "newClientHolder.client"),
38+
runOpsNewReplica: lazyProxy(newClientHolder, "newClientHolder.client"),
39+
runOpsLegacyPrisma: replicaProxy,
40+
runOpsLegacyReplica: replicaProxy,
41+
// The real schema constant is derived from DATABASE_URL's `schema=` search param. `public` keeps
42+
// the query runnable against the testcontainer while still exercising qualification.
43+
sqlDatabaseSchema: Prisma.sql([`public`]),
44+
DATABASE_SCHEMA: "public",
45+
};
46+
});
47+
48+
vi.mock("~/services/clickhouse/clickhouseFactoryInstance.server", () => ({
49+
clickhouseFactory: {
50+
getClickhouseForOrganization: async () => ({}),
51+
},
52+
}));
53+
54+
vi.mock("~/presenters/v3/NextRunListPresenter.server", () => ({
55+
NextRunListPresenter: class {
56+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
57+
constructor(..._args: unknown[]) {}
58+
async call(_organizationId: string, _environmentId: string, opts: { runId?: string[] }) {
59+
return {
60+
runs: (opts.runId ?? []).map((friendlyId) => ({
61+
friendlyId,
62+
taskIdentifier: "echoed",
63+
})),
64+
};
65+
}
66+
},
67+
}));
68+
69+
import { heteroRunOpsPostgresTest } from "@internal/testcontainers";
70+
import { type PrismaClient } from "@trigger.dev/database";
71+
import type { RunOpsPrismaClient } from "@internal/run-ops-database";
72+
import { WaitpointPresenter } from "~/presenters/v3/WaitpointPresenter.server";
73+
74+
vi.setConfig({ testTimeout: 90_000 });
75+
76+
type SeedContext = {
77+
organizationId: string;
78+
projectId: string;
79+
environmentId: string;
80+
};
81+
82+
async function seedParents(prisma: PrismaClient, slug: string): Promise<SeedContext> {
83+
const organization = await prisma.organization.create({
84+
data: { title: `org-${slug}`, slug: `org-${slug}` },
85+
});
86+
const project = await prisma.project.create({
87+
data: {
88+
name: `proj-${slug}`,
89+
slug: `proj-${slug}`,
90+
organizationId: organization.id,
91+
externalRef: `proj-${slug}`,
92+
},
93+
});
94+
const runtimeEnvironment = await prisma.runtimeEnvironment.create({
95+
data: {
96+
slug: `env-${slug}`,
97+
type: "DEVELOPMENT",
98+
projectId: project.id,
99+
organizationId: organization.id,
100+
apiKey: `tr_dev_${slug}`,
101+
pkApiKey: `pk_dev_${slug}`,
102+
shortcode: `sc-${slug}`,
103+
},
104+
});
105+
return {
106+
organizationId: organization.id,
107+
projectId: project.id,
108+
environmentId: runtimeEnvironment.id,
109+
};
110+
}
111+
112+
async function seedRun(
113+
prisma: PrismaClient | RunOpsPrismaClient,
114+
ctx: SeedContext,
115+
friendlyId: string
116+
) {
117+
return (prisma as PrismaClient).taskRun.create({
118+
data: {
119+
friendlyId,
120+
taskIdentifier: "my-task",
121+
status: "PENDING",
122+
payload: JSON.stringify({ foo: friendlyId }),
123+
payloadType: "application/json",
124+
traceId: friendlyId,
125+
spanId: friendlyId,
126+
queue: "test",
127+
runtimeEnvironmentId: ctx.environmentId,
128+
projectId: ctx.projectId,
129+
organizationId: ctx.organizationId,
130+
environmentType: "DEVELOPMENT",
131+
engine: "V2",
132+
},
133+
});
134+
}
135+
136+
// Capture the SQL text of every `$queryRawUnsafe` call, delegating unchanged to the real client
137+
// (the DB still runs the query -- pure instrumentation, never a mock). Everything else passes
138+
// straight through to the real client.
139+
function capturingQueryRaw<T extends object>(real: T): { client: T; sqls: string[] } {
140+
const sqls: string[] = [];
141+
const client = new Proxy(real, {
142+
get(target, prop, receiver) {
143+
if (prop === "$queryRawUnsafe") {
144+
return (query: string, ...params: unknown[]) => {
145+
sqls.push(query);
146+
return (target as any).$queryRawUnsafe(query, ...params);
147+
};
148+
}
149+
return Reflect.get(target, prop, receiver);
150+
},
151+
}) as T;
152+
return { client, sqls };
153+
}
154+
155+
describe("WaitpointPresenter#connectedRunIdsOn schema-qualifies every raw-SQL table reference", () => {
156+
heteroRunOpsPostgresTest(
157+
"both the dedicated WaitpointRunConnection and legacy _WaitpointRunConnections branches qualify tables with sqlDatabaseSchema",
158+
async ({ prisma14, prisma17 }) => {
159+
const ctx = await seedParents(prisma14, "schemaqual");
160+
161+
// Waitpoint resident on LEGACY (drives the implicit-M2M `_WaitpointRunConnections` branch).
162+
const waitpoint = await prisma14.waitpoint.create({
163+
data: {
164+
friendlyId: "waitpoint_schemaqual",
165+
type: "MANUAL",
166+
status: "COMPLETED",
167+
idempotencyKey: "idem-waitpoint_schemaqual",
168+
userProvidedIdempotencyKey: false,
169+
outputType: "application/json",
170+
outputIsError: false,
171+
completedAt: new Date(),
172+
tags: [],
173+
projectId: ctx.projectId,
174+
environmentId: ctx.environmentId,
175+
},
176+
});
177+
178+
// A connected run resident + joined on NEW (dedicated `WaitpointRunConnection` branch).
179+
const newRun = await seedRun(prisma17, ctx, "run_schemaqual_new");
180+
await prisma17.waitpointRunConnection.create({
181+
data: { taskRunId: newRun.id, waitpointId: waitpoint.id },
182+
});
183+
184+
// A connected run resident + joined on LEGACY via the implicit M2M.
185+
const legacyRun = await seedRun(prisma14, ctx, "run_schemaqual_legacy");
186+
await prisma14.waitpoint.update({
187+
where: { id: waitpoint.id },
188+
data: { connectedRuns: { connect: [{ id: legacyRun.id }] } },
189+
});
190+
191+
legacyReplicaHolder.client = prisma14;
192+
newClientHolder.client = prisma17;
193+
194+
const legacy = capturingQueryRaw(prisma14 as unknown as object);
195+
const dedicated = capturingQueryRaw(prisma17 as unknown as object);
196+
197+
const presenter = new WaitpointPresenter(undefined, undefined, {
198+
splitEnabled: true,
199+
newClient: dedicated.client as unknown as PrismaClient,
200+
legacyReplica: legacy.client as unknown as PrismaClient,
201+
});
202+
203+
const result = await presenter.call({
204+
friendlyId: waitpoint.friendlyId,
205+
environmentId: ctx.environmentId,
206+
projectId: ctx.projectId,
207+
});
208+
209+
// The read still works end-to-end (schema-qualified names resolve against the testcontainer's
210+
// public schema), and both stores contributed a connected run.
211+
const returnedIds = (result?.connectedRuns ?? []).map((r) => r.friendlyId).sort();
212+
expect(returnedIds).toEqual(["run_schemaqual_legacy", "run_schemaqual_new"]);
213+
214+
const allSqls = [...dedicated.sqls, ...legacy.sqls];
215+
// Sanity: both raw-SQL branches actually ran.
216+
expect(dedicated.sqls.length).toBeGreaterThan(0);
217+
expect(legacy.sqls.length).toBeGreaterThan(0);
218+
219+
// Every table reference must be schema-qualified. The buggy (unqualified) code emits e.g.
220+
// `FROM "WaitpointRunConnection"` / `JOIN "TaskRun"`, so these fail RED.
221+
for (const sql of allSqls) {
222+
// No unqualified table references (a `"` or `.` must precede any of these table names).
223+
expect(sql).not.toMatch(/(?<![.\w"])"WaitpointRunConnection"/);
224+
expect(sql).not.toMatch(/(?<![.\w"])"_WaitpointRunConnections"/);
225+
expect(sql).not.toMatch(/(?<![.\w"])"TaskRun"/);
226+
// Every JOIN to TaskRun is schema-qualified.
227+
expect(sql).toMatch(/public\."TaskRun"/);
228+
}
229+
230+
const dedicatedSql = dedicated.sqls.join("\n");
231+
const legacySql = legacy.sqls.join("\n");
232+
expect(dedicatedSql).toMatch(/public\."WaitpointRunConnection"/);
233+
expect(legacySql).toMatch(/public\."_WaitpointRunConnections"/);
234+
}
235+
);
236+
});

apps/webapp/test/waitpointPresenter.splitConnectedRuns.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ vi.mock("~/db.server", async () => {
3333
runOpsLegacyPrisma: replicaProxy,
3434
runOpsLegacyReplica: replicaProxy,
3535
sqlDatabaseSchema: Prisma.sql([`public`]),
36+
DATABASE_SCHEMA: "public",
3637
};
3738
});
3839

0 commit comments

Comments
 (0)