authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-02-15 13:20:36-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-02-15 15:51:05-08:00
log7aa717c5c21e74607b4f5e796491af417dc90ccf
treed0eedc5dc1e9c1aebaaf2e1a1f148218d01fead1
parent8beba3e16b9bc6066d20f64cf672c05e0576df40

Harden keybinding parsing and config resolution

Co-authored-by: codex <codex@users.noreply.github.com>

4 files changed, 266 insertions(+), 6 deletions(-)

apps/server/src/wsServer.test.ts+86
...@@ -402,6 +402,92 @@ describe("WebSocket Server", () => {...@@ -402,6 +402,92 @@ describe("WebSocket Server", () => {
402 ).toBe(true);402 ).toBe(true);
403 });403 });
404404
405 it("warns and ignores keybindings with malformed when expressions", async () => {
406 const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
407 const fakeHome = makeTempDir("t3code-home-invalid-when-");
408 const configDir = path.join(fakeHome, ".t3");
409 fs.mkdirSync(configDir, { recursive: true });
410 fs.writeFileSync(
411 path.join(configDir, "keybindings.json"),
412 JSON.stringify([{ key: "mod+j", command: "terminal.toggle", when: "terminalFocus && (" }]),
413 "utf8",
414 );
415 vi.spyOn(os, "homedir").mockReturnValue(fakeHome);
416
417 server = createTestServer({ cwd: "/my/workspace" });
418 await server.start();
419 const addr = server.httpServer.address();
420 const port = typeof addr === "object" && addr !== null ? addr.port : 0;
421
422 const ws = await connectWs(port);
423 connections.push(ws);
424
425 await waitForMessage(ws);
426
427 const response = await sendRequest(ws, WS_METHODS.serverGetConfig);
428 expect(response.error).toBeUndefined();
429 expect(response.result).toEqual({
430 cwd: "/my/workspace",
431 keybindings: [
432 { key: "mod+j", command: "terminal.toggle" },
433 { key: "mod+d", command: "terminal.split", when: "terminalFocus" },
434 { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" },
435 { key: "mod+shift+o", command: "chat.new" },
436 { key: "mod+o", command: "editor.openFavorite" },
437 ],
438 });
439 expect(
440 warnSpy.mock.calls.some(([message]) =>
441 String(message).includes("ignoring invalid keybinding entries"),
442 ),
443 ).toBe(true);
444 });
445
446 it("reads keybindings once at startup and caches the resolved config", async () => {
447 const fakeHome = makeTempDir("t3code-home-cached-config-");
448 const configDir = path.join(fakeHome, ".t3");
449 fs.mkdirSync(configDir, { recursive: true });
450 const keybindingsPath = path.join(configDir, "keybindings.json");
451 fs.writeFileSync(
452 keybindingsPath,
453 JSON.stringify([{ key: "cmd+j", command: "terminal.toggle" }]),
454 "utf8",
455 );
456 vi.spyOn(os, "homedir").mockReturnValue(fakeHome);
457
458 server = createTestServer({ cwd: "/my/workspace" });
459 await server.start();
460 const addr = server.httpServer.address();
461 const port = typeof addr === "object" && addr !== null ? addr.port : 0;
462
463 const ws = await connectWs(port);
464 connections.push(ws);
465 await waitForMessage(ws);
466
467 const firstResponse = await sendRequest(ws, WS_METHODS.serverGetConfig);
468 expect(firstResponse.error).toBeUndefined();
469 expect(firstResponse.result).toEqual({
470 cwd: "/my/workspace",
471 keybindings: [
472 { key: "mod+d", command: "terminal.split", when: "terminalFocus" },
473 { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" },
474 { key: "mod+shift+o", command: "chat.new" },
475 { key: "mod+o", command: "editor.openFavorite" },
476 { key: "cmd+j", command: "terminal.toggle" },
477 ],
478 });
479
480 fs.writeFileSync(
481 keybindingsPath,
482 JSON.stringify([{ key: "cmd+k", command: "terminal.toggle" }]),
483 "utf8",
484 );
485
486 const secondResponse = await sendRequest(ws, WS_METHODS.serverGetConfig);
487 expect(secondResponse.error).toBeUndefined();
488 expect(secondResponse.result).toEqual(firstResponse.result);
489 });
490
405 it("warns and ignores unsupported keybindings config format", async () => {491 it("warns and ignores unsupported keybindings config format", async () => {
406 const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});492 const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
407 const fakeHome = makeTempDir("t3code-home-unsupported-format-");493 const fakeHome = makeTempDir("t3code-home-unsupported-format-");
apps/server/src/wsServer.ts+122-1
...@@ -78,6 +78,122 @@ const DEFAULT_KEYBINDINGS: KeybindingsConfig = [...@@ -78,6 +78,122 @@ const DEFAULT_KEYBINDINGS: KeybindingsConfig = [
78 { key: "mod+o", command: "editor.openFavorite" },78 { key: "mod+o", command: "editor.openFavorite" },
79];79];
8080
81type WhenTokenType = "identifier" | "not" | "and" | "or" | "lparen" | "rparen";
82
83interface WhenToken {
84 type: WhenTokenType;
85}
86
87function tokenizeWhenExpression(expression: string): WhenToken[] | null {
88 const tokens: WhenToken[] = [];
89 let index = 0;
90
91 while (index < expression.length) {
92 const current = expression[index];
93 if (!current) break;
94
95 if (/\s/.test(current)) {
96 index += 1;
97 continue;
98 }
99 if (expression.startsWith("&&", index)) {
100 tokens.push({ type: "and" });
101 index += 2;
102 continue;
103 }
104 if (expression.startsWith("||", index)) {
105 tokens.push({ type: "or" });
106 index += 2;
107 continue;
108 }
109 if (current === "!") {
110 tokens.push({ type: "not" });
111 index += 1;
112 continue;
113 }
114 if (current === "(") {
115 tokens.push({ type: "lparen" });
116 index += 1;
117 continue;
118 }
119 if (current === ")") {
120 tokens.push({ type: "rparen" });
121 index += 1;
122 continue;
123 }
124
125 const identifier = /^[A-Za-z_][A-Za-z0-9_.-]*/.exec(expression.slice(index));
126 if (!identifier) {
127 return null;
128 }
129 tokens.push({ type: "identifier" });
130 index += identifier[0].length;
131 }
132
133 return tokens;
134}
135
136function isValidWhenExpression(when: string): boolean {
137 const tokens = tokenizeWhenExpression(when);
138 if (!tokens || tokens.length === 0) return false;
139 let index = 0;
140
141 const parsePrimary = (): boolean => {
142 const token = tokens[index];
143 if (!token) return false;
144
145 if (token.type === "identifier") {
146 index += 1;
147 return true;
148 }
149
150 if (token.type === "lparen") {
151 index += 1;
152 const parsed = parseOr();
153 const close = tokens[index];
154 if (!parsed || !close || close.type !== "rparen") return false;
155 index += 1;
156 return true;
157 }
158
159 return false;
160 };
161
162 const parseUnary = (): boolean => {
163 const token = tokens[index];
164 if (token?.type === "not") {
165 index += 1;
166 return parseUnary();
167 }
168 return parsePrimary();
169 };
170
171 const parseAnd = (): boolean => {
172 if (!parseUnary()) return false;
173
174 while (tokens[index]?.type === "and") {
175 index += 1;
176 if (!parseUnary()) return false;
177 }
178
179 return true;
180 };
181
182 const parseOr = (): boolean => {
183 if (!parseAnd()) return false;
184
185 while (tokens[index]?.type === "or") {
186 index += 1;
187 if (!parseAnd()) return false;
188 }
189
190 return true;
191 };
192
193 if (!parseOr()) return false;
194 return index === tokens.length;
195}
196
81function mergeWithDefaultKeybindings(custom: KeybindingsConfig): KeybindingsConfig {197function mergeWithDefaultKeybindings(custom: KeybindingsConfig): KeybindingsConfig {
82 if (custom.length === 0) {198 if (custom.length === 0) {
83 return [...DEFAULT_KEYBINDINGS];199 return [...DEFAULT_KEYBINDINGS];
...@@ -114,6 +230,10 @@ function readKeybindingsConfig(logger: ReturnType<typeof createLogger>): Keybind...@@ -114,6 +230,10 @@ function readKeybindingsConfig(logger: ReturnType<typeof createLogger>): Keybind
114 for (const entry of parsed) {230 for (const entry of parsed) {
115 const result = keybindingRuleSchema.safeParse(entry);231 const result = keybindingRuleSchema.safeParse(entry);
116 if (result.success) {232 if (result.success) {
233 if (result.data.when && !isValidWhenExpression(result.data.when)) {
234 invalidEntries += 1;
235 continue;
236 }
117 sanitized.push(result.data);237 sanitized.push(result.data);
118 continue;238 continue;
119 }239 }
...@@ -174,6 +294,7 @@ export function createServer(options: ServerOptions) {...@@ -174,6 +294,7 @@ export function createServer(options: ServerOptions) {
174 const logger = createLogger("ws");294 const logger = createLogger("ws");
175 const logWebSocketEvents =295 const logWebSocketEvents =
176 explicitLogWsEvents ?? parseBooleanEnv(process.env.T3CODE_LOG_WS_EVENTS) ?? Boolean(devUrl);296 explicitLogWsEvents ?? parseBooleanEnv(process.env.T3CODE_LOG_WS_EVENTS) ?? Boolean(devUrl);
297 const keybindingsConfig = readKeybindingsConfig(logger);
177298
178 function logOutgoingPush(push: WsPush, recipients: number) {299 function logOutgoingPush(push: WsPush, recipients: number) {
179 if (!logWebSocketEvents) return;300 if (!logWebSocketEvents) return;
...@@ -496,7 +617,7 @@ export function createServer(options: ServerOptions) {...@@ -496,7 +617,7 @@ export function createServer(options: ServerOptions) {
496 case WS_METHODS.serverGetConfig:617 case WS_METHODS.serverGetConfig:
497 return {618 return {
498 cwd,619 cwd,
499 keybindings: readKeybindingsConfig(logger),620 keybindings: keybindingsConfig,
500 };621 };
501622
502 default:623 default:
apps/web/src/keybindings.test.ts+39
...@@ -110,6 +110,24 @@ describe("split/new terminal shortcuts", () => {...@@ -110,6 +110,24 @@ describe("split/new terminal shortcuts", () => {
110 }),110 }),
111 );111 );
112 });112 });
113
114 it("supports when boolean literals", () => {
115 const keybindings: KeybindingsConfig = [
116 { key: "mod+n", command: "terminal.new", when: "true" },
117 { key: "mod+m", command: "terminal.new", when: "false" },
118 ];
119
120 assert.isTrue(
121 isTerminalNewShortcut(event({ key: "n", ctrlKey: true }), keybindings, {
122 platform: "Linux",
123 }),
124 );
125 assert.isFalse(
126 isTerminalNewShortcut(event({ key: "m", ctrlKey: true }), keybindings, {
127 platform: "Linux",
128 }),
129 );
130 });
113});131});
114132
115describe("shortcutLabelForCommand", () => {133describe("shortcutLabelForCommand", () => {
...@@ -220,6 +238,11 @@ describe("formatShortcutLabel", () => {...@@ -220,6 +238,11 @@ describe("formatShortcutLabel", () => {
220 it("formats labels for non-macOS", () => {238 it("formats labels for non-macOS", () => {
221 assert.strictEqual(formatShortcutLabel("mod+shift+d", "Linux"), "Ctrl+Shift+D");239 assert.strictEqual(formatShortcutLabel("mod+shift+d", "Linux"), "Ctrl+Shift+D");
222 });240 });
241
242 it("formats labels for plus key", () => {
243 assert.strictEqual(formatShortcutLabel("mod++", "MacIntel"), "⌘+");
244 assert.strictEqual(formatShortcutLabel("mod++", "Linux"), "Ctrl++");
245 });
223});246});
224247
225describe("isTerminalClearShortcut", () => {248describe("isTerminalClearShortcut", () => {
...@@ -232,3 +255,19 @@ describe("isTerminalClearShortcut", () => {...@@ -232,3 +255,19 @@ describe("isTerminalClearShortcut", () => {
232 assert.isTrue(isTerminalClearShortcut(event({ key: "k", metaKey: true }), "MacIntel"));255 assert.isTrue(isTerminalClearShortcut(event({ key: "k", metaKey: true }), "MacIntel"));
233 });256 });
234});257});
258
259describe("plus key parsing", () => {
260 it("matches the plus key shortcut", () => {
261 const plusBindings: KeybindingsConfig = [{ key: "mod++", command: "terminal.toggle" }];
262 assert.isTrue(
263 isTerminalToggleShortcut(event({ key: "+", metaKey: true }), plusBindings, {
264 platform: "MacIntel",
265 }),
266 );
267 assert.isTrue(
268 isTerminalToggleShortcut(event({ key: "+", ctrlKey: true }), plusBindings, {
269 platform: "Linux",
270 }),
271 );
272 });
273});
apps/web/src/keybindings.ts+19-5
...@@ -45,6 +45,7 @@ type WhenToken =...@@ -45,6 +45,7 @@ type WhenToken =
45 | { type: "rparen" };45 | { type: "rparen" };
4646
47const WHEN_AST_CACHE = new Map<string, WhenNode | null>();47const WHEN_AST_CACHE = new Map<string, WhenNode | null>();
48const MAX_WHEN_AST_CACHE_ENTRIES = 512;
4849
49function isMacPlatform(platform: string): boolean {50function isMacPlatform(platform: string): boolean {
50 return /mac|iphone|ipad|ipod/i.test(platform);51 return /mac|iphone|ipad|ipod/i.test(platform);
...@@ -57,11 +58,19 @@ function normalizeKeyToken(token: string): string {...@@ -57,11 +58,19 @@ function normalizeKeyToken(token: string): string {
57}58}
5859
59function parseShortcutValue(value: string): ParsedShortcut | null {60function parseShortcutValue(value: string): ParsedShortcut | null {
60 const tokens = value61 const rawTokens = value.toLowerCase().split("+").map((token) => token.trim());
61 .toLowerCase()62 const tokens = [...rawTokens];
62 .split("+")63 let trailingEmptyCount = 0;
63 .map((token) => token.trim())64 while (tokens[tokens.length - 1] === "") {
64 .filter((token) => token.length > 0);65 trailingEmptyCount += 1;
66 tokens.pop();
67 }
68 if (trailingEmptyCount > 0) {
69 tokens.push("+");
70 }
71 if (tokens.some((token) => token.length === 0)) {
72 return null;
73 }
65 if (tokens.length === 0) return null;74 if (tokens.length === 0) return null;
6675
67 let key: string | null = null;76 let key: string | null = null;
...@@ -264,6 +273,8 @@ function parseWhenExpression(expression: string): WhenNode | null {...@@ -264,6 +273,8 @@ function parseWhenExpression(expression: string): WhenNode | null {
264function evaluateWhenNode(node: WhenNode, context: ShortcutMatchContext): boolean {273function evaluateWhenNode(node: WhenNode, context: ShortcutMatchContext): boolean {
265 switch (node.type) {274 switch (node.type) {
266 case "identifier":275 case "identifier":
276 if (node.name === "true") return true;
277 if (node.name === "false") return false;
267 return Boolean(context[node.name]);278 return Boolean(context[node.name]);
268 case "not":279 case "not":
269 return !evaluateWhenNode(node.node, context);280 return !evaluateWhenNode(node.node, context);
...@@ -280,6 +291,9 @@ function matchesWhenExpression(when: string | undefined, context: ShortcutMatchC...@@ -280,6 +291,9 @@ function matchesWhenExpression(when: string | undefined, context: ShortcutMatchC
280 if (normalized.length === 0) return true;291 if (normalized.length === 0) return true;
281292
282 if (!WHEN_AST_CACHE.has(normalized)) {293 if (!WHEN_AST_CACHE.has(normalized)) {
294 if (WHEN_AST_CACHE.size >= MAX_WHEN_AST_CACHE_ENTRIES) {
295 WHEN_AST_CACHE.clear();
296 }
283 WHEN_AST_CACHE.set(normalized, parseWhenExpression(normalized));297 WHEN_AST_CACHE.set(normalized, parseWhenExpression(normalized));
284 }298 }
285 const ast = WHEN_AST_CACHE.get(normalized);299 const ast = WHEN_AST_CACHE.get(normalized);