From 05324d64048aa8e0895de5685d557e7a6e3361fe Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:54:49 +0530 Subject: [PATCH 1/4] feat(acp): surface safe browser tool metadata --- internal/acp/permission.go | 6 ++- internal/acp/permission_test.go | 14 +++++++ internal/acp/translate.go | 53 +++++++++++++++++++++++++- internal/acp/translate_test.go | 66 +++++++++++++++++++++++++++++++++ internal/acp/types.go | 16 ++++++++ 5 files changed, 153 insertions(+), 2 deletions(-) diff --git a/internal/acp/permission.go b/internal/acp/permission.go index 90c0c960c..5b830342a 100644 --- a/internal/acp/permission.go +++ b/internal/acp/permission.go @@ -142,13 +142,17 @@ func actionOffered(optionID string, offered []PermissionOption) bool { // session/request_permission request from a ZERO permission request. func permissionToolCall(req agent.PermissionRequest) ToolCallUpdate { args := marshalArgs(req.Args) - return ToolCallUpdate{ + upd := ToolCallUpdate{ ToolCallID: req.ToolCallID, Title: toolTitle(req.ToolName, string(args)), Kind: toolKindFor(req.ToolName), Status: ToolStatusPending, RawInput: rawInputBytes(args), } + if browser, ok := browserToolDetails(req.ToolName); ok { + upd.Browser = browser + } + return upd } func marshalArgs(args map[string]any) []byte { diff --git a/internal/acp/permission_test.go b/internal/acp/permission_test.go index d17ab3384..7f3ce40fb 100644 --- a/internal/acp/permission_test.go +++ b/internal/acp/permission_test.go @@ -82,3 +82,17 @@ func TestPermissionToolCall(t *testing.T) { t.Error("expected rawInput from args") } } + +func TestPermissionToolCallKeepsTheBrowserDescriptor(t *testing.T) { + call := permissionToolCall(agent.PermissionRequest{ + ToolCallID: "browser-1", + ToolName: "browser_connect", + Args: map[string]any{"target": "127.0.0.1:9222"}, + }) + if call.Browser == nil || call.Browser.Version != 1 || call.Browser.Command != "connect" { + t.Fatalf("browser descriptor = %#v", call.Browser) + } + if call.Title != "browser connect" { + t.Fatalf("title = %q", call.Title) + } +} diff --git a/internal/acp/translate.go b/internal/acp/translate.go index 565174904..8869c3d60 100644 --- a/internal/acp/translate.go +++ b/internal/acp/translate.go @@ -2,6 +2,7 @@ package acp import ( "encoding/json" + "net/url" "strings" "unicode/utf8" @@ -44,12 +45,55 @@ func toolKindFor(name string) string { // toolTitle builds a concise human title, e.g. "read_file src/main.go". func toolTitle(name, rawArgs string) string { + if browser, ok := browserToolDetails(name); ok { + return browserToolTitle(browser.Command, rawArgs) + } if hint := primaryArgHint(rawArgs); hint != "" { return name + " " + hint } return name } +// browserToolDetails identifies ZERO's local browser helpers without treating +// similarly named MCP tools as browser automation. The descriptor intentionally +// contains no request data: ACP tool input is already protocol-visible, but a +// durable UI must not need to retain text, local CDP targets, or full URLs just +// to recognise the browser operation. +func browserToolDetails(name string) (*BrowserToolDetails, bool) { + const prefix = "browser_" + command, ok := strings.CutPrefix(name, prefix) + if !ok { + return nil, false + } + switch command { + case "install", "launch", "connect", "open", "snapshot", "click", "type", "press", "action": + return &BrowserToolDetails{Version: 1, Command: command}, true + default: + return nil, false + } +} + +// browserToolTitle avoids putting browser_type text, an attached DevTools +// endpoint, or a URL query/fragment in a tool-card title. Those values can +// carry credentials or session data; the UI only needs the operation and, for +// navigation, a human-recognisable origin. +func browserToolTitle(command, rawArgs string) string { + if command != "open" { + return "browser " + command + } + var args struct { + URL string `json:"url"` + } + if json.Unmarshal([]byte(rawArgs), &args) != nil { + return "browser open" + } + u, err := url.Parse(strings.TrimSpace(args.URL)) + if err != nil || u.Scheme == "" || u.Host == "" { + return "browser open" + } + return "browser open " + u.Scheme + "://" + u.Host +} + // primaryArgHint extracts the most relevant argument (path/pattern/command) from // raw JSON arguments. Best-effort; returns "" when it can't parse. func primaryArgHint(rawArgs string) string { @@ -89,7 +133,7 @@ func rawInput(args string) json.RawMessage { // toolCallStart maps an advertised ZERO tool call to the initial ACP "tool_call" // update (status in_progress — ZERO executes immediately after advertising). func toolCallStart(call agent.ToolCall) ToolCallUpdate { - return ToolCallUpdate{ + upd := ToolCallUpdate{ SessionUpdate: UpdateToolCall, ToolCallID: call.ID, Title: toolTitle(call.Name, call.Arguments), @@ -97,6 +141,10 @@ func toolCallStart(call agent.ToolCall) ToolCallUpdate { Status: ToolStatusInProgress, RawInput: rawInput(call.Arguments), } + if browser, ok := browserToolDetails(call.Name); ok { + upd.Browser = browser + } + return upd } // toolCallResult maps a finished ZERO tool result to a "tool_call_update". @@ -116,6 +164,9 @@ func toolCallResult(result agent.ToolResult) ToolCallUpdate { if locs := toolResultLocations(result); len(locs) > 0 { upd.Locations = locs } + if browser, ok := browserToolDetails(result.Name); ok { + upd.Browser = browser + } return upd } diff --git a/internal/acp/translate_test.go b/internal/acp/translate_test.go index 4a9adc16d..ac62b57af 100644 --- a/internal/acp/translate_test.go +++ b/internal/acp/translate_test.go @@ -1,6 +1,7 @@ package acp import ( + "encoding/json" "strings" "testing" "unicode/utf8" @@ -56,6 +57,71 @@ func TestToolTitleAndHint(t *testing.T) { } } +func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { + start := toolCallStart(agent.ToolCall{ + ID: "browser-1", + Name: "browser_open", + Arguments: `{"url":"https://example.com/settings?token=not-for-a-title#account"}`, + }) + if start.Browser == nil || start.Browser.Version != 1 || start.Browser.Command != "open" { + t.Fatalf("browser descriptor = %#v, want open", start.Browser) + } + if start.Title != "browser open https://example.com" { + t.Fatalf("browser title = %q", start.Title) + } + if strings.Contains(start.Title, "token=") || strings.Contains(start.Title, "#account") { + t.Fatalf("browser title leaked URL-sensitive data: %q", start.Title) + } + encoded, err := json.Marshal(start) + if err != nil { + t.Fatal(err) + } + var wire struct { + Browser BrowserToolDetails `json:"browser"` + } + if err := json.Unmarshal(encoded, &wire); err != nil { + t.Fatal(err) + } + if wire.Browser != (BrowserToolDetails{Version: 1, Command: "open"}) { + t.Fatalf("browser wire descriptor = %#v", wire.Browser) + } + + typed := toolCallStart(agent.ToolCall{ + ID: "browser-2", + Name: "browser_type", + Arguments: `{"ref":"email","text":"secret@example.test"}`, + }) + if typed.Browser == nil || typed.Browser.Command != "type" { + t.Fatalf("browser type descriptor = %#v", typed.Browser) + } + if typed.Title != "browser type" || strings.Contains(typed.Title, "secret@example.test") { + t.Fatalf("browser type title = %q", typed.Title) + } + + result := toolCallResult(agent.ToolResult{ + ToolCallID: "browser-2", + Name: "browser_type", + Status: tools.StatusOK, + }) + if result.Browser == nil || result.Browser.Command != "type" { + t.Fatalf("browser result descriptor = %#v", result.Browser) + } +} + +func TestBrowserDescriptorDoesNotClaimSimilarlyNamedMCPTools(t *testing.T) { + start := toolCallStart(agent.ToolCall{ID: "mcp-1", Name: "browser_plugin_open", Arguments: `{}`}) + if start.Browser != nil { + t.Fatalf("MCP-like tool received built-in browser descriptor: %#v", start.Browser) + } + encoded, err := json.Marshal(start) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(encoded), `"browser"`) { + t.Fatalf("non-browser tool encoded browser field: %s", encoded) + } +} + func TestToolCallStart(t *testing.T) { upd := toolCallStart(agent.ToolCall{ID: "tc1", Name: "read_file", Arguments: `{"path":"a.go"}`}) if upd.SessionUpdate != UpdateToolCall { diff --git a/internal/acp/types.go b/internal/acp/types.go index b00bf672a..c038f4559 100644 --- a/internal/acp/types.go +++ b/internal/acp/types.go @@ -210,6 +210,22 @@ type ToolCallUpdate struct { RawInput json.RawMessage `json:"rawInput,omitempty"` Content []ToolCallContent `json:"content,omitempty"` Locations []ToolCallLocation `json:"locations,omitempty"` + // Browser is present only for ZERO's built-in browser helper tools. It is a + // deliberately narrow presentation descriptor for ACP clients: the raw + // request may contain typed text, a full URL, or a local DevTools endpoint, + // none of which belongs in a durable browser-status surface. + Browser *BrowserToolDetails `json:"browser,omitempty"` +} + +// BrowserToolDetails identifies the browser helper operation behind a tool +// call. Version is the schema version for this optional ZERO extension; +// Command is one of install, launch, connect, open, snapshot, click, type, +// press, or action. Future fields must remain display-safe and must not +// include browser profile data, cookies, typed text, URL paths/queries, or +// DevTools endpoints. +type BrowserToolDetails struct { + Version int `json:"version"` + Command string `json:"command"` } // ToolCallContent is a tool call's rendered output. ZERO emits "content" (a From 0b9c996ebb60300cbaca5363ac6d8d26ca04b5e7 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:31:10 +0530 Subject: [PATCH 2/4] fix(acp): align browser permission titles --- internal/acp/translate.go | 55 +++++++++++++++++++++++++++------ internal/acp/translate_test.go | 30 ++++++++++++++++++ internal/tools/local_browser.go | 21 +++++++++++-- 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/internal/acp/translate.go b/internal/acp/translate.go index 8869c3d60..969b1edfb 100644 --- a/internal/acp/translate.go +++ b/internal/acp/translate.go @@ -78,20 +78,57 @@ func browserToolDetails(name string) (*BrowserToolDetails, bool) { // carry credentials or session data; the UI only needs the operation and, for // navigation, a human-recognisable origin. func browserToolTitle(command, rawArgs string) string { - if command != "open" { + switch command { + case "action": + action, ok := exactJSONStringArg(rawArgs, "command") + if !ok { + return "browser action" + } + if action, ok := tools.NormalizedBrowserActionCommand(action); ok { + return "browser action " + action + } + return "browser action" + case "open": + rawURL, ok := exactJSONStringArg(rawArgs, "url") + if !ok { + return "browser open" + } + normalized, err := tools.NormalizeBrowserOpenURL(rawURL) + if err != nil { + return "browser open" + } + u, err := url.Parse(normalized) + if err != nil || u.Scheme == "" || u.Host == "" { + return "browser open" + } + origin := u.Scheme + "://" + u.Host + if !utf8.ValidString(origin) { + return "browser open" + } + return "browser open " + truncateHint(origin) + default: return "browser " + command } - var args struct { - URL string `json:"url"` - } +} + +// exactJSONStringArg mirrors ZERO's map-based tool argument decoding: only the +// exact JSON key is considered, and a non-string value is invalid. In +// particular, an incidental "URL" key must not change a permission title when +// browser_open will only read "url". +func exactJSONStringArg(rawArgs, key string) (string, bool) { + var args map[string]json.RawMessage if json.Unmarshal([]byte(rawArgs), &args) != nil { - return "browser open" + return "", false + } + raw, ok := args[key] + if !ok { + return "", false } - u, err := url.Parse(strings.TrimSpace(args.URL)) - if err != nil || u.Scheme == "" || u.Host == "" { - return "browser open" + var value string + if json.Unmarshal(raw, &value) != nil { + return "", false } - return "browser open " + u.Scheme + "://" + u.Host + return value, true } // primaryArgHint extracts the most relevant argument (path/pattern/command) from diff --git a/internal/acp/translate_test.go b/internal/acp/translate_test.go index ac62b57af..976c90e04 100644 --- a/internal/acp/translate_test.go +++ b/internal/acp/translate_test.go @@ -98,6 +98,15 @@ func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { t.Fatalf("browser type title = %q", typed.Title) } + action := toolCallStart(agent.ToolCall{ + ID: "browser-3", + Name: "browser_action", + Arguments: `{"command":"keyboard_insert_text","args":["secret@example.test"]}`, + }) + if action.Title != "browser action keyboard_insert_text" { + t.Fatalf("browser action title = %q", action.Title) + } + result := toolCallResult(agent.ToolResult{ ToolCallID: "browser-2", Name: "browser_type", @@ -108,6 +117,27 @@ func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { } } +func TestBrowserPermissionTitlesMirrorSafeToolArguments(t *testing.T) { + if got := browserToolTitle("open", `{"url":"evil.example.test/pay?token=hidden#fragment"}`); got != "browser open https://evil.example.test" { + t.Fatalf("bare-host title = %q", got) + } + if got := browserToolTitle("open", `{"URL":"https://different.example.test"}`); got != "browser open" { + t.Fatalf("case-variant URL title = %q", got) + } + if got := browserToolTitle("open", `{"URL":"https://different.example.test","url":"https://actual.example.test/path"}`); got != "browser open https://actual.example.test" { + t.Fatalf("exact URL key title = %q", got) + } + if got := browserToolTitle("action", `{"command":"not an action"}`); got != "browser action" { + t.Fatalf("unknown browser action title = %q", got) + } + + longHost := "https://" + strings.Repeat("a", 200) + ".example.test/path?token=hidden" + title := browserToolTitle("open", `{"url":"`+longHost+`"}`) + if !utf8.ValidString(title) || utf8.RuneCountInString(title) > len("browser open ")+61 || strings.Contains(title, "token=") { + t.Fatalf("bounded browser origin title = %q", title) + } +} + func TestBrowserDescriptorDoesNotClaimSimilarlyNamedMCPTools(t *testing.T) { start := toolCallStart(agent.ToolCall{ID: "mcp-1", Name: "browser_plugin_open", Arguments: `{}`}) if start.Browser != nil { diff --git a/internal/tools/local_browser.go b/internal/tools/local_browser.go index 44cd6d524..672057e84 100644 --- a/internal/tools/local_browser.go +++ b/internal/tools/local_browser.go @@ -548,11 +548,11 @@ func browserActionArgs(args map[string]any) (string, []string, error) { if err != nil { return "", nil, err } - command = strings.ToLower(strings.TrimSpace(command)) - spec, ok := browserActionSpecs[command] + command, ok := NormalizedBrowserActionCommand(command) if !ok { return "", nil, fmt.Errorf("command must be one of: %s", strings.Join(browserActionCommandNames(), ", ")) } + spec := browserActionSpecs[command] values, err := stringArrayArg(args, "args") if err != nil { return "", nil, err @@ -570,6 +570,16 @@ func browserActionArgs(args map[string]any) (string, []string, error) { return command, commandArgs, nil } +// NormalizedBrowserActionCommand returns the exact action that browser_action +// will execute after normalizing its command argument. ACP uses it only for a +// permission title, so an unrecognised command is never reflected as text that +// the tool would reject. +func NormalizedBrowserActionCommand(command string) (string, bool) { + command = strings.ToLower(strings.TrimSpace(command)) + _, ok := browserActionSpecs[command] + return command, ok +} + func browserActionCommandArgs(command string, spec browserActionSpec, values []string) ([]string, error) { switch command { case "connect": @@ -657,6 +667,13 @@ func browserOpenURLArg(args map[string]any) (string, error) { if err != nil { return "", err } + return NormalizeBrowserOpenURL(rawURL) +} + +// NormalizeBrowserOpenURL applies the browser_open URL rules before execution. +// Keeping this exported within the internal package lets permission displays +// describe the same destination the browser helper will open. +func NormalizeBrowserOpenURL(rawURL string) (string, error) { normalized := strings.TrimSpace(rawURL) if !strings.Contains(normalized, "://") { normalized = "https://" + normalized From 9abf27e1d20cee35aeb1029baef5632b8a71f6e0 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:35:28 +0530 Subject: [PATCH 3/4] fix(acp): namespace browser metadata --- internal/acp/permission.go | 4 +- internal/acp/permission_test.go | 4 +- internal/acp/translate.go | 25 ++++++-- internal/acp/translate_test.go | 104 ++++++++++++++++++++++++++++---- internal/acp/types.go | 9 ++- 5 files changed, 119 insertions(+), 27 deletions(-) diff --git a/internal/acp/permission.go b/internal/acp/permission.go index 5b830342a..e243f7806 100644 --- a/internal/acp/permission.go +++ b/internal/acp/permission.go @@ -149,9 +149,7 @@ func permissionToolCall(req agent.PermissionRequest) ToolCallUpdate { Status: ToolStatusPending, RawInput: rawInputBytes(args), } - if browser, ok := browserToolDetails(req.ToolName); ok { - upd.Browser = browser - } + attachBrowserToolDetails(&upd, req.ToolName) return upd } diff --git a/internal/acp/permission_test.go b/internal/acp/permission_test.go index 7f3ce40fb..e2211e741 100644 --- a/internal/acp/permission_test.go +++ b/internal/acp/permission_test.go @@ -89,8 +89,8 @@ func TestPermissionToolCallKeepsTheBrowserDescriptor(t *testing.T) { ToolName: "browser_connect", Args: map[string]any{"target": "127.0.0.1:9222"}, }) - if call.Browser == nil || call.Browser.Version != 1 || call.Browser.Command != "connect" { - t.Fatalf("browser descriptor = %#v", call.Browser) + if got := browserDescriptor(t, call); got != (BrowserToolDetails{Version: 1, Command: "connect"}) { + t.Fatalf("browser descriptor = %#v", got) } if call.Title != "browser connect" { t.Fatalf("title = %q", call.Title) diff --git a/internal/acp/translate.go b/internal/acp/translate.go index 969b1edfb..e2eec2243 100644 --- a/internal/acp/translate.go +++ b/internal/acp/translate.go @@ -73,6 +73,23 @@ func browserToolDetails(name string) (*BrowserToolDetails, bool) { } } +const zeroBrowserMetaKey = "github.com/Gitlawb/zero/browser" + +// attachBrowserToolDetails stores ZERO's browser descriptor in ACP's reserved +// extension channel. Keeping this in one helper prevents start, result, and +// permission payloads from drifting onto different wire shapes. +func attachBrowserToolDetails(update *ToolCallUpdate, name string) { + browser, ok := browserToolDetails(name) + if !ok { + return + } + raw, err := json.Marshal(browser) + if err != nil { + return + } + update.Meta = map[string]json.RawMessage{zeroBrowserMetaKey: raw} +} + // browserToolTitle avoids putting browser_type text, an attached DevTools // endpoint, or a URL query/fragment in a tool-card title. Those values can // carry credentials or session data; the UI only needs the operation and, for @@ -178,9 +195,7 @@ func toolCallStart(call agent.ToolCall) ToolCallUpdate { Status: ToolStatusInProgress, RawInput: rawInput(call.Arguments), } - if browser, ok := browserToolDetails(call.Name); ok { - upd.Browser = browser - } + attachBrowserToolDetails(&upd, call.Name) return upd } @@ -201,9 +216,7 @@ func toolCallResult(result agent.ToolResult) ToolCallUpdate { if locs := toolResultLocations(result); len(locs) > 0 { upd.Locations = locs } - if browser, ok := browserToolDetails(result.Name); ok { - upd.Browser = browser - } + attachBrowserToolDetails(&upd, result.Name) return upd } diff --git a/internal/acp/translate_test.go b/internal/acp/translate_test.go index 976c90e04..73ec7965d 100644 --- a/internal/acp/translate_test.go +++ b/internal/acp/translate_test.go @@ -10,6 +10,19 @@ import ( "github.com/Gitlawb/zero/internal/tools" ) +func browserDescriptor(t *testing.T, update ToolCallUpdate) BrowserToolDetails { + t.Helper() + raw, ok := update.Meta[zeroBrowserMetaKey] + if !ok { + t.Fatalf("browser metadata = %#v, want %q", update.Meta, zeroBrowserMetaKey) + } + var details BrowserToolDetails + if err := json.Unmarshal(raw, &details); err != nil { + t.Fatalf("decode browser metadata: %v", err) + } + return details +} + func TestAgentMessageAndThoughtChunks(t *testing.T) { m := agentMessageChunk("hello") if m.SessionUpdate != UpdateAgentMessageChunk || m.Content.Type != "text" || m.Content.Text != "hello" { @@ -63,8 +76,8 @@ func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { Name: "browser_open", Arguments: `{"url":"https://example.com/settings?token=not-for-a-title#account"}`, }) - if start.Browser == nil || start.Browser.Version != 1 || start.Browser.Command != "open" { - t.Fatalf("browser descriptor = %#v, want open", start.Browser) + if got := browserDescriptor(t, start); got != (BrowserToolDetails{Version: 1, Command: "open"}) { + t.Fatalf("browser descriptor = %#v, want open", got) } if start.Title != "browser open https://example.com" { t.Fatalf("browser title = %q", start.Title) @@ -77,13 +90,13 @@ func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { t.Fatal(err) } var wire struct { - Browser BrowserToolDetails `json:"browser"` + Meta map[string]json.RawMessage `json:"_meta"` } if err := json.Unmarshal(encoded, &wire); err != nil { t.Fatal(err) } - if wire.Browser != (BrowserToolDetails{Version: 1, Command: "open"}) { - t.Fatalf("browser wire descriptor = %#v", wire.Browser) + if _, ok := wire.Meta[zeroBrowserMetaKey]; !ok { + t.Fatalf("browser wire metadata = %#v", wire.Meta) } typed := toolCallStart(agent.ToolCall{ @@ -91,8 +104,8 @@ func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { Name: "browser_type", Arguments: `{"ref":"email","text":"secret@example.test"}`, }) - if typed.Browser == nil || typed.Browser.Command != "type" { - t.Fatalf("browser type descriptor = %#v", typed.Browser) + if got := browserDescriptor(t, typed); got.Command != "type" { + t.Fatalf("browser type descriptor = %#v", got) } if typed.Title != "browser type" || strings.Contains(typed.Title, "secret@example.test") { t.Fatalf("browser type title = %q", typed.Title) @@ -112,8 +125,77 @@ func TestBrowserToolUpdatesAreStructuredAndPresentationSafe(t *testing.T) { Name: "browser_type", Status: tools.StatusOK, }) - if result.Browser == nil || result.Browser.Command != "type" { - t.Fatalf("browser result descriptor = %#v", result.Browser) + if got := browserDescriptor(t, result); got.Command != "type" { + t.Fatalf("browser result descriptor = %#v", got) + } +} + +func TestBrowserDescriptorSurvivesProtocolShapedRoundTrip(t *testing.T) { + updates := []ToolCallUpdate{ + toolCallStart(agent.ToolCall{ + ID: "start", + Name: "browser_open", + Arguments: `{"url":"https://user:password@example.test/private?token=secret#fragment"}`, + }), + toolCallResult(agent.ToolResult{ + ToolCallID: "result", + Name: "browser_type", + Status: tools.StatusOK, + }), + permissionToolCall(agent.PermissionRequest{ + ToolCallID: "permission", + ToolName: "browser_connect", + Args: map[string]any{"target": "127.0.0.1:9222"}, + }), + } + + type protocolToolCallUpdate struct { + SessionUpdate string `json:"sessionUpdate,omitempty"` + ToolCallID string `json:"toolCallId"` + Title string `json:"title,omitempty"` + Kind string `json:"kind,omitempty"` + Status string `json:"status,omitempty"` + RawInput json.RawMessage `json:"rawInput,omitempty"` + Content []ToolCallContent `json:"content,omitempty"` + Locations []ToolCallLocation `json:"locations,omitempty"` + Meta map[string]json.RawMessage `json:"_meta,omitempty"` + } + + for _, update := range updates { + encoded, err := json.Marshal(update) + if err != nil { + t.Fatal(err) + } + var root map[string]json.RawMessage + if err := json.Unmarshal(encoded, &root); err != nil { + t.Fatal(err) + } + if _, ok := root["browser"]; ok { + t.Fatalf("browser descriptor escaped ACP _meta: %s", encoded) + } + + var protocol protocolToolCallUpdate + if err := json.Unmarshal(encoded, &protocol); err != nil { + t.Fatal(err) + } + forwarded, err := json.Marshal(protocol) + if err != nil { + t.Fatal(err) + } + var roundTripped ToolCallUpdate + if err := json.Unmarshal(forwarded, &roundTripped); err != nil { + t.Fatal(err) + } + details := browserDescriptor(t, roundTripped) + if details.Version != 1 || details.Command == "" { + t.Fatalf("round-tripped browser descriptor = %#v", details) + } + descriptorJSON := string(roundTripped.Meta[zeroBrowserMetaKey]) + for _, secret := range []string{"password", "private", "token", "fragment", "127.0.0.1", "9222"} { + if strings.Contains(descriptorJSON, secret) { + t.Fatalf("browser descriptor leaked %q: %s", secret, descriptorJSON) + } + } } } @@ -140,8 +222,8 @@ func TestBrowserPermissionTitlesMirrorSafeToolArguments(t *testing.T) { func TestBrowserDescriptorDoesNotClaimSimilarlyNamedMCPTools(t *testing.T) { start := toolCallStart(agent.ToolCall{ID: "mcp-1", Name: "browser_plugin_open", Arguments: `{}`}) - if start.Browser != nil { - t.Fatalf("MCP-like tool received built-in browser descriptor: %#v", start.Browser) + if len(start.Meta) != 0 { + t.Fatalf("MCP-like tool received built-in browser metadata: %#v", start.Meta) } encoded, err := json.Marshal(start) if err != nil { diff --git a/internal/acp/types.go b/internal/acp/types.go index c038f4559..680476aec 100644 --- a/internal/acp/types.go +++ b/internal/acp/types.go @@ -210,11 +210,10 @@ type ToolCallUpdate struct { RawInput json.RawMessage `json:"rawInput,omitempty"` Content []ToolCallContent `json:"content,omitempty"` Locations []ToolCallLocation `json:"locations,omitempty"` - // Browser is present only for ZERO's built-in browser helper tools. It is a - // deliberately narrow presentation descriptor for ACP clients: the raw - // request may contain typed text, a full URL, or a local DevTools endpoint, - // none of which belongs in a durable browser-status surface. - Browser *BrowserToolDetails `json:"browser,omitempty"` + // Meta is ACP's extension channel. ZERO-owned values must remain beneath a + // namespaced key so protocol-shaped clients can preserve them while decoding + // and re-encoding a tool call. + Meta map[string]json.RawMessage `json:"_meta,omitempty"` } // BrowserToolDetails identifies the browser helper operation behind a tool From 33d81a6a2b9f42b32356234e2d355c449afddcfe Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:59:41 +0530 Subject: [PATCH 4/4] fix(acp): reject unsafe browser title text --- internal/acp/translate.go | 19 +++++++++++++- internal/acp/translate_test.go | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/internal/acp/translate.go b/internal/acp/translate.go index e2eec2243..7f1f461fa 100644 --- a/internal/acp/translate.go +++ b/internal/acp/translate.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/url" "strings" + "unicode" "unicode/utf8" "github.com/Gitlawb/zero/internal/agent" @@ -119,7 +120,7 @@ func browserToolTitle(command, rawArgs string) string { return "browser open" } origin := u.Scheme + "://" + u.Host - if !utf8.ValidString(origin) { + if !browserTitleTextSafe(origin) { return "browser open" } return "browser open " + truncateHint(origin) @@ -128,6 +129,22 @@ func browserToolTitle(command, rawArgs string) string { } } +// browserTitleTextSafe validates text after URL parsing has decoded escaped +// UTF-8 in the host. Valid UTF-8 alone is not presentation-safe: control, +// format/bidi, and line/paragraph separator runes can reorder or split the +// permission label shown to a user. The execution URL remains unchanged. +func browserTitleTextSafe(text string) bool { + if !utf8.ValidString(text) { + return false + } + for _, r := range text { + if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) { + return false + } + } + return true +} + // exactJSONStringArg mirrors ZERO's map-based tool argument decoding: only the // exact JSON key is considered, and a non-string value is invalid. In // particular, an incidental "URL" key must not change a permission title when diff --git a/internal/acp/translate_test.go b/internal/acp/translate_test.go index 73ec7965d..89dbcc73a 100644 --- a/internal/acp/translate_test.go +++ b/internal/acp/translate_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "strings" "testing" + "unicode" "unicode/utf8" "github.com/Gitlawb/zero/internal/agent" @@ -220,6 +221,53 @@ func TestBrowserPermissionTitlesMirrorSafeToolArguments(t *testing.T) { } } +func TestBrowserOpenTitlesRejectDecodedUnicodePresentationControls(t *testing.T) { + for _, rawURL := range []string{ + "https://safe.example%E2%80%AEevil.test/path", + "https://safe.example%E2%81%A6evil.test/path", + "https://safe.example%C2%85evil.test/path", + "https://safe.example%E2%80%A8evil.test/path", + "https://safe.example%E2%80%A9evil.test/path", + } { + t.Run(rawURL, func(t *testing.T) { + normalized, err := tools.NormalizeBrowserOpenURL(rawURL) + if err != nil { + t.Fatalf("execution URL rejected: %v", err) + } + if normalized != rawURL { + t.Fatalf("execution URL = %q, want unchanged %q", normalized, rawURL) + } + + args, err := json.Marshal(map[string]any{"url": rawURL}) + if err != nil { + t.Fatal(err) + } + updates := []ToolCallUpdate{ + toolCallStart(agent.ToolCall{ID: "start", Name: "browser_open", Arguments: string(args)}), + permissionToolCall(agent.PermissionRequest{ToolCallID: "permission", ToolName: "browser_open", Args: map[string]any{"url": rawURL}}), + } + for _, update := range updates { + encoded, err := json.Marshal(update) + if err != nil { + t.Fatal(err) + } + var decoded ToolCallUpdate + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatal(err) + } + if decoded.Title != "browser open" { + t.Fatalf("unsafe browser title survived wire round trip: %q", decoded.Title) + } + for _, r := range decoded.Title { + if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) { + t.Fatalf("browser title contains unsafe presentation rune %U: %q", r, decoded.Title) + } + } + } + }) + } +} + func TestBrowserDescriptorDoesNotClaimSimilarlyNamedMCPTools(t *testing.T) { start := toolCallStart(agent.ToolCall{ID: "mcp-1", Name: "browser_plugin_open", Arguments: `{}`}) if len(start.Meta) != 0 {