- commit
- 4d398e8
- parent
- b2833e7
- author
- External.Kai-Fronsdal
- date
- 2026-03-04 12:17:55 -0500 EST
fix(run): always single-quote args to avoid bash history expansion bug The previous shellQuote preferred double quotes, but \! does not suppress history expansion inside double quotes in interactive bash. Switch to always using single quotes (matching Python's shlex.quote), which are universally safe since nothing is special inside single quotes except ' itself. Add unit tests for shellNeedsQuoting and shellQuote. Fixes #72 Made-with: Cursor
1 files changed,
+147,
-86
+147,
-86
1@@ -281,25 +281,29 @@ const Daemon = struct {
2 const cur_cmd = self.command orelse self.task_command;
3 if (cur_cmd) |args| {
4 for (args, 0..) |arg, i| {
5- if (i > 0) {
6- if (cmd_len < ipc.MAX_CMD_LEN) {
7- cmd_buf[cmd_len] = ' ';
8- cmd_len += 1;
9+ const quoted = if (shellNeedsQuoting(arg))
10+ shellQuote(self.alloc, arg) catch null
11+ else
12+ null;
13+ defer if (quoted) |q| self.alloc.free(q);
14+ const src = quoted orelse arg;
15+
16+ const need = src.len + @as(usize, if (i > 0) 1 else 0);
17+ if (cmd_len + need > ipc.MAX_CMD_LEN) {
18+ const ellipsis = "...";
19+ if (cmd_len + ellipsis.len <= ipc.MAX_CMD_LEN) {
20+ @memcpy(cmd_buf[cmd_len..][0..ellipsis.len], ellipsis);
21+ cmd_len += ellipsis.len;
22 }
23+ break;
24 }
25- if (shellNeedsQuoting(arg)) {
26- const quoted = shellQuote(self.alloc, arg) catch arg;
27- defer if (quoted.ptr != arg.ptr) self.alloc.free(quoted);
28- const remaining = ipc.MAX_CMD_LEN - cmd_len;
29- const copy_len: u16 = @intCast(@min(quoted.len, remaining));
30- @memcpy(cmd_buf[cmd_len..][0..copy_len], quoted[0..copy_len]);
31- cmd_len += copy_len;
32- } else {
33- const remaining = ipc.MAX_CMD_LEN - cmd_len;
34- const copy_len: u16 = @intCast(@min(arg.len, remaining));
35- @memcpy(cmd_buf[cmd_len..][0..copy_len], arg[0..copy_len]);
36- cmd_len += copy_len;
37+
38+ if (i > 0) {
39+ cmd_buf[cmd_len] = ' ';
40+ cmd_len += 1;
41 }
42+ @memcpy(cmd_buf[cmd_len..][0..src.len], src);
43+ cmd_len += @intCast(src.len);
44 }
45 }
46
47@@ -977,46 +981,15 @@ fn shellNeedsQuoting(arg: []const u8) bool {
48 }
49
50 fn shellQuote(alloc: std.mem.Allocator, arg: []const u8) ![]u8 {
51- // Prefer double quotes when the arg has no double quotes (cleaner output).
52- // Fall back to single quotes with '\'' escaping for embedded single quotes.
53- const has_double_quote = std.mem.indexOfScalar(u8, arg, '"') != null;
54-
55- if (!has_double_quote) {
56- // Double-quote style: escape only $ ` \ ! "
57- var len: usize = 2; // opening and closing "
58- for (arg) |ch| {
59- if (ch == '$' or ch == '`' or ch == '\\' or ch == '!') {
60- len += 2; // backslash + char
61- } else {
62- len += 1;
63- }
64- }
65- const buf = try alloc.alloc(u8, len);
66- var i: usize = 0;
67- buf[i] = '"';
68- i += 1;
69- for (arg) |ch| {
70- if (ch == '$' or ch == '`' or ch == '\\' or ch == '!') {
71- buf[i] = '\\';
72- buf[i + 1] = ch;
73- i += 2;
74- } else {
75- buf[i] = ch;
76- i += 1;
77- }
78- }
79- buf[i] = '"';
80- return buf;
81- }
82-
83- // Single-quote style: escape embedded single quotes as '\''
84+ // Always use single quotes (like Python's shlex.quote). Inside single
85+ // quotes nothing is special except ' itself, which we handle with the
86+ // '\'' trick (end quote, escaped literal quote, reopen quote).
87+ //
88+ // The previous double-quote strategy broke in interactive bash because
89+ // \! does not suppress history expansion inside double quotes.
90 var len: usize = 2;
91 for (arg) |ch| {
92- if (ch == '\'') {
93- len += 4;
94- } else {
95- len += 1;
96- }
97+ len += if (ch == '\'') 4 else 1;
98 }
99 const buf = try alloc.alloc(u8, len);
100 var i: usize = 0;
101@@ -1024,10 +997,7 @@ fn shellQuote(alloc: std.mem.Allocator, arg: []const u8) ![]u8 {
102 i += 1;
103 for (arg) |ch| {
104 if (ch == '\'') {
105- buf[i] = '\'';
106- buf[i + 1] = '\\';
107- buf[i + 2] = '\'';
108- buf[i + 3] = '\'';
109+ @memcpy(buf[i..][0..4], "'\\''");
110 i += 4;
111 } else {
112 buf[i] = ch;
113@@ -1067,42 +1037,25 @@ fn run(daemon: *Daemon, command_args: [][]const u8) !void {
114 "echo ZMX_TASK_COMPLETED:$?";
115
116 if (command_args.len > 0) {
117- var parts: std.ArrayList([]const u8) = .empty;
118- defer {
119- for (parts.items) |part| alloc.free(part);
120- parts.deinit(alloc);
121- }
122+ var cmd_list = std.ArrayList(u8).empty;
123+ defer cmd_list.deinit(alloc);
124
125- var total_len: usize = 0;
126- for (command_args) |arg| {
127+ for (command_args, 0..) |arg, i| {
128+ if (i > 0) try cmd_list.append(alloc, ' ');
129 if (shellNeedsQuoting(arg)) {
130 const quoted = try shellQuote(alloc, arg);
131- try parts.append(alloc, quoted);
132- total_len += quoted.len + 1;
133+ defer alloc.free(quoted);
134+ try cmd_list.appendSlice(alloc, quoted);
135 } else {
136- const duped = try alloc.dupe(u8, arg);
137- try parts.append(alloc, duped);
138- total_len += duped.len + 1;
139+ try cmd_list.appendSlice(alloc, arg);
140 }
141 }
142
143- total_len += inline_task_marker.len + 1;
144-
145- const cmd_buf = try alloc.alloc(u8, total_len);
146- allocated_cmd = cmd_buf;
147+ try cmd_list.appendSlice(alloc, inline_task_marker);
148+ try cmd_list.append(alloc, '\n');
149
150- var offset: usize = 0;
151- for (parts.items) |part| {
152- @memcpy(cmd_buf[offset .. offset + part.len], part);
153- offset += part.len;
154- cmd_buf[offset] = ' ';
155- offset += 1;
156- }
157-
158- @memcpy(cmd_buf[offset .. offset + inline_task_marker.len], inline_task_marker);
159- offset += inline_task_marker.len;
160- cmd_buf[offset] = '\n';
161- cmd_to_send = cmd_buf;
162+ cmd_to_send = try cmd_list.toOwnedSlice(alloc);
163+ allocated_cmd = @constCast(cmd_to_send.?);
164 } else {
165 const stdin_fd = posix.STDIN_FILENO;
166 if (!std.posix.isatty(stdin_fd)) {
167@@ -1913,6 +1866,114 @@ fn serializeTerminal(alloc: std.mem.Allocator, term: *ghostty_vt.Terminal, forma
168 };
169 }
170
171+test "shellNeedsQuoting" {
172+ try std.testing.expect(shellNeedsQuoting(""));
173+ try std.testing.expect(shellNeedsQuoting("hello world"));
174+ try std.testing.expect(shellNeedsQuoting("hello!"));
175+ try std.testing.expect(shellNeedsQuoting("$PATH"));
176+ try std.testing.expect(shellNeedsQuoting("it's"));
177+ try std.testing.expect(shellNeedsQuoting("a|b"));
178+ try std.testing.expect(shellNeedsQuoting("a;b"));
179+ try std.testing.expect(!shellNeedsQuoting("hello"));
180+ try std.testing.expect(!shellNeedsQuoting("bash"));
181+ try std.testing.expect(!shellNeedsQuoting("-c"));
182+ try std.testing.expect(!shellNeedsQuoting("/usr/bin/env"));
183+}
184+
185+test "shellQuote" {
186+ const alloc = std.testing.allocator;
187+
188+ const empty = try shellQuote(alloc, "");
189+ defer alloc.free(empty);
190+ try std.testing.expectEqualStrings("''", empty);
191+
192+ const space = try shellQuote(alloc, "hello world");
193+ defer alloc.free(space);
194+ try std.testing.expectEqualStrings("'hello world'", space);
195+
196+ const bang = try shellQuote(alloc, "hello!");
197+ defer alloc.free(bang);
198+ try std.testing.expectEqualStrings("'hello!'", bang);
199+
200+ const dollar = try shellQuote(alloc, "$PATH");
201+ defer alloc.free(dollar);
202+ try std.testing.expectEqualStrings("'$PATH'", dollar);
203+
204+ const sq = try shellQuote(alloc, "it's");
205+ defer alloc.free(sq);
206+ try std.testing.expectEqualStrings("'it'\\''s'", sq);
207+
208+ const dq = try shellQuote(alloc, "say \"hi\"");
209+ defer alloc.free(dq);
210+ try std.testing.expectEqualStrings("'say \"hi\"'", dq);
211+
212+ const both = try shellQuote(alloc, "it's \"cool\"");
213+ defer alloc.free(both);
214+ try std.testing.expectEqualStrings("'it'\\''s \"cool\"'", both);
215+
216+ // just a single quote
217+ const lone_sq = try shellQuote(alloc, "'");
218+ defer alloc.free(lone_sq);
219+ try std.testing.expectEqualStrings("''\\'''", lone_sq);
220+
221+ // multiple consecutive single quotes
222+ const triple_sq = try shellQuote(alloc, "'''");
223+ defer alloc.free(triple_sq);
224+ try std.testing.expectEqualStrings("''\\'''\\'''\\'''", triple_sq);
225+
226+ // backtick command substitution
227+ const backtick = try shellQuote(alloc, "`whoami`");
228+ defer alloc.free(backtick);
229+ try std.testing.expectEqualStrings("'`whoami`'", backtick);
230+
231+ // dollar command substitution
232+ const dollar_cmd = try shellQuote(alloc, "$(whoami)");
233+ defer alloc.free(dollar_cmd);
234+ try std.testing.expectEqualStrings("'$(whoami)'", dollar_cmd);
235+
236+ // glob
237+ const glob = try shellQuote(alloc, "*.txt");
238+ defer alloc.free(glob);
239+ try std.testing.expectEqualStrings("'*.txt'", glob);
240+
241+ // tilde
242+ const tilde = try shellQuote(alloc, "~/file");
243+ defer alloc.free(tilde);
244+ try std.testing.expectEqualStrings("'~/file'", tilde);
245+
246+ // trailing backslash
247+ const trailing_bs = try shellQuote(alloc, "path\\");
248+ defer alloc.free(trailing_bs);
249+ try std.testing.expectEqualStrings("'path\\'", trailing_bs);
250+
251+ // semicolon (command injection)
252+ const semi = try shellQuote(alloc, "; rm -rf /");
253+ defer alloc.free(semi);
254+ try std.testing.expectEqualStrings("'; rm -rf /'", semi);
255+
256+ // embedded newline
257+ const newline = try shellQuote(alloc, "line1\nline2");
258+ defer alloc.free(newline);
259+ try std.testing.expectEqualStrings("'line1\nline2'", newline);
260+
261+ // parentheses (subshell)
262+ const parens = try shellQuote(alloc, "(echo hi)");
263+ defer alloc.free(parens);
264+ try std.testing.expectEqualStrings("'(echo hi)'", parens);
265+
266+ // heredoc marker
267+ const heredoc = try shellQuote(alloc, "<<EOF");
268+ defer alloc.free(heredoc);
269+ try std.testing.expectEqualStrings("'<<EOF'", heredoc);
270+
271+ // no quoting needed -- plain word should still be quoted
272+ // (shellQuote is only called when shellNeedsQuoting returns true,
273+ // but verify it produces valid output anyway)
274+ const plain = try shellQuote(alloc, "hello");
275+ defer alloc.free(plain);
276+ try std.testing.expectEqualStrings("'hello'", plain);
277+}
278+
279 test "isKittyCtrlBackslash" {
280 try std.testing.expect(isKittyCtrlBackslash("\x1b[92;5u"));
281 try std.testing.expect(isKittyCtrlBackslash("\x1b[92;5:1u"));