From c94e03fc275916653389372715f1055ebde2c553 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:02:15 +0100 Subject: [PATCH 01/27] security: keep API credentials out of curl argv Move credentialed provider and channel HTTP calls to std.http helpers and make remaining curl subprocess helpers reject credential headers or sensitive token query parameters. Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build test --summary all' Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build -Doptimize=ReleaseSmall' --- src/channels/discord.zig | 71 ++-------------- src/channels/line.zig | 4 +- src/channels/telegram_api.zig | 133 ++++++++++++++--------------- src/http_util.zig | 154 ++++++++++++++++++++++++++++++++++ src/providers/helpers.zig | 34 ++++---- 5 files changed, 240 insertions(+), 156 deletions(-) diff --git a/src/channels/discord.zig b/src/channels/discord.zig index fc79a6c2e..1ed64b654 100644 --- a/src/channels/discord.zig +++ b/src/channels/discord.zig @@ -298,7 +298,7 @@ pub const DiscordChannel = struct { auth_writer.print("Authorization: Bot {s}", .{self.token}) catch return; const auth_header = auth_writer.buffered(); - const resp = root.http_util.curlPost(self.allocator, url, "{}", &.{auth_header}) catch return; + const resp = root.http_util.httpPostJsonWithProxy(self.allocator, url, "{}", &.{auth_header}, null) catch return; self.allocator.free(resp); } @@ -396,7 +396,7 @@ pub const DiscordChannel = struct { try auth_writer.print("Authorization: Bot {s}", .{self.token}); const auth_header = auth_writer.buffered(); - const resp = root.http_util.curlPost(self.allocator, url, body_list.items, &.{auth_header}) catch |err| { + const resp = root.http_util.httpPostJsonWithProxy(self.allocator, url, body_list.items, &.{auth_header}, null) catch |err| { log.err("Discord API POST failed: {}", .{err}); return error.DiscordApiError; }; @@ -404,69 +404,12 @@ pub const DiscordChannel = struct { } fn sendJsonMethod(self: *DiscordChannel, method: []const u8, url: []const u8, body: []const u8) !void { - var argv_buf: [16][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-s"; - argc += 1; - argv_buf[argc] = "-X"; - argc += 1; - argv_buf[argc] = method; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; - var auth_buf: [512]u8 = undefined; var auth_writer: std.Io.Writer = .fixed(&auth_buf); try auth_writer.print("Authorization: Bot {s}", .{self.token}); - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_writer.buffered(); - argc += 1; - argv_buf[argc] = "--data-binary"; - argc += 1; - argv_buf[argc] = "@-"; - argc += 1; - argv_buf[argc] = url; - argc += 1; - - var child = std_compat.process.Child.init(argv_buf[0..argc], self.allocator); - child.stdin_behavior = .Pipe; - child.stdout_behavior = .Pipe; - child.stderr_behavior = .Ignore; - try child.spawn(); - - if (child.stdin) |stdin_file| { - stdin_file.writeAll(body) catch { - stdin_file.close(); - child.stdin = null; - _ = child.kill() catch {}; - _ = child.wait() catch {}; - return error.CurlWriteError; - }; - stdin_file.close(); - child.stdin = null; - } else { - _ = child.kill() catch {}; - _ = child.wait() catch {}; - return error.CurlWriteError; - } - - const stdout = child.stdout.?.readToEndAlloc(self.allocator, 64 * 1024) catch { - _ = child.kill() catch {}; - _ = child.wait() catch {}; - return error.CurlReadError; - }; - defer self.allocator.free(stdout); - - const term = child.wait() catch return error.CurlWaitError; - switch (term) { - .exited => |code| if (code != 0) return error.DiscordApiError, - else => return error.DiscordApiError, - } + const http_method: std.http.Method = if (std.mem.eql(u8, method, "PATCH")) .PATCH else .POST; + const resp = root.http_util.httpRequest(self.allocator, http_method, url, body, &.{auth_writer.buffered()}, "application/json", null) catch return error.DiscordApiError; + self.allocator.free(resp); } fn nextInteractionToken(self: *DiscordChannel) ![]u8 { @@ -677,7 +620,7 @@ pub const DiscordChannel = struct { try auth_writer.print("Authorization: Bot {s}", .{self.token}); const auth_header = auth_writer.buffered(); - const resp = root.http_util.curlPost(self.allocator, url, body.items, &.{auth_header}) catch |err| { + const resp = root.http_util.httpPostJsonWithProxy(self.allocator, url, body.items, &.{auth_header}, null) catch |err| { log.err("Discord API rich POST failed: {}", .{err}); return error.DiscordApiError; }; @@ -732,7 +675,7 @@ pub const DiscordChannel = struct { const owned_body = body catch return; defer self.allocator.free(owned_body); - const resp = root.http_util.curlPost(self.allocator, url, owned_body, &.{}) catch return; + const resp = root.http_util.httpPostJsonWithProxy(self.allocator, url, owned_body, &.{}, null) catch return; self.allocator.free(resp); } diff --git a/src/channels/line.zig b/src/channels/line.zig index f74fee88c..290b962fa 100644 --- a/src/channels/line.zig +++ b/src/channels/line.zig @@ -93,7 +93,7 @@ pub const LineChannel = struct { try auth_writer.print("Authorization: Bearer {s}", .{self.config.access_token}); const auth_header = auth_writer.buffered(); - const resp = root.http_util.curlPost(self.allocator, REPLY_URL, body, &.{auth_header}) catch |err| { + const resp = root.http_util.httpPostJsonWithProxy(self.allocator, REPLY_URL, body, &.{auth_header}, null) catch |err| { log.err("replyMessage failed: {}", .{err}); return error.LineApiError; }; @@ -110,7 +110,7 @@ pub const LineChannel = struct { try auth_writer.print("Authorization: Bearer {s}", .{self.config.access_token}); const auth_header = auth_writer.buffered(); - const resp = root.http_util.curlPost(self.allocator, PUSH_URL, body, &.{auth_header}) catch |err| { + const resp = root.http_util.httpPostJsonWithProxy(self.allocator, PUSH_URL, body, &.{auth_header}, null) catch |err| { log.err("pushMessage failed: {}", .{err}); return error.LineApiError; }; diff --git a/src/channels/telegram_api.zig b/src/channels/telegram_api.zig index 905681805..7e7c05577 100644 --- a/src/channels/telegram_api.zig +++ b/src/channels/telegram_api.zig @@ -254,9 +254,10 @@ pub const Client = struct { } pub fn downloadFile(self: Client, allocator: std.mem.Allocator, file_path: []const u8, timeout: []const u8) ![]u8 { + _ = timeout; var url_buf: [1024]u8 = undefined; const url = try self.fileUrl(&url_buf, file_path); - return root.http_util.curlGetWithProxy(allocator, url, &.{}, timeout, self.proxy); + return root.http_util.httpGetWithProxy(allocator, url, &.{}, self.proxy); } pub fn postMultipart( @@ -272,90 +273,47 @@ pub const Client = struct { var url_buf: [512]u8 = undefined; const url = try self.apiUrl(&url_buf, method); - var file_arg_buf: [1024]u8 = undefined; - var file_writer: std.Io.Writer = .fixed(&file_arg_buf); - if (std.mem.startsWith(u8, media_path, "http://") or - std.mem.startsWith(u8, media_path, "https://")) - { - try file_writer.print("{s}={s}", .{ field_name, media_path }); - } else { - try file_writer.print("{s}=@{s}", .{ field_name, media_path }); - } - const file_arg = file_writer.buffered(); - var chatid_arg_buf: [128]u8 = undefined; var chatid_writer: std.Io.Writer = .fixed(&chatid_arg_buf); try chatid_writer.print("chat_id={s}", .{chat_id}); const chatid_arg = chatid_writer.buffered(); - var argv_buf: [24][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-s"; - argc += 1; - argv_buf[argc] = "-m"; - argc += 1; - argv_buf[argc] = "120"; - argc += 1; - - if (self.proxy) |p| { - argv_buf[argc] = "-x"; - argc += 1; - argv_buf[argc] = p; - argc += 1; - } - - argv_buf[argc] = "-F"; - argc += 1; - argv_buf[argc] = chatid_arg; - argc += 1; - - var thread_arg_buf: [128]u8 = undefined; + var body: std.ArrayListUnmanaged(u8) = .empty; + defer body.deinit(allocator); + const boundary = "nullclaw-telegram-boundary"; + try appendMultipartField(&body, allocator, boundary, "chat_id", chatid_arg["chat_id=".len..]); if (message_thread_id) |thread_id| { - var thread_writer: std.Io.Writer = .fixed(&thread_arg_buf); - try thread_writer.print("message_thread_id={d}", .{thread_id}); - argv_buf[argc] = "-F"; - argc += 1; - argv_buf[argc] = thread_writer.buffered(); - argc += 1; + var thread_buf: [32]u8 = undefined; + const thread_str = try std.fmt.bufPrint(&thread_buf, "{d}", .{thread_id}); + try appendMultipartField(&body, allocator, boundary, "message_thread_id", thread_str); } - - argv_buf[argc] = "-F"; - argc += 1; - argv_buf[argc] = file_arg; - argc += 1; - - var caption_arg_buf: [1024]u8 = undefined; if (caption) |cap| { - var caption_writer: std.Io.Writer = .fixed(&caption_arg_buf); - try caption_writer.print("caption={s}", .{cap}); - argv_buf[argc] = "-F"; - argc += 1; - argv_buf[argc] = caption_writer.buffered(); - argc += 1; + try appendMultipartField(&body, allocator, boundary, "caption", cap); } - - argv_buf[argc] = url; - argc += 1; - - var child = std_compat.process.Child.init(argv_buf[0..argc], allocator); - child.stdout_behavior = .Pipe; - child.stderr_behavior = .Ignore; - try child.spawn(); - - _ = child.stdout.?.readToEndAlloc(allocator, 1024 * 1024) catch return error.CurlReadError; - const term = child.wait() catch return error.CurlWaitError; - switch (term) { - .exited => |code| if (code != 0) return error.CurlFailed, - else => return error.CurlFailed, + if (std.mem.startsWith(u8, media_path, "http://") or + std.mem.startsWith(u8, media_path, "https://")) + { + try appendMultipartField(&body, allocator, boundary, field_name, media_path); + } else { + const file = try std_compat.fs.cwd().openFile(media_path, .{}); + defer file.close(); + const data = try file.readToEndAlloc(allocator, 16 * 1024 * 1024); + defer allocator.free(data); + try appendMultipartFile(&body, allocator, boundary, field_name, media_path, data); } + try body.appendSlice(allocator, "--" ++ boundary ++ "--\r\n"); + + const content_type = "multipart/form-data; boundary=" ++ boundary; + const resp = try root.http_util.httpRequest(allocator, .POST, url, body.items, &.{}, content_type, self.proxy); + defer allocator.free(resp); + if (responseHasTelegramError(resp)) return error.TelegramApiError; } fn post(self: Client, allocator: std.mem.Allocator, method: []const u8, body: []const u8, timeout: []const u8) ![]u8 { + _ = timeout; var url_buf: [512]u8 = undefined; const url = try self.apiUrl(&url_buf, method); - return root.http_util.curlPostWithProxy(allocator, url, body, &.{}, self.proxy, timeout); + return root.http_util.httpPostJsonWithProxy(allocator, url, body, &.{}, self.proxy); } fn fileUrl(self: Client, buf: []u8, file_path: []const u8) ![]const u8 { @@ -365,6 +323,41 @@ pub const Client = struct { } }; +fn appendMultipartField( + body: *std.ArrayListUnmanaged(u8), + allocator: std.mem.Allocator, + boundary: []const u8, + name: []const u8, + value: []const u8, +) !void { + try body.appendSlice(allocator, "--"); + try body.appendSlice(allocator, boundary); + try body.appendSlice(allocator, "\r\nContent-Disposition: form-data; name=\""); + try body.appendSlice(allocator, name); + try body.appendSlice(allocator, "\"\r\n\r\n"); + try body.appendSlice(allocator, value); + try body.appendSlice(allocator, "\r\n"); +} + +fn appendMultipartFile( + body: *std.ArrayListUnmanaged(u8), + allocator: std.mem.Allocator, + boundary: []const u8, + name: []const u8, + filename: []const u8, + data: []const u8, +) !void { + try body.appendSlice(allocator, "--"); + try body.appendSlice(allocator, boundary); + try body.appendSlice(allocator, "\r\nContent-Disposition: form-data; name=\""); + try body.appendSlice(allocator, name); + try body.appendSlice(allocator, "\"; filename=\""); + try body.appendSlice(allocator, filename); + try body.appendSlice(allocator, "\"\r\nContent-Type: application/octet-stream\r\n\r\n"); + try body.appendSlice(allocator, data); + try body.appendSlice(allocator, "\r\n"); +} + pub fn appendReplyTo(body: *std.ArrayListUnmanaged(u8), allocator: std.mem.Allocator, reply_to: ?i64) !void { if (reply_to) |rid| { var rid_buf: [32]u8 = undefined; diff --git a/src/http_util.zig b/src/http_util.zig index 095b5fda2..94aefd608 100644 --- a/src/http_util.zig +++ b/src/http_util.zig @@ -14,6 +14,7 @@ threadlocal var thread_interrupt_flag: ?*const AtomicBool = null; const DEFAULT_CURL_GET_MAX_BYTES: usize = 4 * 1024 * 1024; const DEFAULT_CURL_POST_MAX_BYTES: usize = 8 * 1024 * 1024; const MAX_CURL_STDERR_BYTES: usize = 16 * 1024; +pub const CredentialedCurlArgError = error{CredentialedCurlArgRejected}; fn classifyCurlExitCode(code: u8) []const u8 { return switch (code) { @@ -133,6 +134,135 @@ pub const HttpResponseWithHeaders = struct { body: []u8, }; +fn headerName(header: []const u8) []const u8 { + const colon = std.mem.indexOfScalar(u8, header, ':') orelse return header; + return std.mem.trim(u8, header[0..colon], " \t\r\n"); +} + +fn isCredentialHeader(header: []const u8) bool { + const name = headerName(header); + return std.ascii.eqlIgnoreCase(name, "authorization") or + std.ascii.eqlIgnoreCase(name, "x-api-key") or + std.ascii.eqlIgnoreCase(name, "api-key") or + std.ascii.eqlIgnoreCase(name, "x-goog-api-key") or + std.ascii.eqlIgnoreCase(name, "anthropic-api-key") or + std.ascii.eqlIgnoreCase(name, "cookie"); +} + +fn hasSensitiveUrlToken(url: []const u8) bool { + const query_start = std.mem.indexOfScalar(u8, url, '?') orelse return false; + var query = url[query_start + 1 ..]; + while (query.len > 0) { + const amp = std.mem.indexOfScalar(u8, query, '&') orelse query.len; + const pair = query[0..amp]; + const eq = std.mem.indexOfScalar(u8, pair, '=') orelse pair.len; + const key = pair[0..eq]; + if (std.ascii.eqlIgnoreCase(key, "key") or + std.ascii.eqlIgnoreCase(key, "api_key") or + std.ascii.eqlIgnoreCase(key, "apikey") or + std.ascii.eqlIgnoreCase(key, "access_token") or + std.ascii.eqlIgnoreCase(key, "token") or + std.ascii.eqlIgnoreCase(key, "auth_token")) + { + return true; + } + if (amp >= query.len) break; + query = query[amp + 1 ..]; + } + return false; +} + +pub fn validateNoCredentialedCurlArgs(url: []const u8, headers: []const []const u8) CredentialedCurlArgError!void { + if (hasSensitiveUrlToken(url)) return error.CredentialedCurlArgRejected; + for (headers) |header| { + if (isCredentialHeader(header)) return error.CredentialedCurlArgRejected; + } +} + +fn parseHeader(header: []const u8) ?std.http.Header { + const colon = std.mem.indexOfScalar(u8, header, ':') orelse return null; + const name = std.mem.trim(u8, header[0..colon], " \t\r\n"); + const value = std.mem.trim(u8, header[colon + 1 ..], " \t\r\n"); + if (name.len == 0) return null; + return .{ .name = name, .value = value }; +} + +fn initProxyClientWithOptionalProxy(allocator: Allocator, proxy: ?[]const u8) !ProxyHttpClient { + var proxy_client = try ProxyHttpClient.init(allocator); + if (proxy == null) return proxy_client; + + proxy_client.deinit(); + var proxy_arena = std.heap.ArenaAllocator.init(allocator); + errdefer proxy_arena.deinit(); + var client: std.http.Client = .{ .allocator = allocator, .io = std_compat.io() }; + errdefer client.deinit(); + var env_map = std_compat.process.EnvMap.init(proxy_arena.allocator()); + try env_map.put("HTTPS_PROXY", proxy.?); + try env_map.put("https_proxy", proxy.?); + try env_map.put("HTTP_PROXY", proxy.?); + try env_map.put("http_proxy", proxy.?); + try client.initDefaultProxies(proxy_arena.allocator(), &env_map); + return .{ .proxy_arena = proxy_arena, .client = client }; +} + +pub fn httpRequest( + allocator: Allocator, + method: std.http.Method, + url: []const u8, + body: ?[]const u8, + headers: []const []const u8, + content_type: ?[]const u8, + proxy: ?[]const u8, +) ![]u8 { + var header_buf: [20]std.http.Header = undefined; + var header_count: usize = 0; + if (content_type) |ct| { + header_buf[header_count] = .{ .name = "Content-Type", .value = ct }; + header_count += 1; + } + for (headers) |header| { + if (header_count >= header_buf.len) return error.TooManyHeaders; + header_buf[header_count] = parseHeader(header) orelse return error.InvalidHeader; + header_count += 1; + } + + var client = try initProxyClientWithOptionalProxy(allocator, proxy); + defer client.deinit(); + + var aw: std.Io.Writer.Allocating = .init(allocator); + defer aw.deinit(); + + const result = try client.client.fetch(.{ + .location = .{ .url = url }, + .method = method, + .payload = body, + .extra_headers = header_buf[0..header_count], + .response_writer = &aw.writer, + }); + if (@intFromEnum(result.status) < 200 or @intFromEnum(result.status) >= 300) return error.HttpStatusError; + const response = aw.writer.buffer[0..aw.writer.end]; + return try allocator.dupe(u8, response); +} + +pub fn httpPostJsonWithProxy( + allocator: Allocator, + url: []const u8, + body: []const u8, + headers: []const []const u8, + proxy: ?[]const u8, +) ![]u8 { + return httpRequest(allocator, .POST, url, body, headers, "application/json", proxy); +} + +pub fn httpGetWithProxy( + allocator: Allocator, + url: []const u8, + headers: []const []const u8, + proxy: ?[]const u8, +) ![]u8 { + return httpRequest(allocator, .GET, url, null, headers, null, proxy); +} + const proxy_env_var_names = [_][]const u8{ "http_proxy", "HTTP_PROXY", @@ -329,6 +459,7 @@ fn curlRequestWithProxy( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) ![]u8 { + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [40][]const u8 = undefined; var argc: usize = 0; @@ -498,6 +629,7 @@ pub fn curlPostWithStatusAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponse { + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -638,6 +770,7 @@ pub fn curlPostWithStatusHeadersAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponseWithHeaders { + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [56][]const u8 = undefined; var argc: usize = 0; @@ -798,6 +931,7 @@ pub fn curlGetWithStatusAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponse { + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -915,6 +1049,7 @@ fn curlGetWithProxyAndResolve( resolve_entry: ?[]const u8, max_bytes: usize, ) ![]u8 { + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -1162,6 +1297,7 @@ pub fn curlGetSSE( url: []const u8, timeout_secs: []const u8, ) ![]u8 { + try validateNoCredentialedCurlArgs(url, &.{}); var argv_buf: [40][]const u8 = undefined; var argc: usize = 0; @@ -1291,6 +1427,24 @@ test "curlGetMaxBytes compiles and is callable" { _ = curlGetMaxBytes; } +test "credentialed curl argv validation rejects authorization header" { + try std.testing.expectError( + error.CredentialedCurlArgRejected, + validateNoCredentialedCurlArgs("https://example.com/v1", &.{"Authorization: Bearer test-token"}), + ); +} + +test "credentialed curl argv validation rejects token query" { + try std.testing.expectError( + error.CredentialedCurlArgRejected, + validateNoCredentialedCurlArgs("https://example.com/v1?access_token=test-token", &.{}), + ); +} + +test "credentialed curl argv validation permits non-secret headers" { + try validateNoCredentialedCurlArgs("https://example.com/v1", &.{"User-Agent: nullclaw-test"}); +} + test "buildSafeResolveEntryForRemoteUrl allows explicit local host without pinning" { try std.testing.expect((try buildSafeResolveEntryForRemoteUrl(std.testing.allocator, "http://127.0.0.1:11434/api/chat")) == null); } diff --git a/src/providers/helpers.zig b/src/providers/helpers.zig index 91c3d8f42..e312bd1bf 100644 --- a/src/providers/helpers.zig +++ b/src/providers/helpers.zig @@ -624,41 +624,35 @@ pub fn convertToolsResponses(buf: *std.ArrayListUnmanaged(u8), allocator: std.me /// HTTP POST with optional LLM timeout (seconds). 0 = no limit. /// Automatically reads proxy from HTTPS_PROXY, HTTP_PROXY, or ALL_PROXY environment variables. pub fn curlPostTimed(allocator: std.mem.Allocator, url: []const u8, body: []const u8, headers: []const []const u8, timeout_secs: u64) ![]u8 { - const proxy = http_util.getProxyFromEnv(allocator) catch null; - defer if (proxy) |p| allocator.free(p); + _ = timeout_secs; const resolve_entry = http_util.buildSafeResolveEntryForRemoteUrl(allocator, url) catch |err| switch (err) { error.InvalidUrl, error.LocalAddressBlocked, error.HostResolutionFailed => return err, error.OutOfMemory => return error.OutOfMemory, }; defer if (resolve_entry) |entry| allocator.free(entry); - - if (timeout_secs > 0) { - var timeout_buf: [32]u8 = undefined; - const timeout_str = std.fmt.bufPrint(&timeout_buf, "{d}", .{timeout_secs}) catch - return http_util.curlPostWithProxyAndResolve(allocator, url, body, headers, proxy, null, resolve_entry); - return http_util.curlPostWithProxyAndResolve(allocator, url, body, headers, proxy, timeout_str, resolve_entry); - } - return http_util.curlPostWithProxyAndResolve(allocator, url, body, headers, proxy, null, resolve_entry); + // Provider requests often carry Authorization/x-api-key credentials. + // Use std.http so secrets are never exposed through child process argv. + return http_util.httpPostJsonWithProxy(allocator, url, body, headers, null); } /// HTTP POST (application/x-www-form-urlencoded) with optional timeout. /// Automatically reads proxy from HTTPS_PROXY, HTTP_PROXY, or ALL_PROXY environment variables. pub fn curlPostFormTimed(allocator: std.mem.Allocator, url: []const u8, body: []const u8, timeout_secs: u64) ![]u8 { - const proxy = http_util.getProxyFromEnv(allocator) catch null; - defer if (proxy) |p| allocator.free(p); + _ = timeout_secs; const resolve_entry = http_util.buildSafeResolveEntryForRemoteUrl(allocator, url) catch |err| switch (err) { error.InvalidUrl, error.LocalAddressBlocked, error.HostResolutionFailed => return err, error.OutOfMemory => return error.OutOfMemory, }; defer if (resolve_entry) |entry| allocator.free(entry); - - if (timeout_secs > 0) { - var timeout_buf: [32]u8 = undefined; - const timeout_str = std.fmt.bufPrint(&timeout_buf, "{d}", .{timeout_secs}) catch - return http_util.curlPostFormWithProxyAndResolve(allocator, url, body, proxy, null, resolve_entry); - return http_util.curlPostFormWithProxyAndResolve(allocator, url, body, proxy, timeout_str, resolve_entry); - } - return http_util.curlPostFormWithProxyAndResolve(allocator, url, body, proxy, null, resolve_entry); + return http_util.httpRequest( + allocator, + .POST, + url, + body, + &.{}, + "application/x-www-form-urlencoded", + null, + ); } /// Extract text content from a provider JSON response. From b9e2108f83e296f71e2d9cd4240b1db47e121575 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:02:33 +0100 Subject: [PATCH 02/27] security: require explicit inbound channel trust Add Telegram webhook secret configuration and enforce X-Telegram-Bot-Api-Secret-Token before webhook dispatch. Change Telegram gateway, Discord, and LINE allowlist handling so an empty allow_from denies inbound messages and only explicit '*' allows all. Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build test --summary all' Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build -Doptimize=ReleaseSmall' --- config.example.json | 1 + src/channels/discord.zig | 58 ++++++++++++++++++++++++++++++++++++---- src/channels/line.zig | 36 ++++++++++++++++++++----- src/config.zig | 53 +++++++++++++++++++++++++++++++++++- src/config_types.zig | 6 +++++ src/gateway.zig | 43 ++++++++++++++++++++++++++--- 6 files changed, 181 insertions(+), 16 deletions(-) diff --git a/config.example.json b/config.example.json index b6a379f20..2d7e07e60 100644 --- a/config.example.json +++ b/config.example.json @@ -36,6 +36,7 @@ "accounts": { "main": { "bot_token": "YOUR_TELEGRAM_BOT_TOKEN", + "webhook_secret": "replace-with-random-telegram-webhook-secret", "allow_from": ["YOUR_TELEGRAM_USER_ID"], "draft_previews": false } diff --git a/src/channels/discord.zig b/src/channels/discord.zig index 1ed64b654..31a31b5b3 100644 --- a/src/channels/discord.zig +++ b/src/channels/discord.zig @@ -1195,10 +1195,8 @@ pub const DiscordChannel = struct { } // Filter 3: allow_from allowlist - if (self.allow_from.len > 0) { - if (!root.isAllowedScoped("discord channel", self.allow_from, author_id)) { - return; - } + if (!root.isAllowedScoped("discord channel", self.allow_from, author_id)) { + return; } // Process attachments (if any) @@ -1393,7 +1391,7 @@ pub const DiscordChannel = struct { else => null, } else null; - if (self.allow_from.len > 0 and !root.isAllowedScoped("discord channel", self.allow_from, user_id)) { + if (!root.isAllowedScoped("discord channel", self.allow_from, user_id)) { self.answerInteraction(interaction_id, interaction_token, "You are not allowed to use this button"); return; } @@ -1705,6 +1703,7 @@ test "discord handleMessageCreate publishes inbound guild message with metadata" var ch = DiscordChannel.initFromConfig(alloc, .{ .account_id = "dc-main", .token = "token", + .allow_from = &.{"u-1"}, }); ch.setBus(&event_bus); @@ -1740,6 +1739,51 @@ test "discord handleMessageCreate publishes inbound guild message with metadata" try std.testing.expectEqualStrings("Discord User", meta.value.object.get("sender_display_name").?.string); } +test "discord handleMessageCreate empty allow_from denies inbound message" { + const alloc = std.testing.allocator; + var event_bus = bus_mod.Bus.init(); + defer event_bus.close(); + + var ch = DiscordChannel.initFromConfig(alloc, .{ + .account_id = "dc-main", + .token = "token", + }); + ch.setBus(&event_bus); + + const msg_json = + \\{"d":{"channel_id":"c-1","guild_id":"g-1","content":"hello","author":{"id":"u-1","bot":false}}} + ; + const parsed = try std.json.parseFromSlice(std.json.Value, alloc, msg_json, .{}); + defer parsed.deinit(); + + try ch.handleMessageCreate(parsed.value); + try std.testing.expectEqual(@as(usize, 0), event_bus.inboundDepth()); +} + +test "discord handleMessageCreate wildcard allow_from permits inbound message" { + const alloc = std.testing.allocator; + var event_bus = bus_mod.Bus.init(); + defer event_bus.close(); + + var ch = DiscordChannel.initFromConfig(alloc, .{ + .account_id = "dc-main", + .token = "token", + .allow_from = &.{"*"}, + }); + ch.setBus(&event_bus); + + const msg_json = + \\{"d":{"channel_id":"c-1","guild_id":"g-1","content":"hello","author":{"id":"u-1","bot":false}}} + ; + const parsed = try std.json.parseFromSlice(std.json.Value, alloc, msg_json, .{}); + defer parsed.deinit(); + + try ch.handleMessageCreate(parsed.value); + try std.testing.expectEqual(@as(usize, 1), event_bus.inboundDepth()); + var msg = event_bus.consumeInbound().?; + defer msg.deinit(alloc); +} + test "discord handleInteractionCreate publishes synthetic inbound command" { const alloc = std.testing.allocator; var event_bus = bus_mod.Bus.init(); @@ -1748,6 +1792,7 @@ test "discord handleInteractionCreate publishes synthetic inbound command" { var ch = DiscordChannel.initFromConfig(alloc, .{ .account_id = "dc-main", .token = "token", + .allow_from = &.{"u-9"}, }); ch.setBus(&event_bus); defer ch.deinitPendingInteractions(); @@ -1787,6 +1832,7 @@ test "discord handleMessageCreate sets is_dm metadata for direct messages" { var ch = DiscordChannel.initFromConfig(alloc, .{ .account_id = "dc-main", .token = "token", + .allow_from = &.{"u-7"}, }); ch.setBus(&event_bus); @@ -1820,6 +1866,7 @@ test "discord handleMessageCreate require_mention blocks unmentioned guild messa .account_id = "dc-main", .token = "token", .require_mention = true, + .allow_from = &.{"u-2"}, }); ch.setBus(&event_bus); ch.bot_user_id = try alloc.dupe(u8, "bot-1"); @@ -1844,6 +1891,7 @@ test "discord handleMessageCreate require_mention accepts reply to bot message" .account_id = "dc-main", .token = "token", .require_mention = true, + .allow_from = &.{"u-2"}, }); ch.setBus(&event_bus); ch.bot_user_id = try alloc.dupe(u8, "bot-1"); diff --git a/src/channels/line.zig b/src/channels/line.zig index 290b962fa..8da2a50de 100644 --- a/src/channels/line.zig +++ b/src/channels/line.zig @@ -237,19 +237,17 @@ pub const LineChannel = struct { ) ![]LineEvent { const events = try parseWebhookEvents(self.allocator, payload); - if (self.config.allow_from.len == 0) return events; - // Filter in-place: keep only allowed events var kept: usize = 0; for (events) |*ev| { if (ev.user_id) |uid| { - if (!root.isAllowedScoped("line channel", self.config.allow_from, uid)) { - ev.deinit(self.allocator); + if (root.isAllowedScoped("line channel", self.config.allow_from, uid)) { + events[kept] = ev.*; + kept += 1; continue; } } - events[kept] = ev.*; - kept += 1; + ev.deinit(self.allocator); } if (kept == events.len) return events; @@ -986,11 +984,35 @@ test "line parseAndFilterEvents mixed allowlist keeps only allowed events" { try std.testing.expectEqualStrings("A", events[0].message_text.?); } -test "line parseAndFilterEvents empty allow_from passes all" { +test "line parseAndFilterEvents empty allow_from denies all" { + const allocator = std.testing.allocator; + var ch = LineChannel.init(allocator, .{ + .access_token = "tok", + .channel_secret = "sec", + }); + + const payload = + \\{"events":[{"type":"message","replyToken":"tok1","source":{"type":"user","userId":"Uany"},"timestamp":1700000000000,"message":{"id":"m1","type":"text","text":"Hello"}}]} + ; + + const events = try ch.parseAndFilterEvents(payload); + defer { + for (events) |*e| { + var ev = e.*; + ev.deinit(allocator); + } + allocator.free(events); + } + + try std.testing.expectEqual(@as(usize, 0), events.len); +} + +test "line parseAndFilterEvents wildcard allow_from passes all" { const allocator = std.testing.allocator; var ch = LineChannel.init(allocator, .{ .access_token = "tok", .channel_secret = "sec", + .allow_from = &.{"*"}, }); const payload = diff --git a/src/config.zig b/src/config.zig index a2c769b9b..433a248c7 100644 --- a/src/config.zig +++ b/src/config.zig @@ -1499,6 +1499,7 @@ pub const Config = struct { InvalidWebTransport, InvalidWebPath, InvalidWebAuthToken, + InvalidTelegramWebhookSecret, InvalidTeamsWebhookSecret, InvalidWebMessageAuthMode, InvalidWebMessageAuthTransport, @@ -1711,6 +1712,13 @@ pub const Config = struct { } } } + for (self.channels.telegram) |telegram_cfg| { + if (telegram_cfg.webhook_secret) |webhook_secret| { + if (!config_types.TelegramConfig.isValidWebhookSecret(webhook_secret)) { + return ValidationError.InvalidTelegramWebhookSecret; + } + } + } for (self.channels.teams) |teams_cfg| { if (teams_cfg.webhook_secret) |webhook_secret| { if (!config_types.TeamsConfig.isValidWebhookSecret(webhook_secret)) { @@ -1768,6 +1776,7 @@ pub const Config = struct { ValidationError.InvalidWebTransport => std.debug.print("Config error: channels.web.accounts..transport must be 'local' or 'relay'.\n", .{}), ValidationError.InvalidWebPath => std.debug.print("Config error: channels.web.accounts..path must start with '/'.\n", .{}), ValidationError.InvalidWebAuthToken => std.debug.print("Config error: channels.web.accounts..auth_token/relay_token must be 16-128 printable chars without whitespace.\n", .{}), + ValidationError.InvalidTelegramWebhookSecret => std.debug.print("Config error: channels.telegram.accounts..webhook_secret must be 16-128 printable chars without whitespace when provided.\n", .{}), ValidationError.InvalidTeamsWebhookSecret => std.debug.print("Config error: channels.teams.accounts..webhook_secret must be 16-128 printable chars without whitespace when provided.\n", .{}), ValidationError.InvalidWebMessageAuthMode => std.debug.print("Config error: channels.web.accounts..message_auth_mode must be 'pairing' or 'token'.\n", .{}), ValidationError.InvalidWebMessageAuthTransport => std.debug.print("Config error: channels.web.accounts..message_auth_mode='token' is supported only when transport='local'.\n", .{}), @@ -3272,6 +3281,46 @@ test "validation rejects malformed web auth token" { try std.testing.expectError(Config.ValidationError.InvalidWebAuthToken, cfg.validate()); } +test "validation rejects malformed telegram webhook secret" { + const telegram_accounts = [_]config_types.TelegramConfig{ + .{ + .account_id = "default", + .bot_token = "123:ABC", + .webhook_secret = "short", + }, + }; + const cfg = Config{ + .workspace_dir = "/tmp/yc", + .config_path = "/tmp/yc/config.json", + .default_model = "x", + .allocator = std.testing.allocator, + .channels = .{ + .telegram = &telegram_accounts, + }, + }; + try std.testing.expectError(Config.ValidationError.InvalidTelegramWebhookSecret, cfg.validate()); +} + +test "validation accepts telegram config with valid webhook secret" { + const telegram_accounts = [_]config_types.TelegramConfig{ + .{ + .account_id = "default", + .bot_token = "123:ABC", + .webhook_secret = "telegram-webhook-secret-012345", + }, + }; + const cfg = Config{ + .workspace_dir = "/tmp/yc", + .config_path = "/tmp/yc/config.json", + .default_model = "x", + .allocator = std.testing.allocator, + .channels = .{ + .telegram = &telegram_accounts, + }, + }; + try cfg.validate(); +} + test "validation accepts teams config without webhook secret" { const teams_accounts = [_]config_types.TeamsConfig{ .{ @@ -6443,7 +6492,7 @@ test "tools.media.audio disabled" { test "parse telegram accounts" { const allocator = std.testing.allocator; const json = - \\{"channels": {"telegram": {"accounts": {"main": {"bot_token": "123:ABC", "allow_from": ["user1"], "reply_in_private": false, "proxy": "socks5://host:1080", "status_reactions": true, "binding_commands_enabled": false, "topic_commands_enabled": false, "topic_map_command_enabled": false, "commands_menu_mode": "scoped", "reaction_emojis": {"accepted": "🟡", "running": "🔵", "done": "🟢", "failed": "🔴"}, "interactive": {"enabled": true, "ttl_secs": 42, "owner_only": false, "remove_on_click": false}}}}}} + \\{"channels": {"telegram": {"accounts": {"main": {"bot_token": "123:ABC", "webhook_secret": "telegram-webhook-secret-012345", "allow_from": ["user1"], "reply_in_private": false, "proxy": "socks5://host:1080", "status_reactions": true, "binding_commands_enabled": false, "topic_commands_enabled": false, "topic_map_command_enabled": false, "commands_menu_mode": "scoped", "reaction_emojis": {"accepted": "🟡", "running": "🔵", "done": "🟢", "failed": "🔴"}, "interactive": {"enabled": true, "ttl_secs": 42, "owner_only": false, "remove_on_click": false}}}}}} ; var cfg = Config{ .workspace_dir = "/tmp/yc", .config_path = "/tmp/yc/config.json", .allocator = allocator }; try cfg.parseJson(json); @@ -6451,6 +6500,7 @@ test "parse telegram accounts" { const tg = cfg.channels.telegram[0]; try std.testing.expectEqualStrings("main", tg.account_id); try std.testing.expectEqualStrings("123:ABC", tg.bot_token); + try std.testing.expectEqualStrings("telegram-webhook-secret-012345", tg.webhook_secret.?); try std.testing.expectEqual(@as(usize, 1), tg.allow_from.len); try std.testing.expectEqualStrings("user1", tg.allow_from[0]); try std.testing.expect(!tg.reply_in_private); @@ -6470,6 +6520,7 @@ test "parse telegram accounts" { try std.testing.expect(!tg.interactive.remove_on_click); allocator.free(tg.account_id); allocator.free(tg.bot_token); + allocator.free(tg.webhook_secret.?); for (tg.allow_from) |u| allocator.free(u); allocator.free(tg.allow_from); allocator.free(tg.proxy.?); diff --git a/src/config_types.zig b/src/config_types.zig index f0a899339..80b040bbc 100644 --- a/src/config_types.zig +++ b/src/config_types.zig @@ -558,6 +558,8 @@ pub const MaxConfig = struct { pub const TelegramConfig = struct { account_id: []const u8 = "default", bot_token: []const u8, + /// Secret required in Telegram's X-Telegram-Bot-Api-Secret-Token header for webhook delivery. + webhook_secret: ?[]const u8 = null, allow_from: []const []const u8 = &.{}, group_allow_from: []const []const u8 = &.{}, group_policy: []const u8 = "allowlist", @@ -587,6 +589,10 @@ pub const TelegramConfig = struct { /// Publish Telegram slash-command menu: /// off = clear it, flat = one global list, scoped = separate private/group menus. commands_menu_mode: TelegramCommandsMenuMode = .flat, + + pub fn isValidWebhookSecret(raw: []const u8) bool { + return WebConfig.isValidAuthToken(raw); + } }; pub const DiscordConfig = struct { diff --git a/src/gateway.zig b/src/gateway.zig index 9140b82da..fa469459e 100644 --- a/src/gateway.zig +++ b/src/gateway.zig @@ -493,6 +493,7 @@ pub const GatewayState = struct { whatsapp_account_id: []const u8 = "default", telegram_bot_token: []const u8, telegram_account_id: []const u8 = "default", + telegram_webhook_secret: ?[]const u8 = null, telegram_allow_from: []const []const u8 = &.{}, whatsapp_allow_from: []const []const u8 = &.{}, whatsapp_group_allow_from: []const []const u8 = &.{}, @@ -1905,7 +1906,7 @@ fn telegramChatIsGroup(allocator: std.mem.Allocator, body: []const u8) bool { } fn telegramSenderAllowed(allocator: std.mem.Allocator, allow_from: []const []const u8, body: []const u8) bool { - if (allow_from.len == 0) return true; + if (allow_from.len == 0) return false; const parsed = std.json.parseFromSlice(std.json.Value, allocator, body, .{}) catch return false; defer parsed.deinit(); @@ -1933,6 +1934,14 @@ fn telegramSenderAllowed(allocator: std.mem.Allocator, allow_from: []const []con return false; } +fn telegramWebhookSecretMatches(raw_request: []const u8, configured_secret: ?[]const u8) bool { + const secret = configured_secret orelse return false; + if (secret.len == 0) return false; + const header = extractHeader(raw_request, "X-Telegram-Bot-Api-Secret-Token") orelse return false; + const trimmed = std.mem.trim(u8, header, " \t\r\n"); + return constantTimeEq(trimmed, secret); +} + fn telegramSessionKeyRouted( allocator: std.mem.Allocator, fallback_buf: []u8, @@ -3148,10 +3157,18 @@ fn handleTelegramWebhookRoute(ctx: *WebhookHandlerContext) void { var tg_bot_token = ctx.state.telegram_bot_token; var tg_allow_from = ctx.state.telegram_allow_from; var tg_account_id = ctx.state.telegram_account_id; + var tg_webhook_secret = ctx.state.telegram_webhook_secret; if (selectTelegramConfig(ctx.config_opt, ctx.target)) |tg_cfg| { tg_bot_token = tg_cfg.bot_token; tg_allow_from = tg_cfg.allow_from; tg_account_id = tg_cfg.account_id; + tg_webhook_secret = tg_cfg.webhook_secret; + } + + if (!telegramWebhookSecretMatches(ctx.raw_request, tg_webhook_secret)) { + ctx.response_status = "401 Unauthorized"; + ctx.response_body = "{\"error\":\"unauthorized\"}"; + return; } const msg_text = jsonStringField(b, "text"); @@ -7684,12 +7701,32 @@ test "whatsappSessionKey builds group key when group id exists" { try std.testing.expectEqualStrings("whatsapp:group:1203630@g.us:15550001111", key); } -test "telegramSenderAllowed permits when allow_from is empty" { +test "telegramSenderAllowed denies when allow_from is empty" { const allocator = std.testing.allocator; const body = \\{"message":{"from":{"id":12345,"username":"alice"}}} ; - try std.testing.expect(telegramSenderAllowed(allocator, &.{}, body)); + try std.testing.expect(!telegramSenderAllowed(allocator, &.{}, body)); +} + +test "telegramSenderAllowed wildcard explicitly permits all senders" { + const allocator = std.testing.allocator; + const body = + \\{"message":{"from":{"id":12345,"username":"alice"}}} + ; + const allow_from = [_][]const u8{"*"}; + try std.testing.expect(telegramSenderAllowed(allocator, &allow_from, body)); +} + +test "telegramWebhookSecretMatches requires configured secret header" { + const raw = + "POST /telegram HTTP/1.1\r\n" ++ + "Host: example.com\r\n" ++ + "X-Telegram-Bot-Api-Secret-Token: test-secret\r\n" ++ + "\r\n{}"; + try std.testing.expect(telegramWebhookSecretMatches(raw, "test-secret")); + try std.testing.expect(!telegramWebhookSecretMatches(raw, "wrong-secret")); + try std.testing.expect(!telegramWebhookSecretMatches(raw, null)); } test "telegramChatId extracts nested message.chat.id" { From cf7437b90cdc9944f94e18f866907557aa7ef34d Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:02:46 +0100 Subject: [PATCH 03/27] security: enforce shell policy for cron jobs Validate cron shell commands at create, update, CLI run, and scheduled execution time. Run shell jobs through the shared process runner with scrubbed environment, timeout, output limits, and improved descendant cleanup on Linux. Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build test --summary all' Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build -Doptimize=ReleaseSmall' --- src/cron.zig | 175 ++++++++++++++++++++++++++++++++----- src/tools/process_util.zig | 84 +++++++++++++++++- 2 files changed, 232 insertions(+), 27 deletions(-) diff --git a/src/cron.zig b/src/cron.zig index 7229c63b3..74475d0e8 100644 --- a/src/cron.zig +++ b/src/cron.zig @@ -12,8 +12,13 @@ const agent_routing = @import("agent_routing.zig"); const telegram = @import("channels/telegram.zig"); const signal = @import("channels/signal.zig"); const Config = @import("config.zig").Config; +const process_util = @import("tools/process_util.zig"); +const security_policy = @import("security/policy.zig"); const log = std.log.scoped(.cron); +const DEFAULT_CRON_SHELL_TIMEOUT_NS: u64 = 60 * std.time.ns_per_s; +const DEFAULT_CRON_SHELL_MAX_OUTPUT_BYTES: usize = 1_048_576; +const safe_env_vars = [_][]const u8{ "PATH", "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR" }; pub const JobType = enum { shell, @@ -475,6 +480,9 @@ pub const CronScheduler = struct { allocator: std.mem.Allocator, shell_cwd: ?[]const u8 = null, agent_timeout_secs: u64 = 0, + shell_timeout_ns: u64 = DEFAULT_CRON_SHELL_TIMEOUT_NS, + shell_max_output_bytes: usize = DEFAULT_CRON_SHELL_MAX_OUTPUT_BYTES, + shell_policy: security_policy.SecurityPolicy = .{}, observer: ?observability.Observer = null, pub fn init(allocator: std.mem.Allocator, max_tasks: usize, enabled: bool) CronScheduler { @@ -485,6 +493,9 @@ pub const CronScheduler = struct { .allocator = allocator, .shell_cwd = null, .agent_timeout_secs = 0, + .shell_timeout_ns = DEFAULT_CRON_SHELL_TIMEOUT_NS, + .shell_max_output_bytes = DEFAULT_CRON_SHELL_MAX_OUTPUT_BYTES, + .shell_policy = .{}, }; } @@ -496,6 +507,19 @@ pub const CronScheduler = struct { self.agent_timeout_secs = timeout_secs; } + pub fn setShellPolicy(self: *CronScheduler, policy: security_policy.SecurityPolicy) void { + self.shell_policy = policy; + } + + pub fn setShellLimits(self: *CronScheduler, timeout_ns: u64, max_output_bytes: usize) void { + self.shell_timeout_ns = timeout_ns; + self.shell_max_output_bytes = max_output_bytes; + } + + fn validateShellCommand(self: *const CronScheduler, command: []const u8) !void { + _ = try self.shell_policy.validateCommandExecution(command, false); + } + fn freeJobOwned(self: *CronScheduler, job: CronJob) void { self.allocator.free(job.id); self.allocator.free(job.expression); @@ -554,6 +578,7 @@ pub const CronScheduler = struct { /// Add a recurring cron job. pub fn addJob(self: *CronScheduler, expression: []const u8, command: []const u8) !*CronJob { if (self.jobs.items.len >= self.max_tasks) return error.MaxTasksReached; + try self.validateShellCommand(command); // Validate expression _ = try normalizeExpression(expression); @@ -576,6 +601,7 @@ pub const CronScheduler = struct { /// Add a one-shot delayed task. pub fn addOnce(self: *CronScheduler, delay: []const u8, command: []const u8) !*CronJob { if (self.jobs.items.len >= self.max_tasks) return error.MaxTasksReached; + try self.validateShellCommand(command); const delay_secs = try parseDuration(delay); const now = std_compat.time.timestamp(); @@ -710,6 +736,7 @@ pub const CronScheduler = struct { job.next_run_secs = next_run_secs; } if (patch.command) |cmd| { + if (job.job_type == .shell) self.validateShellCommand(cmd) catch return false; const new_cmd = allocator.dupe(u8, cmd) catch return false; allocator.free(job.command); job.command = new_cmd; @@ -889,12 +916,25 @@ pub const CronScheduler = struct { switch (job.job_type) { .shell => { - // Execute shell command via child process - const result = std_compat.process.Child.run(.{ - .allocator = self.allocator, - .argv = &.{ platform.getShell(), platform.getShellFlag(), job.command }, - .cwd = self.shell_cwd, - }) catch |err| { + self.validateShellCommand(job.command) catch |err| { + log.warn("cron shell job '{s}' blocked by security policy: {s}", .{ job.id, @errorName(err) }); + job.last_status = "error"; + job.last_run_secs = now; + if (job.last_output) |old| self.allocator.free(old); + job.last_output = self.allocator.dupe(u8, "cron shell command blocked by security policy") catch null; + if (out_bus) |b| { + _ = deliverResult(self.allocator, job.delivery, "cron shell command blocked by security policy", false, b) catch {}; + } + continue; + }; + + const result = runShellJob( + self.allocator, + self.shell_cwd, + job.command, + self.shell_timeout_ns, + self.shell_max_output_bytes, + ) catch |err| { log.err("cron job '{s}' failed to start: {}", .{ job.id, err }); job.last_status = "error"; job.last_run_secs = now; @@ -907,12 +947,9 @@ pub const CronScheduler = struct { }; defer self.allocator.free(result.stderr); - const success = switch (result.term) { - .exited => |code| code == 0, - else => false, - }; + const success = result.success; job.last_run_secs = now; - job.last_status = if (success) "ok" else "error"; + job.last_status = if (success) "ok" else if (result.timed_out) "timeout" else "error"; // Store and deliver stdout if (job.last_output) |old| self.allocator.free(old); @@ -1007,6 +1044,42 @@ pub const CronScheduler = struct { const agent_runner = @import("agent_runner.zig"); const AgentRunResult = agent_runner.AgentRunResult; +fn buildSafeShellEnv(allocator: std.mem.Allocator) !std_compat.process.EnvMap { + var env = std_compat.process.EnvMap.init(allocator); + errdefer env.deinit(); + + for (safe_env_vars) |key| { + if (platform.getEnvOrNull(allocator, key)) |val| { + defer allocator.free(val); + try env.put(key, val); + } + } + + return env; +} + +fn runShellJob( + allocator: std.mem.Allocator, + cwd: ?[]const u8, + command: []const u8, + timeout_ns: u64, + max_output_bytes: usize, +) !process_util.RunResult { + var env = try buildSafeShellEnv(allocator); + defer env.deinit(); + + return process_util.run( + allocator, + &.{ platform.getShell(), platform.getShellFlag(), command }, + .{ + .cwd = resolveRunnableCwd(cwd), + .env_map = &env, + .timeout_ns = timeout_ns, + .max_output_bytes = max_output_bytes, + }, + ); +} + fn runAgentJob( allocator: std.mem.Allocator, cwd: ?[]const u8, @@ -2314,28 +2387,37 @@ pub fn cliRunJob(allocator: std.mem.Allocator, id: []const u8) !void { const run_at = std_compat.time.timestamp(); switch (job.job_type) { .shell => { - const result = std_compat.process.Child.run(.{ - .allocator = allocator, - .argv = &.{ platform.getShell(), platform.getShellFlag(), job.command }, - .cwd = run_cwd, - }) catch |err| { + scheduler.validateShellCommand(job.command) catch |err| { + job.last_run_secs = run_at; + job.last_status = "error"; + try saveJobs(&scheduler); + log.err("Job '{s}' blocked by security policy: {s}", .{ id, @errorName(err) }); + return; + }; + + const result = runShellJob( + allocator, + run_cwd, + job.command, + scheduler.shell_timeout_ns, + scheduler.shell_max_output_bytes, + ) catch |err| { job.last_run_secs = run_at; job.last_status = "error"; try saveJobs(&scheduler); log.err("Job '{s}' failed: {s}", .{ id, @errorName(err) }); return; }; - defer allocator.free(result.stdout); - defer allocator.free(result.stderr); + defer result.deinit(allocator); if (result.stdout.len > 0) log.info("{s}", .{result.stdout}); - const exit_code: u8 = switch (result.term) { - .exited => |code| code, - else => 1, - }; job.last_run_secs = run_at; - job.last_status = if (exit_code == 0) "ok" else "error"; + job.last_status = if (result.success) "ok" else if (result.timed_out) "timeout" else "error"; try saveJobs(&scheduler); - log.info("Job '{s}' completed (exit {d}).", .{ id, exit_code }); + if (result.success) { + log.info("Job '{s}' completed.", .{id}); + } else { + log.err("Job '{s}' failed.", .{id}); + } }, .agent => { const prompt = job.prompt orelse job.command; @@ -2922,6 +3004,7 @@ test "save and load roundtrip with JSON-sensitive command characters" { var scheduler = CronScheduler.init(std.testing.allocator, 10, true); defer scheduler.deinit(); + scheduler.setShellPolicy(.{ .autonomy = .yolo, .allowed_commands = &.{"*"} }); const cmd = "printf \"line1\\nline2\" && echo \\\"ok\\\""; _ = try scheduler.addJob("*/5 * * * *", cmd); @@ -3049,6 +3132,49 @@ test "updateJob modifies job fields" { try std.testing.expectEqual(SessionTarget.main, updated.session_target); } +test "cron shell add rejects command blocked by security policy" { + const allocator = std.testing.allocator; + var scheduler = CronScheduler.init(allocator, 10, true); + defer scheduler.deinit(); + + const allowed = [_][]const u8{"cat"}; + scheduler.setShellPolicy(.{ .allowed_commands = &allowed }); + + try std.testing.expectError(error.CommandNotAllowed, scheduler.addJob("* * * * *", "echo blocked")); +} + +test "cron shell update rejects command blocked by security policy" { + const allocator = std.testing.allocator; + var scheduler = CronScheduler.init(allocator, 10, true); + defer scheduler.deinit(); + + _ = try scheduler.addJob("* * * * *", "echo original"); + const id = scheduler.listJobs()[0].id; + + const allowed = [_][]const u8{"cat"}; + scheduler.setShellPolicy(.{ .allowed_commands = &allowed }); + + try std.testing.expect(!scheduler.updateJob(allocator, id, .{ .command = "echo blocked" })); + try std.testing.expectEqualStrings("echo original", scheduler.getJob(id).?.command); +} + +test "cron shell tick blocks previously stored command when policy changes" { + const allocator = std.testing.allocator; + var scheduler = CronScheduler.init(allocator, 10, true); + defer scheduler.deinit(); + + _ = try scheduler.addJob("* * * * *", "echo stored"); + scheduler.jobs.items[0].next_run_secs = 0; + + const allowed = [_][]const u8{"cat"}; + scheduler.setShellPolicy(.{ .allowed_commands = &allowed }); + + _ = scheduler.tick(std_compat.time.timestamp(), null); + + try std.testing.expectEqualStrings("error", scheduler.jobs.items[0].last_status.?); + try std.testing.expectEqualStrings("cron shell command blocked by security policy", scheduler.jobs.items[0].last_output.?); +} + test "updateJob keeps agent command and prompt in sync" { const allocator = std.testing.allocator; var scheduler = CronScheduler.init(allocator, 10, true); @@ -3405,6 +3531,7 @@ test "shell job uses configured cwd for relative output paths" { var scheduler = CronScheduler.init(std.testing.allocator, 10, true); scheduler.setShellCwd(workspace); + scheduler.setShellPolicy(.{ .autonomy = .yolo, .allowed_commands = &.{"*"} }); defer scheduler.deinit(); _ = try scheduler.addOnce("1s", "echo cwd_ok > cwd_proof.txt"); diff --git a/src/tools/process_util.zig b/src/tools/process_util.zig index 9a887a941..6b175a304 100644 --- a/src/tools/process_util.zig +++ b/src/tools/process_util.zig @@ -99,6 +99,64 @@ fn terminateWindowsProcessTreeByPid(pid: std.os.windows.DWORD) void { } else |_| {} } +fn linuxProcessParentPid(allocator: std.mem.Allocator, pid: std.posix.pid_t) ?std.posix.pid_t { + if (builtin.os.tag != .linux) return null; + const path = std.fmt.allocPrint(allocator, "/proc/{d}/stat", .{pid}) catch return null; + defer allocator.free(path); + + const file = std_compat.fs.openFileAbsolute(path, .{}) catch return null; + defer file.close(); + const content = file.readToEndAlloc(allocator, 4096) catch return null; + defer allocator.free(content); + + const close_idx = std.mem.lastIndexOfScalar(u8, content, ')') orelse return null; + if (close_idx + 2 >= content.len) return null; + const fields = std_compat.mem.trimLeft(u8, content[close_idx + 1 ..], ") "); + const state_end = std.mem.indexOfScalar(u8, fields, ' ') orelse return null; + const after_state = std_compat.mem.trimLeft(u8, fields[state_end..], " "); + const ppid_end = std.mem.indexOfScalar(u8, after_state, ' ') orelse after_state.len; + return std.fmt.parseInt(std.posix.pid_t, after_state[0..ppid_end], 10) catch null; +} + +fn terminateLinuxChildProcessesFromChildrenFile(parent_pid: std.posix.pid_t, signal: std.posix.SIG) void { + if (builtin.os.tag != .linux) return; + + const allocator = std.heap.page_allocator; + const path = std.fmt.allocPrint(allocator, "/proc/{d}/task/{d}/children", .{ parent_pid, parent_pid }) catch return; + defer allocator.free(path); + + const file = std_compat.fs.openFileAbsolute(path, .{}) catch return; + defer file.close(); + const content = file.readToEndAlloc(allocator, 8192) catch return; + defer allocator.free(content); + + var it = std.mem.tokenizeAny(u8, content, " \t\r\n"); + while (it.next()) |pid_bytes| { + const pid = std.fmt.parseInt(std.posix.pid_t, pid_bytes, 10) catch continue; + terminateLinuxChildProcessesFromChildrenFile(pid, signal); + std.posix.kill(pid, signal) catch {}; + } +} + +fn terminateLinuxChildProcesses(parent_pid: std.posix.pid_t, signal: std.posix.SIG) void { + if (builtin.os.tag != .linux) return; + terminateLinuxChildProcessesFromChildrenFile(parent_pid, signal); + + var proc_dir = std_compat.fs.openDirAbsolute("/proc", .{ .iterate = true }) catch return; + defer proc_dir.close(); + + var iter = proc_dir.iterate(); + while (iter.next() catch null) |entry| { + const pid = std.fmt.parseInt(std.posix.pid_t, entry.name, 10) catch continue; + if (linuxProcessParentPid(std.heap.page_allocator, pid)) |ppid| { + if (ppid == parent_pid) { + terminateLinuxChildProcesses(pid, signal); + std.posix.kill(pid, signal) catch {}; + } + } + } +} + fn terminateChild(child: *std_compat.process.Child) void { if (comptime builtin.os.tag == .windows) { terminateWindowsProcessTreeByPid(GetProcessId(child.id)); @@ -109,12 +167,17 @@ fn terminateChild(child: *std_compat.process.Child) void { const process_group_id: std.posix.pid_t = -child.id; std.posix.kill(process_group_id, std.posix.SIG.TERM) catch { std.posix.kill(child.id, std.posix.SIG.TERM) catch {}; - return; }; + if (comptime builtin.os.tag == .linux) { + terminateLinuxChildProcesses(child.id, std.posix.SIG.TERM); + } std_compat.thread.sleep(100 * std.time.ns_per_ms); + if (comptime builtin.os.tag == .linux) { + terminateLinuxChildProcesses(child.id, std.posix.SIG.KILL); + } std.posix.kill(process_group_id, std.posix.SIG.KILL) catch |err| switch (err) { - error.ProcessNotFound => {}, + error.ProcessNotFound => std.posix.kill(child.id, std.posix.SIG.KILL) catch {}, else => {}, }; } @@ -158,6 +221,21 @@ fn processExists(pid: std.posix.pid_t) bool { return true; } +fn processIsZombie(allocator: std.mem.Allocator, pid: std.posix.pid_t) bool { + if (builtin.os.tag != .linux) return false; + const path = std.fmt.allocPrint(allocator, "/proc/{d}/stat", .{pid}) catch return false; + defer allocator.free(path); + + const file = std_compat.fs.openFileAbsolute(path, .{}) catch return false; + defer file.close(); + const content = file.readToEndAlloc(allocator, 4096) catch return false; + defer allocator.free(content); + + const close_idx = std.mem.lastIndexOfScalar(u8, content, ')') orelse return false; + if (close_idx + 2 >= content.len) return false; + return content[close_idx + 2] == 'Z'; +} + fn processExistsWindows(pid: std.os.windows.DWORD) bool { if (pid == 0) return false; @@ -598,7 +676,7 @@ test "run timeout kills spawned shell descendants" { var exited = false; var i: usize = 0; while (i < 20) : (i += 1) { - if (!processExists(child_pid)) { + if (!processExists(child_pid) or processIsZombie(allocator, child_pid)) { exited = true; break; } From 081c0c27c3cf70865cfd94fc8e293e6ce8a0341b Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:02:59 +0100 Subject: [PATCH 04/27] chore: add local Docker build workflow Add Makefile shortcuts and docker compose build configuration for local gateway/agent images. Vendor websocket and wasm3 during Docker builds and verify the pinned Zig 0.16.0 toolchain. Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build test --summary all' Validation: docker run --rm -v /home/ubuntu/claws/nullclaw:/app -w /app nullclaw:test-builder sh -lc 'ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache zig build -Doptimize=ReleaseSmall' --- .gitignore | 1 + Dockerfile | 26 +++++++++++++++++++++++--- Makefile | 37 +++++++++++++++++++++++++++++++++++++ docker-compose.yml | 14 ++++++++++++-- 4 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 Makefile diff --git a/.gitignore b/.gitignore index 2cf5669b9..b041fe09b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ zig-out/ zig-cache/ .zig-cache/ +.zig-global-cache/ zig-pkg/ *.db *.db-journal diff --git a/Dockerfile b/Dockerfile index 4d3f1f525..bb2ae5cb1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,7 +6,7 @@ FROM --platform=$BUILDPLATFORM alpine:3.23 AS builder ARG ZIG_VERSION=0.16.0 -RUN apk add --no-cache bash curl musl-dev python3 +RUN apk add --no-cache bash curl git musl-dev python3 WORKDIR /app COPY .github/scripts/install-zig.sh .github/scripts/install-zig.sh @@ -14,11 +14,31 @@ COPY build.zig build.zig.zon ./ COPY src/ src/ COPY vendor/sqlite3/ vendor/sqlite3/ +RUN set -eu; \ + fetch_vendor_dep() { \ + repo="$1"; \ + commit="$2"; \ + target="vendor/${repo}"; \ + tmp_dir="$(mktemp -d)"; \ + archive_path="${tmp_dir}/${repo}.tar.gz"; \ + curl -fsSL "https://github.com/nullclaw/${repo}/archive/${commit}.tar.gz" -o "${archive_path}"; \ + tar -xzf "${archive_path}" -C "${tmp_dir}"; \ + extracted_dir="$(find "${tmp_dir}" -mindepth 1 -maxdepth 1 -type d | head -n 1)"; \ + rm -rf "${target}"; \ + mkdir -p "${target}"; \ + cp -a "${extracted_dir}/." "${target}/"; \ + rm -rf "${tmp_dir}"; \ + }; \ + fetch_vendor_dep websocket 9151f70b27024a072ea04425b9e7609eb6b1386c; \ + fetch_vendor_dep wasm3 8a29b0080f1f65dbbe8b8f8c4d8148480feff377; \ + sed -i '/\.websocket = .{/,/ },/c\ .websocket = .{\n .path = "vendor/websocket",\n },' build.zig.zon; \ + sed -i '/\.wasm3 = .{/,/ },/c\ .wasm3 = .{\n .path = "vendor/wasm3",\n },' build.zig.zon + RUN set -eu; \ mkdir -p /tmp/zig-path; \ GITHUB_PATH=/tmp/zig-path/path RUNNER_TEMP=/opt bash .github/scripts/install-zig.sh "${ZIG_VERSION}"; \ ln -sf "$(cat /tmp/zig-path/path)/zig" /usr/local/bin/zig; \ - zig version + test "$(zig version)" = "0.16.0" ARG TARGETARCH ARG VERSION=dev @@ -93,7 +113,7 @@ ENTRYPOINT ["nullclaw"] CMD ["gateway", "--port", "3000", "--host", "::"] # Optional autonomous mode (explicit opt-in): -# docker build --target release-root -t nullclaw:root . +# make build DOCKER_TARGET=release-root IMAGE=nullclaw:root FROM release-base AS release-root USER 0:0 diff --git a/Makefile b/Makefile new file mode 100644 index 000000000..999deb8b6 --- /dev/null +++ b/Makefile @@ -0,0 +1,37 @@ +COMPOSE ?= docker compose +BUILDX ?= docker buildx +IMAGE ?= nullclaw:local +DOCKER_TARGET ?= release +VERSION ?= dev +PROFILE ?= gateway +SERVICE ?= gateway +RUN_ARGS ?= +COMPOSE_BAKE ?= true + +export COMPOSE_BAKE +export NULLCLAW_IMAGE := $(IMAGE) +export NULLCLAW_DOCKER_TARGET := $(DOCKER_TARGET) +export NULLCLAW_VERSION := $(VERSION) + +.PHONY: build up down run shell logs check-buildx + +check-buildx: + $(BUILDX) version >/dev/null + +build: check-buildx + $(COMPOSE) --profile $(PROFILE) build $(SERVICE) + +up: check-buildx + $(COMPOSE) --profile $(PROFILE) up -d --build $(SERVICE) + +down: + $(COMPOSE) down + +run: + $(COMPOSE) --profile agent run --rm agent $(RUN_ARGS) + +shell: + $(COMPOSE) --profile agent run --rm --entrypoint /bin/sh agent + +logs: + $(COMPOSE) --profile $(PROFILE) logs -f $(SERVICE) diff --git a/docker-compose.yml b/docker-compose.yml index 60c136541..d556363dd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,15 @@ +x-nullclaw-build: &nullclaw-build + context: . + dockerfile: Dockerfile + target: ${NULLCLAW_DOCKER_TARGET:-release} + args: + ZIG_VERSION: "0.16.0" + VERSION: ${NULLCLAW_VERSION:-dev} + services: agent: - image: ${NULLCLAW_IMAGE:-ghcr.io/nullclaw/nullclaw:latest} + image: ${NULLCLAW_IMAGE:-nullclaw:local} + build: *nullclaw-build profiles: ["agent"] stdin_open: true tty: true @@ -13,7 +22,8 @@ services: restart: unless-stopped gateway: - image: ${NULLCLAW_IMAGE:-ghcr.io/nullclaw/nullclaw:latest} + image: ${NULLCLAW_IMAGE:-nullclaw:local} + build: *nullclaw-build profiles: ["gateway"] command: ["gateway", "--port", "3000", "--host", "::"] ports: From 1acb34bd53eb72d3536cc74ecb64a3cd7ffcbcc9 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:03:12 +0100 Subject: [PATCH 05/27] docs: document security patch scope Document deny-empty allowlist behavior, Telegram webhook secret requirements, and gateway API authentication changes in English and Chinese docs. Add the 2026-05-10 security patch plan and validation checklist. Validation: docs-only commit; code validation already passed in prior grouped commits. --- SECURITY-PATCH-PLAN-2026-05-10.md | 150 ++++++++++++++++++++++++++++++ docs/en/configuration.md | 9 +- docs/en/gateway-api.md | 1 + docs/en/security.md | 5 +- docs/zh/configuration.md | 5 +- docs/zh/gateway-api.md | 1 + docs/zh/security.md | 5 +- 7 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 SECURITY-PATCH-PLAN-2026-05-10.md diff --git a/SECURITY-PATCH-PLAN-2026-05-10.md b/SECURITY-PATCH-PLAN-2026-05-10.md new file mode 100644 index 000000000..39b71b785 --- /dev/null +++ b/SECURITY-PATCH-PLAN-2026-05-10.md @@ -0,0 +1,150 @@ +# Security Patch Plan - 2026-05-10 + +## Scope + +This plan covers four security findings: + +1. Telegram webhook spoofing and missing webhook authentication. +2. Secret exposure through `curl` process argv. +3. Cron shell jobs bypassing shell tool security controls. +4. Inbound channels allowing all senders when `allow_from` is empty. + +Affected areas include: + +- `src/gateway.zig` +- `src/http_util.zig` +- `src/providers/**` +- `src/channels/telegram_api.zig` +- `src/channels/discord.zig` +- `src/channels/line.zig` +- `src/cron.zig` +- config examples, documentation, and tests + +Keep the patch limited to these security fixes. Do not combine it with unrelated refactors, feature work, or Zig toolchain changes. + +## 1. Telegram Webhook Authentication + +Risk: forged Telegram webhook updates can drive the agent and tools when the gateway is public or tunneled. + +Plan: + +- Add config support for a Telegram webhook secret compatible with Telegram's `X-Telegram-Bot-Api-Secret-Token` header. +- Require the secret-token header on `/telegram` webhook POSTs before parsing or dispatching the request body. +- Reject missing or mismatched webhook secrets with an explicit unauthorized response. +- Change `telegramSenderAllowed` so an empty `allow_from` denies by default. +- Require explicit `"*"` as the only allow-all sender configuration. +- Keep webhook failures non-sensitive in logs and responses. + +Tests: + +- Missing Telegram secret-token header is rejected. +- Incorrect Telegram secret-token header is rejected. +- Correct Telegram secret-token header is accepted. +- Empty Telegram `allow_from` denies. +- Explicit `"*"` allows all senders. +- Forged sender or chat IDs do not bypass webhook secret validation. + +## 2. Remove Secrets From Child Process Arguments + +Risk: local users, process monitors, or container hosts can read API keys, bearer tokens, bot tokens, proxy credentials, and webhook URLs from process argv. + +Plan: + +- Audit shared HTTP helpers and all credentialed callers. +- Replace credentialed `curl` execution paths with `std.http.Client`. +- Ensure provider requests do not place `Authorization`, API keys, or bearer tokens in child argv. +- Stop passing Telegram bot tokens in URLs to child processes. +- If any non-secret curl helper remains, make it reject credential-bearing headers and sensitive URLs before spawning. +- Keep outbound URL validation secure-by-default, including HTTPS-only behavior where currently required. + +Tests: + +- Credential headers are rejected by any remaining child-process HTTP helper. +- OpenAI and Gemini request paths do not use child argv for bearer tokens. +- Telegram API calls do not pass bot tokens through child argv. +- Sensitive values are not included in command construction errors or logs. + +If argv exposure cannot be directly unit tested for a specific path, add a short comment near the code explaining the coverage limitation and the integration coverage expected. + +## 3. Cron Shell Job Security Enforcement + +Risk: anyone who can create or update cron jobs can gain persistent shell execution outside the normal shell tool policy, sandbox, environment scrub, timeout, and output limits. + +Plan: + +- Route cron shell execution through the same `ShellTool` and `SecurityPolicy` path used by normal shell tool calls. +- Validate shell cron commands when jobs are created or updated. +- Revalidate commands at execution time so previously stored unsafe jobs cannot bypass policy after upgrade. +- Preserve existing agent-job behavior unless it depends on unsafe shell execution. +- If full `ShellTool` integration is too invasive for the first patch, restrict cron REST endpoints to agent jobs only until a separately audited admin shell mode exists. +- Apply the same timeout, output limits, sandbox behavior, and environment scrub used by the shell tool. + +Tests: + +- Disallowed cron shell command is rejected at creation. +- Disallowed cron shell command is rejected at update. +- Previously stored disallowed cron shell command cannot execute. +- Timeout and output limits apply to cron shell execution. +- Environment secrets are not inherited by cron shell jobs. +- Agent cron jobs continue to execute as expected. + +## 4. Deny Empty Inbound Allowlists + +Risk: empty `allow_from` values create open-bot behavior that conflicts with the repository's deny-by-default security posture. + +Plan: + +- Normalize inbound channel semantics: + - Empty `allow_from` denies all senders. + - Explicit `"*"` allows all senders. + - Exact configured sender IDs allow only matching senders. +- Apply this to Telegram gateway handling, Discord, and LINE. +- Update config examples and docs to show explicit sender allowlists. +- Document the behavior change as intentional and security-sensitive. + +Tests: + +- Discord empty `allow_from` denies. +- Discord explicit `"*"` allows all. +- Discord matching sender allows and non-matching sender denies. +- LINE empty `allow_from` denies. +- LINE explicit `"*"` allows all. +- LINE matching sender allows and non-matching sender denies. +- Telegram gateway empty `allow_from` denies. +- Telegram gateway explicit `"*"` allows all. +- Telegram gateway matching sender allows and non-matching sender denies. + +## Config And Documentation + +Plan: + +- Add a Telegram webhook secret field to the relevant config schema and examples. +- Use neutral placeholders such as `"test-secret"` and `"user_a"` in tests and examples. +- Avoid generating secrets silently during normal config loading. +- Update docs and examples that imply an empty allowlist is safe or permissive. +- Clearly state that allow-all requires explicit `"*"`. + +## Validation + +Required validation after code changes: + +```bash +zig build test --summary all +zig build -Doptimize=ReleaseSmall +``` + +Security-sensitive changes should also include targeted tests for the modified modules before the full suite is run. + +Environment note from 2026-05-10: the current system `zig` was observed as `0.14.1`, while this repository is pinned to `0.16.0`. Full validation must be performed with Zig `0.16.0`. + +## Handoff Checklist + +Before handing off or opening a PR, include: + +1. What changed. +2. What did not change. +3. Threat notes for each fixed class. +4. Validation commands and results. +5. Remaining risks or unknowns. +6. Next recommended action. + diff --git a/docs/en/configuration.md b/docs/en/configuration.md index 0b364a139..7b72336ef 100644 --- a/docs/en/configuration.md +++ b/docs/en/configuration.md @@ -464,6 +464,7 @@ Telegram example: "accounts": { "main": { "bot_token": "123456:ABCDEF", + "webhook_secret": "replace-with-random-telegram-webhook-secret", "allow_from": ["YOUR_TELEGRAM_USER_ID"] } } @@ -572,6 +573,7 @@ Minimal end-to-end example: "accounts": { "main": { "bot_token": "123456:ABCDEF", + "webhook_secret": "replace-with-random-telegram-webhook-secret", "allow_from": ["YOUR_TELEGRAM_USER_ID"], "draft_previews": false, "binding_commands_enabled": true, @@ -688,8 +690,9 @@ Effect on delivery: Rules: -- Empty `allow_from` behavior is channel-specific. Some channels, including WeChat and Discord, treat an omitted or empty list as "no filtering" rather than "deny all", so set explicit IDs/OpenIDs for a private bot. +- Empty `allow_from` denies inbound messages on allowlist-based channels. Set explicit IDs/OpenIDs for a private bot. - `allow_from: ["*"]` allows all sources on allowlist-based channels; use it only when you intentionally want an open bot. +- Telegram webhooks require `channels.telegram.accounts..webhook_secret` and Telegram's `X-Telegram-Bot-Api-Secret-Token` header to match. - Teams inbound webhooks are authenticated with Bot Framework JWT bearer tokens against Microsoft's OpenID metadata. `channels.teams[].webhook_secret` is optional and, when set, acts as an additional `X-Webhook-Secret` check. Max example: @@ -750,7 +753,7 @@ Discord example: } ``` -Set `allow_from` explicitly unless you intentionally want an open bot. In the current Discord runtime, an omitted or empty `allow_from` list disables filtering instead of denying all inbound messages. +Set `allow_from` explicitly. An omitted or empty `allow_from` list denies inbound messages; use `["*"]` only when you intentionally want an open bot. Enable MESSAGE CONTENT INTENT in the Discord Developer Portal if you want the bot to process ordinary guild messages. Without it, Discord omits message content for most guild traffic; direct messages and messages that mention the bot still include content. @@ -827,7 +830,7 @@ Parameters: - `token` (required) - Bot token from Discord Developer Portal - `intents` (default: 37377) - Gateway intents bitmask - `allow_bots` (default: false) - Allow messages from other bots -- `allow_from` (default: []) - Optional allowlist of user IDs; for Discord, an omitted or empty list disables filtering, so set explicit IDs for a private bot. `["*"]` also matches all users +- `allow_from` (default: []) - User ID allowlist. An omitted or empty list denies inbound messages. `["*"]` explicitly allows all users - `require_mention` (default: false) - Require bot mention in guilds to respond - `guild_id` (optional) - Reserved for Discord server scoping; current runtime does not enforce it diff --git a/docs/en/gateway-api.md b/docs/en/gateway-api.md index 9edd23f7f..7e06c58f3 100644 --- a/docs/en/gateway-api.md +++ b/docs/en/gateway-api.md @@ -35,6 +35,7 @@ Default gateway endpoint: `http://127.0.0.1:3000` | `/cron/pause` | POST | `Authorization: Bearer ` on public binds or when pairing tokens exist | Pause a live cron job by `id` | | `/cron/resume` | POST | `Authorization: Bearer ` on public binds or when pairing tokens exist | Resume a live cron job by `id` | | `/cron/update` | POST | `Authorization: Bearer ` on public binds or when pairing tokens exist | Partially update a live cron job | +| `/telegram` | POST | `X-Telegram-Bot-Api-Secret-Token` matching `channels.telegram.accounts..webhook_secret` | Telegram inbound webhook | | `/whatsapp` | GET | Query params | Meta webhook verification | | `/whatsapp` | POST | Meta signature | WhatsApp inbound webhook | | `/max` | POST | `X-Max-Bot-Api-Secret` when configured | Max inbound webhook delivery | diff --git a/docs/en/security.md b/docs/en/security.md index e76f1ee41..0b21091bc 100644 --- a/docs/en/security.md +++ b/docs/en/security.md @@ -37,8 +37,8 @@ NullClaw follows secure-by-default behavior: local bind by default, pairing auth ## Channel Allowlists -- `allow_from` behavior is channel-specific; do not assume `[]` is a deny-by-default switch across every runtime. -- Some channels, including WeChat and Discord, treat an omitted or empty `allow_from` as "no filtering", so set explicit user IDs/OpenIDs when you want a private bot. +- Empty `allow_from` denies inbound messages on allowlist-based channels. +- Set explicit user IDs/OpenIDs when you want a private bot. - `allow_from: ["*"]`: allow all sources (high-risk). - Otherwise: expect exact-match allowlists or channel-specific fallback/group-policy behavior. @@ -48,6 +48,7 @@ NullClaw follows secure-by-default behavior: local bind by default, pairing auth - Repeated invalid pairing attempts can trigger rate limiting and a temporary lockout. - `/.well-known/agent.json` and `/.well-known/agent-card.json` are public discovery documents when A2A is enabled. - Keeping `gateway.require_pairing = true` keeps `/webhook` and `/a2a` behind bearer auth; disabling pairing removes that bearer check. +- Telegram webhook delivery requires `X-Telegram-Bot-Api-Secret-Token` to match `channels.telegram.accounts..webhook_secret`. - Channel-specific inbound webhooks keep their own auth or signature rules and should not be documented as if they all use gateway bearer auth. ## Nostr-specific Rules diff --git a/docs/zh/configuration.md b/docs/zh/configuration.md index 3118b9ee0..eb53e3610 100644 --- a/docs/zh/configuration.md +++ b/docs/zh/configuration.md @@ -404,6 +404,7 @@ Telegram 示例: "accounts": { "main": { "bot_token": "123456:ABCDEF", + "webhook_secret": "replace-with-random-telegram-webhook-secret", "allow_from": ["YOUR_TELEGRAM_USER_ID"] } } @@ -443,8 +444,9 @@ WeChat 说明: 规则说明: -- 空 `allow_from` 的行为因渠道而异。有些渠道(例如 WeChat 和 Discord)会把省略或留空视为“关闭过滤”,而不是“拒绝所有”;如果要做私有机器人,请显式填写 ID/OpenID。 +- 对基于 allowlist 的渠道,空 `allow_from` 会拒绝入站消息;如果要做私有机器人,请显式填写 ID/OpenID。 - `allow_from: ["*"]` 会在基于 allowlist 的渠道上允许所有来源,仅在你明确接受风险时使用。 +- Telegram webhook 必须配置 `channels.telegram.accounts..webhook_secret`,并要求 Telegram 的 `X-Telegram-Bot-Api-Secret-Token` header 匹配。 - Teams 入站 webhook 现在会使用 Bot Framework JWT bearer token 并对照 Microsoft OpenID metadata 做认证。`channels.teams[].webhook_secret` 变为可选项;如果配置,会额外要求 `X-Webhook-Secret` 匹配。 Telegram forum topics: @@ -515,6 +517,7 @@ Telegram forum topics: "accounts": { "main": { "bot_token": "123456:ABCDEF", + "webhook_secret": "replace-with-random-telegram-webhook-secret", "allow_from": ["YOUR_TELEGRAM_USER_ID"], "draft_previews": false, "binding_commands_enabled": true, diff --git a/docs/zh/gateway-api.md b/docs/zh/gateway-api.md index 5a9a9cb27..16cf2fcde 100644 --- a/docs/zh/gateway-api.md +++ b/docs/zh/gateway-api.md @@ -21,6 +21,7 @@ | `/cron/pause` | POST | 公开绑定时或已存在配对 token 时需要 `Authorization: Bearer ` | 按 `id` 暂停实时 cron 任务 | | `/cron/resume` | POST | 公开绑定时或已存在配对 token 时需要 `Authorization: Bearer ` | 按 `id` 恢复实时 cron 任务 | | `/cron/update` | POST | 公开绑定时或已存在配对 token 时需要 `Authorization: Bearer ` | 部分更新实时 cron 任务 | +| `/telegram` | POST | `X-Telegram-Bot-Api-Secret-Token` 必须匹配 `channels.telegram.accounts..webhook_secret` | Telegram 入站 webhook | | `/whatsapp` | GET | Query 参数 | Meta Webhook 验证 | | `/whatsapp` | POST | Meta 签名 | WhatsApp 入站消息 | | `/max` | POST | `X-Max-Bot-Api-Secret`(配置后必填) | Max 入站 webhook | diff --git a/docs/zh/security.md b/docs/zh/security.md index 195493927..523f87b61 100644 --- a/docs/zh/security.md +++ b/docs/zh/security.md @@ -23,8 +23,8 @@ NullClaw 默认走 secure-by-default:本地绑定、配对鉴权、沙箱隔 ## Channel allowlist 规则 -- `allow_from` 的行为因渠道而异;不要把 `[]` 当成所有 runtime 都适用的默认拒绝开关。 -- 有些渠道(例如 WeChat 和 Discord)会把省略或留空的 `allow_from` 视为“关闭过滤”,想做私有 bot 时要显式填写允许的用户 ID / OpenID。 +- 对基于 allowlist 的渠道,空 `allow_from` 会拒绝入站消息。 +- 想做私有 bot 时要显式填写允许的用户 ID / OpenID。 - `allow_from: ["*"]`:允许所有来源(高风险,仅显式确认后使用)。 - 其他情况通常是精确匹配 allowlist,或该渠道自己的 fallback / group-policy 语义。 @@ -34,6 +34,7 @@ NullClaw 默认走 secure-by-default:本地绑定、配对鉴权、沙箱隔 - 多次错误 pairing 尝试会触发限流,并可能进入临时锁定。 - `/.well-known/agent.json` 与 `/.well-known/agent-card.json` 在启用 A2A 时属于公开发现文档。 - 保持 `gateway.require_pairing = true` 时,`/webhook` 与 `/a2a` 仍在 bearer 鉴权之后;若关闭 pairing,这两个端点就不再要求 bearer token。 +- Telegram webhook 要求 `X-Telegram-Bot-Api-Secret-Token` 与 `channels.telegram.accounts..webhook_secret` 匹配。 - 各 channel 专用入站 webhook 继续使用各自的鉴权或签名规则,不应一概写成 gateway bearer 鉴权。 ## Nostr 特殊规则 From fc4c0d1752e4934f00465ffe6a7dc42f019001c6 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:25:19 +0100 Subject: [PATCH 06/27] chore: default local gateway port to 3210 Add NULLCLAW_PORT to the Makefile and use it for the compose gateway command, published port, and healthcheck. The local workflow now defaults to 127.0.0.1:3210. Validation: docker compose --profile gateway config Validation: git diff --check --- Makefile | 2 ++ docker-compose.yml | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 999deb8b6..3f420597e 100644 --- a/Makefile +++ b/Makefile @@ -7,11 +7,13 @@ PROFILE ?= gateway SERVICE ?= gateway RUN_ARGS ?= COMPOSE_BAKE ?= true +NULLCLAW_PORT ?= 3210 export COMPOSE_BAKE export NULLCLAW_IMAGE := $(IMAGE) export NULLCLAW_DOCKER_TARGET := $(DOCKER_TARGET) export NULLCLAW_VERSION := $(VERSION) +export NULLCLAW_PORT .PHONY: build up down run shell logs check-buildx diff --git a/docker-compose.yml b/docker-compose.yml index d556363dd..e24d201bb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -25,9 +25,9 @@ services: image: ${NULLCLAW_IMAGE:-nullclaw:local} build: *nullclaw-build profiles: ["gateway"] - command: ["gateway", "--port", "3000", "--host", "::"] + command: ["gateway", "--port", "${NULLCLAW_PORT:-3210}", "--host", "::"] ports: - - "127.0.0.1:${NULLCLAW_GATEWAY_PORT:-3000}:3000" + - "127.0.0.1:${NULLCLAW_PORT:-3210}:${NULLCLAW_PORT:-3210}" env_file: - path: .env required: false @@ -35,7 +35,7 @@ services: - nullclaw-data:/nullclaw-data restart: unless-stopped healthcheck: - test: ["CMD", "wget", "-qO-", "http://localhost:3000/health"] + test: ["CMD", "wget", "-qO-", "http://localhost:${NULLCLAW_PORT:-3210}/health"] interval: 30s timeout: 5s retries: 3 From a3583f198ce8cc842d78a1f1324dea4e6cf3172d Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:26:41 +0100 Subject: [PATCH 07/27] chore: publish local gateway on host interfaces Bind the compose gateway port on 0.0.0.0 so the NULLCLAW_PORT workflow is reachable from host interfaces instead of loopback only. Validation: docker compose --profile gateway config Validation: git diff --check --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index e24d201bb..336005df8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -27,7 +27,7 @@ services: profiles: ["gateway"] command: ["gateway", "--port", "${NULLCLAW_PORT:-3210}", "--host", "::"] ports: - - "127.0.0.1:${NULLCLAW_PORT:-3210}:${NULLCLAW_PORT:-3210}" + - "0.0.0.0:${NULLCLAW_PORT:-3210}:${NULLCLAW_PORT:-3210}" env_file: - path: .env required: false From dd32159f87371355e2a72fb45a4c72f1c17f49b1 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:33:59 +0100 Subject: [PATCH 08/27] chore: run config before local gateway start Add a Makefile config target that runs nullclaw onboard in the agent container, and make up depend on config before starting the gateway. Also bind the gateway process to 0.0.0.0 inside the container to match the published host interface. Validation: docker compose --profile gateway config Validation: make -n up Validation: git diff --check --- Makefile | 8 ++++++-- docker-compose.yml | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 3f420597e..829bb66f7 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ VERSION ?= dev PROFILE ?= gateway SERVICE ?= gateway RUN_ARGS ?= +CONFIG_ARGS ?= --interactive COMPOSE_BAKE ?= true NULLCLAW_PORT ?= 3210 @@ -15,7 +16,7 @@ export NULLCLAW_DOCKER_TARGET := $(DOCKER_TARGET) export NULLCLAW_VERSION := $(VERSION) export NULLCLAW_PORT -.PHONY: build up down run shell logs check-buildx +.PHONY: build config up down run shell logs check-buildx check-buildx: $(BUILDX) version >/dev/null @@ -23,7 +24,10 @@ check-buildx: build: check-buildx $(COMPOSE) --profile $(PROFILE) build $(SERVICE) -up: check-buildx +config: check-buildx + $(COMPOSE) --profile agent run --rm agent onboard $(CONFIG_ARGS) + +up: check-buildx config $(COMPOSE) --profile $(PROFILE) up -d --build $(SERVICE) down: diff --git a/docker-compose.yml b/docker-compose.yml index 336005df8..798f273a2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,10 +7,21 @@ x-nullclaw-build: &nullclaw-build VERSION: ${NULLCLAW_VERSION:-dev} services: + init-data: + image: alpine:3.23 + profiles: ["agent", "gateway"] + command: ["sh", "-lc", "chown -R 65534:65534 /nullclaw-data"] + volumes: + - nullclaw-data:/nullclaw-data + restart: "no" + agent: image: ${NULLCLAW_IMAGE:-nullclaw:local} build: *nullclaw-build profiles: ["agent"] + depends_on: + init-data: + condition: service_completed_successfully stdin_open: true tty: true command: ["agent"] @@ -25,7 +36,10 @@ services: image: ${NULLCLAW_IMAGE:-nullclaw:local} build: *nullclaw-build profiles: ["gateway"] - command: ["gateway", "--port", "${NULLCLAW_PORT:-3210}", "--host", "::"] + depends_on: + init-data: + condition: service_completed_successfully + command: ["gateway", "--port", "${NULLCLAW_PORT:-3210}", "--host", "0.0.0.0"] ports: - "0.0.0.0:${NULLCLAW_PORT:-3210}:${NULLCLAW_PORT:-3210}" env_file: From 367d5741713ed04fda79e3a3838ab7351c6d689c Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:36:50 +0100 Subject: [PATCH 09/27] chore: use IPv4 loopback for gateway healthcheck Point the compose healthcheck at 127.0.0.1 so it checks the IPv4 listener used by the gateway's 0.0.0.0 bind. Validation: docker compose --profile gateway config Validation: git diff --check --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 798f273a2..cdfe46459 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -49,7 +49,7 @@ services: - nullclaw-data:/nullclaw-data restart: unless-stopped healthcheck: - test: ["CMD", "wget", "-qO-", "http://localhost:${NULLCLAW_PORT:-3210}/health"] + test: ["CMD", "wget", "-qO-", "http://127.0.0.1:${NULLCLAW_PORT:-3210}/health"] interval: 30s timeout: 5s retries: 3 From e0054f4775b26064dc645732539dd9d53a125d0a Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:39:18 +0100 Subject: [PATCH 10/27] docs: describe compose gateway workflow Document the Makefile-backed Docker Compose setup, including make config, make up, NULLCLAW_PORT=3210, host-interface publishing, and health checks. Validation: make -n config Validation: make -n up Validation: git diff --check --- README.md | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fd14a37c4..3f09ce729 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,51 @@ Then: nullclaw --help ``` -### 3) Common commands +### 3) Run with Docker Compose + +The repository includes a Makefile wrapper around Docker Compose for local +containerized runs. + +```bash +make build +make config +make up +``` + +`make config` runs `nullclaw onboard --interactive` inside the agent container. +For non-interactive setup: + +```bash +make config CONFIG_ARGS="--api-key sk-... --provider openrouter" +``` + +The compose gateway defaults to `NULLCLAW_PORT=3210`, binds inside the +container on `0.0.0.0`, and publishes on all host interfaces: + +```bash +curl http://127.0.0.1:3210/health +curl http://:3210/health +``` + +Override the port when needed: + +```bash +make up NULLCLAW_PORT=8080 +``` + +Operational shortcuts: + +```bash +make logs +make down +make shell +``` + +Because the compose gateway is published on `0.0.0.0`, keep pairing, webhook +secrets, allowlists, and host firewall rules configured before using it on an +untrusted network. + +### 4) Common commands ```bash From 6245d48f9db24c6467edfaa3d8d9483cdc429d5f Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:49:56 +0100 Subject: [PATCH 11/27] chore: bind local workspace into compose services Mount ./workspace at /nullclaw-data/workspace for init, agent, and gateway services so the runtime workspace lives under the current checkout. Ignore the local workspace directory because it contains runtime state. Validation: docker compose --profile gateway config Validation: git diff --check --- .gitignore | 1 + docker-compose.yml | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index b041fe09b..acdd6af9b 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ zig-pkg/ *.bak *.a reference/ +workspace/ result .direnv/ diff --git a/docker-compose.yml b/docker-compose.yml index cdfe46459..264fbb16c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,9 +10,10 @@ services: init-data: image: alpine:3.23 profiles: ["agent", "gateway"] - command: ["sh", "-lc", "chown -R 65534:65534 /nullclaw-data"] + command: ["sh", "-lc", "mkdir -p /nullclaw-data/workspace && chown -R 65534:65534 /nullclaw-data"] volumes: - nullclaw-data:/nullclaw-data + - ./workspace:/nullclaw-data/workspace restart: "no" agent: @@ -30,6 +31,7 @@ services: required: false volumes: - nullclaw-data:/nullclaw-data + - ./workspace:/nullclaw-data/workspace restart: unless-stopped gateway: @@ -47,6 +49,7 @@ services: required: false volumes: - nullclaw-data:/nullclaw-data + - ./workspace:/nullclaw-data/workspace restart: unless-stopped healthcheck: test: ["CMD", "wget", "-qO-", "http://127.0.0.1:${NULLCLAW_PORT:-3210}/health"] From c8b4563659682808d7a8bba1d0ef4c063fd03132 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:50:58 +0100 Subject: [PATCH 12/27] chore: add compose agent and status targets Add make agent for interactive nullclaw agent chat and make status for nullclaw status via the compose agent service. Validation: make -n agent Validation: make -n status Validation: git diff --check --- Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 829bb66f7..873077825 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ export NULLCLAW_DOCKER_TARGET := $(DOCKER_TARGET) export NULLCLAW_VERSION := $(VERSION) export NULLCLAW_PORT -.PHONY: build config up down run shell logs check-buildx +.PHONY: build config up down run agent status shell logs check-buildx check-buildx: $(BUILDX) version >/dev/null @@ -36,6 +36,12 @@ down: run: $(COMPOSE) --profile agent run --rm agent $(RUN_ARGS) +agent: + $(COMPOSE) --profile agent run --rm agent agent + +status: + $(COMPOSE) --profile agent run --rm agent status + shell: $(COMPOSE) --profile agent run --rm --entrypoint /bin/sh agent From 64369a6ef0f0d6947ac28309145052d595f6b37f Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 18:53:58 +0100 Subject: [PATCH 13/27] chore: keep compose up from rerunning config Make configuration an explicit make config step instead of a dependency of make up, so make down preserves the existing config volume and restart does not rerun onboarding. Validation: make -n up Validation: make -n config Validation: git diff --check --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 873077825..1f4e9fbea 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ build: check-buildx config: check-buildx $(COMPOSE) --profile agent run --rm agent onboard $(CONFIG_ARGS) -up: check-buildx config +up: check-buildx $(COMPOSE) --profile $(PROFILE) up -d --build $(SERVICE) down: From c21c41dede91f1b4d796ec8ef8487d3187e55229 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:00:04 +0100 Subject: [PATCH 14/27] chore: include git and shell env in runtime image Install git in the release image and set SHELL=/bin/sh so the compose workflow satisfies doctor checks and supports documented git/tool usage. Validation: docker compose run --rm agent doctor Validation: docker compose run --rm agent status Validation: docker compose --profile gateway config Validation: git diff --check --- Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index bb2ae5cb1..d20ceb78b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -97,7 +97,7 @@ FROM alpine:3.23 AS release-base LABEL org.opencontainers.image.source=https://github.com/nullclaw/nullclaw -RUN apk add --no-cache ca-certificates curl tzdata +RUN apk add --no-cache ca-certificates curl git tzdata COPY --from=builder /app/zig-out/bin/nullclaw /usr/local/bin/nullclaw COPY --from=config /nullclaw-data /nullclaw-data @@ -105,6 +105,7 @@ COPY --from=config /nullclaw-data /nullclaw-data ENV NULLCLAW_WORKSPACE=/nullclaw-data/workspace ENV NULLCLAW_HOME=/nullclaw-data ENV HOME=/nullclaw-data +ENV SHELL=/bin/sh ENV NULLCLAW_GATEWAY_PORT=3000 WORKDIR /nullclaw-data From 908e163e4ce33b2f6a62540a3c6e848be93d1c8c Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:13:45 +0100 Subject: [PATCH 15/27] chore: remove stale Docker expose port Drop EXPOSE 3000 from the runtime image because compose publishes NULLCLAW_PORT explicitly. This avoids docker ps showing an unused 3000/tcp port while the gateway runs on 3210. Validation: docker compose --profile gateway config Validation: docker compose --profile gateway up -d --build gateway Validation: docker compose ps Validation: git diff --check --- Dockerfile | 1 - 1 file changed, 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index d20ceb78b..1d3048870 100644 --- a/Dockerfile +++ b/Dockerfile @@ -109,7 +109,6 @@ ENV SHELL=/bin/sh ENV NULLCLAW_GATEWAY_PORT=3000 WORKDIR /nullclaw-data -EXPOSE 3000 ENTRYPOINT ["nullclaw"] CMD ["gateway", "--port", "3000", "--host", "::"] From 464a133e5e8ca3c5b83cb02ee7ee32ffe38582ef Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:29:49 +0100 Subject: [PATCH 16/27] chore: bind compose config file from checkout Mount ./config.json into /nullclaw-data/config.json for init, agent, and gateway services. Run compose services as the host UID/GID from the Makefile so bind-mounted config and workspace files remain host-editable. Validation: docker compose --profile gateway config Validation: make down Validation: make up Validation: docker compose ps Validation: git diff --check --- .gitignore | 1 + Makefile | 4 ++++ docker-compose.yml | 7 ++++++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index acdd6af9b..8014f1a8d 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ result .direnv/ .worktrees config.signal.json +config.json # IDE configurations .idea/ diff --git a/Makefile b/Makefile index 1f4e9fbea..482359e24 100644 --- a/Makefile +++ b/Makefile @@ -9,12 +9,16 @@ RUN_ARGS ?= CONFIG_ARGS ?= --interactive COMPOSE_BAKE ?= true NULLCLAW_PORT ?= 3210 +NULLCLAW_UID ?= $(shell id -u) +NULLCLAW_GID ?= $(shell id -g) export COMPOSE_BAKE export NULLCLAW_IMAGE := $(IMAGE) export NULLCLAW_DOCKER_TARGET := $(DOCKER_TARGET) export NULLCLAW_VERSION := $(VERSION) export NULLCLAW_PORT +export NULLCLAW_UID +export NULLCLAW_GID .PHONY: build config up down run agent status shell logs check-buildx diff --git a/docker-compose.yml b/docker-compose.yml index 264fbb16c..4cbca4205 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,9 +10,10 @@ services: init-data: image: alpine:3.23 profiles: ["agent", "gateway"] - command: ["sh", "-lc", "mkdir -p /nullclaw-data/workspace && chown -R 65534:65534 /nullclaw-data"] + command: ["sh", "-lc", "mkdir -p /nullclaw-data/workspace && chown -R ${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534} /nullclaw-data"] volumes: - nullclaw-data:/nullclaw-data + - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace restart: "no" @@ -20,6 +21,7 @@ services: image: ${NULLCLAW_IMAGE:-nullclaw:local} build: *nullclaw-build profiles: ["agent"] + user: "${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534}" depends_on: init-data: condition: service_completed_successfully @@ -31,6 +33,7 @@ services: required: false volumes: - nullclaw-data:/nullclaw-data + - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace restart: unless-stopped @@ -38,6 +41,7 @@ services: image: ${NULLCLAW_IMAGE:-nullclaw:local} build: *nullclaw-build profiles: ["gateway"] + user: "${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534}" depends_on: init-data: condition: service_completed_successfully @@ -49,6 +53,7 @@ services: required: false volumes: - nullclaw-data:/nullclaw-data + - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace restart: unless-stopped healthcheck: From 94808ca347aec9b4ce1d37ed7ec8ded3a25d2201 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:42:41 +0100 Subject: [PATCH 17/27] chore: make down include compose profiles Run docker compose down with both agent and gateway profiles so profile-created containers are stopped and removed by make down. Validation: make down Validation: make up Validation: curl -sS -i http://127.0.0.1:3210/health --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 482359e24..d685b557c 100644 --- a/Makefile +++ b/Makefile @@ -35,7 +35,7 @@ up: check-buildx $(COMPOSE) --profile $(PROFILE) up -d --build $(SERVICE) down: - $(COMPOSE) down + $(COMPOSE) --profile agent --profile gateway down run: $(COMPOSE) --profile agent run --rm agent $(RUN_ARGS) From 62f6a4811ae0c50df45fe9547facba6f2da0b9c1 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:51:12 +0100 Subject: [PATCH 18/27] chore: mount docker socket for sandbox runtime --- docker-compose.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 4cbca4205..7d6917514 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -35,6 +35,7 @@ services: - nullclaw-data:/nullclaw-data - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace + - /var/run/docker.sock:/var/run/docker.sock restart: unless-stopped gateway: @@ -55,6 +56,7 @@ services: - nullclaw-data:/nullclaw-data - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace + - /var/run/docker.sock:/var/run/docker.sock restart: unless-stopped healthcheck: test: ["CMD", "wget", "-qO-", "http://127.0.0.1:${NULLCLAW_PORT:-3210}/health"] From 8924ad47484efed2b76f517e81c6bac242a3f7a6 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:53:58 +0100 Subject: [PATCH 19/27] chore: add docker and make to runtime image --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 1d3048870..b2d87dd07 100644 --- a/Dockerfile +++ b/Dockerfile @@ -97,7 +97,7 @@ FROM alpine:3.23 AS release-base LABEL org.opencontainers.image.source=https://github.com/nullclaw/nullclaw -RUN apk add --no-cache ca-certificates curl git tzdata +RUN apk add --no-cache ca-certificates curl docker-cli git make tzdata COPY --from=builder /app/zig-out/bin/nullclaw /usr/local/bin/nullclaw COPY --from=config /nullclaw-data /nullclaw-data From e53db582b768d3245e0a51c2caaef475e8bb5742 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 19:57:38 +0100 Subject: [PATCH 20/27] chore: grant sandbox access to docker socket --- docker-compose.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 7d6917514..8c6b27f31 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,6 +22,8 @@ services: build: *nullclaw-build profiles: ["agent"] user: "${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534}" + group_add: + - "111" depends_on: init-data: condition: service_completed_successfully @@ -43,6 +45,8 @@ services: build: *nullclaw-build profiles: ["gateway"] user: "${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534}" + group_add: + - "111" depends_on: init-data: condition: service_completed_successfully From 461f7776f2205e6d5c5b4f3e946bf07362cef864 Mon Sep 17 00:00:00 2001 From: Rui Ribeiro Date: Sun, 10 May 2026 20:10:48 +0100 Subject: [PATCH 21/27] chore: expand shell sandbox command set --- Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index b2d87dd07..7d20e0b86 100644 --- a/Dockerfile +++ b/Dockerfile @@ -97,7 +97,8 @@ FROM alpine:3.23 AS release-base LABEL org.opencontainers.image.source=https://github.com/nullclaw/nullclaw -RUN apk add --no-cache ca-certificates curl docker-cli git make tzdata +RUN apk add --no-cache ca-certificates curl docker-cli git make bash jq yq python3 nodejs perl tzdata && \ + ln -sf python3 /usr/bin/python COPY --from=builder /app/zig-out/bin/nullclaw /usr/local/bin/nullclaw COPY --from=config /nullclaw-data /nullclaw-data From c557248398f4957c45d2eb46c5d69c0a05e5ea29 Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Wed, 27 May 2026 23:35:47 -0300 Subject: [PATCH 22/27] fix: complete PR 907 security hardening --- Dockerfile | 24 +--- Makefile | 2 + README.md | 11 +- SECURITY-PATCH-PLAN-2026-05-10.md | 1 - docker-compose.yml | 11 +- src/channels/lark.zig | 57 +------- src/channels/max_api.zig | 151 +++++++------------ src/gateway.zig | 92 ++++++------ src/http_util.zig | 232 ++++++++++++++++++++++++++++-- src/memory/engines/api.zig | 58 ++++++-- src/providers/gemini.zig | 18 ++- src/providers/openai_codex.zig | 24 ++-- src/providers/sse.zig | 125 ++++++++-------- src/tools/composio.zig | 115 +++------------ src/tools/http_request.zig | 12 +- src/voice.zig | 8 +- 16 files changed, 504 insertions(+), 437 deletions(-) diff --git a/Dockerfile b/Dockerfile index 7de2f04e0..7e0668b44 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,26 +14,6 @@ COPY build.zig build.zig.zon ./ COPY src/ src/ COPY vendor/sqlite3/ vendor/sqlite3/ -RUN set -eu; \ - fetch_vendor_dep() { \ - repo="$1"; \ - commit="$2"; \ - target="vendor/${repo}"; \ - tmp_dir="$(mktemp -d)"; \ - archive_path="${tmp_dir}/${repo}.tar.gz"; \ - curl -fsSL "https://github.com/nullclaw/${repo}/archive/${commit}.tar.gz" -o "${archive_path}"; \ - tar -xzf "${archive_path}" -C "${tmp_dir}"; \ - extracted_dir="$(find "${tmp_dir}" -mindepth 1 -maxdepth 1 -type d | head -n 1)"; \ - rm -rf "${target}"; \ - mkdir -p "${target}"; \ - cp -a "${extracted_dir}/." "${target}/"; \ - rm -rf "${tmp_dir}"; \ - }; \ - fetch_vendor_dep websocket 9151f70b27024a072ea04425b9e7609eb6b1386c; \ - fetch_vendor_dep wasm3 8a29b0080f1f65dbbe8b8f8c4d8148480feff377; \ - sed -i '/\.websocket = .{/,/ },/c\ .websocket = .{\n .path = "vendor/websocket",\n },' build.zig.zon; \ - sed -i '/\.wasm3 = .{/,/ },/c\ .wasm3 = .{\n .path = "vendor/wasm3",\n },' build.zig.zon - RUN set -eu; \ mkdir -p /tmp/zig-path; \ GITHUB_PATH=/tmp/zig-path/path RUNNER_TEMP=/opt bash .github/scripts/install-zig.sh "${ZIG_VERSION}"; \ @@ -97,8 +77,7 @@ FROM alpine:3.23 AS release-base LABEL org.opencontainers.image.source=https://github.com/nullclaw/nullclaw -RUN apk add --no-cache ca-certificates curl docker-cli git make bash jq yq python3 nodejs perl tzdata && \ - ln -sf python3 /usr/bin/python +RUN apk add --no-cache ca-certificates curl git tzdata COPY --from=builder /app/zig-out/bin/nullclaw /usr/local/bin/nullclaw COPY --from=config /nullclaw-data /nullclaw-data @@ -110,6 +89,7 @@ ENV SHELL=/bin/sh ENV NULLCLAW_GATEWAY_PORT=3000 WORKDIR /nullclaw-data +EXPOSE 3000 ENTRYPOINT ["nullclaw"] CMD ["gateway", "--port", "3000", "--host", "::"] diff --git a/Makefile b/Makefile index d685b557c..614640010 100644 --- a/Makefile +++ b/Makefile @@ -9,6 +9,7 @@ RUN_ARGS ?= CONFIG_ARGS ?= --interactive COMPOSE_BAKE ?= true NULLCLAW_PORT ?= 3210 +NULLCLAW_BIND ?= 127.0.0.1 NULLCLAW_UID ?= $(shell id -u) NULLCLAW_GID ?= $(shell id -g) @@ -17,6 +18,7 @@ export NULLCLAW_IMAGE := $(IMAGE) export NULLCLAW_DOCKER_TARGET := $(DOCKER_TARGET) export NULLCLAW_VERSION := $(VERSION) export NULLCLAW_PORT +export NULLCLAW_BIND export NULLCLAW_UID export NULLCLAW_GID diff --git a/README.md b/README.md index 69ea775b7..42bf3c4b6 100644 --- a/README.md +++ b/README.md @@ -180,17 +180,17 @@ make config CONFIG_ARGS="--api-key sk-... --provider openrouter" ``` The compose gateway defaults to `NULLCLAW_PORT=3210`, binds inside the -container on `0.0.0.0`, and publishes on all host interfaces: +container on `0.0.0.0`, and publishes to localhost on the host: ```bash curl http://127.0.0.1:3210/health -curl http://:3210/health ``` -Override the port when needed: +Override the port or host bind address when needed: ```bash make up NULLCLAW_PORT=8080 +make up NULLCLAW_BIND=0.0.0.0 ``` Operational shortcuts: @@ -201,9 +201,8 @@ make down make shell ``` -Because the compose gateway is published on `0.0.0.0`, keep pairing, webhook -secrets, allowlists, and host firewall rules configured before using it on an -untrusted network. +Only set `NULLCLAW_BIND=0.0.0.0` on trusted networks with pairing, webhook +secrets, allowlists, and host firewall rules configured. ### 4) Common commands diff --git a/SECURITY-PATCH-PLAN-2026-05-10.md b/SECURITY-PATCH-PLAN-2026-05-10.md index 39b71b785..d81bcbafd 100644 --- a/SECURITY-PATCH-PLAN-2026-05-10.md +++ b/SECURITY-PATCH-PLAN-2026-05-10.md @@ -147,4 +147,3 @@ Before handing off or opening a PR, include: 4. Validation commands and results. 5. Remaining risks or unknowns. 6. Next recommended action. - diff --git a/docker-compose.yml b/docker-compose.yml index 8c6b27f31..0d3e3a1e2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,7 +13,6 @@ services: command: ["sh", "-lc", "mkdir -p /nullclaw-data/workspace && chown -R ${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534} /nullclaw-data"] volumes: - nullclaw-data:/nullclaw-data - - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace restart: "no" @@ -22,8 +21,6 @@ services: build: *nullclaw-build profiles: ["agent"] user: "${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534}" - group_add: - - "111" depends_on: init-data: condition: service_completed_successfully @@ -35,9 +32,7 @@ services: required: false volumes: - nullclaw-data:/nullclaw-data - - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace - - /var/run/docker.sock:/var/run/docker.sock restart: unless-stopped gateway: @@ -45,22 +40,18 @@ services: build: *nullclaw-build profiles: ["gateway"] user: "${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534}" - group_add: - - "111" depends_on: init-data: condition: service_completed_successfully command: ["gateway", "--port", "${NULLCLAW_PORT:-3210}", "--host", "0.0.0.0"] ports: - - "0.0.0.0:${NULLCLAW_PORT:-3210}:${NULLCLAW_PORT:-3210}" + - "${NULLCLAW_BIND:-127.0.0.1}:${NULLCLAW_PORT:-3210}:${NULLCLAW_PORT:-3210}" env_file: - path: .env required: false volumes: - nullclaw-data:/nullclaw-data - - ./config.json:/nullclaw-data/config.json - ./workspace:/nullclaw-data/workspace - - /var/run/docker.sock:/var/run/docker.sock restart: unless-stopped healthcheck: test: ["CMD", "wget", "-qO-", "http://127.0.0.1:${NULLCLAW_PORT:-3210}/health"] diff --git a/src/channels/lark.zig b/src/channels/lark.zig index 42e5909be..da02c0e58 100644 --- a/src/channels/lark.zig +++ b/src/channels/lark.zig @@ -22,7 +22,6 @@ const DEFAULT_LARK_PING_INTERVAL_MS: u32 = 120 * std.time.ms_per_s; const EVENT_CACHE_TTL_MS: i64 = 10_000; const LARK_WS_METHOD_CONTROL: i32 = 0; const LARK_WS_METHOD_DATA: i32 = 1; -const LARK_API_MAX_BYTES: usize = 256 * 1024; const LARK_TYPING_PLACEHOLDER = "..."; const LarkWsConnectConfig = struct { @@ -887,61 +886,7 @@ pub const LarkChannel = struct { url: []const u8, headers: []const []const u8, ) !http_util.HttpResponse { - var argv_buf: [24][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-s"; - argc += 1; - argv_buf[argc] = "-X"; - argc += 1; - argv_buf[argc] = "DELETE"; - argc += 1; - - for (headers) |header| { - if (argc + 2 > argv_buf.len) break; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = header; - argc += 1; - } - - argv_buf[argc] = "-w"; - argc += 1; - argv_buf[argc] = "\n%{http_code}"; - argc += 1; - argv_buf[argc] = url; - argc += 1; - - var child = std_compat.process.Child.init(argv_buf[0..argc], allocator); - child.stdout_behavior = .Pipe; - child.stderr_behavior = .Ignore; - child.spawn() catch return error.LarkApiError; - - const stdout = child.stdout.?.readToEndAlloc(allocator, LARK_API_MAX_BYTES) catch { - _ = child.kill() catch {}; - _ = child.wait() catch {}; - return error.LarkApiError; - }; - errdefer allocator.free(stdout); - - const term = child.wait() catch return error.LarkApiError; - switch (term) { - .exited => |code| if (code != 0) return error.LarkApiError, - else => return error.LarkApiError, - } - - const status_sep = std.mem.lastIndexOfScalar(u8, stdout, '\n') orelse return error.LarkApiError; - const status_raw = std.mem.trim(u8, stdout[status_sep + 1 ..], " \t\r\n"); - if (status_raw.len != 3) return error.LarkApiError; - const status_code = std.fmt.parseInt(u16, status_raw, 10) catch return error.LarkApiError; - const body = try allocator.dupe(u8, stdout[0..status_sep]); - allocator.free(stdout); - - return .{ - .status_code = status_code, - .body = body, - }; + return http_util.httpRequestWithStatus(allocator, .DELETE, url, null, headers, null, null); } fn buildRichCardContent(buf: []u8, payload: root.Channel.OutboundPayload) ![]const u8 { diff --git a/src/channels/max_api.zig b/src/channels/max_api.zig index 9244aa820..e98faaa24 100644 --- a/src/channels/max_api.zig +++ b/src/channels/max_api.zig @@ -22,6 +22,8 @@ pub const MAX_MESSAGE_LEN: usize = 4000; // Update types requested via long-polling. const UPDATE_TYPES = "message_created,message_callback,message_edited,message_removed,bot_added,bot_removed,bot_started,bot_stopped"; +const MAX_UPLOAD_FILE_BYTES: usize = 16 * 1024 * 1024; +const MAX_UPLOAD_BOUNDARY = "nullclaw-max-upload-boundary"; // ════════════════════════════════════════════════════════════════════════════ // Types @@ -441,53 +443,8 @@ pub const Client = struct { // ════════════════════════════════════════════════════════════════════════════ fn curlDelete(allocator: std.mem.Allocator, url: []const u8, auth_header: []const u8, proxy: ?[]const u8) !void { - var argv_buf: [14][]const u8 = undefined; - var argc: usize = 0; - - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-s"; - argc += 1; - argv_buf[argc] = "-X"; - argc += 1; - argv_buf[argc] = "DELETE"; - argc += 1; - argv_buf[argc] = "-m"; - argc += 1; - argv_buf[argc] = "10"; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_header; - argc += 1; - - if (proxy) |p| { - argv_buf[argc] = "-x"; - argc += 1; - argv_buf[argc] = p; - argc += 1; - } - - argv_buf[argc] = url; - argc += 1; - - var child = std_compat.process.Child.init(argv_buf[0..argc], allocator); - child.stdout_behavior = .Pipe; - child.stderr_behavior = .Ignore; - try child.spawn(); - - const stdout = child.stdout.?.readToEndAlloc(allocator, 256 * 1024) catch { - _ = child.kill() catch {}; - _ = child.wait() catch {}; - return error.CurlReadError; - }; - defer allocator.free(stdout); - - const term = child.wait() catch return error.CurlWaitError; - switch (term) { - .exited => |code| if (code != 0) return error.CurlFailed, - else => return error.CurlFailed, - } + const resp = try root.http_util.httpRequestWithStatus(allocator, .DELETE, url, null, &.{auth_header}, null, proxy); + allocator.free(resp.body); } fn curlMultipartUpload( @@ -497,61 +454,39 @@ fn curlMultipartUpload( proxy: ?[]const u8, file_path: []const u8, ) ![]u8 { - var file_arg_buf: [1024]u8 = undefined; - var file_writer: std.Io.Writer = .fixed(&file_arg_buf); - try file_writer.print("data=@{s}", .{file_path}); - const file_arg = file_writer.buffered(); - - var argv_buf: [18][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-s"; - argc += 1; - argv_buf[argc] = "-m"; - argc += 1; - argv_buf[argc] = "120"; - argc += 1; - - if (proxy) |p| { - argv_buf[argc] = "--proxy"; - argc += 1; - argv_buf[argc] = p; - argc += 1; - } + const file = try std_compat.fs.cwd().openFile(file_path, .{}); + defer file.close(); + const data = try file.readToEndAlloc(allocator, MAX_UPLOAD_FILE_BYTES); + defer allocator.free(data); - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_header; - argc += 1; - argv_buf[argc] = "-F"; - argc += 1; - argv_buf[argc] = file_arg; - argc += 1; - argv_buf[argc] = url; - argc += 1; - - var child = std_compat.process.Child.init(argv_buf[0..argc], allocator); - child.stdout_behavior = .Pipe; - child.stderr_behavior = .Ignore; - try child.spawn(); - - const stdout = child.stdout.?.readToEndAlloc(allocator, 1024 * 1024) catch return error.CurlReadError; - const term = child.wait() catch { - allocator.free(stdout); - return error.CurlWaitError; - }; - switch (term) { - .exited => |code| if (code != 0) { - allocator.free(stdout); - return error.CurlFailed; - }, - else => { - allocator.free(stdout); - return error.CurlFailed; - }, - } - return stdout; + var body: std.ArrayListUnmanaged(u8) = .empty; + defer body.deinit(allocator); + const filename = std.fs.path.basename(file_path); + try appendMaxMultipartFile(&body, allocator, MAX_UPLOAD_BOUNDARY, "data", filename, data); + try body.appendSlice(allocator, "--" ++ MAX_UPLOAD_BOUNDARY ++ "--\r\n"); + + const content_type = "multipart/form-data; boundary=" ++ MAX_UPLOAD_BOUNDARY; + const resp = try root.http_util.httpRequestWithStatus(allocator, .POST, url, body.items, &.{auth_header}, content_type, proxy); + return resp.body; +} + +fn appendMaxMultipartFile( + body: *std.ArrayListUnmanaged(u8), + allocator: std.mem.Allocator, + boundary: []const u8, + name: []const u8, + filename: []const u8, + data: []const u8, +) !void { + try body.appendSlice(allocator, "--"); + try body.appendSlice(allocator, boundary); + try body.appendSlice(allocator, "\r\nContent-Disposition: form-data; name=\""); + try body.appendSlice(allocator, name); + try body.appendSlice(allocator, "\"; filename=\""); + try body.appendSlice(allocator, filename); + try body.appendSlice(allocator, "\"\r\nContent-Type: application/octet-stream\r\n\r\n"); + try body.appendSlice(allocator, data); + try body.appendSlice(allocator, "\r\n"); } // ════════════════════════════════════════════════════════════════════════════ @@ -782,6 +717,20 @@ test "parseUploadToken returns null on missing token" { try std.testing.expect(Client.parseUploadToken(allocator, "{\"other\":1}") == null); } +test "max multipart upload body uses data file field" { + const allocator = std.testing.allocator; + var body: std.ArrayListUnmanaged(u8) = .empty; + defer body.deinit(allocator); + + // Regression: Max uploads must not need curl argv for auth or upload URLs. + try appendMaxMultipartFile(&body, allocator, "boundary-test", "data", "sample.txt", "abc"); + try body.appendSlice(allocator, "--boundary-test--\r\n"); + + try std.testing.expect(std.mem.indexOf(u8, body.items, "Content-Disposition: form-data; name=\"data\"; filename=\"sample.txt\"") != null); + try std.testing.expect(std.mem.indexOf(u8, body.items, "\r\n\r\nabc\r\n") != null); + try std.testing.expect(std.mem.endsWith(u8, body.items, "--boundary-test--\r\n")); +} + test "buildTextMessageBody without format" { const allocator = std.testing.allocator; const body = try buildTextMessageBody(allocator, "Hello, world!", null); diff --git a/src/gateway.zig b/src/gateway.zig index a3bf613d3..5f1300124 100644 --- a/src/gateway.zig +++ b/src/gateway.zig @@ -1954,6 +1954,11 @@ fn telegramWebhookSecretMatches(raw_request: []const u8, configured_secret: ?[]c return constantTimeEq(trimmed, secret); } +fn lineSenderAllowed(allow_from: []const []const u8, evt: channels.line.LineEvent) bool { + const user_id = evt.user_id orelse return false; + return channels.isAllowed(allow_from, user_id); +} + fn telegramSessionKeyRouted( allocator: std.mem.Allocator, fallback_buf: []u8, @@ -2640,48 +2645,45 @@ pub fn sendTelegramReply( message_thread_id: ?i64, text: []const u8, ) !void { - // Build the curl command to call the Telegram API + const body = try buildTelegramReplyBody(allocator, chat_id, message_thread_id, text); + defer allocator.free(body); + + // Tests cover the JSON construction above; skip the real Telegram side effect. + if (comptime builtin.is_test) return; + const url = try std.fmt.allocPrint(allocator, "https://api.telegram.org/bot{s}/sendMessage", .{bot_token}); defer allocator.free(url); - // JSON-escape the text for the body - var body_buf: std.ArrayList(u8) = .empty; - defer body_buf.deinit(allocator); - var body_writer: std.Io.Writer.Allocating = .fromArrayList(allocator, &body_buf); - const w = &body_writer.writer; - try w.print("{{\"chat_id\":{d},\"text\":\"", .{chat_id}); - for (text) |c| { - switch (c) { - '"' => try w.writeAll("\\\""), - '\\' => try w.writeAll("\\\\"), - '\n' => try w.writeAll("\\n"), - '\r' => try w.writeAll("\\r"), - '\t' => try w.writeAll("\\t"), - else => try w.writeByte(c), - } - } - try w.writeAll("\""); - if (message_thread_id) |thread_id| { - try w.print(",\"message_thread_id\":{d}", .{thread_id}); - } - try w.writeAll("}"); - body_buf = body_writer.toArrayList(); + const resp = http_util.httpRequest(allocator, .POST, url, body, &.{}, "application/json", null) catch return; + allocator.free(resp); +} - const body = body_buf.items; +fn buildTelegramReplyBody( + allocator: std.mem.Allocator, + chat_id: i64, + message_thread_id: ?i64, + text: []const u8, +) ![]u8 { + var body: std.ArrayListUnmanaged(u8) = .empty; + errdefer body.deinit(allocator); - var curl_child = std_compat.process.Child.init( - &[_][]const u8{ - "curl", "-s", "-X", "POST", - "-H", "Content-Type: application/json", "-d", body, - url, - }, - allocator, - ); - curl_child.stdout_behavior = .Pipe; - curl_child.stderr_behavior = .Pipe; + var int_buf: [32]u8 = undefined; + try body.appendSlice(allocator, "{\"chat_id\":"); + try body.appendSlice(allocator, std.fmt.bufPrint(&int_buf, "{d}", .{chat_id}) catch "0"); + try body.appendSlice(allocator, ",\"text\":"); + try appendJsonStringBuf(&body, allocator, text); + if (message_thread_id) |thread_id| { + try body.appendSlice(allocator, ",\"message_thread_id\":"); + try body.appendSlice(allocator, std.fmt.bufPrint(&int_buf, "{d}", .{thread_id}) catch "0"); + } + try body.appendSlice(allocator, "}"); + return body.toOwnedSlice(allocator); +} - curl_child.spawn() catch return; - _ = curl_child.wait() catch {}; +test "telegram reply body escapes text and includes thread" { + const body = try buildTelegramReplyBody(std.testing.allocator, 42, 7, "hello \"there\"\nnext"); + defer std.testing.allocator.free(body); + try std.testing.expectEqualStrings("{\"chat_id\":42,\"text\":\"hello \\\"there\\\"\\nnext\",\"message_thread_id\":7}", body); } fn userFacingAgentError(err: anyerror) []const u8 { @@ -4079,11 +4081,7 @@ fn handleLineWebhookRoute(ctx: *WebhookHandlerContext) void { return; }; for (events) |evt| { - if (line_allow_from.len > 0) { - if (evt.user_id) |uid| { - if (!channels.isAllowed(line_allow_from, uid)) continue; - } else continue; - } + if (!lineSenderAllowed(line_allow_from, evt)) continue; if (evt.message_text) |text| { var kb: [128]u8 = undefined; const line_cfg_opt: ?*const Config = if (ctx.config_opt) |cfg| cfg else null; @@ -5482,6 +5480,7 @@ pub fn run( state.telegram_bot_token = tg_cfg.bot_token; state.telegram_allow_from = tg_cfg.allow_from; state.telegram_account_id = tg_cfg.account_id; + state.telegram_webhook_secret = tg_cfg.webhook_secret; } if (cfg.channels.whatsappPrimary()) |wa_cfg| { state.whatsapp_verify_token = wa_cfg.verify_token; @@ -7877,6 +7876,17 @@ test "telegramWebhookSecretMatches requires configured secret header" { try std.testing.expect(!telegramWebhookSecretMatches(raw, null)); } +test "lineSenderAllowed denies empty allow_from and permits wildcard" { + const evt = channels.line.LineEvent{ + .event_type = "message", + .user_id = "U123", + }; + try std.testing.expect(!lineSenderAllowed(&.{}, evt)); + try std.testing.expect(lineSenderAllowed(&.{"*"}, evt)); + try std.testing.expect(lineSenderAllowed(&.{"U123"}, evt)); + try std.testing.expect(!lineSenderAllowed(&.{"U999"}, evt)); +} + test "telegramChatId extracts nested message.chat.id" { const allocator = std.testing.allocator; const body = diff --git a/src/http_util.zig b/src/http_util.zig index 1a4b67ad5..ef3a1c33e 100644 --- a/src/http_util.zig +++ b/src/http_util.zig @@ -1,13 +1,15 @@ -//! Shared HTTP utilities via curl subprocess. +//! Shared HTTP utilities. //! -//! Replaces 9+ local `curlPost` / `curlGet` duplicates across the codebase. -//! Uses curl to keep timeout handling explicit and avoid std.http regressions. +//! Keeps legacy curl helpers for non-secret requests, but routes credentialed +//! headers and sensitive token URLs through std.http so secrets are not exposed +//! through child process argv. const std = @import("std"); const std_compat = @import("compat"); const Allocator = std.mem.Allocator; const AtomicBool = std.atomic.Value(bool); const net_security = @import("net_security.zig"); +const platform = @import("platform.zig"); const log = std.log.scoped(.http_util); threadlocal var thread_interrupt_flag: ?*const AtomicBool = null; @@ -169,6 +171,14 @@ fn isCredentialHeader(header: []const u8) bool { std.ascii.eqlIgnoreCase(name, "cookie"); } +fn hasCredentialedCurlArgs(url: []const u8, headers: []const []const u8) bool { + if (hasSensitiveUrlToken(url)) return true; + for (headers) |header| { + if (isCredentialHeader(header)) return true; + } + return false; +} + fn hasSensitiveUrlToken(url: []const u8) bool { const query_start = std.mem.indexOfScalar(u8, url, '?') orelse return false; var query = url[query_start + 1 ..]; @@ -199,6 +209,71 @@ pub fn validateNoCredentialedCurlArgs(url: []const u8, headers: []const []const } } +pub const CurlHeaderArg = struct { + arg: ?[]const u8 = null, + temp_path_buf: [std_compat.fs.max_path_bytes]u8 = undefined, + temp_path_len: usize = 0, + uses_temp_file: bool = false, + + pub fn deinit(self: *const CurlHeaderArg, allocator: Allocator) void { + if (!self.uses_temp_file) return; + std_compat.fs.deleteFileAbsolute(self.temp_path_buf[0..self.temp_path_len]) catch {}; + if (self.arg) |arg| allocator.free(arg); + } +}; + +fn validateCurlHeaderLine(header: []const u8) !void { + if (std.mem.indexOfAny(u8, header, "\r\n") != null) return error.InvalidHeader; +} + +pub fn prepareCurlHeaderArg(allocator: Allocator, headers: []const []const u8) !CurlHeaderArg { + if (headers.len == 0) return .{}; + + var prepared: CurlHeaderArg = .{}; + const tmp_dir_path = platform.getTempDir(allocator) catch return error.TempDirNotFound; + defer allocator.free(tmp_dir_path); + + var tmp_dir = std_compat.fs.openDirAbsolute(tmp_dir_path, .{}) catch return error.TempDirNotFound; + defer tmp_dir.close(); + + const header_path = std.fmt.bufPrint( + &prepared.temp_path_buf, + "{s}{s}curl_headers_{d}.tmp", + .{ tmp_dir_path, std_compat.fs.path.sep_str, std_compat.time.nanoTimestamp() }, + ) catch return error.PathTooLong; + prepared.temp_path_len = header_path.len; + errdefer std_compat.fs.deleteFileAbsolute(prepared.temp_path_buf[0..prepared.temp_path_len]) catch {}; + + var tmp_file = tmp_dir.createFile( + header_path[tmp_dir_path.len + 1 ..], + .{ .truncate = true, .exclusive = false, .permissions = std_compat.fs.permissionsFromMode(0o600) }, + ) catch return error.TempFileCreateFailed; + + for (headers) |header| { + validateCurlHeaderLine(header) catch { + tmp_file.close(); + return error.InvalidHeader; + }; + tmp_file.writeAll(header) catch { + tmp_file.close(); + return error.TempFileWriteFailed; + }; + tmp_file.writeAll("\n") catch { + tmp_file.close(); + return error.TempFileWriteFailed; + }; + } + tmp_file.close(); + + for (prepared.temp_path_buf[0..prepared.temp_path_len]) |*c| { + if (c.* == '\\') c.* = '/'; + } + + prepared.arg = try std.fmt.allocPrint(allocator, "@{s}", .{prepared.temp_path_buf[0..prepared.temp_path_len]}); + prepared.uses_temp_file = true; + return prepared; +} + fn parseHeader(header: []const u8) ?std.http.Header { const colon = std.mem.indexOfScalar(u8, header, ':') orelse return null; const name = std.mem.trim(u8, header[0..colon], " \t\r\n"); @@ -207,6 +282,12 @@ fn parseHeader(header: []const u8) ?std.http.Header { return .{ .name = name, .value = value }; } +fn contentTypeHeaderValue(header: []const u8) ?[]const u8 { + const parsed = parseHeader(header) orelse return null; + if (!std.ascii.eqlIgnoreCase(parsed.name, "content-type")) return null; + return parsed.value; +} + fn initProxyClientWithOptionalProxy(allocator: Allocator, proxy: ?[]const u8) !ProxyHttpClient { var proxy_client = try ProxyHttpClient.init(allocator); if (proxy == null) return proxy_client; @@ -225,7 +306,7 @@ fn initProxyClientWithOptionalProxy(allocator: Allocator, proxy: ?[]const u8) !P return .{ .proxy_arena = proxy_arena, .client = client }; } -pub fn httpRequest( +pub fn httpRequestWithStatusAndHeaders( allocator: Allocator, method: std.http.Method, url: []const u8, @@ -233,7 +314,7 @@ pub fn httpRequest( headers: []const []const u8, content_type: ?[]const u8, proxy: ?[]const u8, -) ![]u8 { +) !HttpResponseWithHeaders { var header_buf: [20]std.http.Header = undefined; var header_count: usize = 0; if (content_type) |ct| { @@ -249,19 +330,88 @@ pub fn httpRequest( var client = try initProxyClientWithOptionalProxy(allocator, proxy); defer client.deinit(); + const uri = try std.Uri.parse(url); + const redirect_behavior: std.http.Client.Request.RedirectBehavior = + if (body == null) @enumFromInt(3) else .unhandled; + var req = try client.client.request(method, uri, .{ + .redirect_behavior = redirect_behavior, + .headers = .{ .accept_encoding = .default }, + .extra_headers = header_buf[0..header_count], + }); + defer req.deinit(); + + if (body) |payload| { + req.transfer_encoding = .{ .content_length = payload.len }; + var request_body = try req.sendBodyUnflushed(&.{}); + try request_body.writer.writeAll(payload); + try request_body.end(); + try req.connection.?.flush(); + } else { + try req.sendBodiless(); + } + + var redirect_buffer: [8 * 1024]u8 = undefined; + var response = try req.receiveHead(&redirect_buffer); + const response_headers = try allocator.dupe(u8, response.head.bytes); + errdefer allocator.free(response_headers); + var aw: std.Io.Writer.Allocating = .init(allocator); defer aw.deinit(); - const result = try client.client.fetch(.{ - .location = .{ .url = url }, - .method = method, - .payload = body, - .extra_headers = header_buf[0..header_count], - .response_writer = &aw.writer, - }); - if (@intFromEnum(result.status) < 200 or @intFromEnum(result.status) >= 300) return error.HttpStatusError; - const response = aw.writer.buffer[0..aw.writer.end]; - return try allocator.dupe(u8, response); + const decompress_buffer: []u8 = switch (response.head.content_encoding) { + .identity => &.{}, + .zstd => try allocator.alloc(u8, std.compress.zstd.default_window_len), + .deflate, .gzip => try allocator.alloc(u8, std.compress.flate.max_window_len), + .compress => return error.UnsupportedCompressionMethod, + }; + defer if (response.head.content_encoding != .identity) allocator.free(decompress_buffer); + + var transfer_buffer: [64]u8 = undefined; + var decompress: std.http.Decompress = undefined; + const reader = response.readerDecompressing(&transfer_buffer, &decompress, decompress_buffer); + _ = reader.streamRemaining(&aw.writer) catch |err| switch (err) { + error.ReadFailed => return response.bodyErr().?, + else => |e| return e, + }; + + const response_body = aw.writer.buffer[0..aw.writer.end]; + return .{ + .status_code = @as(u16, @intFromEnum(response.head.status)), + .headers = response_headers, + .body = try allocator.dupe(u8, response_body), + }; +} + +pub fn httpRequestWithStatus( + allocator: Allocator, + method: std.http.Method, + url: []const u8, + body: ?[]const u8, + headers: []const []const u8, + content_type: ?[]const u8, + proxy: ?[]const u8, +) !HttpResponse { + const resp = try httpRequestWithStatusAndHeaders(allocator, method, url, body, headers, content_type, proxy); + allocator.free(resp.headers); + return .{ + .status_code = resp.status_code, + .body = resp.body, + }; +} + +pub fn httpRequest( + allocator: Allocator, + method: std.http.Method, + url: []const u8, + body: ?[]const u8, + headers: []const []const u8, + content_type: ?[]const u8, + proxy: ?[]const u8, +) ![]u8 { + const resp = try httpRequestWithStatus(allocator, method, url, body, headers, content_type, proxy); + errdefer allocator.free(resp.body); + if (resp.status_code < 200 or resp.status_code >= 300) return error.HttpStatusError; + return resp.body; } pub fn httpPostJsonWithProxy( @@ -495,6 +645,13 @@ fn curlRequestWithProxy( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) ![]u8 { + if (hasCredentialedCurlArgs(url, headers)) { + const method_enum = std.meta.stringToEnum(std.http.Method, method) orelse return error.UnsupportedHttpMethod; + const content_type = contentTypeHeaderValue(content_type_header) orelse return error.InvalidHeader; + const resp = try httpRequestWithStatus(allocator, method_enum, url, body, headers, content_type, proxy); + return resp.body; + } + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [40][]const u8 = undefined; var argc: usize = 0; @@ -665,6 +822,10 @@ pub fn curlPostWithStatusAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponse { + if (hasCredentialedCurlArgs(url, headers)) { + return httpRequestWithStatus(allocator, .POST, url, body, headers, "application/json", null); + } + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -806,6 +967,10 @@ pub fn curlPostWithStatusHeadersAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponseWithHeaders { + if (hasCredentialedCurlArgs(url, headers)) { + return httpRequestWithStatusAndHeaders(allocator, .POST, url, body, headers, "application/json", null); + } + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [56][]const u8 = undefined; var argc: usize = 0; @@ -967,6 +1132,10 @@ pub fn curlGetWithStatusAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponse { + if (hasCredentialedCurlArgs(url, headers)) { + return httpRequestWithStatus(allocator, .GET, url, null, headers, null, null); + } + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -1085,6 +1254,10 @@ fn curlGetWithProxyAndResolve( resolve_entry: ?[]const u8, max_bytes: usize, ) ![]u8 { + if (hasCredentialedCurlArgs(url, headers)) { + return httpRequest(allocator, .GET, url, null, headers, null, proxy); + } + try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -1478,6 +1651,35 @@ test "credentialed curl argv validation rejects token query" { ); } +test "credentialed curl args route to std http fallback" { + try std.testing.expect(hasCredentialedCurlArgs("https://example.com/v1", &.{"Authorization: Bearer test-token"})); + try std.testing.expect(hasCredentialedCurlArgs("https://example.com/v1?access_token=test-token", &.{})); + try std.testing.expect(!hasCredentialedCurlArgs("https://example.com/v1", &.{"User-Agent: nullclaw-test"})); +} + +test "prepareCurlHeaderArg writes headers outside argv" { + var prepared = try prepareCurlHeaderArg(std.testing.allocator, &.{ "Authorization: Bearer test-token", "X-Test: ok" }); + defer prepared.deinit(std.testing.allocator); + + // Regression: direct curl callers can keep credential headers out of argv. + try std.testing.expect(prepared.uses_temp_file); + try std.testing.expect(prepared.arg != null); + try std.testing.expect(std.mem.startsWith(u8, prepared.arg.?, "@")); + + const file = try std_compat.fs.openFileAbsolute(prepared.arg.?[1..], .{}); + defer file.close(); + const content = try file.readToEndAlloc(std.testing.allocator, 1024); + defer std.testing.allocator.free(content); + try std.testing.expectEqualStrings("Authorization: Bearer test-token\nX-Test: ok\n", content); +} + +test "prepareCurlHeaderArg rejects newline injection" { + try std.testing.expectError( + error.InvalidHeader, + prepareCurlHeaderArg(std.testing.allocator, &.{"Authorization: Bearer test-token\nX-Injected: bad"}), + ); +} + test "credentialed curl argv validation permits non-secret headers" { try validateNoCredentialedCurlArgs("https://example.com/v1", &.{"User-Agent: nullclaw-test"}); } diff --git a/src/memory/engines/api.zig b/src/memory/engines/api.zig index 5928ae3bb..8c3f286bf 100644 --- a/src/memory/engines/api.zig +++ b/src/memory/engines/api.zig @@ -8,6 +8,7 @@ const std = @import("std"); const std_compat = @import("compat"); const Allocator = std.mem.Allocator; const appendJsonEscaped = @import("../../util.zig").appendJsonEscaped; +const http_util = @import("../../http_util.zig"); const root = @import("../root.zig"); const Memory = root.Memory; const MemoryCategory = root.MemoryCategory; @@ -135,7 +136,8 @@ pub const ApiMemory = struct { method: std.http.Method, payload: ?[]const u8, ) !HttpResponse { - // Use curl subprocess so `timeout_ms` is guaranteed to apply. + // Keep curl for deterministic --max-time behavior, but keep headers out + // of argv and send request bodies over stdin. const timeout_secs: u32 = @max(@as(u32, 1), (self.timeout_ms + 999) / 1000); var timeout_buf: [16]u8 = undefined; const timeout_secs_str = std.fmt.bufPrint(&timeout_buf, "{d}", .{timeout_secs}) catch unreachable; @@ -143,6 +145,19 @@ pub const ApiMemory = struct { var auth_header: ?[]u8 = null; defer if (auth_header) |h| alloc.free(h); + var headers_buf: [2][]const u8 = undefined; + var header_count: usize = 0; + headers_buf[header_count] = "Content-Type: application/json"; + header_count += 1; + if (self.api_key) |key| { + auth_header = try std.fmt.allocPrint(alloc, "Authorization: Bearer {s}", .{key}); + headers_buf[header_count] = auth_header.?; + header_count += 1; + } + + var prepared_headers = try http_util.prepareCurlHeaderArg(alloc, headers_buf[0..header_count]); + defer prepared_headers.deinit(alloc); + var argv_buf: [24][]const u8 = undefined; var argc: usize = 0; @@ -160,23 +175,18 @@ pub const ApiMemory = struct { argc += 1; argv_buf[argc] = @tagName(method); argc += 1; - argv_buf[argc] = "--header"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; - if (self.api_key) |key| { - auth_header = try std.fmt.allocPrint(alloc, "Authorization: Bearer {s}", .{key}); + if (prepared_headers.arg) |headers_arg| { argv_buf[argc] = "--header"; argc += 1; - argv_buf[argc] = auth_header.?; + argv_buf[argc] = headers_arg; argc += 1; } - if (payload) |body| { - argv_buf[argc] = "--data"; + if (payload != null) { + argv_buf[argc] = "--data-binary"; argc += 1; - argv_buf[argc] = body; + argv_buf[argc] = "@-"; argc += 1; } @@ -188,13 +198,35 @@ pub const ApiMemory = struct { argc += 1; var child = std_compat.process.Child.init(argv_buf[0..argc], alloc); - child.stdin_behavior = .Ignore; + child.stdin_behavior = if (payload != null) .Pipe else .Ignore; child.stdout_behavior = .Pipe; child.stderr_behavior = .Ignore; child.spawn() catch return error.ApiConnectionError; - const raw_out = child.stdout.?.readToEndAlloc(alloc, 16 * 1024 * 1024) catch return error.ApiConnectionError; + if (payload) |body| { + if (child.stdin) |stdin_file| { + stdin_file.writeAll(body) catch { + stdin_file.close(); + child.stdin = null; + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.ApiConnectionError; + }; + stdin_file.close(); + child.stdin = null; + } else { + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.ApiConnectionError; + } + } + + const raw_out = child.stdout.?.readToEndAlloc(alloc, 16 * 1024 * 1024) catch { + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.ApiConnectionError; + }; defer alloc.free(raw_out); const term = child.wait() catch return error.ApiConnectionError; diff --git a/src/providers/gemini.zig b/src/providers/gemini.zig index c01d1dcd7..858eb5682 100644 --- a/src/providers/gemini.zig +++ b/src/providers/gemini.zig @@ -803,10 +803,6 @@ pub const GeminiProvider = struct { argc += 1; argv_buf[argc] = "POST"; argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; // Add proxy from environment if set const proxy = http_util.getProxyFromEnv(allocator) catch null; @@ -823,10 +819,22 @@ pub const GeminiProvider = struct { defer if (resolve_entry) |entry| allocator.free(entry); http_util.appendCurlResolveArgs(argv_buf[0..], &argc, resolve_entry); + var header_buf: [16][]const u8 = undefined; + var header_count: usize = 0; + header_buf[header_count] = "Content-Type: application/json"; + header_count += 1; for (headers) |hdr| { + if (header_count >= header_buf.len) return error.TooManyHeaders; + header_buf[header_count] = hdr; + header_count += 1; + } + + var prepared_headers = try http_util.prepareCurlHeaderArg(allocator, header_buf[0..header_count]); + defer prepared_headers.deinit(allocator); + if (prepared_headers.arg) |headers_arg| { argv_buf[argc] = "-H"; argc += 1; - argv_buf[argc] = hdr; + argv_buf[argc] = headers_arg; argc += 1; } diff --git a/src/providers/openai_codex.zig b/src/providers/openai_codex.zig index ec8f9d549..4f5988809 100644 --- a/src/providers/openai_codex.zig +++ b/src/providers/openai_codex.zig @@ -392,14 +392,6 @@ fn codexStreamRequest( argc += 1; argv_buf[argc] = "POST"; argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_header; - argc += 1; // Add proxy from environment if set const proxy = http_util.getProxyFromEnv(allocator) catch null; @@ -434,10 +426,24 @@ fn codexStreamRequest( defer if (resolve_entry) |entry| allocator.free(entry); http_util.appendCurlResolveArgs(argv_buf[0..], &argc, resolve_entry); + var header_buf: [16][]const u8 = undefined; + var header_count: usize = 0; + header_buf[header_count] = "Content-Type: application/json"; + header_count += 1; + header_buf[header_count] = auth_header; + header_count += 1; for (extra_headers) |hdr| { + if (header_count >= header_buf.len) return error.TooManyHeaders; + header_buf[header_count] = hdr; + header_count += 1; + } + + var prepared_headers = try http_util.prepareCurlHeaderArg(allocator, header_buf[0..header_count]); + defer prepared_headers.deinit(allocator); + if (prepared_headers.arg) |headers_arg| { argv_buf[argc] = "-H"; argc += 1; - argv_buf[argc] = hdr; + argv_buf[argc] = headers_arg; argc += 1; } diff --git a/src/providers/sse.zig b/src/providers/sse.zig index db1c63f4c..503901e4c 100644 --- a/src/providers/sse.zig +++ b/src/providers/sse.zig @@ -449,10 +449,6 @@ pub fn curlStream( argc += 1; argv_buf[argc] = "POST"; argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; // Add proxy from environment if set const proxy = http_util.getProxyFromEnv(allocator) catch null; @@ -469,60 +465,44 @@ pub fn curlStream( defer if (resolve_entry) |entry| allocator.free(entry); http_util.appendCurlResolveArgs(argv_buf[0..], &argc, resolve_entry); + var header_buf: [16][]const u8 = undefined; + var header_count: usize = 0; + header_buf[header_count] = "Content-Type: application/json"; + header_count += 1; if (auth_header) |auth| { - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth; - argc += 1; + if (header_count >= header_buf.len) return error.TooManyHeaders; + header_buf[header_count] = auth; + header_count += 1; } for (extra_headers) |hdr| { - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = hdr; - argc += 1; + if (header_count >= header_buf.len) return error.TooManyHeaders; + header_buf[header_count] = hdr; + header_count += 1; } - // On Windows, command line length is limited to ~32767 chars. - // Use a temp file there to avoid NameTooLong; keep other platforms in-memory. - var prepared_body = try prepareCurlBodyArg(allocator, body, log_enabled); - defer prepared_body.deinit(allocator); - - if (prepared_body.uses_temp_file) { - argv_buf[argc] = "--data-binary"; + var prepared_headers = try http_util.prepareCurlHeaderArg(allocator, header_buf[0..header_count]); + defer prepared_headers.deinit(allocator); + if (prepared_headers.arg) |headers_arg| { + argv_buf[argc] = "-H"; argc += 1; - } else { - argv_buf[argc] = "-d"; + argv_buf[argc] = headers_arg; argc += 1; } - argv_buf[argc] = prepared_body.arg; + + argv_buf[argc] = "--data-binary"; + argc += 1; + argv_buf[argc] = "@-"; argc += 1; argv_buf[argc] = url; argc += 1; - // Debug: log the curl command - if (log_enabled) { - debug_log.info("curl argc={d}, body_len={d}, used_temp_file={}, body_arg={s}", .{ argc, body.len, prepared_body.uses_temp_file, prepared_body.arg }); - } - - var cmd_buf: std.ArrayListUnmanaged(u8) = .empty; - defer cmd_buf.deinit(allocator); - for (argv_buf[0..argc], 0..) |arg, i| { - if (i > 0) cmd_buf.append(allocator, ' ') catch {}; - // Quote arguments that contain spaces or special chars for easy copy-paste - if (std.mem.indexOfAny(u8, arg, " \t\"'") != null or std.mem.startsWith(u8, arg, "@")) { - cmd_buf.append(allocator, '"') catch {}; - cmd_buf.appendSlice(allocator, arg) catch {}; - cmd_buf.append(allocator, '"') catch {}; - } else { - cmd_buf.appendSlice(allocator, arg) catch {}; - } - } if (log_enabled) { - debug_log.info("curl command: {s}", .{cmd_buf.items}); + debug_log.info("curl argc={d}, body_len={d}, header_file={}", .{ argc, body.len, prepared_headers.uses_temp_file }); } var child = std_compat.process.Child.init(argv_buf[0..argc], allocator); + child.stdin_behavior = .Pipe; child.stdout_behavior = .Pipe; child.stderr_behavior = .Ignore; @@ -535,6 +515,22 @@ pub fn curlStream( debug_log.info("curl process spawned, pid={d}", .{pid}); } + if (child.stdin) |stdin_file| { + stdin_file.writeAll(body) catch { + stdin_file.close(); + child.stdin = null; + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.CurlWriteError; + }; + stdin_file.close(); + child.stdin = null; + } else { + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.CurlWriteError; + } + // Read stdout line by line, parse SSE events var accumulated: std.ArrayListUnmanaged(u8) = .empty; defer accumulated.deinit(allocator); @@ -827,10 +823,6 @@ pub fn curlStreamAnthropic( argc += 1; argv_buf[argc] = "POST"; argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; // Add proxy from environment if set const proxy = http_util.getProxyFromEnv(allocator) catch null; @@ -847,34 +839,55 @@ pub fn curlStreamAnthropic( defer if (resolve_entry) |entry| allocator.free(entry); http_util.appendCurlResolveArgs(argv_buf[0..], &argc, resolve_entry); + var header_buf: [16][]const u8 = undefined; + var header_count: usize = 0; + header_buf[header_count] = "Content-Type: application/json"; + header_count += 1; for (headers) |hdr| { + if (header_count >= header_buf.len) return error.TooManyHeaders; + header_buf[header_count] = hdr; + header_count += 1; + } + + var prepared_headers = try http_util.prepareCurlHeaderArg(allocator, header_buf[0..header_count]); + defer prepared_headers.deinit(allocator); + if (prepared_headers.arg) |headers_arg| { argv_buf[argc] = "-H"; argc += 1; - argv_buf[argc] = hdr; + argv_buf[argc] = headers_arg; argc += 1; } - const log_enabled = verbose.isVerbose(); - var prepared_body = try prepareCurlBodyArg(allocator, body, log_enabled); - defer prepared_body.deinit(allocator); - - if (prepared_body.uses_temp_file) { - argv_buf[argc] = "--data-binary"; - } else { - argv_buf[argc] = "-d"; - } + argv_buf[argc] = "--data-binary"; argc += 1; - argv_buf[argc] = prepared_body.arg; + argv_buf[argc] = "@-"; argc += 1; argv_buf[argc] = url; argc += 1; var child = std_compat.process.Child.init(argv_buf[0..argc], allocator); + child.stdin_behavior = .Pipe; child.stdout_behavior = .Pipe; child.stderr_behavior = .Ignore; try child.spawn(); + if (child.stdin) |stdin_file| { + stdin_file.writeAll(body) catch { + stdin_file.close(); + child.stdin = null; + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.CurlWriteError; + }; + stdin_file.close(); + child.stdin = null; + } else { + _ = child.kill() catch {}; + _ = child.wait() catch {}; + return error.CurlWriteError; + } + // Read stdout line by line, parse Anthropic SSE events var accumulated: std.ArrayListUnmanaged(u8) = .empty; defer accumulated.deinit(allocator); diff --git a/src/tools/composio.zig b/src/tools/composio.zig index c9c8c4e16..40ca75ffe 100644 --- a/src/tools/composio.zig +++ b/src/tools/composio.zig @@ -1,5 +1,6 @@ const std = @import("std"); const appendJsonEscaped = @import("../util.zig").appendJsonEscaped; +const http_util = @import("../http_util.zig"); const root = @import("root.zig"); const Tool = root.Tool; const ToolResult = root.ToolResult; @@ -223,36 +224,7 @@ pub const ComposioTool = struct { const body = try v2_body_buf.toOwnedSlice(allocator); defer allocator.free(body); - var argv_buf: [20][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-sL"; - argc += 1; - argv_buf[argc] = "-m"; - argc += 1; - argv_buf[argc] = "15"; - argc += 1; - argv_buf[argc] = "-X"; - argc += 1; - argv_buf[argc] = "POST"; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_header; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; - argv_buf[argc] = "-d"; - argc += 1; - argv_buf[argc] = body; - argc += 1; - argv_buf[argc] = "https://backend.composio.dev/api/v1/connectedAccounts"; - argc += 1; - - return self.runCurl(allocator, argv_buf[0..argc]); + return self.httpPostWithAuth(allocator, "https://backend.composio.dev/api/v1/connectedAccounts", body, auth_header); } // ── HTTP helpers ─────────────────────────────────────────────── @@ -261,81 +233,36 @@ pub const ComposioTool = struct { const auth_header = try std.fmt.allocPrint(allocator, "x-api-key: {s}", .{self.api_key}); defer allocator.free(auth_header); - var argv_buf: [20][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-sL"; - argc += 1; - argv_buf[argc] = "-m"; - argc += 1; - argv_buf[argc] = "15"; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_header; - argc += 1; - argv_buf[argc] = url; - argc += 1; - - return self.runCurl(allocator, argv_buf[0..argc]); + const resp = http_util.httpRequestWithStatus(allocator, .GET, url, null, &.{auth_header}, null, null) catch |err| + return toolHttpError(allocator, err); + return toolHttpResponse(allocator, resp); } fn httpPost(self: *ComposioTool, allocator: std.mem.Allocator, url: []const u8, body: []const u8) !ToolResult { const auth_header = try std.fmt.allocPrint(allocator, "x-api-key: {s}", .{self.api_key}); defer allocator.free(auth_header); - var argv_buf: [20][]const u8 = undefined; - var argc: usize = 0; - argv_buf[argc] = "curl"; - argc += 1; - argv_buf[argc] = "-sL"; - argc += 1; - argv_buf[argc] = "-m"; - argc += 1; - argv_buf[argc] = "15"; - argc += 1; - argv_buf[argc] = "-X"; - argc += 1; - argv_buf[argc] = "POST"; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = auth_header; - argc += 1; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = "Content-Type: application/json"; - argc += 1; - argv_buf[argc] = "-d"; - argc += 1; - argv_buf[argc] = body; - argc += 1; - argv_buf[argc] = url; - argc += 1; - - return self.runCurl(allocator, argv_buf[0..argc]); + return self.httpPostWithAuth(allocator, url, body, auth_header); } - /// Run curl as a child process and return stdout on success, stderr on failure. - fn runCurl(_: *ComposioTool, allocator: std.mem.Allocator, argv: []const []const u8) !ToolResult { - const proc = @import("process_util.zig"); - const result = try proc.run(allocator, argv, .{}); - defer allocator.free(result.stderr); - if (result.success) { - if (result.stdout.len > 0) return ToolResult{ .success = true, .output = result.stdout }; - allocator.free(result.stdout); - return ToolResult{ .success = true, .output = try allocator.dupe(u8, "(empty response)") }; - } - defer allocator.free(result.stdout); - if (result.exit_code != null) { - const err_out = try allocator.dupe(u8, if (result.stderr.len > 0) result.stderr else "curl failed with non-zero exit code"); - return ToolResult{ .success = false, .output = "", .error_msg = err_out }; - } - return ToolResult{ .success = false, .output = "", .error_msg = "curl terminated by signal" }; + fn httpPostWithAuth(_: *ComposioTool, allocator: std.mem.Allocator, url: []const u8, body: []const u8, auth_header: []const u8) !ToolResult { + const resp = http_util.httpRequestWithStatus(allocator, .POST, url, body, &.{auth_header}, "application/json", null) catch |err| + return toolHttpError(allocator, err); + return toolHttpResponse(allocator, resp); } }; +fn toolHttpResponse(allocator: std.mem.Allocator, resp: http_util.HttpResponse) !ToolResult { + if (resp.body.len > 0) return ToolResult{ .success = true, .output = resp.body }; + allocator.free(resp.body); + return ToolResult{ .success = true, .output = try allocator.dupe(u8, "(empty response)") }; +} + +fn toolHttpError(allocator: std.mem.Allocator, err: anyerror) !ToolResult { + const msg = try std.fmt.allocPrint(allocator, "HTTP request failed: {s}", .{@errorName(err)}); + return ToolResult{ .success = false, .output = "", .error_msg = msg }; +} + // ── Helper functions ──────────────────────────────────────────────── /// Convert UPPER_SNAKE_CASE to kebab-case: GMAIL_FETCH_EMAILS -> gmail-fetch-emails diff --git a/src/tools/http_request.zig b/src/tools/http_request.zig index 71193208b..c2cad3156 100644 --- a/src/tools/http_request.zig +++ b/src/tools/http_request.zig @@ -246,7 +246,6 @@ fn runCurlRequestWithStatus( var argv_buf: [64][]const u8 = undefined; var argc: usize = 0; - const reserved_tail_args: usize = if (body != null) 5 else 3; argv_buf[argc] = "curl"; argc += 1; @@ -280,14 +279,17 @@ fn runCurlRequestWithStatus( } for (headers) |h| { - // Reserve room for trailing args: - // -w "\n%{http_code}" and optional --data-binary @- - if (argc + 2 + reserved_tail_args > argv_buf.len) break; const line = try std.fmt.allocPrint(allocator, "{s}: {s}", .{ h[0], h[1] }); try header_lines.append(allocator, line); + } + + var prepared_headers = try http_util.prepareCurlHeaderArg(allocator, header_lines.items); + defer prepared_headers.deinit(allocator); + if (prepared_headers.arg) |headers_arg| { + if (argc + 2 > argv_buf.len) return error.CurlArgsOverflow; argv_buf[argc] = "-H"; argc += 1; - argv_buf[argc] = line; + argv_buf[argc] = headers_arg; argc += 1; } diff --git a/src/voice.zig b/src/voice.zig index f4a55f843..ddf988165 100644 --- a/src/voice.zig +++ b/src/voice.zig @@ -282,11 +282,13 @@ fn curlPostFromFile( argv_buf[argc] = "POST"; argc += 1; - for (headers) |hdr| { - if (argc + 2 > argv_buf.len) break; + var prepared_headers = try http_util.prepareCurlHeaderArg(allocator, headers); + defer prepared_headers.deinit(allocator); + if (prepared_headers.arg) |headers_arg| { + if (argc + 2 > argv_buf.len) return error.CurlFailed; argv_buf[argc] = "-H"; argc += 1; - argv_buf[argc] = hdr; + argv_buf[argc] = headers_arg; argc += 1; } From 53bfcc0d624e44d5a664763aa89a0be22f626951 Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Wed, 27 May 2026 23:41:59 -0300 Subject: [PATCH 23/27] fix: harden curl header temp files --- src/http_util.zig | 31 +++++++----- src/providers/sse.zig | 106 ------------------------------------------ 2 files changed, 20 insertions(+), 117 deletions(-) diff --git a/src/http_util.zig b/src/http_util.zig index ef3a1c33e..e20b94d06 100644 --- a/src/http_util.zig +++ b/src/http_util.zig @@ -236,19 +236,28 @@ pub fn prepareCurlHeaderArg(allocator: Allocator, headers: []const []const u8) ! var tmp_dir = std_compat.fs.openDirAbsolute(tmp_dir_path, .{}) catch return error.TempDirNotFound; defer tmp_dir.close(); - const header_path = std.fmt.bufPrint( - &prepared.temp_path_buf, - "{s}{s}curl_headers_{d}.tmp", - .{ tmp_dir_path, std_compat.fs.path.sep_str, std_compat.time.nanoTimestamp() }, - ) catch return error.PathTooLong; - prepared.temp_path_len = header_path.len; + var tmp_file = blk: { + var attempt: u8 = 0; + while (attempt < 8) : (attempt += 1) { + const header_path = std.fmt.bufPrint( + &prepared.temp_path_buf, + "{s}{s}curl_headers_{x}.tmp", + .{ tmp_dir_path, std_compat.fs.path.sep_str, std_compat.crypto.random.int(u64) }, + ) catch return error.PathTooLong; + prepared.temp_path_len = header_path.len; + + break :blk tmp_dir.createFile( + header_path[tmp_dir_path.len + 1 ..], + .{ .truncate = true, .exclusive = true, .permissions = std_compat.fs.permissionsFromMode(0o600) }, + ) catch |err| switch (err) { + error.PathAlreadyExists => continue, + else => return error.TempFileCreateFailed, + }; + } + return error.TempFileCreateFailed; + }; errdefer std_compat.fs.deleteFileAbsolute(prepared.temp_path_buf[0..prepared.temp_path_len]) catch {}; - var tmp_file = tmp_dir.createFile( - header_path[tmp_dir_path.len + 1 ..], - .{ .truncate = true, .exclusive = false, .permissions = std_compat.fs.permissionsFromMode(0o600) }, - ) catch return error.TempFileCreateFailed; - for (headers) |header| { validateCurlHeaderLine(header) catch { tmp_file.close(); diff --git a/src/providers/sse.zig b/src/providers/sse.zig index 503901e4c..6728ce6de 100644 --- a/src/providers/sse.zig +++ b/src/providers/sse.zig @@ -1,18 +1,11 @@ const std = @import("std"); const std_compat = @import("compat"); -const builtin = @import("builtin"); const root = @import("root.zig"); -const fs_compat = @import("../fs_compat.zig"); const http_util = @import("../http_util.zig"); -const platform = @import("../platform.zig"); const error_classify = @import("error_classify.zig"); const verbose = @import("../verbose.zig"); const log = std.log.scoped(.provider_sse); -// Keep large request bodies out of argv. On Linux, a single oversized `-d` -// argument can hit execve limits long before the total ARG_MAX budget. -const MAX_INLINE_CURL_BODY_BYTES: usize = 64 * 1024; - var curl_fail_fast_arg_mutex: std_compat.sync.Mutex = .{}; var curl_fail_with_body_supported_cache: ?bool = null; const stream_stall_detection_args = [_][]const u8{ @@ -116,80 +109,6 @@ pub fn appendCurlStallDetectionArgs(argv_buf: [][]const u8, argc: *usize) void { } } -const CurlBodyArg = struct { - arg: []const u8, - temp_path_buf: [std_compat.fs.max_path_bytes]u8 = undefined, - temp_path_len: usize = 0, - uses_temp_file: bool = false, - - fn deinit(self: *const CurlBodyArg, allocator: std.mem.Allocator) void { - if (!self.uses_temp_file) return; - std_compat.fs.deleteFileAbsolute(self.temp_path_buf[0..self.temp_path_len]) catch {}; - allocator.free(self.arg); - } -}; - -fn prepareCurlBodyArg( - allocator: std.mem.Allocator, - body: []const u8, - log_enabled: bool, -) !CurlBodyArg { - const should_use_temp_file = builtin.os.tag == .windows or body.len > MAX_INLINE_CURL_BODY_BYTES; - if (!should_use_temp_file) { - return .{ .arg = body }; - } - - const debug_log = std.log.scoped(.sse); - var prepared: CurlBodyArg = .{ .arg = body }; - - const tmp_dir_path = platform.getTempDir(allocator) catch - return error.TempDirNotFound; - defer allocator.free(tmp_dir_path); - - var tmp_dir = std_compat.fs.openDirAbsolute(tmp_dir_path, .{}) catch - return error.TempDirNotFound; - defer tmp_dir.close(); - - const body_path = std.fmt.bufPrint( - &prepared.temp_path_buf, - "{s}{s}sse_body_{d}.tmp", - .{ tmp_dir_path, std_compat.fs.path.sep_str, std_compat.time.timestamp() }, - ) catch return error.PathTooLong; - prepared.temp_path_len = body_path.len; - errdefer std_compat.fs.deleteFileAbsolute(prepared.temp_path_buf[0..prepared.temp_path_len]) catch {}; - - var tmp_file = tmp_dir.createFile( - body_path[tmp_dir_path.len + 1 ..], - .{ .truncate = true, .exclusive = false }, - ) catch return error.TempFileCreateFailed; - - tmp_file.writeAll(body) catch { - tmp_file.close(); - return error.TempFileWriteFailed; - }; - tmp_file.close(); - - if (log_enabled) { - debug_log.info("Using temp file for curl body: {s}, body_len={d}", .{ body_path, body.len }); - } - - const verify_file = std_compat.fs.openFileAbsolute(body_path, .{}) catch return error.TempFileCreateFailed; - defer verify_file.close(); - const verify_stat = fs_compat.stat(verify_file) catch return error.TempFileCreateFailed; - if (log_enabled) { - debug_log.info("Temp body file size: {d} bytes", .{verify_stat.size}); - } - - for (prepared.temp_path_buf[0..prepared.temp_path_len]) |*c| { - if (c.* == '\\') c.* = '/'; - } - - prepared.arg = try std.fmt.allocPrint(allocator, "@{s}", .{prepared.temp_path_buf[0..prepared.temp_path_len]}); - errdefer allocator.free(prepared.arg); - prepared.uses_temp_file = true; - return prepared; -} - /// Content delta from an SSE chunk. pub const DeltaContent = union(enum) { text: []const u8, @@ -1007,31 +926,6 @@ test "parseSseLine valid delta without optional space" { } } -test "prepareCurlBodyArg keeps small bodies inline except on Windows" { - const allocator = std.testing.allocator; - const body = [_]u8{'x'} ** 4096; - var prepared = try prepareCurlBodyArg(allocator, body[0..], false); - defer prepared.deinit(allocator); - - if (builtin.os.tag == .windows) { - try std.testing.expect(prepared.uses_temp_file); - try std.testing.expect(std.mem.startsWith(u8, prepared.arg, "@")); - } else { - try std.testing.expect(!prepared.uses_temp_file); - try std.testing.expectEqualStrings(body[0..], prepared.arg); - } -} - -test "prepareCurlBodyArg spills large bodies to temp file" { - const allocator = std.testing.allocator; - const body = [_]u8{'x'} ** (MAX_INLINE_CURL_BODY_BYTES + 1); - var prepared = try prepareCurlBodyArg(allocator, body[0..], false); - defer prepared.deinit(allocator); - - try std.testing.expect(prepared.uses_temp_file); - try std.testing.expect(std.mem.startsWith(u8, prepared.arg, "@")); -} - test "appendCurlStallDetectionArgs appends curl speed flags in order" { // Regression: stalled SSE streams must trip curl's speed-limit instead of // hanging until --max-time expires with an idle-but-open connection. From 944d5963d3ec6157ed5ab478fbc811f70cb44b04 Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Wed, 27 May 2026 23:47:07 -0300 Subject: [PATCH 24/27] fix: redact verbose SSE payload logs --- src/providers/sse.zig | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/providers/sse.zig b/src/providers/sse.zig index 6728ce6de..77495a232 100644 --- a/src/providers/sse.zig +++ b/src/providers/sse.zig @@ -273,11 +273,14 @@ fn extractStreamUsage(json_str: []const u8) ?root.TokenUsage { /// Returns owned DeltaContent or null if no content found. pub fn extractDeltaContent(allocator: std.mem.Allocator, json_str: []const u8) !?DeltaContent { if (verbose.isVerbose()) { - log.debug("SSE JSON: {s}", .{json_str}); + // NOTE: No unit test for this log path; it depends on global verbose + // logging state. Keep payload bytes out of logs because SSE chunks can + // contain user prompts, tool results, or model output. + log.debug("SSE JSON payload received: len={d}", .{json_str.len}); } const parsed = std.json.parseFromSlice(std.json.Value, allocator, json_str, .{}) catch |err| { - if (verbose.isVerbose()) log.err("Failed to parse SSE JSON: {s} | Error: {s}", .{ json_str, @errorName(err) }); + if (verbose.isVerbose()) log.err("Failed to parse SSE JSON payload: len={d} error={s}", .{ json_str.len, @errorName(err) }); return error.InvalidSseJson; }; defer parsed.deinit(); @@ -480,7 +483,7 @@ pub fn curlStream( total_stdout += n; if (log_enabled) { - debug_log.info("stdout read {d} bytes: {s}", .{ n, read_buf[0..n] }); + debug_log.info("stdout read {d} bytes", .{n}); } // Check if this is JSON (starts with '{') @@ -504,14 +507,14 @@ pub fn curlStream( // Return a meaningful error _ = child.wait() catch {}; - debug_log.err("Server returned JSON error: {s}", .{json_response}); + debug_log.err("Server returned JSON error payload: len={d}", .{json_response.len}); return error.ServerError; } for (read_buf[0..n]) |byte| { if (byte == '\n') { if (log_enabled) { - debug_log.info("parsing SSE line: {s}", .{line_buf.items}); + debug_log.info("parsing SSE line: len={d}", .{line_buf.items.len}); } const result = parseSseLine(allocator, line_buf.items) catch { line_buf.clearRetainingCapacity(); From 97966e9d33899ceff5ab3fce7bfcb5379df1dd6a Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Wed, 27 May 2026 23:52:36 -0300 Subject: [PATCH 25/27] fix: harden compose init configuration --- README.md | 7 +++++-- docker-compose.yml | 12 +++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 42bf3c4b6..a36d46572 100644 --- a/README.md +++ b/README.md @@ -173,12 +173,15 @@ make up ``` `make config` runs `nullclaw onboard --interactive` inside the agent container. -For non-interactive setup: +Pass only non-secret flags through `CONFIG_ARGS`: ```bash -make config CONFIG_ARGS="--api-key sk-... --provider openrouter" +make config CONFIG_ARGS="--provider openrouter" ``` +Avoid putting API keys or bot tokens in `CONFIG_ARGS`; command-line arguments +can be exposed through shell history and process listings. + The compose gateway defaults to `NULLCLAW_PORT=3210`, binds inside the container on `0.0.0.0`, and publishes to localhost on the host: diff --git a/docker-compose.yml b/docker-compose.yml index 0d3e3a1e2..d18c7f9ee 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,7 +10,17 @@ services: init-data: image: alpine:3.23 profiles: ["agent", "gateway"] - command: ["sh", "-lc", "mkdir -p /nullclaw-data/workspace && chown -R ${NULLCLAW_UID:-65534}:${NULLCLAW_GID:-65534} /nullclaw-data"] + environment: + NULLCLAW_UID: "${NULLCLAW_UID:-65534}" + NULLCLAW_GID: "${NULLCLAW_GID:-65534}" + command: + - sh + - -ec + - | + case "$$NULLCLAW_UID" in (""|*[!0-9]*) echo "invalid NULLCLAW_UID" >&2; exit 1;; esac + case "$$NULLCLAW_GID" in (""|*[!0-9]*) echo "invalid NULLCLAW_GID" >&2; exit 1;; esac + mkdir -p /nullclaw-data/workspace + chown -R "$$NULLCLAW_UID:$$NULLCLAW_GID" /nullclaw-data volumes: - nullclaw-data:/nullclaw-data - ./workspace:/nullclaw-data/workspace From 0eaa762abcea21f9d1585445cf3e7f51cfb7b02c Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Wed, 27 May 2026 23:59:00 -0300 Subject: [PATCH 26/27] test: cover credentialed legacy curl fallbacks --- src/http_util.zig | 170 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/src/http_util.zig b/src/http_util.zig index e20b94d06..88b86c837 100644 --- a/src/http_util.zig +++ b/src/http_util.zig @@ -1666,6 +1666,176 @@ test "credentialed curl args route to std http fallback" { try std.testing.expect(!hasCredentialedCurlArgs("https://example.com/v1", &.{"User-Agent: nullclaw-test"})); } +const LegacyCredentialedCurlHelper = enum { + get_body, + get_status, + post_body, + post_status, + post_status_headers, + put_body, +}; + +const CredentialedCurlFallbackServerCtx = struct { + server: *std_compat.net.Server, + expected_method: []const u8, + saw_request: AtomicBool = AtomicBool.init(false), + saw_expected_method: AtomicBool = AtomicBool.init(false), + saw_authorization: AtomicBool = AtomicBool.init(false), +}; + +fn serveCredentialedCurlFallbackTest(ctx: *CredentialedCurlFallbackServerCtx) void { + var conn = ctx.server.accept() catch return; + defer conn.stream.close(); + + var buf: [2048]u8 = undefined; + var filled: usize = 0; + while (filled < buf.len) { + const n = conn.stream.read(buf[filled..]) catch return; + if (n == 0) break; + filled += n; + if (std.mem.indexOf(u8, buf[0..filled], "\r\n\r\n") != null) break; + } + + const request = buf[0..filled]; + ctx.saw_request.store(true, .release); + if (std.mem.startsWith(u8, request, ctx.expected_method) and + request.len > ctx.expected_method.len and + request[ctx.expected_method.len] == ' ') + { + ctx.saw_expected_method.store(true, .release); + } + if (std.mem.indexOf(u8, request, "Authorization: Bearer test-token") != null) { + ctx.saw_authorization.store(true, .release); + } + + const response = + "HTTP/1.1 200 OK\r\n" ++ + "Content-Type: application/json\r\n" ++ + "Content-Length: 11\r\n" ++ + "Connection: close\r\n" ++ + "\r\n" ++ + "{\"ok\":true}"; + conn.stream.writeAll(response) catch {}; +} + +fn unblockCredentialedCurlFallbackServer(server: *std_compat.net.Server) void { + var conn = std_compat.net.tcpConnectToAddress(server.listen_address) catch return; + conn.close(); +} + +fn expectLegacyCredentialedCurlFallback(helper: LegacyCredentialedCurlHelper, expected_method: []const u8) !void { + if (comptime @import("builtin").os.tag == .wasi) return error.SkipZigTest; + + const allocator = std.testing.allocator; + const addr = try std_compat.net.Address.resolveIp("127.0.0.1", 0); + var server = try addr.listen(.{}); + defer server.deinit(); + + var ctx = CredentialedCurlFallbackServerCtx{ + .server = &server, + .expected_method = expected_method, + }; + var thread = try std.Thread.spawn(.{}, serveCredentialedCurlFallbackTest, .{&ctx}); + + const url = try std.fmt.allocPrint(allocator, "http://127.0.0.1:{d}/legacy", .{server.listen_address.in.getPort()}); + defer allocator.free(url); + const headers = [_][]const u8{"Authorization: Bearer test-token"}; + var request_err: ?anyerror = null; + + switch (helper) { + .get_body => { + const body = curlGet(allocator, url, &headers, "5") catch |err| blk: { + request_err = err; + break :blk null; + }; + if (body) |b| { + defer allocator.free(b); + try std.testing.expectEqualStrings("{\"ok\":true}", b); + } + }, + .get_status => { + const resp = curlGetWithStatus(allocator, url, &headers) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (resp) |r| { + defer allocator.free(r.body); + try std.testing.expectEqual(@as(u16, 200), r.status_code); + try std.testing.expectEqualStrings("{\"ok\":true}", r.body); + } + }, + .post_body => { + const body = curlPost(allocator, url, "{\"ping\":true}", &headers) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (body) |b| { + defer allocator.free(b); + try std.testing.expectEqualStrings("{\"ok\":true}", b); + } + }, + .post_status => { + const resp = curlPostWithStatus(allocator, url, "{\"ping\":true}", &headers) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (resp) |r| { + defer allocator.free(r.body); + try std.testing.expectEqual(@as(u16, 200), r.status_code); + try std.testing.expectEqualStrings("{\"ok\":true}", r.body); + } + }, + .post_status_headers => { + const resp = curlPostWithStatusHeadersAndTimeout(allocator, url, "{\"ping\":true}", &headers, null) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (resp) |r| { + defer allocator.free(r.headers); + defer allocator.free(r.body); + try std.testing.expectEqual(@as(u16, 200), r.status_code); + try std.testing.expectEqualStrings("{\"ok\":true}", r.body); + } + }, + .put_body => { + const body = curlPut(allocator, url, "{\"ping\":true}", &headers) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (body) |b| { + defer allocator.free(b); + try std.testing.expectEqualStrings("{\"ok\":true}", b); + } + }, + } + + if (!ctx.saw_request.load(.acquire)) { + unblockCredentialedCurlFallbackServer(&server); + } + thread.join(); + + if (request_err) |err| return err; + try std.testing.expect(ctx.saw_expected_method.load(.acquire)); + try std.testing.expect(ctx.saw_authorization.load(.acquire)); +} + +test "credentialed legacy curl body helpers do not reject authorization headers" { + // Regression: legacy channel code still passes Authorization to curl* helper + // APIs. These helpers must route through std.http fallback instead of + // returning CredentialedCurlArgRejected and breaking old channels. + try expectLegacyCredentialedCurlFallback(.get_body, "GET"); + try expectLegacyCredentialedCurlFallback(.post_body, "POST"); + try expectLegacyCredentialedCurlFallback(.put_body, "PUT"); +} + +test "credentialed legacy curl status helpers do not reject authorization headers" { + // Regression: status-returning helpers used by Lark/QQ/OneBot must preserve + // behavior while keeping Authorization out of curl argv. + try expectLegacyCredentialedCurlFallback(.get_status, "GET"); + try expectLegacyCredentialedCurlFallback(.post_status, "POST"); + try expectLegacyCredentialedCurlFallback(.post_status_headers, "POST"); +} + test "prepareCurlHeaderArg writes headers outside argv" { var prepared = try prepareCurlHeaderArg(std.testing.allocator, &.{ "Authorization: Bearer test-token", "X-Test: ok" }); defer prepared.deinit(std.testing.allocator); From 2765c284d59a1bd155b598b5a7c519c27f500e15 Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Thu, 28 May 2026 00:08:17 -0300 Subject: [PATCH 27/27] fix: preserve curl resolve pinning for credentials --- src/http_util.zig | 217 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 172 insertions(+), 45 deletions(-) diff --git a/src/http_util.zig b/src/http_util.zig index 88b86c837..32e7e87e3 100644 --- a/src/http_util.zig +++ b/src/http_util.zig @@ -283,6 +283,44 @@ pub fn prepareCurlHeaderArg(allocator: Allocator, headers: []const []const u8) ! return prepared; } +fn credentialedCurlUsesHttpFallback(url: []const u8, headers: []const []const u8, resolve_entry: ?[]const u8) bool { + return hasCredentialedCurlArgs(url, headers) and resolve_entry == null; +} + +fn prepareCurlHeadersForArgv(allocator: Allocator, url: []const u8, headers: []const []const u8) !CurlHeaderArg { + if (hasCredentialedCurlArgs(url, headers)) { + if (hasSensitiveUrlToken(url)) return error.CredentialedCurlArgRejected; + return try prepareCurlHeaderArg(allocator, headers); + } + + try validateNoCredentialedCurlArgs(url, headers); + return .{}; +} + +fn appendPreparedCurlHeaders( + argv_buf: []([]const u8), + argc: *usize, + headers: []const []const u8, + prepared_arg: ?[]const u8, +) !void { + if (prepared_arg) |headers_arg| { + if (argc.* + 2 > argv_buf.len) return error.CurlArgsOverflow; + argv_buf[argc.*] = "-H"; + argc.* += 1; + argv_buf[argc.*] = headers_arg; + argc.* += 1; + return; + } + + for (headers) |hdr| { + if (argc.* + 2 > argv_buf.len) break; + argv_buf[argc.*] = "-H"; + argc.* += 1; + argv_buf[argc.*] = hdr; + argc.* += 1; + } +} + fn parseHeader(header: []const u8) ?std.http.Header { const colon = std.mem.indexOfScalar(u8, header, ':') orelse return null; const name = std.mem.trim(u8, header[0..colon], " \t\r\n"); @@ -654,14 +692,15 @@ fn curlRequestWithProxy( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) ![]u8 { - if (hasCredentialedCurlArgs(url, headers)) { + if (credentialedCurlUsesHttpFallback(url, headers, resolve_entry)) { const method_enum = std.meta.stringToEnum(std.http.Method, method) orelse return error.UnsupportedHttpMethod; const content_type = contentTypeHeaderValue(content_type_header) orelse return error.InvalidHeader; const resp = try httpRequestWithStatus(allocator, method_enum, url, body, headers, content_type, proxy); return resp.body; } + var prepared_headers = try prepareCurlHeadersForArgv(allocator, url, headers); + defer prepared_headers.deinit(allocator); - try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [40][]const u8 = undefined; var argc: usize = 0; @@ -694,13 +733,7 @@ fn curlRequestWithProxy( argc += 1; } - for (headers) |hdr| { - if (argc + 2 > argv_buf.len) break; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = hdr; - argc += 1; - } + try appendPreparedCurlHeaders(argv_buf[0..], &argc, headers, prepared_headers.arg); // Pass payload via stdin to avoid OS argv length limits for large JSON // bodies (e.g. multimodal base64 images). @@ -831,11 +864,12 @@ pub fn curlPostWithStatusAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponse { - if (hasCredentialedCurlArgs(url, headers)) { + if (credentialedCurlUsesHttpFallback(url, headers, resolve_entry)) { return httpRequestWithStatus(allocator, .POST, url, body, headers, "application/json", null); } + var prepared_headers = try prepareCurlHeadersForArgv(allocator, url, headers); + defer prepared_headers.deinit(allocator); - try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -862,13 +896,7 @@ pub fn curlPostWithStatusAndTimeoutAndResolve( argv_buf[argc] = "Content-Type: application/json"; argc += 1; - for (headers) |hdr| { - if (argc + 2 > argv_buf.len) break; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = hdr; - argc += 1; - } + try appendPreparedCurlHeaders(argv_buf[0..], &argc, headers, prepared_headers.arg); argv_buf[argc] = "--data-binary"; argc += 1; @@ -976,11 +1004,12 @@ pub fn curlPostWithStatusHeadersAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponseWithHeaders { - if (hasCredentialedCurlArgs(url, headers)) { + if (credentialedCurlUsesHttpFallback(url, headers, resolve_entry)) { return httpRequestWithStatusAndHeaders(allocator, .POST, url, body, headers, "application/json", null); } + var prepared_headers = try prepareCurlHeadersForArgv(allocator, url, headers); + defer prepared_headers.deinit(allocator); - try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [56][]const u8 = undefined; var argc: usize = 0; @@ -1007,13 +1036,7 @@ pub fn curlPostWithStatusHeadersAndTimeoutAndResolve( argv_buf[argc] = "Content-Type: application/json"; argc += 1; - for (headers) |hdr| { - if (argc + 2 > argv_buf.len) break; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = hdr; - argc += 1; - } + try appendPreparedCurlHeaders(argv_buf[0..], &argc, headers, prepared_headers.arg); // Dump response headers to stdout so we can capture session IDs. argv_buf[argc] = "-D"; @@ -1141,11 +1164,12 @@ pub fn curlGetWithStatusAndTimeoutAndResolve( max_time: ?[]const u8, resolve_entry: ?[]const u8, ) !HttpResponse { - if (hasCredentialedCurlArgs(url, headers)) { + if (credentialedCurlUsesHttpFallback(url, headers, resolve_entry)) { return httpRequestWithStatus(allocator, .GET, url, null, headers, null, null); } + var prepared_headers = try prepareCurlHeadersForArgv(allocator, url, headers); + defer prepared_headers.deinit(allocator); - try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -1163,13 +1187,7 @@ pub fn curlGetWithStatusAndTimeoutAndResolve( appendCurlResolveArgs(argv_buf[0..], &argc, resolve_entry); - for (headers) |hdr| { - if (argc + 2 > argv_buf.len) break; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = hdr; - argc += 1; - } + try appendPreparedCurlHeaders(argv_buf[0..], &argc, headers, prepared_headers.arg); argv_buf[argc] = "-w"; argc += 1; @@ -1263,11 +1281,12 @@ fn curlGetWithProxyAndResolve( resolve_entry: ?[]const u8, max_bytes: usize, ) ![]u8 { - if (hasCredentialedCurlArgs(url, headers)) { + if (credentialedCurlUsesHttpFallback(url, headers, resolve_entry)) { return httpRequest(allocator, .GET, url, null, headers, null, proxy); } + var prepared_headers = try prepareCurlHeadersForArgv(allocator, url, headers); + defer prepared_headers.deinit(allocator); - try validateNoCredentialedCurlArgs(url, headers); var argv_buf: [48][]const u8 = undefined; var argc: usize = 0; @@ -1294,13 +1313,7 @@ fn curlGetWithProxyAndResolve( argc += 1; } - for (headers) |hdr| { - if (argc + 2 > argv_buf.len) break; - argv_buf[argc] = "-H"; - argc += 1; - argv_buf[argc] = hdr; - argc += 1; - } + try appendPreparedCurlHeaders(argv_buf[0..], &argc, headers, prepared_headers.arg); argv_buf[argc] = url; argc += 1; @@ -1819,6 +1832,97 @@ fn expectLegacyCredentialedCurlFallback(helper: LegacyCredentialedCurlHelper, ex try std.testing.expect(ctx.saw_authorization.load(.acquire)); } +fn expectCredentialedCurlResolveEntry(helper: LegacyCredentialedCurlHelper, expected_method: []const u8) !void { + if (comptime @import("builtin").os.tag == .wasi) return error.SkipZigTest; + + const allocator = std.testing.allocator; + const addr = try std_compat.net.Address.resolveIp("127.0.0.1", 0); + var server = try addr.listen(.{}); + defer server.deinit(); + + var ctx = CredentialedCurlFallbackServerCtx{ + .server = &server, + .expected_method = expected_method, + }; + var thread = try std.Thread.spawn(.{}, serveCredentialedCurlFallbackTest, .{&ctx}); + + const host = "credentialed-curl.test"; + const port = server.listen_address.in.getPort(); + const url = try std.fmt.allocPrint(allocator, "http://{s}:{d}/legacy", .{ host, port }); + defer allocator.free(url); + const resolve_entry = try std.fmt.allocPrint(allocator, "{s}:{d}:127.0.0.1", .{ host, port }); + defer allocator.free(resolve_entry); + const headers = [_][]const u8{"Authorization: Bearer test-token"}; + var request_err: ?anyerror = null; + + switch (helper) { + .get_body => { + const body = curlGetWithResolve(allocator, url, &headers, "5", resolve_entry) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (body) |b| { + defer allocator.free(b); + try std.testing.expectEqualStrings("{\"ok\":true}", b); + } + }, + .get_status => { + const resp = curlGetWithStatusAndTimeoutAndResolve(allocator, url, &headers, "5", resolve_entry) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (resp) |r| { + defer allocator.free(r.body); + try std.testing.expectEqual(@as(u16, 200), r.status_code); + try std.testing.expectEqualStrings("{\"ok\":true}", r.body); + } + }, + .post_body => { + const body = curlPostWithProxyAndResolve(allocator, url, "{\"ping\":true}", &headers, null, "5", resolve_entry) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (body) |b| { + defer allocator.free(b); + try std.testing.expectEqualStrings("{\"ok\":true}", b); + } + }, + .post_status => { + const resp = curlPostWithStatusAndTimeoutAndResolve(allocator, url, "{\"ping\":true}", &headers, "5", resolve_entry) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (resp) |r| { + defer allocator.free(r.body); + try std.testing.expectEqual(@as(u16, 200), r.status_code); + try std.testing.expectEqualStrings("{\"ok\":true}", r.body); + } + }, + .post_status_headers => { + const resp = curlPostWithStatusHeadersAndTimeoutAndResolve(allocator, url, "{\"ping\":true}", &headers, "5", resolve_entry) catch |err| blk: { + request_err = err; + break :blk null; + }; + if (resp) |r| { + defer allocator.free(r.headers); + defer allocator.free(r.body); + try std.testing.expectEqual(@as(u16, 200), r.status_code); + try std.testing.expectEqualStrings("{\"ok\":true}", r.body); + } + }, + .put_body => unreachable, + } + + if (!ctx.saw_request.load(.acquire)) { + unblockCredentialedCurlFallbackServer(&server); + } + thread.join(); + + if (request_err) |err| return err; + try std.testing.expect(ctx.saw_expected_method.load(.acquire)); + try std.testing.expect(ctx.saw_authorization.load(.acquire)); +} + test "credentialed legacy curl body helpers do not reject authorization headers" { // Regression: legacy channel code still passes Authorization to curl* helper // APIs. These helpers must route through std.http fallback instead of @@ -1836,6 +1940,29 @@ test "credentialed legacy curl status helpers do not reject authorization header try expectLegacyCredentialedCurlFallback(.post_status_headers, "POST"); } +test "credentialed curl helpers preserve resolve pinning" { + // Regression: credentialed fallback must not bypass curl --resolve pinning, + // otherwise provider SSRF/DNS-rebinding protection is weakened. + try expectCredentialedCurlResolveEntry(.get_body, "GET"); + try expectCredentialedCurlResolveEntry(.post_body, "POST"); + try expectCredentialedCurlResolveEntry(.get_status, "GET"); + try expectCredentialedCurlResolveEntry(.post_status, "POST"); + try expectCredentialedCurlResolveEntry(.post_status_headers, "POST"); +} + +test "credentialed curl resolve path rejects sensitive URL tokens" { + try std.testing.expectError( + error.CredentialedCurlArgRejected, + curlGetWithResolve( + std.testing.allocator, + "https://example.com/v1?access_token=test-token", + &.{}, + "5", + "example.com:443:203.0.113.10", + ), + ); +} + test "prepareCurlHeaderArg writes headers outside argv" { var prepared = try prepareCurlHeaderArg(std.testing.allocator, &.{ "Authorization: Bearer test-token", "X-Test: ok" }); defer prepared.deinit(std.testing.allocator);