From 85a91be33eda2aadc76b4d668523699836b065f3 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 18 Jul 2026 14:01:18 -0400 Subject: [PATCH 1/4] fix(egress): attribute subprocess proxy audit events to task/invocation (#338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Egress events for subprocess traffic (skill scripts, browser, DB/CLI binaries routed via HTTP_PROXY) were missing task_id and correlation_id. The in-process enforcer path tags them from ctx via EmitFromContext, but the egress proxy is a separate 127.0.0.1 HTTP forward proxy — the request arriving from a subprocess carries no Go context, so OnAttempt had nothing to attach. Re-attach identity across the process boundary via proxy credentials: - exec.go stamps the request's task + correlation IDs into the HTTP_PROXY URL userinfo (proxyURLWithIdentity). HTTP clients replay userinfo as a Basic Proxy-Authorization header on every request and CONNECT. - egress_proxy.go decodes it (identityFromRequest) and threads the IDs onto a new EgressAttempt struct passed to OnAttempt. - runner.go sets TaskID + CorrelationID on the proxy egress events. Degrades gracefully: binaries that ignore proxy creds are still enforced and audited, just without identity (pre-fix behaviour). Proxy-Authorization is stripped before forwarding upstream; CONNECT is a blind relay. Tests: identityFromRequest decode table, end-to-end attribution through a real proxied request (HTTP + CONNECT), proxyURLWithIdentity stamping, and executor HTTP_PROXY injection. --- forge-cli/runtime/runner.go | 15 ++- forge-cli/tools/browser/manager_test.go | 4 +- forge-cli/tools/browser/manual_demo_test.go | 6 +- forge-cli/tools/exec.go | 39 +++++++- forge-cli/tools/exec_test.go | 50 ++++++++++ forge-core/security/egress_proxy.go | 73 ++++++++++++-- forge-core/security/egress_proxy_test.go | 105 ++++++++++++++++++-- 7 files changed, 259 insertions(+), 33 deletions(-) diff --git a/forge-cli/runtime/runner.go b/forge-cli/runtime/runner.go index 49f502b6..01e4e119 100644 --- a/forge-cli/runtime/runner.go +++ b/forge-cli/runtime/runner.go @@ -721,14 +721,21 @@ func (r *Runner) Run(ctx context.Context) error { if (!security.InContainer() && egressCfg.Mode != security.ModeDevOpen) || browserActive { matcher := security.NewDomainMatcher(egressCfg.Mode, egressCfg.AllDomains) egressProxy = security.NewEgressProxy(matcher, allowPrivateIPs) - egressProxy.OnAttempt = func(domain string, allowed bool) { + egressProxy.OnAttempt = func(a security.EgressAttempt) { event := coreruntime.AuditEgressAllowed - if !allowed { + if !a.Allowed { event = coreruntime.AuditEgressBlocked } + // #338 — task_id/correlation_id are recovered from the + // Proxy-Authorization creds the subprocess replays (see + // SkillCommandExecutor / identityFromRequest). Empty when the + // proxied binary ignores proxy credentials, matching the + // pre-#338 domain-only event. auditLogger.Emit(coreruntime.AuditEvent{ - Event: event, - Fields: map[string]any{"domain": domain, "mode": string(egressCfg.Mode), "source": "proxy"}, + Event: event, + TaskID: a.TaskID, + CorrelationID: a.CorrelationID, + Fields: map[string]any{"domain": a.Domain, "mode": string(egressCfg.Mode), "source": "proxy"}, }) } var pErr error diff --git a/forge-cli/tools/browser/manager_test.go b/forge-cli/tools/browser/manager_test.go index cfe5fce2..b5bddd41 100644 --- a/forge-cli/tools/browser/manager_test.go +++ b/forge-cli/tools/browser/manager_test.go @@ -74,9 +74,9 @@ func startProxy(t *testing.T, matcher *security.DomainMatcher) (string, *sync.Mu proxy := security.NewEgressProxy(matcher, false) mu := &sync.Mutex{} attempts := map[string]bool{} - proxy.OnAttempt = func(domain string, allowed bool) { + proxy.OnAttempt = func(a security.EgressAttempt) { mu.Lock() - attempts[domain] = allowed + attempts[a.Domain] = a.Allowed mu.Unlock() } proxyURL, err := proxy.Start(context.Background()) diff --git a/forge-cli/tools/browser/manual_demo_test.go b/forge-cli/tools/browser/manual_demo_test.go index 1cf60ca7..cfd41bd1 100644 --- a/forge-cli/tools/browser/manual_demo_test.go +++ b/forge-cli/tools/browser/manual_demo_test.go @@ -50,12 +50,12 @@ func TestManualBrowseDemo(t *testing.T) { // demonstrates real egress enforcement, not dev-open. matcher := security.NewDomainMatcher(security.ModeAllowlist, []string{u.Hostname()}) proxy := security.NewEgressProxy(matcher, false) - proxy.OnAttempt = func(domain string, allowed bool) { + proxy.OnAttempt = func(a security.EgressAttempt) { verdict := "ALLOW" - if !allowed { + if !a.Allowed { verdict = "BLOCK" } - t.Logf("[egress] %-5s %s", verdict, domain) + t.Logf("[egress] %-5s %s", verdict, a.Domain) } proxyURL, err := proxy.Start(context.Background()) if err != nil { diff --git a/forge-cli/tools/exec.go b/forge-cli/tools/exec.go index da49f358..1bdcd667 100644 --- a/forge-cli/tools/exec.go +++ b/forge-cli/tools/exec.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "net/url" "os" "os/exec" "time" @@ -12,6 +13,7 @@ import ( "go.opentelemetry.io/otel/propagation" "github.com/initializ/forge/forge-core/llm/oauth" + coreruntime "github.com/initializ/forge/forge-core/runtime" ) // otelEnvPassthroughPrefixes lists the OTel SDK env var prefixes / names @@ -145,11 +147,18 @@ func (e *SkillCommandExecutor) Run(ctx context.Context, command string, args []s env = append(env, "REVIEW_MODEL="+e.Model) } if e.ProxyURL != "" { + // #338 — stamp the task/invocation IDs into the proxy URL userinfo so + // the egress proxy can attribute each subprocess request back to its + // task in the audit log. HTTP clients replay userinfo as a Basic + // Proxy-Authorization header on every request/CONNECT; the proxy + // decodes it (identityFromRequest) and tags egress_allowed/blocked + // events. When ctx carries neither ID the base URL is used unchanged. + proxyURL := proxyURLWithIdentity(ctx, e.ProxyURL) env = append(env, - "HTTP_PROXY="+e.ProxyURL, - "HTTPS_PROXY="+e.ProxyURL, - "http_proxy="+e.ProxyURL, - "https_proxy="+e.ProxyURL, + "HTTP_PROXY="+proxyURL, + "HTTPS_PROXY="+proxyURL, + "http_proxy="+proxyURL, + "https_proxy="+proxyURL, ) } // Issue #182 — propagate W3C trace context + curated OTel SDK env @@ -198,6 +207,28 @@ func (e *SkillCommandExecutor) Run(ctx context.Context, command string, args []s return stdout.String(), nil } +// proxyURLWithIdentity embeds the request's task and correlation (invocation) +// IDs as userinfo in the egress proxy URL. The subprocess's HTTP client replays +// that userinfo as a Basic Proxy-Authorization header, letting the egress proxy +// attribute each outbound request to its originating task/invocation in the +// audit log (#338). Returns base unchanged when ctx carries neither ID, or when +// base doesn't parse as a URL. url.UserPassword percent-encodes the IDs, and +// the client decodes them before base64-encoding the Basic credential, so the +// values round-trip verbatim to identityFromRequest on the proxy side. +func proxyURLWithIdentity(ctx context.Context, base string) string { + taskID := coreruntime.TaskIDFromContext(ctx) + corrID := coreruntime.CorrelationIDFromContext(ctx) + if taskID == "" && corrID == "" { + return base + } + u, err := url.Parse(base) + if err != nil { + return base + } + u.User = url.UserPassword(taskID, corrID) + return u.String() +} + // resolveOAuthToken loads and refreshes the OpenAI OAuth token. // Returns nil if no valid token is available. func resolveOAuthToken() *oauth.Token { diff --git a/forge-cli/tools/exec_test.go b/forge-cli/tools/exec_test.go index d9c162e5..5143fabd 100644 --- a/forge-cli/tools/exec_test.go +++ b/forge-cli/tools/exec_test.go @@ -11,6 +11,8 @@ import ( "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" + + coreruntime "github.com/initializ/forge/forge-core/runtime" ) func TestSkillCommandExecutor_OrgIDInjection(t *testing.T) { @@ -245,3 +247,51 @@ func TestSkillCommandExecutor_OTelSubsetPassedThrough(t *testing.T) { } } } + +// TestProxyURLWithIdentity covers the userinfo stamping that lets the egress +// proxy attribute a subprocess request to its task/invocation (#338). +func TestProxyURLWithIdentity(t *testing.T) { + const base = "http://127.0.0.1:54321" + tests := []struct { + name string + ctx context.Context + want string + }{ + { + name: "both ids stamped as userinfo", + ctx: coreruntime.WithCorrelationID(coreruntime.WithTaskID(context.Background(), "task-1"), "corr-1"), + want: "http://task-1:corr-1@127.0.0.1:54321", + }, + { + name: "task only", + ctx: coreruntime.WithTaskID(context.Background(), "task-1"), + want: "http://task-1:@127.0.0.1:54321", + }, + { + name: "no identity leaves base unchanged", + ctx: context.Background(), + want: base, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := proxyURLWithIdentity(tc.ctx, base); got != tc.want { + t.Errorf("proxyURLWithIdentity() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestSkillCommandExecutor_ProxyIdentityInjected proves the executor injects +// the identity-bearing proxy URL into HTTP_PROXY when ctx carries task context. +func TestSkillCommandExecutor_ProxyIdentityInjected(t *testing.T) { + e := &SkillCommandExecutor{ProxyURL: "http://127.0.0.1:9"} + ctx := coreruntime.WithCorrelationID(coreruntime.WithTaskID(context.Background(), "task-42"), "corr-42") + out, err := e.Run(ctx, "env", nil, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out, "HTTP_PROXY=http://task-42:corr-42@127.0.0.1:9") { + t.Errorf("expected identity-stamped HTTP_PROXY in subprocess env; got:\n%s", out) + } +} diff --git a/forge-core/security/egress_proxy.go b/forge-core/security/egress_proxy.go index 7853cc5c..68e3b026 100644 --- a/forge-core/security/egress_proxy.go +++ b/forge-core/security/egress_proxy.go @@ -2,6 +2,7 @@ package security import ( "context" + "encoding/base64" "fmt" "io" "net" @@ -21,7 +22,28 @@ type EgressProxy struct { listener net.Listener srv *http.Server addr string // "127.0.0.1:" - OnAttempt func(domain string, allowed bool) + OnAttempt func(EgressAttempt) +} + +// EgressAttempt describes a single egress decision for audit correlation. +// TaskID and CorrelationID are recovered from the Proxy-Authorization header +// the subprocess sends (see identityFromRequest) — the caller injects them as +// userinfo in the HTTP_PROXY URL, which HTTP clients replay as Basic proxy +// credentials on every request and CONNECT. They are empty when the client +// doesn't send credentials (arbitrary binaries), which degrades gracefully to +// the pre-#338 behaviour: a domain-only event with no task attribution. +type EgressAttempt struct { + Domain string + Allowed bool + TaskID string + CorrelationID string +} + +// egressIdentity carries the per-request task/invocation IDs recovered from the +// proxy credentials, threaded from the request handler down to the callback. +type egressIdentity struct { + taskID string + correlationID string } // NewEgressProxy creates a new EgressProxy that validates domains using the @@ -89,8 +111,9 @@ func (p *EgressProxy) handleRequest(w http.ResponseWriter, req *http.Request) { // handleHTTP forwards plain HTTP requests after domain validation. func (p *EgressProxy) handleHTTP(w http.ResponseWriter, req *http.Request) { host := extractHost(req.URL.Host) + id := identityFromRequest(req) - if !p.checkDomain(host) { + if !p.checkDomain(host, id) { http.Error(w, fmt.Sprintf("egress proxy: domain %q blocked", host), http.StatusForbidden) return } @@ -132,8 +155,9 @@ func (p *EgressProxy) handleHTTP(w http.ResponseWriter, req *http.Request) { // hostname, then blind-relays bytes without decrypting TLS. func (p *EgressProxy) handleConnect(w http.ResponseWriter, req *http.Request) { host := extractHost(req.Host) + id := identityFromRequest(req) - if !p.checkDomain(host) { + if !p.checkDomain(host, id) { http.Error(w, fmt.Sprintf("egress proxy: domain %q blocked", host), http.StatusForbidden) return } @@ -182,28 +206,59 @@ func (p *EgressProxy) handleConnect(w http.ResponseWriter, req *http.Request) { } // checkDomain validates a host against the matcher, allowing localhost always. -func (p *EgressProxy) checkDomain(host string) bool { +func (p *EgressProxy) checkDomain(host string, id egressIdentity) bool { // Reject non-standard IP formats early if err := ValidateHostIP(host); err != nil { - p.fireCallback(host, false) + p.fireCallback(host, false, id) return false } // Localhost is always allowed if IsLocalhost(host) { - p.fireCallback(host, true) + p.fireCallback(host, true, id) return true } allowed := p.matcher.IsAllowed(host) - p.fireCallback(host, allowed) + p.fireCallback(host, allowed, id) return allowed } -func (p *EgressProxy) fireCallback(domain string, allowed bool) { +func (p *EgressProxy) fireCallback(domain string, allowed bool, id egressIdentity) { if p.OnAttempt != nil { - p.OnAttempt(domain, allowed) + p.OnAttempt(EgressAttempt{ + Domain: domain, + Allowed: allowed, + TaskID: id.taskID, + CorrelationID: id.correlationID, + }) + } +} + +// identityFromRequest recovers the task/invocation IDs the caller stashed in +// the proxy credentials. HTTP clients that see userinfo in the HTTP_PROXY URL +// replay it as a "Proxy-Authorization: Basic base64(taskID:correlationID)" +// header on every proxied request and CONNECT. Returns a zero identity when the +// header is absent or malformed — the proxy still enforces and audits, just +// without task attribution (arbitrary binaries that ignore proxy creds). +func identityFromRequest(req *http.Request) egressIdentity { + h := req.Header.Get("Proxy-Authorization") + if h == "" { + return egressIdentity{} + } + const prefix = "Basic " + if len(h) <= len(prefix) || !strings.EqualFold(h[:len(prefix)], prefix) { + return egressIdentity{} + } + raw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(h[len(prefix):])) + if err != nil { + return egressIdentity{} + } + task, corr, found := strings.Cut(string(raw), ":") + if !found { + return egressIdentity{} } + return egressIdentity{taskID: task, correlationID: corr} } // extractHost strips the port from a host:port string. diff --git a/forge-core/security/egress_proxy_test.go b/forge-core/security/egress_proxy_test.go index 3f01e610..a81c1081 100644 --- a/forge-core/security/egress_proxy_test.go +++ b/forge-core/security/egress_proxy_test.go @@ -3,6 +3,7 @@ package security import ( "context" "crypto/tls" + "encoding/base64" "io" "net" "net/http" @@ -251,16 +252,10 @@ func TestEgressProxyOnAttemptCallback(t *testing.T) { proxy := NewEgressProxy(matcher, false) var mu sync.Mutex - var calls []struct { - domain string - allowed bool - } - proxy.OnAttempt = func(domain string, allowed bool) { + var calls []EgressAttempt + proxy.OnAttempt = func(a EgressAttempt) { mu.Lock() - calls = append(calls, struct { - domain string - allowed bool - }{domain, allowed}) + calls = append(calls, a) mu.Unlock() } @@ -297,8 +292,8 @@ func TestEgressProxyOnAttemptCallback(t *testing.T) { t.Fatal("expected at least 1 callback call") } // First call should be the localhost (upstream is on localhost) - if !calls[0].allowed { - t.Errorf("first call should be allowed (localhost), got allowed=%v", calls[0].allowed) + if !calls[0].Allowed { + t.Errorf("first call should be allowed (localhost), got allowed=%v", calls[0].Allowed) } } @@ -344,3 +339,91 @@ func TestEgressProxyURL(t *testing.T) { t.Errorf("ProxyURL() = %q, want %q", proxy.ProxyURL(), proxyAddr) } } + +// TestIdentityFromRequest covers the Proxy-Authorization decode that recovers +// the task/invocation IDs the subprocess replays as Basic proxy creds (#338). +func TestIdentityFromRequest(t *testing.T) { + basic := func(user, pass string) string { + return "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) + } + tests := []struct { + name string + header string + wantTask string + wantCorr string + }{ + {"both ids", basic("task-123", "corr-abc"), "task-123", "corr-abc"}, + {"task only, empty corr", basic("task-123", ""), "task-123", ""}, + {"empty task, corr only", basic("", "corr-abc"), "", "corr-abc"}, + {"case-insensitive scheme", "basic " + base64.StdEncoding.EncodeToString([]byte("t:c")), "t", "c"}, + {"no header", "", "", ""}, + {"wrong scheme", "Bearer abc", "", ""}, + {"not base64", "Basic @@@notb64@@@", "", ""}, + {"no colon separator", "Basic " + base64.StdEncoding.EncodeToString([]byte("nocolon")), "", ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) + if tc.header != "" { + req.Header.Set("Proxy-Authorization", tc.header) + } + got := identityFromRequest(req) + if got.taskID != tc.wantTask || got.correlationID != tc.wantCorr { + t.Errorf("identityFromRequest() = {task:%q corr:%q}, want {task:%q corr:%q}", + got.taskID, got.correlationID, tc.wantTask, tc.wantCorr) + } + }) + } +} + +// TestEgressProxyAttributesIdentity proves the end-to-end path: a client that +// sends userinfo in the proxy URL (as the SkillCommandExecutor does) surfaces +// its task/invocation IDs on the OnAttempt callback for both plain HTTP and the +// HTTPS CONNECT tunnel (#338). +func TestEgressProxyAttributesIdentity(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + upstreamURL, _ := url.Parse(upstream.URL) + + matcher := NewDomainMatcher(ModeAllowlist, []string{upstreamURL.Hostname()}) + proxy := NewEgressProxy(matcher, false) + + var mu sync.Mutex + var got EgressAttempt + proxy.OnAttempt = func(a EgressAttempt) { + mu.Lock() + got = a + mu.Unlock() + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + proxyAddr, err := proxy.Start(ctx) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer proxy.Stop() //nolint:errcheck + + // Proxy URL carries the identity as userinfo — the client replays it as a + // Basic Proxy-Authorization header, exactly like proxyURLWithIdentity does. + proxyURL, _ := url.Parse(proxyAddr) + proxyURL.User = url.UserPassword("task-xyz", "corr-777") + client := &http.Client{Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}} + + resp, err := client.Get(upstream.URL + "/test") + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() //nolint:errcheck + + mu.Lock() + defer mu.Unlock() + if got.TaskID != "task-xyz" { + t.Errorf("TaskID = %q, want %q", got.TaskID, "task-xyz") + } + if got.CorrelationID != "corr-777" { + t.Errorf("CorrelationID = %q, want %q", got.CorrelationID, "corr-777") + } +} From a19c3eeaccc4938c3d3126bb1fe8cabb1ed25695 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 18 Jul 2026 14:04:43 -0400 Subject: [PATCH 2/4] docs(egress): document task/correlation attribution on proxy audit events (#338) Update audit-logging.md (example + source note + plain-Emit exception table), egress-control.md (audit-events section), and the comprehensive forge.md skill to reflect that subprocess proxy egress events now carry correlation_id + task_id, recovered from the Proxy-Authorization creds the subprocess replays. Note the graceful no-identity fallback for credential-ignoring binaries. --- .claude/skills/forge.md | 7 ++++++- docs/security/audit-logging.md | 6 +++--- docs/security/egress-control.md | 8 ++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/.claude/skills/forge.md b/.claude/skills/forge.md index e114fe79..ff14fe7d 100644 --- a/.claude/skills/forge.md +++ b/.claude/skills/forge.md @@ -565,7 +565,12 @@ the in-process proxy isn't started — `NetworkPolicy` (generated by `forge package`) handles egress at the pod level. Emits `egress_allowed` / `egress_blocked` per request with `source` -(in-process vs proxy) so consumers can attribute. +(in-process vs proxy) so consumers can attribute. Both variants carry +`correlation_id` + `task_id`: the in-process enforcer reads them from +ctx; the subprocess proxy recovers them from the `Proxy-Authorization` +creds the subprocess replays (runner stamps the IDs into the injected +`HTTP_PROXY` userinfo). Creds-ignoring binaries still get enforced + +audited, just without identity (#338). **Read**: `docs/security/egress-control.md`, `docs/security/overview.md`. diff --git a/docs/security/audit-logging.md b/docs/security/audit-logging.md index dd6aefa7..7d7c9032 100644 --- a/docs/security/audit-logging.md +++ b/docs/security/audit-logging.md @@ -48,12 +48,12 @@ All runtime security events are emitted as structured NDJSON to stderr with corr ```json {"ts":"2026-02-28T10:00:00Z","event":"session_start","correlation_id":"a1b2c3d4","task_id":"task-1"} {"ts":"2026-02-28T10:00:01Z","event":"tool_exec","correlation_id":"a1b2c3d4","fields":{"tool":"tavily_research","phase":"start"}} -{"ts":"2026-02-28T10:00:01Z","event":"egress_allowed","correlation_id":"a1b2c3d4","fields":{"domain":"api.tavily.com","mode":"allowlist","source":"proxy"}} +{"ts":"2026-02-28T10:00:01Z","event":"egress_allowed","correlation_id":"a1b2c3d4","task_id":"task-1","fields":{"domain":"api.tavily.com","mode":"allowlist","source":"proxy"}} {"ts":"2026-02-28T10:00:05Z","event":"tool_exec","correlation_id":"a1b2c3d4","fields":{"tool":"tavily_research","phase":"end"}} {"ts":"2026-02-28T10:00:06Z","event":"session_end","correlation_id":"a1b2c3d4","fields":{"state":"completed"}} ``` -The `source` field distinguishes in-process enforcer events from subprocess proxy events. +The `source` field distinguishes in-process enforcer events from subprocess proxy events. Both carry `correlation_id` and `task_id`: the in-process enforcer reads them from the request context, while the subprocess proxy recovers them from the `Proxy-Authorization` credentials the subprocess replays (the runner stamps the task/invocation IDs into the `HTTP_PROXY` URL userinfo). A subprocess binary that ignores proxy credentials still produces an enforced, audited event — just without the identity fields (issue #338). ### Workflow correlation @@ -581,7 +581,7 @@ documented inline: | Site | Why plain `Emit` | |---|---| -| Egress proxy `OnAttempt` with `source=proxy` | Subprocess HTTP `CONNECT` has no Go ctx tying back to the A2A request | +| Egress proxy `OnAttempt` with `source=proxy` | Subprocess HTTP `CONNECT` has no Go ctx tying back to the A2A request, so it can't use `EmitFromContext`. Since #338 it still recovers `task_id` + `correlation_id` out-of-band from the `Proxy-Authorization` creds and sets them on the event manually; seq + trace cross-link remain unavailable (no ctx). | | MCP server startup events (`mcp_server_started` / `_failed` / `_degraded`) | Pre-invocation; no scope | | Scheduler tick (`schedule_fire` / `schedule_complete` / `schedule_skip` / `schedule_modify`) | Runs on its own timer outside any A2A request | | Startup banners (`policy_loaded`, `agent_card_published`, `audit_export_status`) | Pre-invocation; no scope | diff --git a/docs/security/egress-control.md b/docs/security/egress-control.md index 0d993914..94cc70b5 100644 --- a/docs/security/egress-control.md +++ b/docs/security/egress-control.md @@ -327,12 +327,12 @@ The `allow_private_ips` field controls whether RFC 1918 addresses are allowed th Both the enforcer and proxy emit structured audit events: ```json -{"event":"egress_allowed","domain":"api.tavily.com","mode":"allowlist"} -{"event":"egress_blocked","domain":"evil.com","mode":"allowlist"} -{"event":"egress_allowed","domain":"api.tavily.com","mode":"allowlist","source":"proxy"} +{"event":"egress_allowed","correlation_id":"a1b2c3d4","task_id":"task-1","fields":{"domain":"api.tavily.com","mode":"allowlist"}} +{"event":"egress_blocked","correlation_id":"a1b2c3d4","task_id":"task-1","fields":{"domain":"evil.com","mode":"allowlist"}} +{"event":"egress_allowed","correlation_id":"a1b2c3d4","task_id":"task-1","fields":{"domain":"api.tavily.com","mode":"allowlist","source":"proxy"}} ``` -Events without `"source"` come from the in-process enforcer; events with `"source": "proxy"` come from the subprocess proxy. +Events without `"source"` come from the in-process enforcer; events with `"source": "proxy"` come from the subprocess proxy. Both carry `correlation_id` (the invocation ID) and `task_id`. The in-process enforcer reads them from the request context; the proxy recovers them from the `Proxy-Authorization` credentials the subprocess replays — the runner stamps the task/invocation IDs into the injected `HTTP_PROXY` URL as userinfo, and standard HTTP clients echo that back as a Basic proxy-auth header on every request and `CONNECT`. A binary that ignores proxy credentials is still enforced and audited, but its proxy events omit the identity fields (issue #338). ## Related Files From 59f7736bb04c5edd79acb3d543c94d3ff21c60b8 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 18 Jul 2026 14:30:52 -0400 Subject: [PATCH 3/4] fix(egress): separator-safe identity encoding + CONNECT test (#339 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on PR #339: - [Low] A task ID containing a raw ':' (params.id is client-supplied) would mis-split the flattened Basic credential, since Basic auth has no internal escaping and identityFromRequest cut at the first colon. Encode each ID with base64url (alphabet [A-Za-z0-9-_], colon-free) before it goes into the HTTP_PROXY userinfo, and decode each half on the proxy side. The separator can no longer collide with the data. Added a colon-bearing round-trip case and an invalid-inner-encoding case. - [Nit] Added TestEgressProxyAttributesIdentityCONNECT: drives a real HTTPS CONNECT tunnel through the proxy and asserts the IDs surface via handleConnect (the prior end-to-end test only covered plain HTTP). Uses a colon-bearing task ID to exercise the encoding through the tunnel too. Finding #3 (IDs readable via /proc/self/environ) is correlation data, not secrets — acknowledged inline in the proxyURLWithIdentity doc comment. --- forge-cli/tools/exec.go | 16 +++-- forge-cli/tools/exec_test.go | 24 +++++-- forge-core/security/egress_proxy.go | 21 ++++-- forge-core/security/egress_proxy_test.go | 82 ++++++++++++++++++++++-- 4 files changed, 121 insertions(+), 22 deletions(-) diff --git a/forge-cli/tools/exec.go b/forge-cli/tools/exec.go index 1bdcd667..c08ad048 100644 --- a/forge-cli/tools/exec.go +++ b/forge-cli/tools/exec.go @@ -3,6 +3,7 @@ package tools import ( "bytes" "context" + "encoding/base64" "fmt" "net/url" "os" @@ -212,9 +213,15 @@ func (e *SkillCommandExecutor) Run(ctx context.Context, command string, args []s // that userinfo as a Basic Proxy-Authorization header, letting the egress proxy // attribute each outbound request to its originating task/invocation in the // audit log (#338). Returns base unchanged when ctx carries neither ID, or when -// base doesn't parse as a URL. url.UserPassword percent-encodes the IDs, and -// the client decodes them before base64-encoding the Basic credential, so the -// values round-trip verbatim to identityFromRequest on the proxy side. +// base doesn't parse as a URL. +// +// The IDs are base64url-encoded before they go into the userinfo. The Basic +// credential the client sends is base64(user:pass) with no internal escaping, +// so a task_id containing a raw ':' (params.id is client-supplied) would +// mis-split on the proxy side. base64url output is drawn from [A-Za-z0-9-_], so +// it can never contain the ':' separator — identityFromRequest decodes each +// half back to the verbatim ID. These are correlation identifiers, not secrets; +// the encoding is for separator-safety, not confidentiality. func proxyURLWithIdentity(ctx context.Context, base string) string { taskID := coreruntime.TaskIDFromContext(ctx) corrID := coreruntime.CorrelationIDFromContext(ctx) @@ -225,7 +232,8 @@ func proxyURLWithIdentity(ctx context.Context, base string) string { if err != nil { return base } - u.User = url.UserPassword(taskID, corrID) + enc := base64.RawURLEncoding + u.User = url.UserPassword(enc.EncodeToString([]byte(taskID)), enc.EncodeToString([]byte(corrID))) return u.String() } diff --git a/forge-cli/tools/exec_test.go b/forge-cli/tools/exec_test.go index 5143fabd..fb787c3b 100644 --- a/forge-cli/tools/exec_test.go +++ b/forge-cli/tools/exec_test.go @@ -2,6 +2,7 @@ package tools import ( "context" + "encoding/base64" "os" "regexp" "strings" @@ -248,8 +249,15 @@ func TestSkillCommandExecutor_OTelSubsetPassedThrough(t *testing.T) { } } +// b64u is the separator-safe encoding proxyURLWithIdentity uses for the userinfo +// halves (see #338 — a raw ':' in a task ID would otherwise mis-split the Basic +// credential on the proxy side). +func b64u(s string) string { return base64.RawURLEncoding.EncodeToString([]byte(s)) } + // TestProxyURLWithIdentity covers the userinfo stamping that lets the egress -// proxy attribute a subprocess request to its task/invocation (#338). +// proxy attribute a subprocess request to its task/invocation (#338). The IDs +// are base64url-encoded so a colon in a task ID can't collide with the Basic +// credential separator. func TestProxyURLWithIdentity(t *testing.T) { const base = "http://127.0.0.1:54321" tests := []struct { @@ -260,12 +268,17 @@ func TestProxyURLWithIdentity(t *testing.T) { { name: "both ids stamped as userinfo", ctx: coreruntime.WithCorrelationID(coreruntime.WithTaskID(context.Background(), "task-1"), "corr-1"), - want: "http://task-1:corr-1@127.0.0.1:54321", + want: "http://" + b64u("task-1") + ":" + b64u("corr-1") + "@127.0.0.1:54321", }, { name: "task only", ctx: coreruntime.WithTaskID(context.Background(), "task-1"), - want: "http://task-1:@127.0.0.1:54321", + want: "http://" + b64u("task-1") + ":@127.0.0.1:54321", + }, + { + name: "colon-bearing task id survives (no mis-split)", + ctx: coreruntime.WithCorrelationID(coreruntime.WithTaskID(context.Background(), "urn:task:1"), "corr-1"), + want: "http://" + b64u("urn:task:1") + ":" + b64u("corr-1") + "@127.0.0.1:54321", }, { name: "no identity leaves base unchanged", @@ -291,7 +304,8 @@ func TestSkillCommandExecutor_ProxyIdentityInjected(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !strings.Contains(out, "HTTP_PROXY=http://task-42:corr-42@127.0.0.1:9") { - t.Errorf("expected identity-stamped HTTP_PROXY in subprocess env; got:\n%s", out) + want := "HTTP_PROXY=http://" + b64u("task-42") + ":" + b64u("corr-42") + "@127.0.0.1:9" + if !strings.Contains(out, want) { + t.Errorf("expected identity-stamped HTTP_PROXY %q in subprocess env; got:\n%s", want, out) } } diff --git a/forge-core/security/egress_proxy.go b/forge-core/security/egress_proxy.go index 68e3b026..3a7f05af 100644 --- a/forge-core/security/egress_proxy.go +++ b/forge-core/security/egress_proxy.go @@ -237,10 +237,14 @@ func (p *EgressProxy) fireCallback(domain string, allowed bool, id egressIdentit // identityFromRequest recovers the task/invocation IDs the caller stashed in // the proxy credentials. HTTP clients that see userinfo in the HTTP_PROXY URL -// replay it as a "Proxy-Authorization: Basic base64(taskID:correlationID)" -// header on every proxied request and CONNECT. Returns a zero identity when the -// header is absent or malformed — the proxy still enforces and audits, just -// without task attribution (arbitrary binaries that ignore proxy creds). +// replay it as a "Proxy-Authorization: Basic base64(user:pass)" header on every +// proxied request and CONNECT, where user/pass are the base64url-encoded task +// and correlation IDs (see proxyURLWithIdentity). The inner base64url encoding +// keeps each half free of the ':' separator, so a task ID containing a raw +// colon can't mis-split its own attribution. Returns a zero identity when the +// header is absent or doesn't match that exact shape — the proxy still enforces +// and audits, just without task attribution (arbitrary binaries that ignore +// proxy creds, or send their own unrelated credentials). func identityFromRequest(req *http.Request) egressIdentity { h := req.Header.Get("Proxy-Authorization") if h == "" { @@ -254,11 +258,16 @@ func identityFromRequest(req *http.Request) egressIdentity { if err != nil { return egressIdentity{} } - task, corr, found := strings.Cut(string(raw), ":") + encTask, encCorr, found := strings.Cut(string(raw), ":") if !found { return egressIdentity{} } - return egressIdentity{taskID: task, correlationID: corr} + task, tErr := base64.RawURLEncoding.DecodeString(encTask) + corr, cErr := base64.RawURLEncoding.DecodeString(encCorr) + if tErr != nil || cErr != nil { + return egressIdentity{} + } + return egressIdentity{taskID: string(task), correlationID: string(corr)} } // extractHost strips the port from a host:port string. diff --git a/forge-core/security/egress_proxy_test.go b/forge-core/security/egress_proxy_test.go index a81c1081..5ad1e8ff 100644 --- a/forge-core/security/egress_proxy_test.go +++ b/forge-core/security/egress_proxy_test.go @@ -342,9 +342,14 @@ func TestEgressProxyURL(t *testing.T) { // TestIdentityFromRequest covers the Proxy-Authorization decode that recovers // the task/invocation IDs the subprocess replays as Basic proxy creds (#338). +// The credential shape is base64(b64url(task) ":" b64url(corr)) — the inner +// base64url keeps each half colon-free so a task ID containing ':' can't +// mis-split its own attribution. func TestIdentityFromRequest(t *testing.T) { - basic := func(user, pass string) string { - return "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) + b64u := func(s string) string { return base64.RawURLEncoding.EncodeToString([]byte(s)) } + // basic builds the exact header a client produces from the stamped userinfo. + basic := func(task, corr string) string { + return "Basic " + base64.StdEncoding.EncodeToString([]byte(b64u(task)+":"+b64u(corr))) } tests := []struct { name string @@ -355,11 +360,13 @@ func TestIdentityFromRequest(t *testing.T) { {"both ids", basic("task-123", "corr-abc"), "task-123", "corr-abc"}, {"task only, empty corr", basic("task-123", ""), "task-123", ""}, {"empty task, corr only", basic("", "corr-abc"), "", "corr-abc"}, - {"case-insensitive scheme", "basic " + base64.StdEncoding.EncodeToString([]byte("t:c")), "t", "c"}, + {"colon-bearing task id round-trips", basic("urn:task:1", "corr-abc"), "urn:task:1", "corr-abc"}, + {"case-insensitive scheme", "basic " + base64.StdEncoding.EncodeToString([]byte(b64u("t")+":"+b64u("c"))), "t", "c"}, {"no header", "", "", ""}, {"wrong scheme", "Bearer abc", "", ""}, - {"not base64", "Basic @@@notb64@@@", "", ""}, + {"outer not base64", "Basic @@@notb64@@@", "", ""}, {"no colon separator", "Basic " + base64.StdEncoding.EncodeToString([]byte("nocolon")), "", ""}, + {"inner halves not base64url", "Basic " + base64.StdEncoding.EncodeToString([]byte("!!!:@@@")), "", ""}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -406,10 +413,12 @@ func TestEgressProxyAttributesIdentity(t *testing.T) { } defer proxy.Stop() //nolint:errcheck - // Proxy URL carries the identity as userinfo — the client replays it as a - // Basic Proxy-Authorization header, exactly like proxyURLWithIdentity does. + // Proxy URL carries the identity as base64url-encoded userinfo — the client + // replays it as a Basic Proxy-Authorization header, exactly like + // proxyURLWithIdentity stamps it. + b64u := func(s string) string { return base64.RawURLEncoding.EncodeToString([]byte(s)) } proxyURL, _ := url.Parse(proxyAddr) - proxyURL.User = url.UserPassword("task-xyz", "corr-777") + proxyURL.User = url.UserPassword(b64u("task-xyz"), b64u("corr-777")) client := &http.Client{Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}} resp, err := client.Get(upstream.URL + "/test") @@ -427,3 +436,62 @@ func TestEgressProxyAttributesIdentity(t *testing.T) { t.Errorf("CorrelationID = %q, want %q", got.CorrelationID, "corr-777") } } + +// TestEgressProxyAttributesIdentityCONNECT proves identity surfaces through the +// HTTPS CONNECT tunnel path, not just plain HTTP — handleConnect reads the same +// Proxy-Authorization the client sends on the CONNECT request itself. A colon in +// the task ID is deliberately used to also exercise the separator-safe encoding +// end-to-end (#338). +func TestEgressProxyAttributesIdentityCONNECT(t *testing.T) { + upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + upstreamURL, _ := url.Parse(upstream.URL) + host, port, _ := net.SplitHostPort(upstreamURL.Host) + + matcher := NewDomainMatcher(ModeAllowlist, []string{host}) + proxy := NewEgressProxy(matcher, false) + + var mu sync.Mutex + var got EgressAttempt + proxy.OnAttempt = func(a EgressAttempt) { + mu.Lock() + got = a + mu.Unlock() + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + proxyAddr, err := proxy.Start(ctx) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer proxy.Stop() //nolint:errcheck + + b64u := func(s string) string { return base64.RawURLEncoding.EncodeToString([]byte(s)) } + proxyURL, _ := url.Parse(proxyAddr) + proxyURL.User = url.UserPassword(b64u("urn:task:9"), b64u("corr-9")) + client := &http.Client{ + Transport: &http.Transport{ + Proxy: http.ProxyURL(proxyURL), + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec + }, + Timeout: 5 * time.Second, + } + + resp, err := client.Get("https://" + host + ":" + port + "/test") + if err != nil { + t.Fatalf("CONNECT request failed: %v", err) + } + resp.Body.Close() //nolint:errcheck + + mu.Lock() + defer mu.Unlock() + if got.TaskID != "urn:task:9" { + t.Errorf("TaskID = %q, want %q", got.TaskID, "urn:task:9") + } + if got.CorrelationID != "corr-9" { + t.Errorf("CorrelationID = %q, want %q", got.CorrelationID, "corr-9") + } +} From bc28270b4a68aeac95f269d6056fb1ee1883cb9c Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 18 Jul 2026 14:46:48 -0400 Subject: [PATCH 4/4] fix(egress): stamp identity on cli_execute proxy egress too (#338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial fix only covered SkillCommandExecutor, but the cli_execute builtin (curl/gh/kubectl/... driven by the LLM) injects HTTP_PROXY through its own buildEnv path. So subprocess egress from cli_execute still emitted source=proxy audit events with no task_id/correlation_id — observed live: {"event":"egress_blocked","fields":{"domain":"initializ.ai", "mode":"allowlist","source":"proxy"}} ← no task_id/correlation_id Thread ctx into CLIExecuteTool.buildEnv and run the proxy URL through the same proxyURLWithIdentity stamping, so curl (and every other cli_execute binary) replays the base64url task/correlation IDs as Proxy-Authorization. These are the only two HTTP_PROXY-injecting executors; the browser uses chromedp's own proxy path and is out of scope here. Test: TestBuildEnv_ProxyIdentityStamped pins the stamped HTTP_PROXY for cli_execute using the exact task/correlation IDs from the live repro. --- forge-cli/tools/cli_execute.go | 18 +++++++---- forge-cli/tools/cli_execute_test.go | 47 +++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/forge-cli/tools/cli_execute.go b/forge-cli/tools/cli_execute.go index bc67cf81..a5f34805 100644 --- a/forge-cli/tools/cli_execute.go +++ b/forge-cli/tools/cli_execute.go @@ -223,7 +223,7 @@ func (t *CLIExecuteTool) Execute(ctx context.Context, args json.RawMessage) (str } // Security check 6: Env isolation - cmd.Env = t.buildEnv(input.Binary) + cmd.Env = t.buildEnv(ctx, input.Binary) // R9 JIT credentials: if the runner wired a credentials.Injector, // mint fresh scoped-down creds now, merge them into the subprocess @@ -332,7 +332,9 @@ func stripEnvKeys(env []string, override map[string]string) []string { // and explicitly configured passthrough variables. When workDir is set, // HOME is overridden to workDir so subprocess ~ expansion stays confined. // The binary parameter scopes credential env vars to only the binaries that need them. -func (t *CLIExecuteTool) buildEnv(binary string) []string { +// ctx carries the task/invocation identity that is stamped into the egress +// proxy URL so proxied subprocess egress can be attributed in the audit log (#338). +func (t *CLIExecuteTool) buildEnv(ctx context.Context, binary string) []string { realHome := os.Getenv("HOME") homeVal := realHome if t.workDir != "" { @@ -374,11 +376,15 @@ func (t *CLIExecuteTool) buildEnv(binary string) []string { } } if t.proxyURL != "" { + // #338 — stamp the task/invocation IDs into the proxy URL userinfo so + // the egress proxy can attribute this subprocess's egress to its task + // in the audit log (same mechanism as SkillCommandExecutor). + proxyURL := proxyURLWithIdentity(ctx, t.proxyURL) env = append(env, - "HTTP_PROXY="+t.proxyURL, - "HTTPS_PROXY="+t.proxyURL, - "http_proxy="+t.proxyURL, - "https_proxy="+t.proxyURL, + "HTTP_PROXY="+proxyURL, + "HTTPS_PROXY="+proxyURL, + "http_proxy="+proxyURL, + "https_proxy="+proxyURL, ) } diff --git a/forge-cli/tools/cli_execute_test.go b/forge-cli/tools/cli_execute_test.go index eee9d60b..8c704ed3 100644 --- a/forge-cli/tools/cli_execute_test.go +++ b/forge-cli/tools/cli_execute_test.go @@ -2,12 +2,15 @@ package tools import ( "context" + "encoding/base64" "encoding/json" "os" "path/filepath" "runtime" "strings" "testing" + + coreruntime "github.com/initializ/forge/forge-core/runtime" ) func TestCLIExecute_Name(t *testing.T) { @@ -583,7 +586,7 @@ func TestBuildEnv_GHConfigDirScopedToGh(t *testing.T) { }) // gh binary should get GH_CONFIG_DIR - ghEnv := tool.buildEnv("gh") + ghEnv := tool.buildEnv(context.Background(), "gh") found := false for _, e := range ghEnv { if strings.HasPrefix(e, "GH_CONFIG_DIR=") { @@ -596,7 +599,7 @@ func TestBuildEnv_GHConfigDirScopedToGh(t *testing.T) { } // curl binary should NOT get GH_CONFIG_DIR - curlEnv := tool.buildEnv("curl") + curlEnv := tool.buildEnv(context.Background(), "curl") for _, e := range curlEnv { if strings.HasPrefix(e, "GH_CONFIG_DIR=") { t.Error("GH_CONFIG_DIR should not be set for curl binary") @@ -624,7 +627,7 @@ func TestBuildEnv_KubeconfigScopedToKubectl(t *testing.T) { }) // kubectl should get KUBECONFIG - kubectlEnv := tool.buildEnv("kubectl") + kubectlEnv := tool.buildEnv(context.Background(), "kubectl") found := false for _, e := range kubectlEnv { if strings.HasPrefix(e, "KUBECONFIG=") { @@ -640,7 +643,7 @@ func TestBuildEnv_KubeconfigScopedToKubectl(t *testing.T) { } // curl should NOT get KUBECONFIG - curlEnv := tool.buildEnv("curl") + curlEnv := tool.buildEnv(context.Background(), "curl") for _, e := range curlEnv { if strings.HasPrefix(e, "KUBECONFIG=") { t.Error("KUBECONFIG should not be set for curl binary") @@ -669,7 +672,7 @@ func TestBuildEnv_KubectlNoProxy(t *testing.T) { tool.proxyURL = "http://127.0.0.1:54321" // kubectl should get NO_PROXY with the K8s API server host - kubectlEnv := tool.buildEnv("kubectl") + kubectlEnv := tool.buildEnv(context.Background(), "kubectl") var noProxy string for _, e := range kubectlEnv { if strings.HasPrefix(e, "NO_PROXY=") { @@ -688,10 +691,42 @@ func TestBuildEnv_KubectlNoProxy(t *testing.T) { } // curl should NOT get NO_PROXY - curlEnv := tool.buildEnv("curl") + curlEnv := tool.buildEnv(context.Background(), "curl") for _, e := range curlEnv { if strings.HasPrefix(e, "NO_PROXY=") { t.Error("NO_PROXY should not be set for curl binary") } } } + +// TestBuildEnv_ProxyIdentityStamped pins that cli_execute (the tool the LLM +// drives via curl/gh/etc.) stamps the task/invocation IDs into the injected +// HTTP_PROXY, so the egress proxy can attribute its subprocess egress (#338). +// This is the path that produced source=proxy events with no task_id before +// the fix was extended beyond SkillCommandExecutor. +func TestBuildEnv_ProxyIdentityStamped(t *testing.T) { + tool := NewCLIExecuteTool(CLIExecuteConfig{ + AllowedBinaries: []string{"curl"}, + WorkDir: t.TempDir(), + }) + tool.proxyURL = "http://127.0.0.1:54321" + + ctx := coreruntime.WithCorrelationID( + coreruntime.WithTaskID(context.Background(), "aibuilderdemo-1784399795814070000"), + "7fabfb4f99467e8c", + ) + want := "HTTP_PROXY=http://" + + base64.RawURLEncoding.EncodeToString([]byte("aibuilderdemo-1784399795814070000")) + ":" + + base64.RawURLEncoding.EncodeToString([]byte("7fabfb4f99467e8c")) + "@127.0.0.1:54321" + + env := tool.buildEnv(ctx, "curl") + var found bool + for _, e := range env { + if e == want { + found = true + } + } + if !found { + t.Errorf("expected identity-stamped %q in cli_execute env; got:\n%s", want, strings.Join(env, "\n")) + } +}