diff --git a/db/asset_intercept_match.go b/db/asset_intercept_match.go index 65a4be1..d919856 100644 --- a/db/asset_intercept_match.go +++ b/db/asset_intercept_match.go @@ -45,6 +45,12 @@ func (r AssetInterceptRule) Reason() string { // matchOne checks whether a single enabled rule matches the given domain/IP/URL // candidates, and returns the concrete matched value. +// +// URL rules match the URL string as given, with no normalisation. Known limit: +// exact_url misses any other spelling of the same URL (case, percent-encoding +// such as %2F, a trailing slash, an added query, /./), and fuzzy_url misses an +// encoded form of its substring. At tool dispatch the string is the URL the +// call declares (intercept, targetCheck.run). func matchOne(r AssetInterceptRule, domains, ips, urls []string) (string, bool) { p := strings.TrimSpace(r.Pattern) if p == "" { diff --git a/guard/target_gate_test.go b/guard/target_gate_test.go index ec61a86..a3ec87f 100644 --- a/guard/target_gate_test.go +++ b/guard/target_gate_test.go @@ -282,18 +282,27 @@ type gateDB struct { task string // task id } -// stubResolver answers every name with the addresses it holds. +// stubResolver answers every name with the addresses it holds, and counts the +// lookups. type stubResolver struct { mu sync.Mutex ips []string err error + n int } func (r *stubResolver) Name() string { return "stub" } +func (r *stubResolver) lookups() int { + r.mu.Lock() + defer r.mu.Unlock() + return r.n +} + func (r *stubResolver) LookupIPs(context.Context, string) ([]netip.Addr, error) { r.mu.Lock() defer r.mu.Unlock() + r.n++ if r.err != nil { return nil, r.err } @@ -576,8 +585,8 @@ func TestGateOffLeavesEveryBranchUnchanged(t *testing.T) { } } -// A deny by a rule blocks first, as before: the gate is not consulted and no -// target request exists. +// A deny by a rule or by the judge blocks first, as before: the gate is not +// consulted, nothing is resolved and no target request exists. func TestDenyRuleWinsBeforeTargetRequest(t *testing.T) { g := newGateDB(t) m := marker(t) @@ -601,8 +610,77 @@ func TestDenyRuleWinsBeforeTargetRequest(t *testing.T) { if block, _ := pre(guard, g.ctx(true, time.Minute), "Bash", `{"command":"curl https://app.example.com/ # `+m+`"}`); !block { t.Fatal("deny did not block") } - if len(g.pending()) != 0 || len(g.pins()) != 0 { - t.Fatalf("pending=%+v pins=%+v, want none", g.pending(), g.pins()) + if len(g.pending()) != 0 || len(g.pins()) != 0 || g.res.lookups() != 0 { + t.Fatalf("pending=%+v pins=%+v lookups=%d, want none", g.pending(), g.pins(), g.res.lookups()) + } + + // A deny by the judge takes the same branch. + g.judge("deny") + input := `{"command":"curl https://judged.example.com/"}` + gate = &fakeGate{t: t, check: unpinnedOnce} + guard = NewWithInterceptor(g.ic) + guard.gate = gate + before := len(g.rows()) + if block, msg := pre(guard, g.ctx(true, time.Minute), "Bash", input); !block || !strings.Contains(msg, "judge says deny") || strings.Contains(msg, "TARGET_") { + t.Fatalf("judge deny: block=%v msg=%q", block, msg) + } + if gate.checks != 0 || len(gate.asked) != 0 { + t.Fatalf("judge deny: the gate was reached: %d checks, %d requests", gate.checks, len(gate.asked)) + } + if rows := g.rows()[before:]; len(rows) != 1 || rows[0].status != "denied" || rows[0].decidedBy != "model" || rows[0].targetKey != "" { + t.Fatalf("judge deny: rows = %+v, want the denied judge row only", rows) + } + guard = NewWithInterceptor(g.ic) + if block, _ := pre(guard, g.ctx(true, time.Minute), "Bash", input); !block { + t.Fatal("judge deny did not block") + } + if len(g.pending()) != 0 || len(g.pins()) != 0 || g.res.lookups() != 0 { + t.Fatalf("judge deny: pending=%+v pins=%+v lookups=%d, want none", g.pending(), g.pins(), g.res.lookups()) + } +} + +// An asset block rule wins over an allow and an ask, of a rule or of the +// judge: the call is blocked, the allow is not logged, no human is asked, and +// the rule hits on what the call declares, so nothing is resolved. A name rule +// and a URL rule behave the same. +func TestBlockRuleWinsOverAllowAndAsk(t *testing.T) { + const input = `{"command":"curl https://app.example.com/blocked-path # %s"}` + for _, block := range []struct{ name, kind, pattern string }{ + {"name rule", "exact_domain", "app.example.com"}, + {"URL rule", "fuzzy_url", "/blocked-path"}, + } { + for _, v := range []struct{ name, by, action string }{ + {"rule allow", "rule", "allow"}, + {"judge allow", "judge", "allow"}, + {"rule ask", "rule", "ask"}, + {"judge ask", "judge", "ask"}, + } { + t.Run(block.name+" over "+v.name, func(t *testing.T) { + g := newGateDB(t) + id, _ := strconv.ParseInt(g.task, 10, 64) + if _, err := g.d.Assets().CreateTaskInterceptRule(id, "block", block.kind, block.pattern, "guard test", true); err != nil { + t.Fatal(err) + } + m := marker(t) + if v.by == "rule" { + g.rule(v.action, m) + } else { + g.judge(v.action) + } + guard := NewWithInterceptor(g.ic) + r := g.result(call(guard, g.ctx(true, time.Minute), "Bash", fmt.Sprintf(input, m))) + if !r.block || !strings.Contains(r.msg, intercept.TargetRuleBlocked+": ") || !strings.Contains(r.msg, block.pattern) { + t.Fatalf("block=%v msg=%q, want %s", r.block, r.msg, intercept.TargetRuleBlocked) + } + rows := g.rows() + if len(rows) != 1 || rows[0].status != "denied" || rows[0].decidedBy != "system" || rows[0].ruleID != 0 { + t.Fatalf("rows = %+v, want the one block row of the gate: no allow logged, nobody asked", rows) + } + if n := g.res.lookups(); n != 0 || len(g.pins()) != 0 { + t.Fatalf("%d lookups, pins %+v; want none", n, g.pins()) + } + }) + } } } diff --git a/intercept/target_declared_test.go b/intercept/target_declared_test.go new file mode 100644 index 0000000..8edb77f --- /dev/null +++ b/intercept/target_declared_test.go @@ -0,0 +1,193 @@ +package intercept + +import ( + "encoding/json" + "slices" + "strings" + "testing" + + "github.com/Autumn-27/artex/db" +) + +// The declared forms of a call decide first: the URL block rules on the URLs +// the call writes, then the block rules on every name, IP literal and range as +// written. Nothing is resolved before that, so a hit costs no lookup. + +func TestExtractDeclaredURLs(t *testing.T) { + raw := func(v any) []byte { + b, _ := json.Marshal(v) + return b + } + for _, c := range []struct { + name string + tool string + input []byte + want []string + }{ + {"WebFetch", "WebFetch", raw(map[string]string{"url": " https://app.example.com/admin?x=1 "}), []string{"https://app.example.com/admin?x=1"}}, + {"WebFetch without a target", "WebFetch", raw(map[string]string{"url": "http:///path"}), nil}, + {"Bash, query kept", "Bash", raw(command("curl -s 'https://a.example.com/admin?x=1'")), []string{"https://a.example.com/admin?x=1"}}, + {"Bash, two URLs", "Bash", raw(command("curl http://a.example.com/x && curl https://b.example.com:8443/y")), + []string{"http://a.example.com/x", "https://b.example.com:8443/y"}}, + {"shell_send", "shell_send", raw(map[string]string{"text": "curl http://a.example.com/x\n"}), []string{"http://a.example.com/x"}}, + {"Monitor", "Monitor", raw(command("watch curl http://198.51.100.9:8080/status")), []string{"http://198.51.100.9:8080/status"}}, + {"MCP string leaves", "mcp__scan__run", []byte(`{"target":"http://a.example.com/p","opts":{"list":["https://b.example.com/q"]}}`), + []string{"http://a.example.com/p", "https://b.example.com/q"}}, + {"ExecuteExtraTool unwrap", executeExtraTool, raw(map[string]any{"tool_name": "WebFetch", "params": map[string]string{"url": "https://a.example.com/z"}}), + []string{"https://a.example.com/z"}}, + {"tool without a target", "web_search", raw(map[string]string{"query": "https://a.example.com/admin"}), nil}, + {"wrapped tool without a target", executeExtraTool, raw(map[string]any{"tool_name": "web_search", "params": map[string]string{"query": "https://a.example.com/admin"}}), nil}, + {"authority that names no host", "Bash", raw(command("curl http://$HOST/x https://${H}:8443/admin")), nil}, + } { + d := extractDeclared(c.tool, c.input) + if !slices.Equal(d.urls, c.want) { + t.Errorf("%s: urls = %q, want %q", c.name, d.urls, c.want) + } + // The targets are those ExtractTargets returns. + if !slices.Equal(d.targets, ExtractTargets(c.tool, c.input)) { + t.Errorf("%s: targets differ from ExtractTargets", c.name) + } + if len(d.urls) > 0 && len(d.targets) == 0 { + t.Errorf("%s: a URL without a target", c.name) + } + } +} + +// A name that a name rule blocks is blocked on the name as written: no lookup +// is made for it, nor for another name of the same call that no rule blocks. +func TestBlockRuleOnDeclaredNameNeedsNoLookup(t *testing.T) { + for _, rule := range []db.AssetInterceptRule{ + {Enabled: true, Kind: "exact_domain", Pattern: "blocked.example.com"}, + {Enabled: true, Kind: "fuzzy_domain", Pattern: "blocked.example"}, + } { + g := newGate(t) + g.res.set("ok.example.com", "203.0.113.7") + g.res.set("blocked.example.com", "203.0.113.8") + g.store.rules = []db.AssetInterceptRule{rule} + for name, v := range map[string]verdict{ + "WebFetch": g.fetch("https://blocked.example.com/"), + // The unblocked name comes first in the call. + "two names": g.check("Bash", `{"command":"curl https://ok.example.com/ https://blocked.example.com/"}`), + } { + res := g.blocked(rule.Kind+", "+name, TargetRuleBlocked, v) + if res.Host != "blocked.example.com" || !strings.Contains(res.Reason, rule.Pattern) { + t.Fatalf("%s, %s: result = %+v", rule.Kind, name, res) + } + } + if n := g.res.total(); n != 0 { + t.Fatalf("%s: %d lookups, want none", rule.Kind, n) + } + for _, row := range g.store.logged { + if a := row.Audit.Targets[0]; a.Outcome != "rule_blocked" || len(a.ResolvedIPs) != 0 { + t.Fatalf("%s: logged target = %+v, want rule_blocked with no address", rule.Kind, a) + } + } + } +} + +// URL rules apply at dispatch, on the URLs the call declares, before anything +// is resolved. A hit blocks with the code of every block rule, opens no +// request, and wins over a pin and an exemption. +func TestURLBlockRuleAtDispatch(t *testing.T) { + g := newGate(t) + g.res.set("app.example.com", "203.0.113.7") + g.res.set("pypi.org", "151.101.0.223") + g.store.exempt = []db.TargetExemption{{Host: "pypi.org", Port: 443}} + g.store.approve(TargetRequest{Host: "app.example.com", Port: 443, ResolvedIPs: []string{"203.0.113.7"}}) + g.passes("pinned", g.fetch("https://app.example.com/admin")) + g.passes("exempt", g.fetch("https://pypi.org/admin")) + + rule := func(kind, pattern string, enabled bool) []db.AssetInterceptRule { + return []db.AssetInterceptRule{{Enabled: enabled, Kind: kind, Pattern: pattern}} + } + // A disabled URL rule blocks nothing. + g.store.rules = rule("exact_url", "https://app.example.com/admin", false) + g.passes("disabled rule", g.fetch("https://app.example.com/admin")) + + lookups, rows := g.res.total(), len(g.store.logged) + for _, c := range []struct { + name string + rules []db.AssetInterceptRule + v func() verdict + host string + port int + matchedURL string + }{ + {"exact_url, WebFetch of a pinned host", rule("exact_url", "https://app.example.com/admin", true), + func() verdict { return g.fetch("https://app.example.com/admin") }, "app.example.com", 443, "https://app.example.com/admin"}, + {"fuzzy_url, exempt host", rule("fuzzy_url", "/admin", true), + func() verdict { return g.fetch("https://pypi.org/admin") }, "pypi.org", 443, "https://pypi.org/admin"}, + {"fuzzy_url, Bash with a query", rule("fuzzy_url", "/admin", true), + func() verdict { return g.check("Bash", `{"command":"curl -s 'https://app.example.com/admin?x=1'"}`) }, "app.example.com", 443, "https://app.example.com/admin?x=1"}, + {"fuzzy_url, second URL of the call", rule("fuzzy_url", "/admin", true), + func() verdict { + return g.check("Bash", `{"command":"curl https://app.example.com/ && curl http://198.51.100.9:8080/admin"}`) + }, "198.51.100.9", 8080, "http://198.51.100.9:8080/admin"}, + {"fuzzy_url, MCP input", rule("fuzzy_url", "/ADMIN", true), + func() verdict { return g.check("mcp__scan__run", `{"target":"https://app.example.com/admin/users"}`) }, "app.example.com", 443, "https://app.example.com/admin/users"}, + {"exact_url, wrapped WebFetch", rule("exact_url", "https://app.example.com/admin", true), + func() verdict { + return g.check(executeExtraTool, `{"tool_name":"WebFetch","params":{"url":"https://app.example.com/admin"}}`) + }, "app.example.com", 443, "https://app.example.com/admin"}, + } { + g.store.rules = c.rules + g.res.set("app.example.com", "198.51.100.200") // a lookup would make a pinned host ask + res := g.blocked(c.name, TargetRuleBlocked, c.v()) + if res.Host != c.host || res.Port != c.port || !strings.Contains(res.Reason, c.matchedURL+" is blocked by an asset rule") || + !strings.Contains(res.Reason, c.rules[0].Pattern) { + t.Errorf("%s: result = %+v", c.name, res) + } + } + if n := g.res.total() - lookups; n != 0 { + t.Fatalf("%d lookups for calls a URL rule blocks, want none", n) + } + if got := g.store.outcomes()[rows:]; len(got) != 6 || slices.ContainsFunc(got, func(o string) bool { return o != "denied/rule_blocked" }) { + t.Fatalf("logged rows = %v, want 6 rule_blocked rows", got) + } + + // The rendered URL of a custom http tool and the URLs of a rendered command + // meet the same rules, with no lookup. + g.store.rules = rule("exact_url", "https://app.example.com/admin", true) + if res := GateRenderedURL(g.ctx(), "probe_http", "https://app.example.com/admin"); res.Code != TargetRuleBlocked || res.Host != "app.example.com" { + t.Fatalf("rendered URL: %+v", res) + } + g.store.rules = rule("fuzzy_url", "/admin", true) + if res := GateRenderedCommand(g.ctx(), "probe_cmd", "curl 'https://app.example.com/admin'"); res.Code != TargetRuleBlocked { + t.Fatalf("rendered command: %+v", res) + } + if n := g.res.total() - lookups; n != 0 { + t.Fatalf("%d lookups for rendered calls a URL rule blocks, want none", n) + } +} + +// Documents the known limit of URL rules: they match the URL as the call +// writes it. Another spelling of the same URL is not blocked. For a host, a +// domain, IP or CIDR rule is the one to write. +func TestExactURLRuleMatchesOnlyAsWritten(t *testing.T) { + g := newGate(t) + g.res.set("app.example.com", "203.0.113.7") + g.store.approve(TargetRequest{Host: "app.example.com", Port: 443, ResolvedIPs: []string{"203.0.113.7"}}) + + g.store.rules = []db.AssetInterceptRule{{Enabled: true, Kind: "exact_url", Pattern: "https://app.example.com/admin"}} + g.blocked("as written", TargetRuleBlocked, g.fetch("https://app.example.com/admin")) + for _, u := range []string{ + "https://APP.example.com/admin", // case of the host + "https://app.example.com/Admin", // case of the path + "https://app.example.com/admin/", // trailing slash + "https://app.example.com/admin?x=1", // added query + "https://app.example.com/%61dmin", // percent-encoding + "https://app.example.com/./admin", // dot segment + "https://app.example.com:443/admin", // default port written out + } { + g.passes("exact_url, "+u, g.fetch(u)) + } + + g.store.rules = []db.AssetInterceptRule{{Enabled: true, Kind: "fuzzy_url", Pattern: "/admin"}} + g.blocked("fuzzy, as written", TargetRuleBlocked, g.fetch("https://app.example.com/x/admin")) + for _, u := range []string{"https://app.example.com/%61dmin", "https://app.example.com/adm%69n", "https://app.example.com//%2fadmin"} { + if strings.Contains(strings.ToLower(u), "/admin") { + t.Fatalf("%s contains the pattern; the case documents an encoded form", u) + } + g.passes("fuzzy_url, "+u, g.fetch(u)) + } +} diff --git a/intercept/target_extract.go b/intercept/target_extract.go index 315ae43..5541d7a 100644 --- a/intercept/target_extract.go +++ b/intercept/target_extract.go @@ -62,6 +62,13 @@ func toolSet(names ...string) map[string]bool { // names on its own. The gate detects a declared target; it does not prevent an // undeclared connection. func ExtractTargets(toolName string, input []byte) []Target { + return extractDeclared(toolName, input).targets +} + +// extractDeclared reads what a tool call declares: its targets, as +// ExtractTargets returns them, and the URLs it writes, as written, for the URL +// block rules. A URL is kept only when it names a target. +func extractDeclared(toolName string, input []byte) targetList { if toolName == executeExtraTool { var in struct { ToolName string `json:"tool_name"` @@ -72,13 +79,15 @@ func ExtractTargets(toolName string, input []byte) []Target { } } if noTargetTools[toolName] { - return nil + return targetList{} } var out targetList switch toolName { case "WebFetch": - if t, ok := NormalizeTarget(declaredFields(input).URL); ok { + u := declaredFields(input).URL + if t, ok := NormalizeTarget(u); ok { out.add(t) + out.urls = append(out.urls, strings.TrimSpace(u)) } case "Bash", "shell_open", "Monitor": out.scan(declaredFields(input).Command) @@ -87,7 +96,7 @@ func ExtractTargets(toolName string, input []byte) []Target { default: out.scanStrings(input) } - return out.targets + return out } // declaredInput holds the fields of the built-in tools that carry a target. @@ -109,6 +118,7 @@ func declaredFields(input []byte) declaredInput { type targetList struct { targets []Target seen map[Target]bool + urls []string // the URLs that name a target, as written } func (l *targetList) add(t Target) { @@ -207,6 +217,10 @@ func (l *targetList) scan(s string) { found = true } } + if found { + // The whole token, path and query included, from the original string. + l.urls = append(l.urls, s[m[0]:m[1]]) + } // The scheme and what follows the authority are never read again. An // authority that named no host is: an address written in it still counts. blank(m[0], m[4]) diff --git a/intercept/target_gate.go b/intercept/target_gate.go index 1c43f22..a99a73f 100644 --- a/intercept/target_gate.go +++ b/intercept/target_gate.go @@ -93,12 +93,12 @@ func (i *Interceptor) CheckTargets(ctx context.Context, scope TargetScope, toolN if scope.Enabled == nil || !scope.Enabled() { return nil, TargetResult{} } - targets := ExtractTargets(toolName, input) - if len(targets) == 0 { + declared := extractDeclared(toolName, input) + if len(declared.targets) == 0 { return nil, TargetResult{} } - c := &targetCheck{i: i, ctx: ctx, scope: scope, tool: toolName, input: input} - return c.run(targets) + c := &targetCheck{i: i, ctx: ctx, scope: scope, tool: toolName, input: input, urls: declared.urls} + return c.run(declared.targets) } // targetCheck is one run of the gate decision. @@ -109,6 +109,7 @@ type targetCheck struct { ref db.TargetScopeRef tool string input []byte + urls []string // the URLs the call declares, as written, for the URL block rules } // checkedTarget is one target of the call and what the gate learned about it. @@ -146,23 +147,47 @@ func (c *targetCheck) run(targets []Target) ([]TargetRequest, TargetResult) { return c.block(TargetGateUnavailable, "gate_unavailable", first, "the exemption list could not be read: "+err.Error()) } - // Block rules come first, on the name and on what it resolves to: an exempt - // or pinned host that matches one is blocked. A name the configuration - // exempts is never resolved. + // Block rules come first, and an exempt or pinned host that matches one is + // blocked. They are matched on what the call declares before anything is + // resolved: a hit there costs no lookup, for any target of the call. + // + // URL rules (exact_url, fuzzy_url) are matched once per decision, on the + // URLs as the call writes them. That is a known limit: exact_url compares + // the string as written, so another case, encoding (%2F), a trailing slash, + // an added query or /./ does not match, and fuzzy_url is evaded by encoding + // the substring; a command can also build its URL at run time. To keep an + // agent away from a host, a domain, IP or CIDR rule is the one to write. + if rule, u, hit := db.MatchAssetInterceptRules(rules, nil, nil, c.urls); hit { + ct := &checkedTarget{Target: targets[0]} + if t, ok := NormalizeTarget(u); ok { + ct.Target = t + } + return c.block(TargetRuleBlocked, "rule_blocked", ct, fmt.Sprintf("%s is blocked by an asset rule. %s", u, rule.Reason())) + } + // Then the declared form of every target: the name as written, the IP + // literal, the range. checked := make([]*checkedTarget, len(targets)) for n, t := range targets { ct := &checkedTarget{Target: t} checked[n] = ct - switch { - case t.Kind != TargetDomain: + if t.Kind != TargetDomain { ct.ips, ct.resolver, ct.resolvedAt = []string{t.Host}, literalResolver, time.Now().UTC() - case !ex.derived[db.TargetKey(t.Host, t.Port)]: - c.resolve(ct, false) } if rule, hit := blockRule(rules, ct); hit { return c.block(TargetRuleBlocked, "rule_blocked", ct, fmt.Sprintf("%s is blocked by an asset rule. %s", t, rule.Reason())) } } + // Only then are the names resolved, and the rules matched on the addresses. + // A name the configuration exempts is never resolved. + for _, ct := range checked { + if ct.Kind != TargetDomain || ex.derived[db.TargetKey(ct.Host, ct.Port)] { + continue + } + c.resolve(ct, false) + if rule, hit := blockRule(rules, ct); hit { + return c.block(TargetRuleBlocked, "rule_blocked", ct, fmt.Sprintf("%s is blocked by an asset rule. %s", ct.Target, rule.Reason())) + } + } i.mu.RLock() proxied := i.upstreamProxy @@ -306,7 +331,8 @@ func (c *targetCheck) pinUsed(ct *checkedTarget, pin *db.TargetPin) { } // blockRule matches the asset block rules against a target: its name, and its -// addresses. A range is matched as written and by its network address. +// addresses. A range is matched as written and by its network address. URL +// rules are not matched here: run matches them once, on the declared URLs. func blockRule(rules []db.AssetInterceptRule, ct *checkedTarget) (db.AssetInterceptRule, bool) { var domains []string ips := ct.ips diff --git a/intercept/target_rendered.go b/intercept/target_rendered.go index 67921b0..220e5a8 100644 --- a/intercept/target_rendered.go +++ b/intercept/target_rendered.go @@ -40,7 +40,7 @@ func GateRenderedURL(ctx context.Context, toolName, rawURL string) TargetResult if err != nil { return undeterminedTarget(ctx, scope, toolName, input, "the target of the rendered URL could not be read ("+err.Error()+")") } - return gateRendered(ctx, scope, toolName, input, []Target{t}) + return gateRendered(ctx, scope, toolName, input, []Target{t}, []string{rawURL}) } // GateRenderedCommand is the target gate for the command a custom command tool @@ -62,7 +62,7 @@ func GateRenderedCommand(ctx context.Context, toolName, command string, readings for _, r := range readings { l.scan(r) } - return gateRendered(ctx, scope, toolName, renderedInput("command", command), l.targets) + return gateRendered(ctx, scope, toolName, renderedInput("command", command), l.targets, l.urls) } func renderedScope(ctx context.Context) (TargetScope, bool) { @@ -79,9 +79,10 @@ func renderedInput(field, value string) []byte { // gateRendered runs the gate decision on the targets of a rendered call, asks // for the approval of those no pin covers, and checks again after the -// decisions. It never merges an ask: the rules and the judge decided at the -// hook, before the tool ran. -func gateRendered(ctx context.Context, scope TargetScope, toolName string, input []byte, targets []Target) TargetResult { +// decisions. urls are the URLs the call renders, for the URL block rules. It +// never merges an ask: the rules and the judge decided at the hook, before the +// tool ran. +func gateRendered(ctx context.Context, scope TargetScope, toolName string, input []byte, targets []Target, urls []string) TargetResult { if len(targets) == 0 { return TargetResult{} } @@ -95,7 +96,7 @@ func gateRendered(ctx context.Context, scope TargetScope, toolName string, input if !scope.Enabled() { return nil, TargetResult{} } - c := &targetCheck{i: i, ctx: ctx, scope: scope, tool: toolName, input: input} + c := &targetCheck{i: i, ctx: ctx, scope: scope, tool: toolName, input: input, urls: urls} return c.run(targets) } unpinned, res := check() diff --git a/server/customtool_target_gate_test.go b/server/customtool_target_gate_test.go index 47b1723..6bcd549 100644 --- a/server/customtool_target_gate_test.go +++ b/server/customtool_target_gate_test.go @@ -571,6 +571,49 @@ func TestDenyRuleWinsBeforeRenderedTarget(t *testing.T) { } } +// Documents a known limit: script tools stay advisory (D9). A custom script +// tool's target is in its stored code, which neither the hook nor the tool +// reads, so in a gated task the call is allowed and the script reaches a target +// nobody approved: no lookup, no request, no row. This test changes only if +// that decision changes. +func TestBypass_CustomScriptToolTargetNotGated(t *testing.T) { + if detectPython() == "" { + t.Skip("python3 unavailable") + } + g := newGateServer(t) + on := true + task := g.newTask(t, db.TaskCreateOptions{TargetGate: &on}) + res := &countingResolver{answers: map[string][]string{"localhost": {"127.0.0.1"}}} + g.m.interceptor.SetResolver(res) + _, hits, _, port := countingServer(t) + + code := "import urllib.request\n" + + "opener = urllib.request.build_opener(urllib.request.ProxyHandler({}))\n" + + "print(opener.open('http://localhost:" + port + "/script', timeout=20).read().decode())\n" + tool := g.s.buildCustomTool(&db.Tool{Key: "probe_script", Kind: "script", Exec: execJSON(t, map[string]any{"code": code})}) + // The gate sees no target in the call: only the parameters reach the hook. + if got := intercept.ExtractTargets("probe_script", []byte(`{}`)); len(got) != 0 { + t.Fatalf("the hook reads %+v; the test needs a target only the stored code names", got) + } + + p := &toolScript{calls: []scriptedCall{{"probe_script", `{}`}}, results: make(chan string, 16)} + done := runScriptedWorker(t, g, task, p, tool) + if r := p.next(t); !strings.Contains(r, "probe-reached") || strings.Contains(r, "TARGET_") { + t.Fatalf("script tool in a gated task: %q, want it allowed and the target reached", r) + } + select { + case <-done: + case <-time.After(60 * time.Second): + t.Fatal("the worker run did not end") + } + if hits.Load() != 1 { + t.Fatalf("%d requests reached the unapproved target, want 1 (the gap)", hits.Load()) + } + if n, rows := res.lookups(), targetRowCount(t, g.m.pg, task.ID); n != 0 || rows != 0 { + t.Fatalf("%d lookups, %d target rows; the gate did see the script's target", n, rows) + } +} + // A task with the gate off (the default) runs as before: a worker's WebFetch // and the custom tools reach their targets, and the gate writes no row and // resolves no name.