History

Harden keybinding parsing and config resolution

Julius Marminge committed 8 months ago

Commit be32ce91 parent c70e2d54 files · +266 −6Browse files

Unified · Split

4 files
  1. apps/server/src/wsServer.test.ts +86 −0
  2. apps/server/src/wsServer.ts +122 −1
  3. apps/web/src/keybindings.test.ts +39 −0
  4. apps/web/src/keybindings.ts +19 −5
apps/server/src/wsServer.test.ts+86−0
@@ -402,6 +402,92 @@ describe("WebSocket Server", () => { · 401 unmodified lines
402402 ).toBe(true);
403403 });
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+
405491 it("warns and ignores unsupported keybindings config format", async () => {
406492 const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
407493 const fakeHome = makeTempDir("t3code-home-unsupported-format-");
apps/server/src/wsServer.ts+122−1
@@ -78,6 +78,122 @@ const DEFAULT_KEYBINDINGS: KeybindingsConfig = [ · 77 unmodified lines
7878 { key: "mod+o", command: "editor.openFavorite" },
7979 ];
8080
81+type WhenTokenType = "identifier" | "not" | "and" | "or" | "lparen" | "rparen";
82+
83+interface WhenToken {
84+ type: WhenTokenType;
85+}
86+
87+function 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+
136+function 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+
81197 function mergeWithDefaultKeybindings(custom: KeybindingsConfig): KeybindingsConfig {
82198 if (custom.length === 0) {
83199 return [...DEFAULT_KEYBINDINGS];
@@ -114,6 +230,10 @@ function readKeybindingsConfig(logger: ReturnType<typeof createLogger>): Keybind · 30 unmodified lines
114230 for (const entry of parsed) {
115231 const result = keybindingRuleSchema.safeParse(entry);
116232 if (result.success) {
233+ if (result.data.when && !isValidWhenExpression(result.data.when)) {
234+ invalidEntries += 1;
235+ continue;
236+ }
117237 sanitized.push(result.data);
118238 continue;
119239 }
@@ -174,6 +294,7 @@ export function createServer(options: ServerOptions) { · 54 unmodified lines
174294 const logger = createLogger("ws");
175295 const logWebSocketEvents =
176296 explicitLogWsEvents ?? parseBooleanEnv(process.env.T3CODE_LOG_WS_EVENTS) ?? Boolean(devUrl);
297+ const keybindingsConfig = readKeybindingsConfig(logger);
177298
178299 function logOutgoingPush(push: WsPush, recipients: number) {
179300 if (!logWebSocketEvents) return;
@@ -496,7 +617,7 @@ export function createServer(options: ServerOptions) { · 316 unmodified lines
496617 case WS_METHODS.serverGetConfig:
497618 return {
498619 cwd,
499− keybindings: readKeybindingsConfig(logger),
620+ keybindings: keybindingsConfig,
500621 };
501622
502623 default:
apps/web/src/keybindings.test.ts+39−0
@@ -110,6 +110,24 @@ describe("split/new terminal shortcuts", () => { · 109 unmodified lines
110110 }),
111111 );
112112 });
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+ });
113131 });
114132
115133 describe("shortcutLabelForCommand", () => {
@@ -220,6 +238,11 @@ describe("formatShortcutLabel", () => { · 104 unmodified lines
220238 it("formats labels for non-macOS", () => {
221239 assert.strictEqual(formatShortcutLabel("mod+shift+d", "Linux"), "Ctrl+Shift+D");
222240 });
241+
242+ it("formats labels for plus key", () => {
243+ assert.strictEqual(formatShortcutLabel("mod++", "MacIntel"), "⌘+");
244+ assert.strictEqual(formatShortcutLabel("mod++", "Linux"), "Ctrl++");
245+ });
223246 });
224247
225248 describe("isTerminalClearShortcut", () => {
@@ -232,3 +255,19 @@ describe("isTerminalClearShortcut", () => { · 6 unmodified lines
232255 assert.isTrue(isTerminalClearShortcut(event({ key: "k", metaKey: true }), "MacIntel"));
233256 });
234257 });
258+
259+describe("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 = · 44 unmodified lines
4545 | { type: "rparen" };
4646
4747 const WHEN_AST_CACHE = new Map<string, WhenNode | null>();
48+const MAX_WHEN_AST_CACHE_ENTRIES = 512;
4849
4950 function isMacPlatform(platform: string): boolean {
5051 return /mac|iphone|ipad|ipod/i.test(platform);
@@ -57,11 +58,19 @@ function normalizeKeyToken(token: string): string { · 6 unmodified lines
5758 }
5859
5960 function parseShortcutValue(value: string): ParsedShortcut | null {
60− const tokens = value
61− .toLowerCase()
62− .split("+")
63− .map((token) => token.trim())
64− .filter((token) => token.length > 0);
61+ const rawTokens = value.toLowerCase().split("+").map((token) => token.trim());
62+ const tokens = [...rawTokens];
63+ let trailingEmptyCount = 0;
64+ while (tokens[tokens.length - 1] === "") {
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+ }
6574 if (tokens.length === 0) return null;
6675
6776 let key: string | null = null;
@@ -264,6 +273,8 @@ function parseWhenExpression(expression: string): WhenNode | null { · 196 unmodified lines
264273 function evaluateWhenNode(node: WhenNode, context: ShortcutMatchContext): boolean {
265274 switch (node.type) {
266275 case "identifier":
276+ if (node.name === "true") return true;
277+ if (node.name === "false") return false;
267278 return Boolean(context[node.name]);
268279 case "not":
269280 return !evaluateWhenNode(node.node, context);
@@ -280,6 +291,9 @@ function matchesWhenExpression(when: string | undefined, context: ShortcutMatchC · 10 unmodified lines
280291 if (normalized.length === 0) return true;
281292
282293 if (!WHEN_AST_CACHE.has(normalized)) {
294+ if (WHEN_AST_CACHE.size >= MAX_WHEN_AST_CACHE_ENTRIES) {
295+ WHEN_AST_CACHE.clear();
296+ }
283297 WHEN_AST_CACHE.set(normalized, parseWhenExpression(normalized));
284298 }
285299 const ast = WHEN_AST_CACHE.get(normalized);