Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/strip-null-bytes-trigger-keys.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { describe, expect, vi } from "vitest";

vi.mock("~/db.server", () => ({
prisma: {},
$replica: {},
runOpsNewPrisma: {},
runOpsLegacyPrisma: {},
runOpsNewReplica: {},
runOpsLegacyReplica: {},
}));
vi.mock("~/v3/runOpsMigration/splitMode.server", () => ({ isSplitEnabled: async () => false }));
vi.mock("~/services/platform.v3.server", async (importOriginal) => {
const actual = (await importOriginal()) as Record<string, unknown>;
return {
...actual,
getEntitlement: vi.fn(),
};
});
Comment thread
ericallam marked this conversation as resolved.

import { setupAuthenticatedEnvironment } from "@internal/run-engine/tests";
import { assertNonNullable, containerTest } from "@internal/testcontainers";
import { trace } from "@opentelemetry/api";
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
import { DefaultQueueManager } from "~/runEngine/concerns/queues.server";
import { RunEngineTriggerTaskService } from "./triggerTask.server";
import {
buildEngine,
CapturingParentRunValidator,
MockPayloadProcessor,
MockTraceEventConcern,
} from "./triggerTask.server.test.helpers";

vi.setConfig({ testTimeout: 60_000 });

const NUL = String.fromCharCode(0);

function buildService(engine: any, prisma: any) {
return new RunEngineTriggerTaskService({
engine,
prisma,
payloadProcessor: new MockPayloadProcessor(),
queueConcern: new DefaultQueueManager(prisma, engine),
idempotencyKeyConcern: new IdempotencyKeyConcern(prisma, engine, new MockTraceEventConcern()),
validator: new CapturingParentRunValidator(),
traceEventConcern: new MockTraceEventConcern(),
tracer: trace.getTracer("test", "0.0.0"),
metadataMaximumSize: 1024 * 1024 * 1,
});
}

describe("RunEngineTriggerTaskService null-byte sanitization", () => {
containerTest(
"strips a NUL from idempotencyKeyOptions.key so the jsonb insert does not 22P05",
async ({ prisma, redisOptions }) => {
const engine = buildEngine(prisma, redisOptions);

try {
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
const service = buildService(engine, prisma);

const result = await service.call({
taskId: "nul-idem-task",
environment,
body: {
payload: { kind: "idem" },
options: {
idempotencyKey: "a".repeat(64),
idempotencyKeyOptions: { key: `acme${NUL}inc`, scope: "run" },
},
},
});
assertNonNullable(result);

const row = await prisma.taskRun.findUniqueOrThrow({ where: { id: result.run.id } });
expect(row.idempotencyKeyOptions).toEqual({ key: "acmeinc", scope: "run" });
} finally {
await engine.quit();
}
}
);

containerTest(
"strips a NUL from debounce.key so the jsonb insert does not 22P05",
async ({ prisma, redisOptions }) => {
const engine = buildEngine(prisma, redisOptions);

try {
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
const service = buildService(engine, prisma);

const result = await service.call({
taskId: "nul-debounce-task",
environment,
body: {
payload: { kind: "debounce" },
options: {
debounce: { key: `grp${NUL}1`, delay: "1s" },
},
},
});
assertNonNullable(result);

const row = await prisma.taskRun.findUniqueOrThrow({ where: { id: result.run.id } });
expect((row.debounce as { key: string }).key).toBe("grp1");
} finally {
await engine.quit();
}
}
);
});
5 changes: 3 additions & 2 deletions apps/webapp/app/runEngine/services/triggerTask.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type { PrismaClientOrTransaction } from "@trigger.dev/database";
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
import { logger } from "~/services/logger.server";
import { parseDelay } from "~/utils/delays";
import { removeNullBytesFromKey } from "~/utils/nullBytes";
import { handleMetadataPacket } from "~/utils/packets";
import { startSpan } from "~/v3/tracing.server";
import { resolveRunIdMintKind } from "~/v3/engineVersion.server";
Expand Down Expand Up @@ -906,7 +907,7 @@ export class RunEngineTriggerTaskService {
environment: args.environment,
idempotencyKey: args.idempotencyKey,
idempotencyKeyExpiresAt: args.idempotencyKey ? args.idempotencyKeyExpiresAt : undefined,
idempotencyKeyOptions: args.body.options?.idempotencyKeyOptions,
idempotencyKeyOptions: removeNullBytesFromKey(args.body.options?.idempotencyKeyOptions),
Comment thread
ericallam marked this conversation as resolved.
taskIdentifier: args.taskId,
payload: args.payloadPacket.data ?? "",
payloadType: args.payloadPacket.dataType,
Expand Down Expand Up @@ -971,7 +972,7 @@ export class RunEngineTriggerTaskService {
planType: args.planType,
realtimeStreamsVersion: args.options.realtimeStreamsVersion,
streamBasinName: args.environment.organization.streamBasinName,
debounce: args.body.options?.debounce,
debounce: removeNullBytesFromKey(args.body.options?.debounce),
annotations: args.annotations,
};
}
Expand Down
36 changes: 36 additions & 0 deletions apps/webapp/app/utils/nullBytes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { describe, expect, it } from "vitest";
import { removeNullBytes, removeNullBytesFromKey } from "./nullBytes";

describe("removeNullBytes", () => {
it("strips every NUL from a string", () => {
expect(removeNullBytes(`a\u0000b\u0000c`)).toBe("abc");
});

it("returns the same reference when there is no NUL", () => {
const clean = "acme-inc";
expect(removeNullBytes(clean)).toBe(clean);
});

it("passes through undefined and null", () => {
expect(removeNullBytes(undefined)).toBeUndefined();
expect(removeNullBytes(null)).toBeNull();
});
});

describe("removeNullBytesFromKey", () => {
it("strips a NUL from the key while preserving other fields", () => {
expect(removeNullBytesFromKey({ key: `k\u00001`, scope: "run" })).toEqual({
key: "k1",
scope: "run",
});
});

it("returns the same object reference when the key is clean", () => {
const opts = { key: "clean", scope: "run" };
expect(removeNullBytesFromKey(opts)).toBe(opts);
});

it("passes through undefined", () => {
expect(removeNullBytesFromKey(undefined)).toBeUndefined();
});
});
26 changes: 26 additions & 0 deletions apps/webapp/app/utils/nullBytes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Removes Unicode NUL (U+0000) from a string. Postgres cannot store a NUL in a
* `text` column (SQLSTATE 22021) and rejects a `\u0000` escape when a JSON value
* is stored as `jsonb` (SQLSTATE 22P05), so a caller-supplied NUL reaching
* `taskRun.create()` fails the insert. The `indexOf` guard keeps the common
* (NUL-free) case allocation-free on the trigger hot path.
*/
export function removeNullBytes<T extends string | undefined | null>(value: T): T {
if (typeof value !== "string" || value.indexOf("\u0000") === -1) {
return value;
}
return value.replace(/\u0000/g, "") as T;
}

/**
* Returns `value` with a NUL-stripped `key`, reusing the original object when no
* NUL is present. Used for the user-supplied idempotency-key and debounce
* options, whose `key` lands in a `jsonb` column on the TaskRun row.
*/
export function removeNullBytesFromKey<T extends { key: string } | undefined>(value: T): T {
if (!value) {
return value;
}
const cleaned = removeNullBytes(value.key);
return cleaned === value.key ? value : { ...value, key: cleaned };
}
Loading