Skip to content
Merged
57 changes: 57 additions & 0 deletions apps/server/src/imageMime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,63 @@ describe("imageMime", () => {
});
});

it("rejects payload with characters outside the base64 alphabet", () => {
expect(parseBase64DataUrl("data:image/png;base64,SGVs!bG8=")).toBeNull();
expect(parseBase64DataUrl("data:image/png;base64,SGVs,bG8=")).toBeNull();
});

it("rejects structurally malformed base64", () => {
// '=' before the trailing padding position
expect(parseBase64DataUrl("data:image/png;base64,AB=CD===")).toBeNull();
expect(parseBase64DataUrl("data:image/png;base64,SGV=bG8=")).toBeNull();
// more than two padding characters
expect(parseBase64DataUrl("data:image/png;base64,SGVsbG8=====AAA")).toBeNull();
// length not a multiple of 4
expect(parseBase64DataUrl("data:image/png;base64,SGVsbG8")).toBeNull();
});

it("accepts base64 with one or two trailing padding characters", () => {
expect(parseBase64DataUrl("data:image/png;base64,SGVsbA==")).toEqual({
mimeType: "image/png",
base64: "SGVsbA==",
});
expect(parseBase64DataUrl("data:image/png;base64,SGVsbG8h")).toEqual({
mimeType: "image/png",
base64: "SGVsbG8h",
});
});

it("rejects empty and whitespace-only payloads", () => {
expect(parseBase64DataUrl("data:image/png;base64,")).toBeNull();
expect(parseBase64DataUrl("data:image/png;base64, \r\n")).toBeNull();
});

it("parses a case-insensitive scheme and mime type", () => {
expect(parseBase64DataUrl("DATA:IMAGE/PNG;BASE64,SGVsbG8=")).toEqual({
mimeType: "image/png",
base64: "SGVsbG8=",
});
});

it("parses a multi-megabyte payload from a deep call stack", () => {
// Regression: matching the payload with a regex borrowed the JS call
// stack, so a ~10 MB image parsed inside fiber execution threw
// "RangeError: Maximum call stack size exceeded".
const dataUrl = `data:image/png;base64,${"A".repeat(14_000_000)}`;
const atDepth = (depth: number): ReturnType<typeof parseBase64DataUrl> =>
depth === 0 ? parseBase64DataUrl(dataUrl) : atDepth(depth - 1);
const findMaxDepth = (depth: number): number => {
try {
return findMaxDepth(depth + 1);
} catch {
return depth;
}
};
const result = atDepth(Math.floor(findMaxDepth(0) * 0.85));
expect(result?.mimeType).toBe("image/png");
expect(result?.base64.length).toBe(14_000_000);
});

it("does not read inherited keys from mime extension map", () => {
expect(inferImageExtension({ mimeType: "constructor" })).toBe(".bin");
});
Expand Down
71 changes: 63 additions & 8 deletions apps/server/src/imageMime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,43 @@ export const SAFE_IMAGE_FILE_EXTENSIONS = new Set([
".webp",
]);

// Whether `code` is a character the base64 payload may contain, aside from
// the whitespace handled separately below.
function isBase64Char(code: number): boolean {
return (
(code >= 0x61 && code <= 0x7a) || // a-z
(code >= 0x41 && code <= 0x5a) || // A-Z
(code >= 0x30 && code <= 0x39) || // 0-9
code === 0x2b || // +
code === 0x2f || // /
code === 0x3d // =
);
}

function isBase64Whitespace(code: number): boolean {
return code === 0x09 || code === 0x0d || code === 0x0a || code === 0x20; // \t \r \n space
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Data URLs carry the full image payload, so this parser must never run a
// regex across the payload: V8's regex engine borrows the JS call stack, and
// matching a multi-megabyte string from a deep call stack (e.g. inside fiber
// execution) throws "Maximum call stack size exceeded".
export function parseBase64DataUrl(
dataUrl: string,
): { readonly mimeType: string; readonly base64: string } | null {
const match = /^data:([^,]+),([a-z0-9+/=\r\n ]+)$/i.exec(dataUrl.trim());
if (!match) return null;
const trimmed = dataUrl.trim();
if (trimmed.slice(0, 5).toLowerCase() !== "data:") return null;

const commaIndex = trimmed.indexOf(",");
if (commaIndex === -1) return null;
const header = trimmed.slice(5, commaIndex);
if (header.length === 0) return null;

const headerParts: Array<string> = [];
for (const part of (match[1] ?? "").split(";")) {
const trimmed = part.trim();
if (trimmed.length > 0) {
headerParts.push(trimmed);
for (const part of header.split(";")) {
const partTrimmed = part.trim();
if (partTrimmed.length > 0) {
headerParts.push(partTrimmed);
}
}
if (headerParts.length < 2) {
Expand All @@ -51,8 +77,37 @@ export function parseBase64DataUrl(
}

const mimeType = headerParts[0]?.toLowerCase();
const base64 = match[2]?.replace(/\s+/g, "");
if (!mimeType || !base64) return null;
if (!mimeType) return null;

const payload = trimmed.slice(commaIndex + 1);
const runs: Array<string> = [];
let runStart = -1;
for (let index = 0; index < payload.length; index += 1) {
const code = payload.charCodeAt(index);
if (isBase64Char(code)) {
if (runStart === -1) runStart = index;
continue;
}
if (!isBase64Whitespace(code)) return null;
if (runStart !== -1) {
runs.push(payload.slice(runStart, index));
runStart = -1;
}
}
if (runStart !== -1) {
runs.push(payload.slice(runStart));
}
const base64 = runs.length === 1 ? runs[0]! : runs.join("");
if (base64.length === 0 || base64.length % 4 !== 0) return null;
const firstPad = base64.indexOf("=");
if (firstPad !== -1) {
// '=' is only valid as one or two trailing padding characters; Node's
// decoder would otherwise silently truncate at the first '='.
if (base64.length - firstPad > 2) return null;
for (let index = firstPad; index < base64.length; index += 1) {
if (base64.charCodeAt(index) !== 0x3d) return null;
}
}

return { mimeType, base64 };
}
Expand Down
22 changes: 22 additions & 0 deletions apps/server/src/keybindings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,28 @@ it.layer(NodeServices.layer)("keybindings", (it) => {
}).pipe(Effect.provide(makeKeybindingsLayer())),
);

it.effect("replacing with a rule that already exists elsewhere does not duplicate it", () =>
Effect.gen(function* () {
const { keybindingsConfigPath } = yield* ServerConfig.ServerConfig;
yield* writeKeybindingsConfig(keybindingsConfigPath, [
{ key: "mod+r", command: "script.run-tests.run" },
{ key: "mod+alt+r", command: "script.run-tests.run" },
]);
yield* Effect.gen(function* () {
const keybindings = yield* Keybindings.Keybindings;
return yield* keybindings.upsertKeybindingRule({
key: "mod+alt+r",
command: "script.run-tests.run",
replace: { key: "mod+r", command: "script.run-tests.run" },
});
});

const persisted = yield* readKeybindingsConfig(keybindingsConfigPath);
const persistedView = persisted.map(({ key, command }) => ({ key, command }));
assert.deepEqual(persistedView, [{ key: "mod+alt+r", command: "script.run-tests.run" }]);
}).pipe(Effect.provide(makeKeybindingsLayer())),
);

it.effect("removes only the targeted custom keybinding", () =>
Effect.gen(function* () {
const { keybindingsConfigPath } = yield* ServerConfig.ServerConfig;
Expand Down
4 changes: 3 additions & 1 deletion apps/server/src/keybindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,9 @@ const make = Effect.gen(function* () {
const nextConfig = [
...customConfig.filter((entry) => {
if (replaceTarget) {
return !isSameKeybindingRule(entry, replaceTarget);
return (
!isSameKeybindingRule(entry, replaceTarget) && !isSameKeybindingRule(entry, rule)
);
}
return !isSameKeybindingRule(entry, rule);
}),
Expand Down
21 changes: 15 additions & 6 deletions apps/server/src/persistence/ProviderSessionRuntime.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as Arr from "effect/Array";
import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
Expand Down Expand Up @@ -280,18 +281,26 @@ export const make = Effect.gen(function* () {
),
),
Effect.flatMap((rows) =>
// Skip rows that no longer decode (e.g. written by an older build)
// instead of failing the whole list — one stale row must not disable
// every consumer that enumerates sessions, such as the reaper.
Effect.forEach(rows, (row) =>
decodeRuntimeRow(row).pipe(
Effect.mapError((cause) =>
PersistenceDecodeError.fromSchemaError(
"ProviderSessionRuntimeRepository.list:decodeRows",
cause,
{ threadId: row.threadId },
),
Effect.map(Option.some),
Effect.catch((cause) =>
Effect.logWarning("provider.session.runtime.row-skipped", {
threadId: row.threadId,
error: PersistenceDecodeError.fromSchemaError(
"ProviderSessionRuntimeRepository.list:decodeRows",
cause,
{ threadId: row.threadId },
).message,
}).pipe(Effect.as(Option.none<ProviderSessionRuntime>())),
),
),
),
),
Effect.map((decoded) => Arr.getSomes(decoded)),
);

const deleteByThreadId: ProviderSessionRuntimeRepository["Service"]["deleteByThreadId"] = (
Expand Down
28 changes: 18 additions & 10 deletions apps/server/src/persistence/RepositoryErrorCorrelation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ describe("persistence error correlation", () => {
}).pipe(Effect.provide(authPairingLinkLayer)),
);

it.effect("correlates provider runtime SQL and per-row decode failures by thread", () =>
it.effect("skips undecodable provider runtime rows and correlates SQL failures by thread", () =>
Effect.gen(function* () {
const runtimes = yield* ProviderSessionRuntime.ProviderSessionRuntimeRepository;
const sql = yield* SqlClient.SqlClient;
Expand Down Expand Up @@ -215,16 +215,24 @@ describe("persistence error correlation", () => {
)
`;

const decodeError = yield* Effect.flip(runtimes.list());
assert.instanceOf(decodeError, PersistenceErrors.PersistenceDecodeError);
assert.deepStrictEqual(decodeError.correlation, { threadId });
assert.equal(
decodeError.message,
`Decode error in ProviderSessionRuntimeRepository.list:decodeRows: ${decodeError.issue}`,
const validThreadId = ThreadId.make("thread-valid");
yield* runtimes.upsert({
threadId: validThreadId,
providerName: "codex",
providerInstanceId: null,
adapterKey: "codex",
runtimeMode: "full-access",
status: "running",
lastSeenAt,
resumeCursor: null,
runtimePayload: null,
});

const listed = yield* runtimes.list();
assert.deepStrictEqual(
listed.map((runtime) => runtime.threadId),
[validThreadId],
);
assert.notInclude(decodeError.issue, runtimePayload);
assert.notInclude(decodeError.message, runtimePayload);
assert.notInclude(decodeError.message, lastSeenAt);

yield* sql`DROP TABLE provider_session_runtime`;
const sqlFailure = yield* Effect.flip(
Expand Down
36 changes: 36 additions & 0 deletions apps/server/src/provider/Drivers/CodexHomeLayout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ it.layer(NodeServices.layer)("CodexHomeLayout", (it) => {

const sessionsTarget = yield* fileSystem.readLink(path.join(shadowHome, "sessions"));
const configTarget = yield* fileSystem.readLink(path.join(shadowHome, "config.toml"));
const mcpOauthLocksTarget = yield* fileSystem.readLink(
path.join(shadowHome, "mcp-oauth-locks"),
);
const modelsCacheExists = yield* fileSystem.exists(
path.join(shadowHome, "models_cache.json"),
);
Expand All @@ -124,12 +127,45 @@ it.layer(NodeServices.layer)("CodexHomeLayout", (it) => {

expect(sessionsTarget).toBe(path.join(sharedHome, "sessions"));
expect(configTarget).toBe(path.join(sharedHome, "config.toml"));
expect(mcpOauthLocksTarget).toBe(path.join(sharedHome, "mcp-oauth-locks"));
expect(modelsCacheExists).toBe(false);
expect(authLinkResult._tag).toBe("Failure");
expect(authContents).toContain("shadow");
}),
);

it.effect("replaces Codex-created local MCP OAuth locks with the shared lock directory", () =>
Effect.gen(function* () {
const fileSystem = yield* FileSystem.FileSystem;
const path = yield* Path.Path;
const sharedHome = yield* makeTempDir("t3code-codex-shared-");
const shadowRoot = yield* makeTempDir("t3code-codex-shadow-root-");
const shadowHome = path.join(shadowRoot, "shadow");
const sharedLocks = path.join(sharedHome, "mcp-oauth-locks");
const shadowLocks = path.join(shadowHome, "mcp-oauth-locks");

yield* writeTextFile(path.join(sharedLocks, "file-store.lock"), "");
yield* writeTextFile(path.join(shadowLocks, "file-store.lock"), "");

const layout = yield* resolveCodexHomeLayout(
decodeCodexSettings({
homePath: sharedHome,
shadowHomePath: shadowHome,
}),
);

yield* materializeCodexShadowHome(layout);

const locksTarget = yield* fileSystem.readLink(shadowLocks);
const sharedLockExists = yield* fileSystem.exists(
path.join(sharedLocks, "file-store.lock"),
);

expect(locksTarget).toBe(sharedLocks);
expect(sharedLockExists).toBe(true);
}),
);

it.effect("accepts Codex-created shadow-local runtime directories", () =>
Effect.gen(function* () {
const fileSystem = yield* FileSystem.FileSystem;
Expand Down
39 changes: 29 additions & 10 deletions apps/server/src/provider/Drivers/CodexHomeLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ const KNOWN_SHARED_DIRECTORIES = [
"plugins",
"cache",
"logs",
"mcp-oauth-locks",
] as const;

const PRIVATE_ENTRY_NAMES = new Set(["auth.json", "models_cache.json"]);
const SHADOW_LOCAL_ENTRY_NAMES = new Set(["log", "memories", "tmp"]);
const REPLACEABLE_SHARED_RUNTIME_DIRECTORIES = new Set(["mcp-oauth-locks"]);

function resolveHomePath(path: Path.Path, value: string | undefined): string {
const expanded =
Expand Down Expand Up @@ -225,16 +227,6 @@ const ensureSymlink = Effect.fn("CodexHomeLayout.ensureSymlink")(function* (inpu
linkPath: link,
});

if (state._tag === "NotSymlink") {
return yield* new CodexShadowHomeEntryConflictError({
sharedHomePath: input.sharedHomePath,
effectiveHomePath: input.effectiveHomePath,
entryName: input.entryName,
linkPath: link,
targetPath: target,
});
}

const createLink = input.fileSystem.symlink(target, link).pipe(
Effect.catchTags({
PlatformError: (cause) =>
Expand All @@ -250,6 +242,33 @@ const ensureSymlink = Effect.fn("CodexHomeLayout.ensureSymlink")(function* (inpu
}),
);

if (state._tag === "NotSymlink") {
if (!REPLACEABLE_SHARED_RUNTIME_DIRECTORIES.has(input.entryName)) {
return yield* new CodexShadowHomeEntryConflictError({
sharedHomePath: input.sharedHomePath,
effectiveHomePath: input.effectiveHomePath,
entryName: input.entryName,
linkPath: link,
targetPath: target,
});
}

yield* input.fileSystem.remove(link, { recursive: true }).pipe(
Effect.catchTags({
PlatformError: (cause) =>
new CodexShadowHomeFileSystemError({
sharedHomePath: input.sharedHomePath,
effectiveHomePath: input.effectiveHomePath,
operation: "remove",
path: link,
entryName: input.entryName,
cause,
}),
}),
);
return yield* createLink;
}

if (state._tag === "Missing") {
return yield* createLink;
}
Expand Down
Loading
Loading