Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/agent-deck/web_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,16 @@ func buildWebServer(profile string, args []string, menuData web.MenuDataLoader,
}
}

confirmLinkOpen := session.GetWebConfirmLinkOpen()
server := web.NewServer(web.Config{
ListenAddr: *listenAddr,
Profile: effectiveProfile,
ReadOnly: *readOnly,
WebMutations: resolveMutationsEnabled(*readOnly),
Token: *token,
InsecureBind: *insecureBind,
TrustedDomains: session.GetWebTrustedDomains(),
ConfirmLinkOpen: &confirmLinkOpen,
MenuData: menuData,
PushVAPIDPublicKey: resolvedPushPublic,
PushVAPIDPrivateKey: resolvedPushPrivate,
Expand Down
111 changes: 111 additions & 0 deletions internal/session/userconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,19 @@ type WebSettings struct {
// MutationsEnabled controls whether POST/PATCH/DELETE endpoints accept
// requests. nil (omitted) defaults to true. Forced off by --read-only.
MutationsEnabled *bool `toml:"mutations_enabled,omitempty"`

// TrustedDomains lists hosts whose links open from the web terminal
// without the "this link could potentially be dangerous" confirm
// (issue #1682). Entries are hosts, not URLs — a full URL is accepted
// and reduced to its host. A leading `*.` matches subdomains only
// (`*.corp.example` matches `git.corp.example`, not `corp.example`).
// Everything not on the list still confirms.
TrustedDomains []string `toml:"trusted_domains,omitempty"`

// ConfirmLinkOpen controls the web terminal's link-open confirm for
// hosts that are NOT on TrustedDomains. nil (omitted) defaults to true.
// Setting it false accepts the risk and opens every link directly.
ConfirmLinkOpen *bool `toml:"confirm_link_open,omitempty"`
}

// FeedbackSettings controls the in-product feedback prompts.
Expand Down Expand Up @@ -3489,6 +3502,104 @@ func GetWebMutationsEnabled() bool {
return *config.Web.MutationsEnabled
}

// GetWebTrustedDomains returns the normalized `[web].trusted_domains` hosts.
// Links whose host matches an entry skip the web terminal's link-open confirm
// (issue #1682). Returns an empty slice when the key is absent, so the
// confirm stays on for everything by default.
func GetWebTrustedDomains() []string {
config, err := LoadUserConfig()
if err != nil || config == nil {
return nil
}
return NormalizeTrustedDomains(config.Web.TrustedDomains)
}

// GetWebConfirmLinkOpen reports whether the web terminal confirms before
// opening a link whose host is not on `[web].trusted_domains`. Defaults to
// true when `[web].confirm_link_open` is omitted.
func GetWebConfirmLinkOpen() bool {
config, err := LoadUserConfig()
if err != nil || config == nil || config.Web.ConfirmLinkOpen == nil {
return true
}
return *config.Web.ConfirmLinkOpen
}

// NormalizeTrustedDomains reduces raw `[web].trusted_domains` entries to
// lowercase hosts suitable for exact comparison against a URL host:
//
// - "https://gitlab.corp.example/group/repo" -> "gitlab.corp.example"
// - "GitLab.Corp.Example:8443" -> "gitlab.corp.example"
// - "*.corp.example" -> "*.corp.example" (subdomains)
//
// Blank entries, bare wildcards ("*", "*."), and entries that carry no host
// are dropped. Duplicates are removed, input order is preserved.
func NormalizeTrustedDomains(entries []string) []string {
out := make([]string, 0, len(entries))
seen := make(map[string]struct{}, len(entries))
for _, raw := range entries {
host := normalizeTrustedDomain(raw)
if host == "" {
continue
}
if _, dup := seen[host]; dup {
continue
}
seen[host] = struct{}{}
out = append(out, host)
}
if len(out) == 0 {
return nil
}
return out
}

// normalizeTrustedDomain normalizes one entry; "" means "unusable, drop it".
func normalizeTrustedDomain(raw string) string {
s := strings.ToLower(strings.TrimSpace(raw))
if s == "" {
return ""
}
// Accept a pasted URL: strip scheme, then anything from the first
// path/query/fragment separator onward.
if i := strings.Index(s, "://"); i >= 0 {
s = s[i+3:]
}
if i := strings.IndexAny(s, "/?#"); i >= 0 {
s = s[:i]
}
// Strip userinfo ("user:pass@host") — the host is what we match on.
if i := strings.LastIndex(s, "@"); i >= 0 {
s = s[i+1:]
}
wildcard := strings.HasPrefix(s, "*.")
if wildcard {
s = s[2:]
}
// Strip the port. Bracketed IPv6 literals keep their brackets so the
// colons inside are not mistaken for a port separator.
if strings.HasPrefix(s, "[") {
if i := strings.Index(s, "]"); i >= 0 {
s = s[:i+1]
}
} else if i := strings.LastIndex(s, ":"); i >= 0 && !strings.Contains(s[i+1:], ":") {
s = s[:i]
}
s = strings.TrimSuffix(s, ".")
if s == "" || strings.ContainsAny(s, " \t*") {
return ""
}
if wildcard {
// A subdomain wildcard needs a registrable base with a dot, else
// "*.example" would silently allow every single-label host.
if !strings.Contains(s, ".") {
return ""
}
return "*." + s
}
return s
}

// GetHotkeyOverrides returns user-configured hotkey overrides from config.toml.
//
// Merge order (issue #434):
Expand Down
88 changes: 88 additions & 0 deletions internal/session/userconfig_web_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,91 @@ mutations_enabled = false
t.Errorf("GetWebMutationsEnabled() = true, want false when explicitly disabled")
}
}

// --- [web] trusted_domains / confirm_link_open (issue #1682) ---------------

func TestWebTrustedDomains_EmptyWhenAbsent(t *testing.T) {
withTempHomeAndConfig(t, `
[web]
mutations_enabled = true
`)
if got := GetWebTrustedDomains(); len(got) != 0 {
t.Errorf("GetWebTrustedDomains() = %v, want empty when the key is absent", got)
}
}

func TestWebTrustedDomains_ReadsAndNormalizes(t *testing.T) {
withTempHomeAndConfig(t, `
[web]
trusted_domains = ["GitLab.Corp.Example", "https://gerrit.corp.example:8443/c/1", " ", "*.ci.corp.example", "gitlab.corp.example"]
`)
got := GetWebTrustedDomains()
want := []string{"gitlab.corp.example", "gerrit.corp.example", "*.ci.corp.example"}
if len(got) != len(want) {
t.Fatalf("GetWebTrustedDomains() = %v, want %v", got, want)
}
for i := range want {
if got[i] != want[i] {
t.Errorf("GetWebTrustedDomains()[%d] = %q, want %q", i, got[i], want[i])
}
}
}

func TestWebConfirmLinkOpen_DefaultsTrue(t *testing.T) {
withTempHomeAndConfig(t, `
[web]
trusted_domains = ["gitlab.corp.example"]
`)
if !GetWebConfirmLinkOpen() {
t.Error("GetWebConfirmLinkOpen() = false, want true when the key is omitted")
}
}

func TestWebConfirmLinkOpen_ExplicitFalse(t *testing.T) {
withTempHomeAndConfig(t, `
[web]
confirm_link_open = false
`)
if GetWebConfirmLinkOpen() {
t.Error("GetWebConfirmLinkOpen() = true, want false when explicitly disabled")
}
}

func TestNormalizeTrustedDomains(t *testing.T) {
tests := []struct {
name string
in []string
want []string
}{
{"nil stays nil", nil, nil},
{"plain host", []string{"gitlab.corp.example"}, []string{"gitlab.corp.example"}},
{"case folded", []string{"GitLab.CORP.example"}, []string{"gitlab.corp.example"}},
{"trims space", []string{" gitlab.corp.example "}, []string{"gitlab.corp.example"}},
{"strips scheme and path", []string{"https://gitlab.corp.example/group/repo/-/merge_requests/7"}, []string{"gitlab.corp.example"}},
{"strips port", []string{"gerrit.corp.example:8443"}, []string{"gerrit.corp.example"}},
{"strips userinfo", []string{"https://user:pw@gitlab.corp.example"}, []string{"gitlab.corp.example"}},
{"strips trailing dot", []string{"gitlab.corp.example."}, []string{"gitlab.corp.example"}},
{"keeps subdomain wildcard", []string{"*.corp.example"}, []string{"*.corp.example"}},
{"wildcard with port", []string{"*.corp.example:8443"}, []string{"*.corp.example"}},
{"ipv6 literal keeps brackets", []string{"[::1]:8443"}, []string{"[::1]"}},
{"drops blanks", []string{"", " ", "\t"}, nil},
{"drops bare wildcard", []string{"*", "*.", "*.example"}, nil},
{"drops embedded wildcard", []string{"git.*.corp.example"}, nil},
{"drops scheme-only", []string{"https://"}, nil},
{"dedupes after normalizing", []string{"gitlab.corp.example", "GITLAB.corp.example:443"}, []string{"gitlab.corp.example"}},
{"preserves order", []string{"b.example", "a.example"}, []string{"b.example", "a.example"}},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := NormalizeTrustedDomains(tc.in)
if len(got) != len(tc.want) {
t.Fatalf("NormalizeTrustedDomains(%v) = %v, want %v", tc.in, got, tc.want)
}
for i := range tc.want {
if got[i] != tc.want[i] {
t.Errorf("NormalizeTrustedDomains(%v)[%d] = %q, want %q", tc.in, i, got[i], tc.want[i])
}
}
})
}
}
6 changes: 6 additions & 0 deletions internal/web/api_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ type SettingsResponse struct {
// show_only_installed_tools ("" mapped to "shell" for web).
HiddenTools []string `json:"hiddenTools"`
PickerTools []string `json:"pickerTools"`

// Link-open policy for the web terminal (issue #1682). TrustedDomains
// are normalized hosts whose links open without a confirm;
// ConfirmLinkOpen reports whether every other host still confirms.
TrustedDomains []string `json:"trustedDomains"`
ConfirmLinkOpen bool `json:"confirmLinkOpen"`
}

// ProfilesResponse is returned by GET /api/profiles.
Expand Down
10 changes: 10 additions & 0 deletions internal/web/handlers_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ func (s *Server) handleSettings(w http.ResponseWriter, r *http.Request) {
return
}

// The link-open allowlist (issue #1682) is always serialized as an array,
// never null, so the client's `Array.isArray` hydration guard cannot be
// tripped by an unconfigured server.
trustedDomains := s.cfg.TrustedDomains
if trustedDomains == nil {
trustedDomains = []string{}
}

// Tool-visibility filter (issue #1259) is read from the process registry at
// request time, so it reflects the current config (re-probed only when config
// changes — see currentRegistry). It is a display filter only.
Expand All @@ -30,6 +38,8 @@ func (s *Server) handleSettings(w http.ResponseWriter, r *http.Request) {
ToolFilterFallback: session.ToolFilterFallbackActive(),
HiddenTools: session.ConfiguredHiddenToolNames(),
PickerTools: session.PickerToolNames(),
TrustedDomains: trustedDomains,
ConfirmLinkOpen: s.cfg.confirmLinkOpen(),
})
}

Expand Down
63 changes: 63 additions & 0 deletions internal/web/handlers_settings_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package web

import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
Expand Down Expand Up @@ -159,3 +160,65 @@ func TestProfilesUnauthorized(t *testing.T) {
t.Errorf("expected UNAUTHORIZED error, got: %s", rr.Body.String())
}
}

// Issue #1682: GET /api/settings carries the terminal link-open policy so the
// web UI can skip the confirm for trusted hosts. A Config that never set
// ConfirmLinkOpen must still report the confirm ON — a false there would
// silently disable the prompt for every host.
func TestSettingsLinkPolicy_DefaultsToConfirmOn(t *testing.T) {
srv := NewServer(Config{ListenAddr: "127.0.0.1:0", Profile: "default"})
srv.menuData = &fakeMenuDataLoader{snapshot: &MenuSnapshot{}}

req := httptest.NewRequest(http.MethodGet, "/api/settings", nil)
rr := httptest.NewRecorder()
srv.Handler().ServeHTTP(rr, req)

if rr.Code != http.StatusOK {
t.Fatalf("expected status %d, got %d: %s", http.StatusOK, rr.Code, rr.Body.String())
}
var got SettingsResponse
if err := json.Unmarshal(rr.Body.Bytes(), &got); err != nil {
t.Fatalf("decode settings: %v", err)
}
if !got.ConfirmLinkOpen {
t.Error("confirmLinkOpen = false with ConfirmLinkOpen unset, want true (confirm stays on)")
}
if len(got.TrustedDomains) != 0 {
t.Errorf("trustedDomains = %v, want empty when unconfigured", got.TrustedDomains)
}
}

func TestSettingsLinkPolicy_ServesConfiguredAllowlistAndToggle(t *testing.T) {
off := false
srv := NewServer(Config{
ListenAddr: "127.0.0.1:0",
Profile: "default",
TrustedDomains: []string{"gitlab.corp.example", "*.ci.corp.example"},
ConfirmLinkOpen: &off,
})
srv.menuData = &fakeMenuDataLoader{snapshot: &MenuSnapshot{}}

req := httptest.NewRequest(http.MethodGet, "/api/settings", nil)
rr := httptest.NewRecorder()
srv.Handler().ServeHTTP(rr, req)

if rr.Code != http.StatusOK {
t.Fatalf("expected status %d, got %d: %s", http.StatusOK, rr.Code, rr.Body.String())
}
var got SettingsResponse
if err := json.Unmarshal(rr.Body.Bytes(), &got); err != nil {
t.Fatalf("decode settings: %v", err)
}
if got.ConfirmLinkOpen {
t.Error("confirmLinkOpen = true, want false when ConfirmLinkOpen is explicitly false")
}
want := []string{"gitlab.corp.example", "*.ci.corp.example"}
if len(got.TrustedDomains) != len(want) {
t.Fatalf("trustedDomains = %v, want %v", got.TrustedDomains, want)
}
for i := range want {
if got.TrustedDomains[i] != want[i] {
t.Errorf("trustedDomains[%d] = %q, want %q", i, got.TrustedDomains[i], want[i])
}
}
}
45 changes: 45 additions & 0 deletions internal/web/issue1682_link_policy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package web

import (
"regexp"
"strings"
"testing"
)

// TestTerminalLinkHandler_WiredIntoTerminal_RegressionFor1682 pins the one
// line that makes `[web].trusted_domains` reachable at all.
//
// The trusted-domain policy lives in static/app/terminalLinks.js and is
// covered behaviorally by tests/web/unit/terminalLinks.test.js (matching
// rules) and tests/web/e2e/trusted-domains.spec.js (allowlisted vs not, in a
// real browser against a real server). Neither can observe whether xterm is
// actually handed the handler: an OSC-8 link only exists once a live tmux
// pane emits one, and xterm exposes no back-reference from its DOM element to
// the Terminal instance.
//
// So this is a source pin, in the same spirit as
// TestWebBundle_ChartNotInInitialPayload_RegressionFor1022. Drop the
// `linkHandler` option from TerminalPanel.js and every link silently falls
// back to xterm's built-in "could potentially be dangerous" confirm — the
// exact regression issue #1682 asks us to prevent.
func TestTerminalLinkHandler_WiredIntoTerminal_RegressionFor1682(t *testing.T) {
panel := readEmbedded(t, "static/app/TerminalPanel.js")

if !strings.Contains(panel, `from './terminalLinks.js'`) {
t.Error("TerminalPanel.js does not import ./terminalLinks.js")
}
wired := regexp.MustCompile(`linkHandler\s*:\s*createTerminalLinkHandler\(\)`)
if !wired.MatchString(panel) {
t.Error("TerminalPanel.js does not pass linkHandler: createTerminalLinkHandler() to new Terminal(...) — " +
"without it xterm's built-in confirm fires on every link and [web].trusted_domains has no effect")
}

// The policy module must keep reading the hydrated signals rather than a
// snapshot captured at terminal-construction time.
links := readEmbedded(t, "static/app/terminalLinks.js")
for _, want := range []string{"trustedDomainsSignal", "confirmLinkOpenSignal"} {
if !strings.Contains(links, want) {
t.Errorf("terminalLinks.js no longer references %s — link policy would ignore /api/settings", want)
}
}
}
Loading
Loading