diff --git a/agent/chat.go b/agent/chat.go index 02ec298..a859dd3 100644 --- a/agent/chat.go +++ b/agent/chat.go @@ -128,13 +128,10 @@ func (c *ChatAgent) Chat(ctx context.Context, agentKey, sessionID, message strin Provider: c.prov, SystemPrompt: system, DynamicBoundary: boundary, - Tools: tools, + Tools: withWebFetch(tools, c.proxyAddr, c.proxyCACert), // WebFetch: routed through the recording proxy for tracing; loads the proxy CA to verify MITM-resigned HTTPS certificates; every redirect hop is gated DeferredTools: def.Deferred, UnlockSet: def.Unlock, PermissionMode: permission.ModeBypass, - EnableWebFetch: true, // routed through the recording proxy for tracing; loads the proxy CA to verify MITM-resigned HTTPS certificates - WebFetchProxy: c.proxyAddr, - WebFetchCACert: c.proxyCACert, // Web search (optional). ddgs needs no key; brave-free needs BraveKey; tavily needs TavilyKey. // WebSearchProxy is a dedicated egress proxy (http/https/socks5), unrelated to the traffic-recording MITM proxy; empty = direct. EnableWebSearch: ws.Enabled, diff --git a/agent/mainagent.go b/agent/mainagent.go index 67b6d41..5b31883 100644 --- a/agent/mainagent.go +++ b/agent/mainagent.go @@ -144,13 +144,10 @@ func (m *MainAgent) Chat(ctx context.Context, taskID int64, mainSeg int, as *db. Provider: m.prov, SystemPrompt: system, DynamicBoundary: boundary, - Tools: tools, + Tools: withWebFetch(tools, m.proxyAddr, m.proxyCACert), // WebFetch: routed through the recording proxy for tracing; loads the proxy CA to verify MITM-resigned HTTPS certificates; every redirect hop is gated DeferredTools: def.Deferred, UnlockSet: def.Unlock, PermissionMode: permission.ModeBypass, - EnableWebFetch: true, // routed through the recording proxy for tracing; loads the proxy CA to verify MITM-resigned HTTPS certificates - WebFetchProxy: m.proxyAddr, - WebFetchCACert: m.proxyCACert, // Web search (optional). ddgs needs no key; brave-free needs BraveKey; tavily needs TavilyKey. // WebSearchProxy is a dedicated egress proxy (http/https/socks5), unrelated to the traffic-recording MITM proxy; empty = direct. EnableWebSearch: m.webSearch.Enabled, diff --git a/agent/planner.go b/agent/planner.go index 696e768..99128b1 100644 --- a/agent/planner.go +++ b/agent/planner.go @@ -426,13 +426,10 @@ func (p *Planner) Plan(ctx context.Context, taskID int64, as *db.AssetStore, ts Provider: p.prov, SystemPrompt: system, DynamicBoundary: boundary, - Tools: tools, + Tools: withWebFetch(tools, p.proxyAddr, p.proxyCACert), // WebFetch: route through the recording proxy for traceability; load the proxy CA to verify MITM-re-signed HTTPS certs; every redirect hop is gated DeferredTools: def.Deferred, UnlockSet: def.Unlock, PermissionMode: permission.ModeBypass, - EnableWebFetch: true, // route through the recording proxy for traceability; load the proxy CA to verify MITM-re-signed HTTPS certs - WebFetchProxy: p.proxyAddr, - WebFetchCACert: p.proxyCACert, // Web search (optional). ddgs needs no key; brave-free needs BraveKey; tavily needs TavilyKey. // WebSearchProxy is a dedicated egress proxy (http/https/socks5), unrelated to the traffic-recording MITM proxy; empty = direct. EnableWebSearch: p.webSearch.Enabled, diff --git a/agent/webfetch.go b/agent/webfetch.go new file mode 100644 index 0000000..5bdcd02 --- /dev/null +++ b/agent/webfetch.go @@ -0,0 +1,19 @@ +package agent + +import ( + "github.com/Autumn-27/artex/intercept" + actool "github.com/Autumn-27/norma/tool" +) + +// withWebFetch returns tools plus the WebFetch tool of an agent session. Every +// agent kind builds WebFetch here instead of through agentcore's EnableWebFetch, +// which has no option for a redirect policy: every redirect hop goes through the +// target gate (intercept.RedirectCheck), on the client that uses proxy and on +// the direct retry client alike. The tool is added after the session's own +// tools, where agentcore adds it, so the tool list keeps its order. caCert is the +// recording proxy's CA, trusted to verify the HTTPS it re-signs. tools is not +// modified. +func withWebFetch(tools []actool.CoreTool, proxy, caCert string) []actool.CoreTool { + webFetch := actool.NewWebFetch(actool.WebFetchConfig{Proxy: proxy, CACert: caCert, CheckRedirect: intercept.RedirectCheck("WebFetch")}) + return append(tools[:len(tools):len(tools)], webFetch) +} diff --git a/agent/webfetch_redirect_test.go b/agent/webfetch_redirect_test.go new file mode 100644 index 0000000..7c99d1d --- /dev/null +++ b/agent/webfetch_redirect_test.go @@ -0,0 +1,228 @@ +package agent + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/Autumn-27/artex/db" + "github.com/Autumn-27/artex/guard" + "github.com/Autumn-27/artex/intercept" + "github.com/Autumn-27/norma/llm" + actool "github.com/Autumn-27/norma/tool" +) + +// Step 1.9: the WebFetch tool of every agent kind checks each redirect hop +// against the target gate, on the primary client and on the direct retry +// client. With the gate off it follows redirects as before. + +// passHooks lets every tool call run. +type passHooks struct{} + +func (passHooks) PreToolUse(context.Context, string, []byte) (bool, string, []byte) { + return false, "", nil +} + +func (passHooks) PostToolUse(context.Context, string, []byte, []byte, bool) {} + +func (passHooks) Stop(context.Context, []llm.Message) (bool, []string, string) { + return false, nil, "" +} + +// redirectPair is a server that redirects to a second one on another port of +// 127.0.0.1, so the hop is another target (an IP literal: never resolved). +type redirectPair struct { + start, final string // URLs + startHits, finalHits *atomic.Int64 + finalKey string // host:port of the redirect target +} + +func newRedirectPair(t *testing.T) redirectPair { + t.Helper() + p := redirectPair{startHits: &atomic.Int64{}, finalHits: &atomic.Int64{}} + final := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p.finalHits.Add(1) + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write([]byte("redirect-target-reached")) + })) + t.Cleanup(final.Close) + start := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p.startHits.Add(1) + http.Redirect(w, r, final.URL+"/final", http.StatusFound) + })) + t.Cleanup(start.Close) + u, _ := url.Parse(final.URL) + p.start, p.final, p.finalKey = start.URL+"/start", final.URL+"/final", u.Host + return p +} + +// gatedScope is a gated target scope on task taskID, whose rows are removed +// afterwards. +func gatedScope(t *testing.T, d *db.DB, taskID string) intercept.TargetScope { + t.Helper() + t.Cleanup(func() { _, _ = d.Exec(`DELETE FROM intercept_pending WHERE task_id=$1`, taskID) }) + return intercept.TargetScope{TaskID: taskID, Enabled: func() bool { return true }, + WaitBound: intercept.DefaultTargetWaitBound, Interceptor: intercept.New(d)} +} + +// openRedirectRequests returns the target keys of the open requests of task. +func openRedirectRequests(t *testing.T, d *db.DB, taskID string) []string { + t.Helper() + rows, err := d.Query(`SELECT target_key FROM intercept_pending WHERE task_id=$1 AND status='pending' AND target_key IS NOT NULL ORDER BY id`, taskID) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + var keys []string + for rows.Next() { + var k string + if err := rows.Scan(&k); err != nil { + t.Fatal(err) + } + keys = append(keys, k) + } + return keys +} + +func fetchInputJSON(u string) string { + b, _ := json.Marshal(map[string]string{"url": u}) + return string(b) +} + +// Each agent kind runs a scripted WebFetch on a URL that redirects to a target +// no pin covers. The hooks let the call run, so the hop is what the gate sees: +// each kind must stop at the hop, leave the redirect target untouched and open +// one request for it. A fifth agent kind added without the redirect policy +// fails this table once it is added to it. +func TestWebFetchRedirectCheckWiredOnEveryAgentKind(t *testing.T) { + for _, kind := range []string{"worker", "planner", "mainagent", "chat"} { + t.Run(kind, func(t *testing.T) { + f := newHookReachFixture(t) + d := testDB(t) + t.Cleanup(func() { _ = d.Close() }) + pair := newRedirectPair(t) + scope := gatedScope(t, d, f.taskID) + ctx := intercept.WithTargetScope(t.Context(), scope) + prov := toolCallThenStop("WebFetch", fetchInputJSON(pair.start)) + + switch kind { + case "worker": + payload := map[string]any{"summary": "redirect probe"} + intentID, err := f.ts.AddNode("intent", payload, 0, "running", "planner", nil) + if err != nil { + t.Fatal(err) + } + raw, _ := json.Marshal(payload) + w := NewWorker(prov, "test-model", f.workDir, nil, 0, 3) + if _, _, err := w.Execute(ctx, "work#1", f.expID, nil, f.ts, &db.Node{ID: intentID, Payload: raw}, passHooks{}, nil, nil, nil); err != nil { + t.Fatal(err) + } + case "planner": + p := NewPlanner(prov, "test-model", f.workDir, nil, 0, 3) + if _, _, err := p.Plan(ctx, f.expID, nil, f.ts, "probe goal", nil, passHooks{}, nil); err != nil { + t.Fatal(err) + } + case "mainagent": + m := NewMainAgent(prov, "test-model", f.workDir, nil, 0, 3) + if _, err := m.Chat(ctx, f.expID, 0, nil, f.ts, "probe goal", "status?", passHooks{}, nil, nil, nil, nil, nil); err != nil { + t.Fatal(err) + } + case "chat": + c := NewChatAgent(prov, "test-model", t.TempDir(), nil, 0) + c.SetGuard(guard.New()) + if _, err := c.Chat(ctx, "assistant", "conv-redirect-probe", "fetch it", 3, 0, false, nil); err != nil { + t.Fatal(err) + } + } + + if pair.startHits.Load() != 1 { + t.Fatalf("the fetch did not run: %d hits on the origin", pair.startHits.Load()) + } + if pair.finalHits.Load() != 0 { + t.Fatal("the redirect was followed to a target no pin covers") + } + if keys := openRedirectRequests(t, d, f.taskID); len(keys) != 1 || keys[0] != pair.finalKey { + t.Fatalf("open requests = %v, want one for %s", keys, pair.finalKey) + } + }) + } +} + +// The direct retry client carries the policy too: with a proxy that refuses +// the connection, WebFetch retries without it, reaches the origin, and the +// hop is still stopped. +func TestWebFetchRedirectCheckOnDirectRetry(t *testing.T) { + d := testDB(t) + t.Cleanup(func() { _ = d.Close() }) + expID, err := d.CreateExploration("redirect retry probe", "the direct retry client checks redirects") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = d.Exec(`DELETE FROM explorations WHERE id=$1`, expID) }) + taskID := fmt.Sprint(expID) + pair := newRedirectPair(t) + ctx := intercept.WithTargetScope(t.Context(), gatedScope(t, d, taskID)) + + out, isErr := callWebFetch(t, ctx, "http://127.0.0.1:1", pair.start) + if !isErr || !strings.Contains(out, intercept.TargetApprovalPending) || !strings.Contains(out, "redirects to "+pair.finalKey) { + t.Fatalf("retried fetch: error=%v out=%q", isErr, out) + } + if pair.startHits.Load() != 1 || pair.finalHits.Load() != 0 { + t.Fatalf("hits origin=%d target=%d, want the origin reached by the retry and the target untouched", pair.startHits.Load(), pair.finalHits.Load()) + } + if keys := openRedirectRequests(t, d, taskID); len(keys) != 1 || keys[0] != pair.finalKey { + t.Fatalf("open requests = %v, want one for %s", keys, pair.finalKey) + } +} + +// With the gate off, or without a scope, WebFetch follows redirects as it did +// with agentcore's own tool: to the end of the chain, and the same error after +// 10 hops. No database. +func TestWebFetchRedirectGateOffUnchanged(t *testing.T) { + loop := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, r.URL.Path+"x", http.StatusFound) + })) + t.Cleanup(loop.Close) + for name, ctx := range map[string]context.Context{ + "no scope": t.Context(), + "flag off": intercept.WithTargetScope(t.Context(), intercept.TargetScope{TaskID: "7", Enabled: func() bool { return false }, + WaitBound: time.Minute, Interceptor: intercept.New(nil)}), + } { + pair := newRedirectPair(t) + for _, proxy := range []string{"", "http://127.0.0.1:1"} { + if out, isErr := callWebFetch(t, ctx, proxy, pair.start); isErr || !strings.Contains(out, "redirect-target-reached") { + t.Fatalf("%s, proxy %q: error=%v out=%q", name, proxy, isErr, out) + } + } + if pair.finalHits.Load() != 2 { + t.Fatalf("%s: %d hits on the redirect target, want 2", name, pair.finalHits.Load()) + } + got, _ := callWebFetch(t, ctx, "", loop.URL+"/l") + want := actool.NewWebFetch(actool.WebFetchConfig{}).Call + res, err := want(ctx, json.RawMessage(fetchInputJSON(loop.URL+"/l")), &actool.ToolContext{}) + if err != nil || got != res.Flatten() || !strings.Contains(got, "stopped after 10 redirects") { + t.Fatalf("%s: redirect loop: %q, want agentcore's %q", name, got, res.Flatten()) + } + } +} + +// callWebFetch runs the WebFetch tool the agents build, with proxy. +func callWebFetch(t *testing.T, ctx context.Context, proxy, u string) (string, bool) { + t.Helper() + tools := withWebFetch(nil, proxy, "") + if len(tools) != 1 || tools[0].Name() != "WebFetch" { + t.Fatalf("withWebFetch built %d tools", len(tools)) + } + res, err := tools[0].Call(ctx, json.RawMessage(fetchInputJSON(u)), &actool.ToolContext{}) + if err != nil { + t.Fatal(err) + } + return res.Flatten(), res.IsError +} diff --git a/agent/worker.go b/agent/worker.go index d69d003..1900918 100644 --- a/agent/worker.go +++ b/agent/worker.go @@ -482,16 +482,14 @@ func (w *Worker) execute(ctx context.Context, name string, taskID int64, as *db. Provider: w.prov, SystemPrompt: system, DynamicBoundary: boundary, - Tools: tools, + Tools: withWebFetch(tools, w.proxyAddr, w.proxyCACert), DeferredTools: def.Deferred, UnlockSet: def.Unlock, PermissionMode: permission.ModeBypass, - // WebFetch goes through the recording proxy, so its HTTP is recorded just like - // curl; loading the proxy CA lets MITM-re-signed HTTPS certs [verify normally] - // (rather than disabling verification). Direct when proxy is empty. - EnableWebFetch: true, - WebFetchProxy: w.proxyAddr, - WebFetchCACert: w.proxyCACert, + // WebFetch (added to Tools above) goes through the recording proxy, so its + // HTTP is recorded just like curl; loading the proxy CA lets MITM-re-signed + // HTTPS certs [verify normally] (rather than disabling verification). Direct + // when proxy is empty. Every redirect hop goes through the target gate. // Web search (optional). ddgs needs no key; brave-free needs BraveKey; tavily needs TavilyKey. // WebSearchProxy is a dedicated egress proxy (http/https/socks5), unrelated to the traffic-recording MITM proxy; empty = direct. EnableWebSearch: w.webSearch.Enabled, diff --git a/db/intercept_detail.go b/db/intercept_detail.go index 9712190..3a8e661 100644 --- a/db/intercept_detail.go +++ b/db/intercept_detail.go @@ -76,6 +76,9 @@ type InterceptTarget struct { // ExemptVoid: a stored exemption names this target, and did not apply because // the name resolved to a private, loopback or otherwise local address. ExemptVoid bool `json:"exempt_void,omitempty"` + // RedirectFrom: the origin (host:port) of the redirect hop that led to this + // target; empty when the call named the target itself. + RedirectFrom string `json:"redirect_from,omitempty"` } type InterceptDetail struct { diff --git a/intercept/target_gate.go b/intercept/target_gate.go index a99a73f..b9d038b 100644 --- a/intercept/target_gate.go +++ b/intercept/target_gate.go @@ -110,6 +110,8 @@ type targetCheck struct { tool string input []byte urls []string // the URLs the call declares, as written, for the URL block rules + // redirectFrom is the origin (host:port) of a redirect hop; empty for a call. + redirectFrom string } // checkedTarget is one target of the call and what the gate learned about it. @@ -303,6 +305,9 @@ func (c *targetCheck) log(status, reason, configDigest string, targets ...db.Int // The row records that a pin or an exemption was used, not this one call. audit.ExecutionStatus = "unknown" } + for n := range targets { + targets[n].RedirectFrom = c.redirectFrom + } audit.Targets = targets _, agentName := taskInfoFromCtx(c.ctx) _ = c.i.targets.LogDecision(db.TargetDecision{Scope: c.ref, AgentName: agentName, ToolName: c.tool, diff --git a/intercept/target_redirect.go b/intercept/target_redirect.go new file mode 100644 index 0000000..cbf487e --- /dev/null +++ b/intercept/target_redirect.go @@ -0,0 +1,107 @@ +package intercept + +import ( + "context" + "encoding/json" + "errors" + "net/http" +) + +// The HTTP clients the platform owns (WebFetch and the custom http tool) +// follow redirects. The hook and the rendered-URL gate decide on the URL a call +// names; a redirect sends the client to a URL nobody checked. RedirectCheck is +// the redirect policy of those clients: the client asks it before it follows +// each hop, and the hop goes through the same target decision as the call, on +// the URL the client is about to request: the asset block rules on that URL and +// on its target, before any lookup, then exemptions, resolution and pins. + +// maxRedirects is the limit net/http applies to a client without a redirect +// policy. Setting a policy replaces it, so RedirectCheck restates it, with the +// same error text. +const maxRedirects = 10 + +// RedirectCheck returns the redirect policy (http.Client.CheckRedirect) of a +// platform-owned client run by the tool toolName. Everything it needs comes +// from the context of the request, which net/http keeps on every hop: the +// target scope of the session and the Interceptor that owns its gate. +// +// With the gate off, or with no scope in the context (the operator's test +// endpoint), it only applies the limit of 10 hops: nothing is parsed, resolved, +// read or written, and the client behaves as with the default policy. +// +// A hop no pin or exemption covers is not followed, and the policy never waits: +// the fetch has its own deadline, which a human decision does not fit in. The +// request row for the hop's target is found or created (it is durable, so an +// approval given later lets the retried call follow), and the call fails at +// once with TARGET_APPROVAL_PENDING. A block rule, a failed lookup or a hop +// whose target cannot be read fails the call with its block code, and opens no +// request. +func RedirectCheck(toolName string) func(req *http.Request, via []*http.Request) error { + return func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return errors.New("stopped after 10 redirects") + } + ctx := req.Context() + scope, on := renderedScope(ctx) + if !on { + return nil + } + return checkRedirectHop(ctx, scope, toolName, req.URL.String(), via[len(via)-1].URL.String()) + } +} + +// checkRedirectHop runs the gate decision on one hop, from the URL that +// redirected (from) to the URL the client will request (hop). +func checkRedirectHop(ctx context.Context, scope TargetScope, toolName, hop, from string) error { + input, _ := json.Marshal(map[string]string{"url": hop, "redirect_from": from}) + blocked := func(res TargetResult) error { + return &redirectError{res: res, from: from, hop: hop, scope: scope.scopeName()} + } + t, err := urlTarget(hop) + if err != nil { + return blocked(undeterminedTarget(ctx, scope, toolName, input, "the target of the redirect could not be read ("+err.Error()+")")) + } + i := scope.Interceptor + if i == nil { + return blocked(TargetResult{Code: TargetGateUnavailable, Host: t.Host, Port: t.Port, Reason: "the session's target scope carries no gate"}) + } + var origin string + if o, err := urlTarget(from); err == nil { + origin = o.String() + } + ctx = WithCall(ctx, toolName, input) + c := &targetCheck{i: i, ctx: ctx, scope: scope, tool: toolName, input: input, urls: []string{hop}, redirectFrom: origin} + unpinned, res := c.run([]Target{t}) + if res.Code == "" && len(unpinned) > 0 { + // Wait bound 0: the request is found or created, and the call does not wait. + scope.WaitBound = 0 + for n := range unpinned { + unpinned[n].RedirectFrom = origin + } + res = i.RequestTargets(ctx, scope, toolName, input, unpinned, nil) + } + if res.Code == "" { + return nil + } + return blocked(res) +} + +// redirectError is what the client returns when a hop is not followed. Both +// tools hand the client error to the model as the tool result. +type redirectError struct { + res TargetResult + from, hop, scope string +} + +func (e *redirectError) Error() string { + target := e.hop + if e.res.Host != "" { + target = targetName(TargetRequest{Host: e.res.Host, Port: e.res.Port}) + } + msg := "[ARTEX platform control, not target defense] " + e.res.Code + ": " + e.from + " redirects to " + target + if e.res.Code == TargetApprovalPending { + return msg + ", which is not approved for " + e.scope + ". The redirect was not followed and an approval was requested. " + + "Retry the same call after the approval. Do not fetch the redirect target directly or with another tool." + } + return msg + ": " + e.res.Reason + ". The redirect was not followed. Do not fetch the redirect target directly or with another tool." +} diff --git a/intercept/target_redirect_pg_test.go b/intercept/target_redirect_pg_test.go new file mode 100644 index 0000000..fb78ab7 --- /dev/null +++ b/intercept/target_redirect_pg_test.go @@ -0,0 +1,95 @@ +package intercept + +import ( + "strconv" + "strings" + "testing" + "time" +) + +// Step 1.9 against PostgreSQL: a hop no pin covers opens the durable request +// and fails the call at once; an approval lets the same call follow. + +// A hop to an unpinned target is not followed: the redirect target receives +// nothing, exactly one request row is opened for it, and the call fails with +// TARGET_APPROVAL_PENDING without waiting, although the scope would wait five +// minutes at the hook. The same call again attaches to that row. After the +// approval it follows. +func TestRedirectToUnpinnedHostNotFollowed(t *testing.T) { + d := targetTestDB(t) + ic, res := pgGate(t, d) + scope := taskScope(t, d, ic, 5*time.Minute) + ctx := WithTargetScope(t.Context(), scope) + final, finalHits, finalKey := hopServer(t, "") + start, startHits, startKey := hopServer(t, final.URL+"/final") + + began := time.Now() + _, err := gatedFetch(ctx, start.URL+"/start") + if err == nil || !strings.Contains(err.Error(), TargetApprovalPending) || + !strings.Contains(err.Error(), start.URL+"/start redirects to "+finalKey) || !strings.Contains(err.Error(), "not followed") { + t.Fatalf("unpinned hop: %v", err) + } + if elapsed := time.Since(began); elapsed > 10*time.Second { + t.Fatalf("the hop waited %v", elapsed) + } + if startHits.Load() != 1 || finalHits.Load() != 0 { + t.Fatalf("hits start=%d final=%d, want the redirect target untouched", startHits.Load(), finalHits.Load()) + } + ids := openRows(t, d, scope) + if len(ids) != 1 || allRows(t, d, scope) != 1 { + t.Fatalf("open requests = %v (%d rows), want one", ids, allRows(t, d, scope)) + } + detail, err := d.GetInterceptDetail(ids[0]) + if err != nil || detail == nil || detail.Audit == nil || len(detail.Audit.Targets) != 1 { + t.Fatalf("request detail = %+v, %v", detail, err) + } + if tgt := detail.Audit.Targets[0]; tgt.Host+":"+strconv.Itoa(tgt.Port) != finalKey || tgt.RedirectFrom != startKey || tgt.Outcome != "requested" { + t.Fatalf("requested target = %+v, want %s from %s", tgt, finalKey, startKey) + } + if !strings.Contains(detail.Reason, "A redirect from "+startKey) || !strings.Contains(string(detail.ToolInput), start.URL+"/start") { + t.Fatalf("request reason %q / input %s, want the origin of the redirect", detail.Reason, detail.ToolInput) + } + + // The retried call stops at the same hop and attaches to the same row. + if _, err := gatedFetch(ctx, start.URL+"/start"); err == nil || !strings.Contains(err.Error(), TargetApprovalPending) || finalHits.Load() != 0 { + t.Fatalf("retry before the approval: %v, final hits %d", err, finalHits.Load()) + } + if again := openRows(t, d, scope); len(again) != 1 || again[0] != ids[0] || allRows(t, d, scope) != 1 { + t.Fatalf("after the retry: open %v, %d rows; want the same single request", again, allRows(t, d, scope)) + } + + approveAll(t, d, ic, scope) + if resp, err := gatedFetch(ctx, start.URL+"/start"); err != nil || resp.StatusCode != 200 || finalHits.Load() != 1 { + t.Fatalf("after the approval: %v, final hits %d", err, finalHits.Load()) + } + if res.total() != 0 { + t.Fatalf("%d lookups for IP literals", res.total()) + } +} + +// http -> https on the same host is another key (port 443, not 80): a pin on +// (host, 80) does not cover it, and the hop asks again. +func TestRedirectHTTPToHTTPSAsksAgain(t *testing.T) { + d := targetTestDB(t) + ic, res := pgGate(t, d) + res.set("app.example.com", "203.0.113.7") + scope := taskScope(t, d, ic, time.Minute) + pinURL(t, d, ic, scope, "http://app.example.com/") + ctx := WithTargetScope(t.Context(), scope) + + if err := hop(ctx, "http://app.example.com/next", "http://app.example.com/"); err != nil { + t.Fatalf("hop on the pinned key: %v", err) + } + err := hop(ctx, "https://app.example.com/", "http://app.example.com/") + if err == nil || !strings.Contains(err.Error(), TargetApprovalPending) || !strings.Contains(err.Error(), "redirects to app.example.com:443") { + t.Fatalf("http -> https: %v", err) + } + ids := openRows(t, d, scope) + if len(ids) != 1 { + t.Fatalf("open requests = %v, want the one for port 443", ids) + } + detail, err := d.GetInterceptDetail(ids[0]) + if err != nil || detail == nil || detail.Audit == nil || detail.Audit.Targets[0].Port != 443 || detail.Audit.Targets[0].RedirectFrom != "app.example.com:80" { + t.Fatalf("request = %+v, %v", detail, err) + } +} diff --git a/intercept/target_redirect_test.go b/intercept/target_redirect_test.go new file mode 100644 index 0000000..61821a2 --- /dev/null +++ b/intercept/target_redirect_test.go @@ -0,0 +1,229 @@ +package intercept + +import ( + "context" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/Autumn-27/artex/db" +) + +// Step 1.9: every redirect hop of a platform-owned client goes through the gate. +// Without a database: a hop that would open a request is covered by the +// PostgreSQL tests. + +// hopServer counts its requests and answers a fixed page, or a redirect to +// location when it is set. +func hopServer(t *testing.T, location string) (srv *httptest.Server, hits *atomic.Int64, key string) { + t.Helper() + hits = &atomic.Int64{} + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + if location != "" { + http.Redirect(w, r, location, http.StatusFound) + return + } + _, _ = w.Write([]byte("hop-reached")) + })) + t.Cleanup(srv.Close) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + return srv, hits, u.Host +} + +// hop calls the redirect policy of tool on one hop, as the client does. +func hop(ctx context.Context, to string, from ...string) error { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, to, nil) + if err != nil { + panic(err) + } + via := make([]*http.Request, len(from)) + for n, f := range from { + if via[n], err = http.NewRequestWithContext(ctx, http.MethodGet, f, nil); err != nil { + panic(err) + } + } + return RedirectCheck("WebFetch")(req, via) +} + +func gatedFetch(ctx context.Context, rawURL string) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, err + } + resp, err := (&http.Client{CheckRedirect: RedirectCheck("WebFetch")}).Do(req) + if resp != nil { + resp.Body.Close() + } + return resp, err +} + +// With the gate off, or without a scope, the policy is net/http's default: +// every hop is followed, up to 10, with the same error after that. Nothing is +// parsed, resolved, read or logged, not even a hop whose target cannot be read. +func TestRedirectGateOffKeepsDefaultPolicy(t *testing.T) { + g := newGate(t) + g.res.set("app.example.com", "203.0.113.7") + chain := make([]string, maxRedirects) + for n := range chain { + chain[n] = "http://app.example.com/" + } + for name, ctx := range map[string]context.Context{ + "no scope": context.Background(), + "no flag": WithTargetScope(context.Background(), TargetScope{TaskID: "7", WaitBound: time.Minute, Interceptor: g.ic}), + "flag off": WithTargetScope(context.Background(), TargetScope{TaskID: "7", Enabled: func() bool { return false }, Interceptor: g.ic}), + } { + if err := hop(ctx, "https://other.example.com/", chain[:1]...); err != nil { + t.Errorf("%s: %v, want the hop followed", name, err) + } + if err := hop(ctx, "http:///no-host", chain[:1]...); err != nil { + t.Errorf("%s, no host: %v, want the hop followed", name, err) + } + if err := hop(ctx, "https://other.example.com/", chain[:maxRedirects-1]...); err != nil { + t.Errorf("%s, hop 9: %v", name, err) + } + if err := hop(ctx, "https://other.example.com/", chain...); err == nil || err.Error() != "stopped after 10 redirects" { + t.Errorf("%s, hop 10: %v, want net/http's limit", name, err) + } + + // Through a real client: a hop to another port, which the gate would ask + // for, is followed. + final, finalHits, _ := hopServer(t, "") + start, _, _ := hopServer(t, final.URL+"/final") + if resp, err := gatedFetch(ctx, start.URL+"/start"); err != nil || resp.StatusCode != http.StatusOK || finalHits.Load() != 1 { + t.Errorf("%s: chain not followed: %v, hits=%d", name, err, finalHits.Load()) + } + loop := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, r.URL.Path+"x", http.StatusFound) + })) + _, gotErr := gatedFetch(ctx, loop.URL+"/l") + _, wantErr := (&http.Client{}).Get(loop.URL + "/l") + loop.Close() + if gotErr == nil || wantErr == nil || gotErr.Error() != wantErr.Error() { + t.Errorf("%s: redirect loop: %v, want the default policy's %v", name, gotErr, wantErr) + } + } + if g.store.reads != 0 || len(g.store.logged) != 0 || g.res.total() != 0 { + t.Errorf("gate off: %d store reads, %d rows, %d lookups, want none", g.store.reads, len(g.store.logged), g.res.total()) + } + // Gate on, the limit holds as well, before anything is read. + if err := hop(g.ctx(), "https://app.example.com/", chain...); err == nil || err.Error() != "stopped after 10 redirects" || g.store.reads != 0 { + t.Errorf("gate on, hop 10: %v, %d store reads", err, g.store.reads) + } +} + +// A hop to a pinned target is followed, through a real client: the redirected +// request keeps the context of the call, so the policy finds the scope. The +// first use of the pin is logged with the origin of the redirect. +func TestRedirectToPinnedHostFollowed(t *testing.T) { + g := newGate(t) + final, finalHits, finalKey := hopServer(t, "") + start, startHits, startKey := hopServer(t, final.URL+"/final") + host, port, _ := net.SplitHostPort(finalKey) + pin := &db.TargetPin{ID: 1, Host: host, Port: atoiPort(t, port), Version: 1, Status: db.TargetPinActive, PinnedIPs: []string{host}} + g.store.pins = []*db.TargetPin{pin} + + resp, err := gatedFetch(g.ctx(), start.URL+"/start") + if err != nil || resp.StatusCode != http.StatusOK || startHits.Load() != 1 || finalHits.Load() != 1 { + t.Fatalf("pinned hop: %v, hits start=%d final=%d", err, startHits.Load(), finalHits.Load()) + } + if got := g.store.outcomes(); len(got) != 1 || got[0] != "allowed/pin_match" { + t.Fatalf("rows = %v, want the first use of the pin", got) + } + row := g.store.logged[0] + if tgt := row.Audit.Targets[0]; tgt.Host != host || tgt.RedirectFrom != startKey || tgt.PinID != pin.ID { + t.Fatalf("logged target = %+v, want %s from %s", tgt, finalKey, startKey) + } + if !strings.Contains(string(row.ToolInput), final.URL+"/final") || !strings.Contains(string(row.ToolInput), start.URL+"/start") { + t.Fatalf("logged input = %s, want the hop and its origin", row.ToolInput) + } + if g.res.total() != 0 { + t.Fatalf("%d lookups for an IP literal", g.res.total()) + } +} + +// A (host, 0) pin covers every port: http -> https on that host is followed. +func TestRedirectToPortZeroPinnedHostFollowed(t *testing.T) { + g := newGate(t) + g.res.set("app.example.com", "203.0.113.7") + g.store.pins = []*db.TargetPin{{ID: 1, Host: "app.example.com", Port: 0, Version: 1, Status: db.TargetPinActive, PinnedIPs: []string{"203.0.113.7"}}} + if err := hop(g.ctx(), "https://app.example.com/login", "http://app.example.com/login"); err != nil { + t.Fatalf("http -> https under a (host, 0) pin: %v", err) + } +} + +// A block rule stops a hop before anything is resolved, whether it names the +// hop's host or its URL, and even when the hop stays on a pinned host. No +// request is opened. +func TestRedirectToBlockedNameNotResolved(t *testing.T) { + g := newGate(t) + g.res.set("app.example.com", "203.0.113.7") + g.res.set("evil.example", "198.51.100.9") + g.store.pins = []*db.TargetPin{{ID: 1, Host: "app.example.com", Port: 443, Version: 1, Status: db.TargetPinActive, PinnedIPs: []string{"203.0.113.7"}}} + g.store.rules = []db.AssetInterceptRule{ + {Enabled: true, Kind: "exact_domain", Pattern: "evil.example"}, + {Enabled: true, Kind: "exact_url", Pattern: "https://app.example.com/admin"}, + } + + err := hop(g.ctx(), "http://evil.example/x", "https://app.example.com/") + if err == nil || !strings.Contains(err.Error(), TargetRuleBlocked) || !strings.Contains(err.Error(), "https://app.example.com/ redirects to evil.example:80") { + t.Fatalf("hop to a blocked name: %v", err) + } + if n := g.res.lookups("evil.example"); n != 0 { + t.Fatalf("the blocked name was resolved %d times", n) + } + err = hop(g.ctx(), "https://app.example.com/admin", "https://app.example.com/") + if err == nil || !strings.Contains(err.Error(), TargetRuleBlocked) { + t.Fatalf("hop to a blocked URL on a pinned host: %v", err) + } + if got := g.store.outcomes(); len(got) != 2 || got[0] != "denied/rule_blocked" || got[1] != "denied/rule_blocked" { + t.Fatalf("rows = %v, want two blocks", got) + } + if from := g.store.logged[0].Audit.Targets[0].RedirectFrom; from != "app.example.com:443" { + t.Fatalf("redirect_from = %q", from) + } + if g.res.total() != 0 { + t.Fatalf("%d lookups, want none", g.res.total()) + } +} + +// A hop whose target cannot be read is blocked with TARGET_GATE_UNAVAILABLE, as +// a rendered URL is, and opens no request. +func TestRedirectUnreadableHopBlocked(t *testing.T) { + g := newGate(t) + for _, to := range []*url.URL{ + {Scheme: "http", Path: "/no-host"}, + {Scheme: "http", Host: "app.example.com:99999", Path: "/"}, + } { + req, _ := http.NewRequestWithContext(g.ctx(), http.MethodGet, "http://placeholder.example/", nil) + req.URL = to + via, _ := http.NewRequestWithContext(g.ctx(), http.MethodGet, "https://app.example.com/", nil) + err := RedirectCheck("WebFetch")(req, []*http.Request{via}) + if err == nil || !strings.Contains(err.Error(), TargetGateUnavailable) { + t.Errorf("%s: %v, want TARGET_GATE_UNAVAILABLE", to, err) + } + } + if got := g.store.outcomes(); len(got) != 2 || got[0] != "denied/gate_unavailable" { + t.Fatalf("rows = %v", got) + } + if g.res.total() != 0 { + t.Fatalf("%d lookups, want none", g.res.total()) + } +} + +func atoiPort(t *testing.T, s string) int { + t.Helper() + p := validPort(s) + if p == 0 { + t.Fatalf("port %q", s) + } + return p +} diff --git a/intercept/target_request.go b/intercept/target_request.go index dde6e68..025fb49 100644 --- a/intercept/target_request.go +++ b/intercept/target_request.go @@ -41,6 +41,9 @@ type TargetRequest struct { Reason string // no_pin | new_ip | revoked Pin *db.TargetPin // the active pin an approval would extend (new_ip); nil otherwise ExemptVoid bool // a stored exemption names the target and was void for this call + // RedirectFrom is the origin (host:port) of the redirect that leads to the + // target; empty when the call names the target itself. + RedirectFrom string } // TargetResult is the outcome of RequestTargets. Code is empty when every @@ -248,6 +251,7 @@ func (i *Interceptor) openTargetRequest(ctx context.Context, ref db.TargetScopeR Host: t.Host, Port: t.Port, Kind: t.Kind, ResolvedIPs: t.ResolvedIPs, Resolver: t.Resolver, ResolvedAt: t.ResolvedAt, Reason: t.Reason, Outcome: "requested", NameOnlyEffective: nameOnly, ExemptVoid: t.ExemptVoid, + RedirectFrom: t.RedirectFrom, } if t.Pin != nil { target.PinID, target.PinVersion, target.PinnedIPs = t.Pin.ID, t.Pin.Version, t.Pin.PinnedIPs @@ -363,6 +367,9 @@ func targetReason(scope TargetScope, t TargetRequest, ask *Decision, nameOnly bo b.WriteString(ask.Message) b.WriteString("\nThis call also names a target that needs approval. ") } + if t.RedirectFrom != "" { + fmt.Fprintf(&b, "A redirect from %s leads to this target; the redirect was not followed. ", t.RedirectFrom) + } name, bound := targetName(t), scope.scopeName() if t.Port == 0 { name += " (port not declared: every port)" diff --git a/server/customtool.go b/server/customtool.go index 5ece010..91fae7f 100644 --- a/server/customtool.go +++ b/server/customtool.go @@ -455,6 +455,8 @@ func (s *Server) runHTTPTool(ctx context.Context, key string, execRaw json.RawMe req.Header.Set(k, renderTemplate(v, params, identity)) } client := &http.Client{Timeout: timeoutOr(spec.TimeoutMs, 30000)} + // Every redirect hop goes through the target gate, as the rendered URL did. + client.CheckRedirect = intercept.RedirectCheck(key) if tr := s.httpProxyTransport(spec); tr != nil { client.Transport = tr } diff --git a/server/target_redirect_test.go b/server/target_redirect_test.go new file mode 100644 index 0000000..b9b3b92 --- /dev/null +++ b/server/target_redirect_test.go @@ -0,0 +1,162 @@ +package server + +import ( + "fmt" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/Autumn-27/artex/db" +) + +// Step 1.9: the redirect hops of the custom http tool and of a worker's +// WebFetch go through the target gate. + +// redirectingServer answers every request with a redirect to location and +// counts the requests. Its host:port is an IP literal: the gate never resolves it. +func redirectingServer(t *testing.T, location string) (srv *httptest.Server, hits *atomic.Int64, key string) { + t.Helper() + hits = &atomic.Int64{} + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + http.Redirect(w, r, location, http.StatusFound) + })) + t.Cleanup(srv.Close) + return srv, hits, srv.Listener.Addr().String() +} + +func decide(t *testing.T, g *gateServer, id int64, decision string) { + t.Helper() + if code, body := g.do(t, "POST", fmt.Sprintf("/api/intercept/pending/%d/decide", id), `{"decision":"`+decision+`"}`); code != 200 { + t.Fatalf("decide %d: %d %v", id, code, body) + } +} + +// A custom http tool whose rendered URL is pinned, and which redirects to a +// target no pin covers: the hop is not followed, one request is opened for its +// target, and the call fails at once. After the approval the same call follows. +func TestCustomHTTPRedirectTargetGated(t *testing.T) { + g := newGateServer(t) + on := true + task := g.newTask(t, db.TaskCreateOptions{TargetGate: &on}) + pg := g.m.pg + final, finalHits, fhost, fport := countingServer(t) + finalKey := net.JoinHostPort(fhost, fport) + start, startHits, startKey := redirectingServer(t, final.URL+"/final") + + tool := g.s.buildCustomTool(&db.Tool{Key: "probe_redirect", Kind: "http", + Exec: execJSON(t, map[string]any{"method": "GET", "url": start.URL + "/start"})}) + // The rendered URL itself first: its request, approved. + if out, isErr := callTool(t, tool, gatedCtx(task, 0), `{}`, nil); !isErr || !strings.Contains(out, "TARGET_APPROVAL_PENDING") || startHits.Load() != 0 { + t.Fatalf("rendered URL: error=%v out=%q", isErr, out) + } + id, _, _, ok := openTargetRequest(t, pg, task.ID, startKey) + if !ok { + t.Fatal("no request for the rendered URL") + } + decide(t, g, id, "allowed") + + // The wait bound is long: the hop must not use it. + began := time.Now() + out, isErr := callTool(t, tool, gatedCtx(task, time.Minute), `{}`, nil) + if !isErr || !strings.Contains(out, "TARGET_APPROVAL_PENDING") || !strings.Contains(out, start.URL+"/start redirects to "+finalKey) { + t.Fatalf("redirect to an unpinned target: error=%v out=%q", isErr, out) + } + if elapsed := time.Since(began); elapsed > 10*time.Second { + t.Fatalf("the hop waited %v", elapsed) + } + if startHits.Load() != 1 || finalHits.Load() != 0 { + t.Fatalf("hits origin=%d target=%d, want the target untouched", startHits.Load(), finalHits.Load()) + } + hopID, toolName, toolInput, ok := openTargetRequest(t, pg, task.ID, finalKey) + if !ok || toolName != "probe_redirect" || !strings.Contains(toolInput, final.URL+"/final") || !strings.Contains(toolInput, start.URL+"/start") { + t.Fatalf("request for the hop: ok=%v tool=%q input=%s", ok, toolName, toolInput) + } + var requests int + if err := pg.QueryRow(`SELECT count(*) FROM intercept_pending WHERE task_id=$1 AND target_key IS NOT NULL`, task.ID).Scan(&requests); err != nil || requests != 2 { + t.Fatalf("%d requests, %v; want the origin's and the hop's", requests, err) + } + + decide(t, g, hopID, "allowed") + if out, isErr := callTool(t, tool, gatedCtx(task, 0), `{}`, nil); isErr || !strings.Contains(out, "probe-reached") || finalHits.Load() != 1 { + t.Fatalf("after the approval: error=%v out=%q target hits=%d", isErr, out, finalHits.Load()) + } +} + +// The custom-tool test endpoint has no approval scope (D14): its redirects are +// followed as before, and nothing is written. +func TestCustomToolTestEndpointRedirectUngated(t *testing.T) { + g := newGateServer(t) + on := true + g.newTask(t, db.TaskCreateOptions{TargetGate: &on}) + final, finalHits, _, _ := countingServer(t) + start, _, _ := redirectingServer(t, final.URL+"/final") + count := func() int { + var n int + if err := g.m.pg.QueryRow(`SELECT count(*) FROM intercept_pending WHERE decision_source='target' OR target_key IS NOT NULL`).Scan(&n); err != nil { + t.Fatal(err) + } + return n + } + before := count() + code, out := g.do(t, "POST", "/api/tools/custom/test", fmt.Sprintf(`{"kind":"http","exec":{"url":%q}}`, start.URL+"/start")) + if code != 200 || out["is_error"] != false || !strings.Contains(fmt.Sprint(out["output"]), "probe-reached") || finalHits.Load() != 1 { + t.Fatalf("test endpoint: %d %v target hits=%d", code, out, finalHits.Load()) + } + if after := count(); after != before { + t.Fatalf("the test endpoint wrote %d target rows", after-before) + } +} + +// A gated worker's WebFetch end to end: the origin is approved at the hook, the +// redirect stops at the hop with TARGET_APPROVAL_PENDING and leaves one request +// open; once it is approved through the API, the same call follows. +func TestRedirectRequestSurvivesAndRetryPasses(t *testing.T) { + g := newGateServer(t) + on := true + task := g.newTask(t, db.TaskCreateOptions{TargetGate: &on}) + pg := g.m.pg + final, finalHits, fhost, fport := countingServer(t) + finalKey := net.JoinHostPort(fhost, fport) + start, startHits, startKey := redirectingServer(t, final.URL+"/final") + p := &toolScript{calls: fetches(start.URL+"/start", start.URL+"/start"), results: make(chan string, 16), + hold: map[int64]chan struct{}{2: make(chan struct{})}} + done := runScriptedWorker(t, g, task, p) + + var id int64 + waitUntil(t, "the request for the origin", func() bool { + var ok bool + id, _, _, ok = openTargetRequest(t, pg, task.ID, startKey) + return ok + }) + decide(t, g, id, "allowed") + if r := p.next(t); !strings.Contains(r, "TARGET_APPROVAL_PENDING") || !strings.Contains(r, "redirects to "+finalKey) { + t.Fatalf("first call: %q", r) + } + if startHits.Load() != 1 || finalHits.Load() != 0 { + t.Fatalf("hits origin=%d target=%d, want the target untouched", startHits.Load(), finalHits.Load()) + } + hopID, toolName, _, ok := openTargetRequest(t, pg, task.ID, finalKey) + if !ok || toolName != "WebFetch" { + t.Fatalf("request for the hop: ok=%v tool=%q", ok, toolName) + } + detail, err := pg.GetInterceptDetail(hopID) + if err != nil || detail == nil || detail.Audit == nil || len(detail.Audit.Targets) != 1 || detail.Audit.Targets[0].RedirectFrom != startKey { + t.Fatalf("hop request detail = %+v, %v", detail, err) + } + + decide(t, g, hopID, "allowed") + close(p.hold[2]) + if r := p.next(t); !strings.Contains(r, "probe-reached") || finalHits.Load() != 1 { + t.Fatalf("the same call after the approval: %q target hits=%d", r, finalHits.Load()) + } + select { + case <-done: + case <-time.After(60 * time.Second): + t.Fatal("the worker run did not end") + } +}