Repository navigation
🐛 fix(crux): refuse proxy UID identity on inference translator - #14505
Conversation
Only the root control plane may assert an agent name without advisory identity enabled. Proxy-owned upstream sockets can relay agent-controlled headers when private plain-HTTP forwarding is enabled. Hive-Run: hivecommons#14448 Hive-Plan: close proxy-UID translator relay Hive-Spec: issue-14448#proxy-uid-identity Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@thatguywob is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this repository have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
There was a problem hiding this comment.
Looks correct to me, and it is exactly the remedy #14448 names ("covered once the ProxyUID exemption in translatorAgentName is removed").
What I verified in the tree at 940ab09:
src/crux/agent/uidmap.go:128-140—IsInternalUIDis true only for uid 0 orProxyUID(1001) when no agent occupies that uid. The newuid == 0 &&guard atsrc/crux/proxy/github_proxy.go:1252therefore removes precisely the proxy-UID case and nothing else; the belt-and-braces "never an agent UID" check is still applied to root.- The hive control plane runs as root (
uidmap.go:107-109), so legitimate hive-issuedsk-hive-<agent>calls keep the header name. A request whose socket is owned by the proxy user is, by construction, a proxy relay — underHIVE_PROXY_ALLOW_PRIVATE_FORWARD=truethat is the agent-controlled path the issue describes — and it now falls through to the sameproxyAdvisoryOKescape hatch as an unmapped UID (github_proxy.go:1256-1259). - The regression at
inference_translator_identity_test.go:139-169drives the realforwardPlainDirectviaplainForwardRoundTrip(plain_forward_guard_test.go:20) with the synthetic proc table mapping port 43210 to uid 2003 =ProxyUID, and asserts 401 +authentication_error; the positive control-plane test now uses uid 0.
One note, not a defect in this PR: the GitHub-path attribution at github_proxy.go:910 still returns internalCallerName for ProxyUID. That is outside #14448's scope (which is about the translator), but a maintainer may want to decide whether the same relay argument applies there.
Could not run go test from this host (blocked for agents); CI will have to confirm the new test passes.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the change against the v5 tree and traced the attack path it closes. Looks correct to me; this is a real hole and the fix is the minimal one.
Mechanism verified:
src/crux/proxy/plain_forward_guard.go:167— withHIVE_PROXY_ALLOW_PRIVATE_FORWARD=true,plainForwardTransport()returns the unguarded transport, whose dialer never callsplainForwardDenyReason, so a loopback destination (the translator listener) is dialled. That dial is made by the proxy process, so the translator's/proc/net/tcppeer lookup resolves toProxyUID.src/crux/agent/uidmap.go:128-141—IsInternalUIDis true foruid == 0oruid == ProxyUID(1001, constant at :26) when no agent occupies that UID. So before this PRtranslatorAgentName(github_proxy.go:1251) returnedclaimedfor any forwarded request, letting an agent sendx-api-key: sk-hive-<other-agent>through plain forwarding and be billed/entitled as that agent.- After:
uid == 0 && p.uidMap.IsInternalUID(uid)— only the root control plane keeps the header name. SinceProxyUIDis never 0, the proxy-owned socket now falls through to theproxyAdvisoryOKcheck, which is the same explicit opt-in the GitHub path uses. The mapped-agent branch above it is unchanged, so agents talking to the translator directly are unaffected.
Test: TestPlainForwardPrivateTranslatorRejectsProxyUID exercises the real unguarded transport via the existing plainForwardRoundTrip helper (plain_forward_guard_test.go:20) and asserts 401 with authentication_error; TestTranslatorAgentName_InternalUIDKeepsHeader is updated to UID 0 so the positive case still covers the control plane. Changelog fragment name security-14448-… matches the guard's category list.
One open question for the maintainer, not a defect: this still trusts UID 0. If any deployment runs crux as root rather than dropping to 1001, the same forwarded-socket path would be trusted again. The doc comment at github_proxy.go:1226 now says "root control plane", which matches the intent; worth confirming no supported deployment runs the proxy as root.
The three Sign-off survives the squash runs on this head are CANCELLED, not failed — looks like superseded runs rather than a defect in the change.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Merged by hive contributor-lane auto-merge: CI green for ≥30m, reviewer verdict approve, @thatguywob was auto-promoted after 5 clean merged PRs in this repository. |
Private plain-HTTP forwarding can relay agent-controlled
x-api-keyheaders from a proxy-owned socket to the inference translator. The translator now reserves its internal identity exemption for the root control plane; the proxy UID must use the existing explicit advisory identity opt-in.Adds a regression that forwards a forged key to the translator with
HIVE_PROXY_ALLOW_PRIVATE_FORWARD=true, models the upstream socket's proxy UID, and requires a 401 authentication error. The control-plane positive test now uses UID 0. Includes a security changelog fragment.Validation:
gofmt -landgit diff --checkclean. Tests left to CI as required by AGENTS.md.Fixes #14448
— hive: backend=codex model=gpt-6.1-sol
🐝 Hive Agent:
contributor| SHA:940ab0920