From 2ca7550947ab12d751e08484f0a0ef25e56b007c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Mon, 4 May 2026 09:54:13 +0200 Subject: [PATCH] agent: restrict getEnv tool to LP_ namespace Limits the getEnv tool to variables starting with LP_ to prevent the model from probing sensitive system environment variables or API keys. --- docs/agent.md | 4 +++ src/browser/tools.zig | 73 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/docs/agent.md b/docs/agent.md index 86e85222..ed4cb3d5 100644 --- a/docs/agent.md +++ b/docs/agent.md @@ -221,6 +221,10 @@ for the LLM. surfaced by a page are not followed unless they match the user's task. - `$LP_*` environment variable references in `TYPE` / `fill` values are resolved at execution time, so credentials never enter the LLM context. +- The `getEnv` tool only reads variables whose name starts with `LP_`. + Everything else (provider API keys, system env, third-party secrets) + reports "not set" so the model can't probe for it. The user controls + what lives under `LP_*`. - `--obey-robots`, `--http-proxy`, `--user-agent`, and the rest of the browser-level CLI flags apply to `agent` the same way they apply to `serve`, `fetch`, and `mcp`. diff --git a/src/browser/tools.zig b/src/browser/tools.zig index fcc062f7..b04de26a 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -303,12 +303,12 @@ pub const tool_defs = [_]ToolDef{ }, .{ .name = "getEnv", - .description = "Read the value of an environment variable. Useful for retrieving credentials or configuration without hardcoding them.", + .description = "Read an environment variable from the lightpanda LP_* namespace. Other names are reported as not set. Use $LP_* placeholders in fill values to inject secrets into the page without exposing them to the model.", .input_schema = minify( \\{ \\ "type": "object", \\ "properties": { - \\ "name": { "type": "string", "description": "The environment variable name to read." } + \\ "name": { "type": "string", "description": "Environment variable name; must start with LP_." } \\ }, \\ "required": ["name"] \\} @@ -854,12 +854,24 @@ fn execFindElement(session: *lp.Session, arena: std.mem.Allocator, registry: *CD fn execGetEnv(arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { const Params = struct { name: []const u8 }; const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + + // Only the LP_ namespace is readable through this tool. Everything else + // (provider API keys, system env, third-party secrets) reports "not set" + // so the LLM can't probe for it. Same pattern as Kakoune's `kak_*`. + if (!isLpNamespace(args.name)) { + return std.fmt.allocPrint(arena, "Environment variable '{s}' is not set", .{args.name}) catch ToolError.InternalError; + } + const name_z = arena.dupeZ(u8, args.name) catch return ToolError.InternalError; const value = std.posix.getenv(name_z) orelse return std.fmt.allocPrint(arena, "Environment variable '{s}' is not set", .{args.name}) catch ToolError.InternalError; return value; } +fn isLpNamespace(name: []const u8) bool { + return name.len >= 3 and std.ascii.eqlIgnoreCase(name[0..3], "LP_"); +} + fn execConsoleLogs( session: *lp.Session, arena: std.mem.Allocator, @@ -1006,3 +1018,60 @@ test "substituteEnvVars bare dollar" { const r = substituteEnvVars(arena.allocator(), "price is $ 5"); try std.testing.expectEqualStrings("price is $ 5", r); } + +test "isLpNamespace" { + try std.testing.expect(isLpNamespace("LP_FOO")); + try std.testing.expect(isLpNamespace("LP_USERNAME")); + try std.testing.expect(isLpNamespace("lp_anything")); + + try std.testing.expect(!isLpNamespace("HOME")); + try std.testing.expect(!isLpNamespace("PATH")); + try std.testing.expect(!isLpNamespace("ANTHROPIC_API_KEY")); + try std.testing.expect(!isLpNamespace("")); + try std.testing.expect(!isLpNamespace("LP")); // too short, no underscore + // "LP_" mid-name must not pass. + try std.testing.expect(!isLpNamespace("HELP_TEXT")); +} + +extern fn setenv(name: [*:0]u8, value: [*:0]u8, override: c_int) c_int; +extern fn unsetenv(name: [*:0]u8) c_int; + +test "execGetEnv reads LP_* values" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const aa = arena.allocator(); + + const var_name = "LP_GETENV_TEST_OK"; + const var_value = "hello-world"; + _ = setenv(@constCast(var_name), @constCast(var_value), 1); + defer _ = unsetenv(@constCast(var_name)); + + var obj: std.json.ObjectMap = .init(aa); + try obj.put("name", .{ .string = var_name }); + const arguments: std.json.Value = .{ .object = obj }; + + const r = try execGetEnv(aa, arguments); + try std.testing.expectEqualStrings(var_value, r); +} + +test "execGetEnv hides non-LP_ values even when set" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const aa = arena.allocator(); + + const var_name = "LIGHTPANDA_GETENV_TEST_OUTSIDE"; + const var_value = "should-not-leak"; + _ = setenv(@constCast(var_name), @constCast(var_value), 1); + defer _ = unsetenv(@constCast(var_name)); + + var obj: std.json.ObjectMap = .init(aa); + try obj.put("name", .{ .string = var_name }); + const arguments: std.json.Value = .{ .object = obj }; + + const r = try execGetEnv(aa, arguments); + try std.testing.expect(std.mem.indexOf(u8, r, var_value) == null); + try std.testing.expectEqualStrings( + "Environment variable '" ++ var_name ++ "' is not set", + r, + ); +}