fix(egress): attribute subprocess proxy audit events to task/invocation (#338)

[initializ-mk]

What

Closes #338. Subprocess egress audit events (egress_allowed / egress_blocked with "source": "proxy") now carry task_id and correlation_id, so proxied traffic from skill scripts, the browser, and DB/CLI binaries can be correlated back to the task/invocation that caused it.

Why they were missing

Two emission paths, only one carried identity:

• In-process enforcer (LLM/MCP/channels) tags IDs from ctx via EmitFromContext — fine.
• Egress proxy is a separate 127.0.0.1 HTTP forward proxy. Subprocesses reach it through HTTP_PROXY, so the request that lands at the proxy has no Go context — OnAttempt(domain, allowed) had nothing to attach.

Approach

Carry identity across the process boundary in the proxy credentials:

1. exec.go stamps the task + correlation IDs into the HTTP_PROXY userinfo — http://<taskID>:<correlationID>@127.0.0.1:<port> (proxyURLWithIdentity).
2. HTTP clients replay userinfo as Proxy-Authorization: Basic base64(taskID:correlationID) on every request and CONNECT.
3. The proxy decodes it (identityFromRequest) and threads the IDs onto a new EgressAttempt struct; runner.go sets TaskID + CorrelationID on the event.

Safety

• Graceful fallback — a binary that ignores proxy creds is still enforced + audited, just with empty IDs (identical to pre-fix).
• No leak — Proxy-Authorization is stripped before the request is forwarded upstream; CONNECT is a blind byte relay (no headers forwarded).
• Known limitation — a compromised subprocess inside the trust boundary could spoof another task's ID (correlation, not authz). The tamper-proof alternative (per-task proxy port) is much heavier; documented in the issue for later if needed.

Tests

• identityFromRequest decode table (both IDs, partial, case-insensitive scheme, malformed, missing).
• End-to-end attribution through a real proxied request (HTTP path; CONNECT exercised via the existing proxy tests + relay).
• proxyURLWithIdentity stamping table + SkillCommandExecutor HTTP_PROXY injection.
• All forge-core/security (incl. -race) and forge-cli/tools + runtime suites green; lint clean.

[initializ-mk]

Review — attribute subprocess proxy audit events to task/invocation (#338)

Approve. Clean, correctly-scoped fix. I traced every load-bearing claim to source and all hold; findings below are Low/nit only.

Claims verified against code (not the description)

• No credential leak upstream — Proxy-Authorization is Del'd on the cloned outbound request (egress_proxy.go:130) while identityFromRequest reads the incoming request first (:114). CONNECT never forwards headers — it's a raw io.Copy byte relay, TLS negotiated inside the tunnel. ✓
• Graceful degradation — zero identity on absent/malformed/wrong-scheme/non-base64/no-colon (all 8 table-tested); emits empty IDs, byte-identical to pre-#338. ✓
• The integration question (would this be silently empty in production?) — SkillTool.Execute forwards ctx verbatim to executor.Run (skill_tool.go:81), and that ctx is the A2A-request-scoped ctx with WithTaskID(params.ID) set in the handlers. This reuses the exact ctx the in-process enforcer already reads successfully — not new plumbing that could silently be empty. ✓
• Good defensive choices: url.UserPassword + u.String() (percent-encodes, so odd IDs can't break the env-var URL); spoofing-within-trust-boundary correctly framed as a known correlation (not authz) limitation.

Findings (all Low / nit)

1. [Low] Colon in a task_id mis-splits attribution — see inline on identityFromRequest.
2. [Nit] CONNECT identity path isn't end-to-end tested — see inline.
3. [Info] IDs now travel in HTTP_PROXY userinfo (readable via /proc/self/environ by the subprocess + children). Correlation identifiers, not secrets — noting only for completeness.

CI green across all 6 platforms + lint + integration. Nice work.

[initializ-mk]

[Low] Colon in a task ID mis-splits attribution. This strings.Cut runs on the flattened Basic credential, and Basic auth has no internal escaping. So even though url.UserPassword percent-encodes the URL correctly, the credential is base64(taskID + ":" + corrID) — a taskID of "a:b" produces base64("a:b:corr"), and Cut at the first : yields task="a", corr="b:corr".

params.id is client-supplied, so a task ID containing : corrupts its own correlation. Impact is limited to self-inflicted attribution noise (and the deliberate-spoofing case is already documented), so this is Low — but worth either a one-line note that IDs must be colon-free, or a fixed-shape encoding that can't collide with the separator.

[initializ-mk]

[Nit] CONNECT identity attribution isn't end-to-end tested. handleConnect calls the same identityFromRequest, but the new TestEgressProxyAttributesIdentity only drives the plain-HTTP path — the PR body's "CONNECT exercised via… relay" covers the tunnel, not identity surfacing through it. Risk is minimal since it's the identical function call, but given the recurring lesson (a handler passing in isolation while the real wired path differs), a CONNECT-tunnel identity assertion would close the loop cheaply.

[initializ-mk]

Thanks for the thorough trace — addressed in 59f7736:

Finding #1 (colon in task ID mis-splits) — fixed. Went with the fixed-shape encoding rather than a doc caveat, since params.id is client-supplied and colons are realistic (urn:task:1, namespace:id). Each ID is now base64url-encoded (RawURLEncoding, alphabet [A-Za-z0-9-_]) before it goes into the HTTP_PROXY userinfo; identityFromRequest decodes each half after the Cut. base64url output can't contain the : separator, so a colon in either ID round-trips verbatim. Covered by a new colon-bearing task id round-trips case plus an inner halves not base64url case (exercises the new decode-failure → empty-identity branch).

Finding #2 (CONNECT identity not end-to-end tested) — fixed. Added TestEgressProxyAttributesIdentityCONNECT: drives a real HTTPS CONNECT tunnel through the proxy and asserts TaskID/CorrelationID surface via handleConnect. It uses a colon-bearing task ID, so it also proves the separator-safe encoding through the tunnel path.

Finding #3 (IDs in /proc/self/environ) — acknowledged. These are correlation identifiers, not secrets; the encoding is for separator-safety, not confidentiality. Noted explicitly in the proxyURLWithIdentity doc comment.

forge-core/security green under -race (incl. both new end-to-end tests); forge-cli/tools green; lint clean on both.

[initializ-mk]

Re-review — fix commit 59f7736 ✅

Both findings from my prior review are resolved cleanly; nothing new introduced.

[Low] Colon in a task ID mis-splits attribution — fixed correctly

The IDs are now base64url-encoded (base64.RawURLEncoding) before entering the userinfo, and decoded on the proxy side (identityFromRequest). This is the right fix and it's structural, not cosmetic: base64url's alphabet is [A-Za-z0-9-_], so an encoded half can never contain the : separator — the Basic-credential ambiguity is eliminated rather than discouraged. Edge cases I checked all hold:

• Empty half (task only / empty task): RawURLEncoding encodes ""→"" and decodes back with no error, so the empty ID round-trips.
• Malformed inner (!!!:@@@): decode fails → zero identity → fail-closed, matching the graceful-degradation contract. New inner halves not base64url case pins it.
• Encoder/decoder are exact inverses: on the wire base64(b64url(task) ":" b64url(corr)) ↔ split-then-RawURLEncoding.Decode at the proxy.

[Nit] CONNECT identity not end-to-end tested — fixed

New TestEgressProxyAttributesIdentityCONNECT drives a real httptest.NewTLSServer through an actual CONNECT tunnel and asserts the ID surfaces on OnAttempt — and it deliberately uses a colon-bearing task ID (urn:task:9), so it exercises the separator-safe encoding through the real HTTPS path in the same test. Exactly the loop-closer I asked for.

The comments on both sides now accurately describe the two-layer scheme and correctly note it's separator-safety, not confidentiality (these are correlation IDs, not secrets) — no misleading claims left behind.

Lint green; Test suite running at time of posting (test-only + a localized encode/decode swap, prior commit was green across all 6 platforms). Approve — ready to merge once Test lands.