Skip to content

Commit 7246f67

Browse files
authored
fix(webapp): strip null bytes from idempotency and debounce keys at trigger (#4527)
## What A trigger request carrying a Unicode NUL (`U+0000`) in the **idempotency key** or **debounce key** reached `prisma.taskRun.create()` and failed the insert, so the caller got an opaque 500 and the run was never created. These two keys are stored in `jsonb` columns (`idempotencyKeyOptions`, `debounce`), and Postgres rejects a NUL inside a `jsonb` value with `SQLSTATE 22P05` ("unsupported Unicode escape sequence ... cannot be converted to text"). This fix strips the NUL from both keys at the single trigger-input chokepoint (`#buildEngineTriggerInput`), which every trigger path flows through (single, batch item, mollified, and drainer replay). Stripping matches the existing precedent for run errors and task events. It does not change dedup behaviour: the idempotency **dedup identity** is the hashed key (a clean 64-char digest), computed independently of the raw key we clean, so dedup keeps working exactly as before. For debounce the key is used directly, so the cleaned key also becomes the grouping key, an acceptable change for input that is already malformed. ## Why not payload / metadata / tags Those are `text` columns fed by `JSON.stringify`, which escapes a NUL to a safe escape sequence, so they do not hit this failure on the normal JSON path. (A raw NUL in a `text` column throws a different code, `22021`, and is not what triggers this issue.) The observed failures are the `jsonb` `22P05` variant, which is only reachable via the two key fields. ## Evidence Red then green (containerTest, real Postgres): with the fix reverted, triggering through the real service with a NUL in `idempotencyKeyOptions.key` / `debounce.key` fails with the exact `22P05` signature; with the fix, the run is created and the stored key has the NUL removed. Full-stack e2e (isolated stack, real HTTP): `POST /api/v1/tasks/:taskId/trigger` with a NUL inside `idempotencyKeyOptions.key` (`"acme<NUL>inc"`) and, separately, `debounce.key` (`"grp<NUL>1"`): - both returned `HTTP 200` with a created run (previously `500`) - stored `idempotencyKeyOptions` = `{ "key": "acmeinc", "scope": "run" }` (7 chars, NUL removed) - stored `debounce.key` = `"grp1"` (4 chars, NUL removed) - both runs render in the dashboard Unit tests cover the helper (strip, no-op fast path, object-reference reuse, null/undefined pass-through). ## Rollout / rollback Server-only webapp change, no flag. Zero behaviour change for clean input; only affects inputs that previously 500'd. Rollback is a straight revert, no data migration. ## Known limitation A raw NUL in a plain-string idempotency key (not created via `idempotencyKeys.create()`) lands in a `text` column and throws `22021` instead. That variant is not addressed here because stripping it would change the dedup identity, so it warrants a separate decision. Not observed in practice. refs TRI-13030
1 parent dc52941 commit 7246f67

5 files changed

Lines changed: 181 additions & 2 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Fixed a rare error where triggering a task could fail if the idempotency key or debounce key contained an invalid null character. The character is now removed automatically and the run is created as normal.
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { describe, expect, vi } from "vitest";
2+
3+
vi.mock("~/db.server", () => ({
4+
prisma: {},
5+
$replica: {},
6+
runOpsNewPrisma: {},
7+
runOpsLegacyPrisma: {},
8+
runOpsNewReplica: {},
9+
runOpsLegacyReplica: {},
10+
}));
11+
vi.mock("~/v3/runOpsMigration/splitMode.server", () => ({ isSplitEnabled: async () => false }));
12+
vi.mock("~/services/platform.v3.server", async (importOriginal) => {
13+
const actual = (await importOriginal()) as Record<string, unknown>;
14+
return {
15+
...actual,
16+
getEntitlement: vi.fn(),
17+
};
18+
});
19+
20+
import { setupAuthenticatedEnvironment } from "@internal/run-engine/tests";
21+
import { assertNonNullable, containerTest } from "@internal/testcontainers";
22+
import { trace } from "@opentelemetry/api";
23+
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
24+
import { DefaultQueueManager } from "~/runEngine/concerns/queues.server";
25+
import { RunEngineTriggerTaskService } from "./triggerTask.server";
26+
import {
27+
buildEngine,
28+
CapturingParentRunValidator,
29+
MockPayloadProcessor,
30+
MockTraceEventConcern,
31+
} from "./triggerTask.server.test.helpers";
32+
33+
vi.setConfig({ testTimeout: 60_000 });
34+
35+
const NUL = String.fromCharCode(0);
36+
37+
function buildService(engine: any, prisma: any) {
38+
return new RunEngineTriggerTaskService({
39+
engine,
40+
prisma,
41+
payloadProcessor: new MockPayloadProcessor(),
42+
queueConcern: new DefaultQueueManager(prisma, engine),
43+
idempotencyKeyConcern: new IdempotencyKeyConcern(prisma, engine, new MockTraceEventConcern()),
44+
validator: new CapturingParentRunValidator(),
45+
traceEventConcern: new MockTraceEventConcern(),
46+
tracer: trace.getTracer("test", "0.0.0"),
47+
metadataMaximumSize: 1024 * 1024 * 1,
48+
});
49+
}
50+
51+
describe("RunEngineTriggerTaskService null-byte sanitization", () => {
52+
containerTest(
53+
"strips a NUL from idempotencyKeyOptions.key so the jsonb insert does not 22P05",
54+
async ({ prisma, redisOptions }) => {
55+
const engine = buildEngine(prisma, redisOptions);
56+
57+
try {
58+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
59+
const service = buildService(engine, prisma);
60+
61+
const result = await service.call({
62+
taskId: "nul-idem-task",
63+
environment,
64+
body: {
65+
payload: { kind: "idem" },
66+
options: {
67+
idempotencyKey: "a".repeat(64),
68+
idempotencyKeyOptions: { key: `acme${NUL}inc`, scope: "run" },
69+
},
70+
},
71+
});
72+
assertNonNullable(result);
73+
74+
const row = await prisma.taskRun.findUniqueOrThrow({ where: { id: result.run.id } });
75+
expect(row.idempotencyKeyOptions).toEqual({ key: "acmeinc", scope: "run" });
76+
} finally {
77+
await engine.quit();
78+
}
79+
}
80+
);
81+
82+
containerTest(
83+
"strips a NUL from debounce.key so the jsonb insert does not 22P05",
84+
async ({ prisma, redisOptions }) => {
85+
const engine = buildEngine(prisma, redisOptions);
86+
87+
try {
88+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
89+
const service = buildService(engine, prisma);
90+
91+
const result = await service.call({
92+
taskId: "nul-debounce-task",
93+
environment,
94+
body: {
95+
payload: { kind: "debounce" },
96+
options: {
97+
debounce: { key: `grp${NUL}1`, delay: "1s" },
98+
},
99+
},
100+
});
101+
assertNonNullable(result);
102+
103+
const row = await prisma.taskRun.findUniqueOrThrow({ where: { id: result.run.id } });
104+
expect((row.debounce as { key: string }).key).toBe("grp1");
105+
} finally {
106+
await engine.quit();
107+
}
108+
}
109+
);
110+
});

apps/webapp/app/runEngine/services/triggerTask.server.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type { PrismaClientOrTransaction } from "@trigger.dev/database";
2525
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
2626
import { logger } from "~/services/logger.server";
2727
import { parseDelay } from "~/utils/delays";
28+
import { removeNullBytesFromKey } from "~/utils/nullBytes";
2829
import { handleMetadataPacket } from "~/utils/packets";
2930
import { startSpan } from "~/v3/tracing.server";
3031
import { resolveRunIdMintKind } from "~/v3/engineVersion.server";
@@ -906,7 +907,7 @@ export class RunEngineTriggerTaskService {
906907
environment: args.environment,
907908
idempotencyKey: args.idempotencyKey,
908909
idempotencyKeyExpiresAt: args.idempotencyKey ? args.idempotencyKeyExpiresAt : undefined,
909-
idempotencyKeyOptions: args.body.options?.idempotencyKeyOptions,
910+
idempotencyKeyOptions: removeNullBytesFromKey(args.body.options?.idempotencyKeyOptions),
910911
taskIdentifier: args.taskId,
911912
payload: args.payloadPacket.data ?? "",
912913
payloadType: args.payloadPacket.dataType,
@@ -971,7 +972,7 @@ export class RunEngineTriggerTaskService {
971972
planType: args.planType,
972973
realtimeStreamsVersion: args.options.realtimeStreamsVersion,
973974
streamBasinName: args.environment.organization.streamBasinName,
974-
debounce: args.body.options?.debounce,
975+
debounce: removeNullBytesFromKey(args.body.options?.debounce),
975976
annotations: args.annotations,
976977
};
977978
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { describe, expect, it } from "vitest";
2+
import { removeNullBytes, removeNullBytesFromKey } from "./nullBytes";
3+
4+
describe("removeNullBytes", () => {
5+
it("strips every NUL from a string", () => {
6+
expect(removeNullBytes(`a\u0000b\u0000c`)).toBe("abc");
7+
});
8+
9+
it("returns the same reference when there is no NUL", () => {
10+
const clean = "acme-inc";
11+
expect(removeNullBytes(clean)).toBe(clean);
12+
});
13+
14+
it("passes through undefined and null", () => {
15+
expect(removeNullBytes(undefined)).toBeUndefined();
16+
expect(removeNullBytes(null)).toBeNull();
17+
});
18+
});
19+
20+
describe("removeNullBytesFromKey", () => {
21+
it("strips a NUL from the key while preserving other fields", () => {
22+
expect(removeNullBytesFromKey({ key: `k\u00001`, scope: "run" })).toEqual({
23+
key: "k1",
24+
scope: "run",
25+
});
26+
});
27+
28+
it("returns the same object reference when the key is clean", () => {
29+
const opts = { key: "clean", scope: "run" };
30+
expect(removeNullBytesFromKey(opts)).toBe(opts);
31+
});
32+
33+
it("passes through undefined", () => {
34+
expect(removeNullBytesFromKey(undefined)).toBeUndefined();
35+
});
36+
});

apps/webapp/app/utils/nullBytes.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Removes Unicode NUL (U+0000) from a string. Postgres cannot store a NUL in a
3+
* `text` column (SQLSTATE 22021) and rejects a `\u0000` escape when a JSON value
4+
* is stored as `jsonb` (SQLSTATE 22P05), so a caller-supplied NUL reaching
5+
* `taskRun.create()` fails the insert. The `indexOf` guard keeps the common
6+
* (NUL-free) case allocation-free on the trigger hot path.
7+
*/
8+
export function removeNullBytes<T extends string | undefined | null>(value: T): T {
9+
if (typeof value !== "string" || value.indexOf("\u0000") === -1) {
10+
return value;
11+
}
12+
return value.replace(/\u0000/g, "") as T;
13+
}
14+
15+
/**
16+
* Returns `value` with a NUL-stripped `key`, reusing the original object when no
17+
* NUL is present. Used for the user-supplied idempotency-key and debounce
18+
* options, whose `key` lands in a `jsonb` column on the TaskRun row.
19+
*/
20+
export function removeNullBytesFromKey<T extends { key: string } | undefined>(value: T): T {
21+
if (!value) {
22+
return value;
23+
}
24+
const cleaned = removeNullBytes(value.key);
25+
return cleaned === value.key ? value : { ...value, key: cleaned };
26+
}

0 commit comments

Comments
 (0)