From 7aa717c5c21e74607b4f5e796491af417dc90ccf Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 15 Feb 2026 13:20:36 -0800 Subject: [PATCH] Harden keybinding parsing and config resolution Co-authored-by: codex --- apps/server/src/wsServer.test.ts | 86 +++++++++++++++++++++ apps/server/src/wsServer.ts | 123 ++++++++++++++++++++++++++++++- apps/web/src/keybindings.test.ts | 39 ++++++++++ apps/web/src/keybindings.ts | 24 ++++-- 4 files changed, 266 insertions(+), 6 deletions(-) diff --git a/apps/server/src/wsServer.test.ts b/apps/server/src/wsServer.test.ts index a769193fb7f6f6eaae8d912badde10e88a5224fe..6eaa7fd73a2cea476c128ad7d8177e705da26f01 100644 --- a/apps/server/src/wsServer.test.ts +++ b/apps/server/src/wsServer.test.ts @@ -402,6 +402,92 @@ describe("WebSocket Server", () => { ).toBe(true); }); + it("warns and ignores keybindings with malformed when expressions", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fakeHome = makeTempDir("t3code-home-invalid-when-"); + const configDir = path.join(fakeHome, ".t3"); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, "keybindings.json"), + JSON.stringify([{ key: "mod+j", command: "terminal.toggle", when: "terminalFocus && (" }]), + "utf8", + ); + vi.spyOn(os, "homedir").mockReturnValue(fakeHome); + + server = createTestServer({ cwd: "/my/workspace" }); + await server.start(); + const addr = server.httpServer.address(); + const port = typeof addr === "object" && addr !== null ? addr.port : 0; + + const ws = await connectWs(port); + connections.push(ws); + + await waitForMessage(ws); + + const response = await sendRequest(ws, WS_METHODS.serverGetConfig); + expect(response.error).toBeUndefined(); + expect(response.result).toEqual({ + cwd: "/my/workspace", + keybindings: [ + { key: "mod+j", command: "terminal.toggle" }, + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, + { key: "mod+shift+o", command: "chat.new" }, + { key: "mod+o", command: "editor.openFavorite" }, + ], + }); + expect( + warnSpy.mock.calls.some(([message]) => + String(message).includes("ignoring invalid keybinding entries"), + ), + ).toBe(true); + }); + + it("reads keybindings once at startup and caches the resolved config", async () => { + const fakeHome = makeTempDir("t3code-home-cached-config-"); + const configDir = path.join(fakeHome, ".t3"); + fs.mkdirSync(configDir, { recursive: true }); + const keybindingsPath = path.join(configDir, "keybindings.json"); + fs.writeFileSync( + keybindingsPath, + JSON.stringify([{ key: "cmd+j", command: "terminal.toggle" }]), + "utf8", + ); + vi.spyOn(os, "homedir").mockReturnValue(fakeHome); + + server = createTestServer({ cwd: "/my/workspace" }); + await server.start(); + const addr = server.httpServer.address(); + const port = typeof addr === "object" && addr !== null ? addr.port : 0; + + const ws = await connectWs(port); + connections.push(ws); + await waitForMessage(ws); + + const firstResponse = await sendRequest(ws, WS_METHODS.serverGetConfig); + expect(firstResponse.error).toBeUndefined(); + expect(firstResponse.result).toEqual({ + cwd: "/my/workspace", + keybindings: [ + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, + { key: "mod+shift+o", command: "chat.new" }, + { key: "mod+o", command: "editor.openFavorite" }, + { key: "cmd+j", command: "terminal.toggle" }, + ], + }); + + fs.writeFileSync( + keybindingsPath, + JSON.stringify([{ key: "cmd+k", command: "terminal.toggle" }]), + "utf8", + ); + + const secondResponse = await sendRequest(ws, WS_METHODS.serverGetConfig); + expect(secondResponse.error).toBeUndefined(); + expect(secondResponse.result).toEqual(firstResponse.result); + }); + it("warns and ignores unsupported keybindings config format", async () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const fakeHome = makeTempDir("t3code-home-unsupported-format-"); diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index b22b906ae6400c730d85defaa7b4309552c336f7..8b9d7e76c2643341a7109781f857686fcd604da2 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -78,6 +78,122 @@ const DEFAULT_KEYBINDINGS: KeybindingsConfig = [ { key: "mod+o", command: "editor.openFavorite" }, ]; +type WhenTokenType = "identifier" | "not" | "and" | "or" | "lparen" | "rparen"; + +interface WhenToken { + type: WhenTokenType; +} + +function tokenizeWhenExpression(expression: string): WhenToken[] | null { + const tokens: WhenToken[] = []; + let index = 0; + + while (index < expression.length) { + const current = expression[index]; + if (!current) break; + + if (/\s/.test(current)) { + index += 1; + continue; + } + if (expression.startsWith("&&", index)) { + tokens.push({ type: "and" }); + index += 2; + continue; + } + if (expression.startsWith("||", index)) { + tokens.push({ type: "or" }); + index += 2; + continue; + } + if (current === "!") { + tokens.push({ type: "not" }); + index += 1; + continue; + } + if (current === "(") { + tokens.push({ type: "lparen" }); + index += 1; + continue; + } + if (current === ")") { + tokens.push({ type: "rparen" }); + index += 1; + continue; + } + + const identifier = /^[A-Za-z_][A-Za-z0-9_.-]*/.exec(expression.slice(index)); + if (!identifier) { + return null; + } + tokens.push({ type: "identifier" }); + index += identifier[0].length; + } + + return tokens; +} + +function isValidWhenExpression(when: string): boolean { + const tokens = tokenizeWhenExpression(when); + if (!tokens || tokens.length === 0) return false; + let index = 0; + + const parsePrimary = (): boolean => { + const token = tokens[index]; + if (!token) return false; + + if (token.type === "identifier") { + index += 1; + return true; + } + + if (token.type === "lparen") { + index += 1; + const parsed = parseOr(); + const close = tokens[index]; + if (!parsed || !close || close.type !== "rparen") return false; + index += 1; + return true; + } + + return false; + }; + + const parseUnary = (): boolean => { + const token = tokens[index]; + if (token?.type === "not") { + index += 1; + return parseUnary(); + } + return parsePrimary(); + }; + + const parseAnd = (): boolean => { + if (!parseUnary()) return false; + + while (tokens[index]?.type === "and") { + index += 1; + if (!parseUnary()) return false; + } + + return true; + }; + + const parseOr = (): boolean => { + if (!parseAnd()) return false; + + while (tokens[index]?.type === "or") { + index += 1; + if (!parseAnd()) return false; + } + + return true; + }; + + if (!parseOr()) return false; + return index === tokens.length; +} + function mergeWithDefaultKeybindings(custom: KeybindingsConfig): KeybindingsConfig { if (custom.length === 0) { return [...DEFAULT_KEYBINDINGS]; @@ -114,6 +230,10 @@ function readKeybindingsConfig(logger: ReturnType): Keybind for (const entry of parsed) { const result = keybindingRuleSchema.safeParse(entry); if (result.success) { + if (result.data.when && !isValidWhenExpression(result.data.when)) { + invalidEntries += 1; + continue; + } sanitized.push(result.data); continue; } @@ -174,6 +294,7 @@ export function createServer(options: ServerOptions) { const logger = createLogger("ws"); const logWebSocketEvents = explicitLogWsEvents ?? parseBooleanEnv(process.env.T3CODE_LOG_WS_EVENTS) ?? Boolean(devUrl); + const keybindingsConfig = readKeybindingsConfig(logger); function logOutgoingPush(push: WsPush, recipients: number) { if (!logWebSocketEvents) return; @@ -496,7 +617,7 @@ export function createServer(options: ServerOptions) { case WS_METHODS.serverGetConfig: return { cwd, - keybindings: readKeybindingsConfig(logger), + keybindings: keybindingsConfig, }; default: diff --git a/apps/web/src/keybindings.test.ts b/apps/web/src/keybindings.test.ts index 0770888ac9d5e060d7e467546a7a24bafc29f05f..d6f99c2f98ef069dc24d9b7ccec05c779fa09056 100644 --- a/apps/web/src/keybindings.test.ts +++ b/apps/web/src/keybindings.test.ts @@ -110,6 +110,24 @@ describe("split/new terminal shortcuts", () => { }), ); }); + + it("supports when boolean literals", () => { + const keybindings: KeybindingsConfig = [ + { key: "mod+n", command: "terminal.new", when: "true" }, + { key: "mod+m", command: "terminal.new", when: "false" }, + ]; + + assert.isTrue( + isTerminalNewShortcut(event({ key: "n", ctrlKey: true }), keybindings, { + platform: "Linux", + }), + ); + assert.isFalse( + isTerminalNewShortcut(event({ key: "m", ctrlKey: true }), keybindings, { + platform: "Linux", + }), + ); + }); }); describe("shortcutLabelForCommand", () => { @@ -220,6 +238,11 @@ describe("formatShortcutLabel", () => { it("formats labels for non-macOS", () => { assert.strictEqual(formatShortcutLabel("mod+shift+d", "Linux"), "Ctrl+Shift+D"); }); + + it("formats labels for plus key", () => { + assert.strictEqual(formatShortcutLabel("mod++", "MacIntel"), "⌘+"); + assert.strictEqual(formatShortcutLabel("mod++", "Linux"), "Ctrl++"); + }); }); describe("isTerminalClearShortcut", () => { @@ -232,3 +255,19 @@ describe("isTerminalClearShortcut", () => { assert.isTrue(isTerminalClearShortcut(event({ key: "k", metaKey: true }), "MacIntel")); }); }); + +describe("plus key parsing", () => { + it("matches the plus key shortcut", () => { + const plusBindings: KeybindingsConfig = [{ key: "mod++", command: "terminal.toggle" }]; + assert.isTrue( + isTerminalToggleShortcut(event({ key: "+", metaKey: true }), plusBindings, { + platform: "MacIntel", + }), + ); + assert.isTrue( + isTerminalToggleShortcut(event({ key: "+", ctrlKey: true }), plusBindings, { + platform: "Linux", + }), + ); + }); +}); diff --git a/apps/web/src/keybindings.ts b/apps/web/src/keybindings.ts index 728e8a11bdf302b2fd7bb4930d3a062b3ca75380..58e84ee8c859f7fefba68ff9047fafa44fe05324 100644 --- a/apps/web/src/keybindings.ts +++ b/apps/web/src/keybindings.ts @@ -45,6 +45,7 @@ type WhenToken = | { type: "rparen" }; const WHEN_AST_CACHE = new Map(); +const MAX_WHEN_AST_CACHE_ENTRIES = 512; function isMacPlatform(platform: string): boolean { return /mac|iphone|ipad|ipod/i.test(platform); @@ -57,11 +58,19 @@ function normalizeKeyToken(token: string): string { } function parseShortcutValue(value: string): ParsedShortcut | null { - const tokens = value - .toLowerCase() - .split("+") - .map((token) => token.trim()) - .filter((token) => token.length > 0); + const rawTokens = value.toLowerCase().split("+").map((token) => token.trim()); + const tokens = [...rawTokens]; + let trailingEmptyCount = 0; + while (tokens[tokens.length - 1] === "") { + trailingEmptyCount += 1; + tokens.pop(); + } + if (trailingEmptyCount > 0) { + tokens.push("+"); + } + if (tokens.some((token) => token.length === 0)) { + return null; + } if (tokens.length === 0) return null; let key: string | null = null; @@ -264,6 +273,8 @@ function parseWhenExpression(expression: string): WhenNode | null { function evaluateWhenNode(node: WhenNode, context: ShortcutMatchContext): boolean { switch (node.type) { case "identifier": + if (node.name === "true") return true; + if (node.name === "false") return false; return Boolean(context[node.name]); case "not": return !evaluateWhenNode(node.node, context); @@ -280,6 +291,9 @@ function matchesWhenExpression(when: string | undefined, context: ShortcutMatchC if (normalized.length === 0) return true; if (!WHEN_AST_CACHE.has(normalized)) { + if (WHEN_AST_CACHE.size >= MAX_WHEN_AST_CACHE_ENTRIES) { + WHEN_AST_CACHE.clear(); + } WHEN_AST_CACHE.set(normalized, parseWhenExpression(normalized)); } const ast = WHEN_AST_CACHE.get(normalized); -- 2.54.0