Don't override a hub-managed agent's real token with dev auth on localhost - #1239
Don't override a hub-managed agent's real token with dev auth on localhost#1239dea-rour wants to merge 1 commit into
Conversation
…lhost
createHubClient() (and the duplicated logic in cmd/hub.go's getHubClient,
plus the display-only getAuthInfo) prefers dev auth over a non-dev
scion-token whenever the Hub endpoint is localhost, to avoid a stale
agent token left over from a previous *remote* hub connection on a
developer's machine.
That exception fired unconditionally, including inside a container the
Runtime Broker itself started (workstation mode always runs the Hub on
localhost). There the scion-token is freshly minted for this exact Hub,
not stale, but got silently swapped for dev auth anyway. Dev auth
resolves to a generic/dev identity rather than any specific agent, so any
Hub endpoint requiring the caller to *be* a particular agent — e.g.
POST /api/v1/agents/{id}/outbound-message, used by `scion message
user:<x>` for an agent to reach a human's inbox — 401s with "Agents can
only send outbound messages as themselves" turning into the generic
"authentication failed, login to hub with 'scion hub auth login'".
Net effect: no hub-managed agent running against a workstation-mode
(loopback) Hub could message a user directly via `scion message`, at
all, ever — a total feature gap for any agent whose harness doesn't have
its own out-of-band relay to the message broker.
Fix: gate the localhost dev-auth-preference exception behind
`!config.IsHubManagedAgent()` (already-existing helper, keys off
SCION_AGENT_ID, which the Runtime Broker sets when it starts an agent
container) in both createHubClient and getHubClient, and apply the same
condition to getAuthInfo so `scion hub auth status` reports accurately
inside an agent container. The original developer-workstation scenario
this exception was written for is untouched — it only ever applied when
SCION_AGENT_ID isn't set.
Added regression tests: TestGetAuthInfo_HubManagedAgentUsesRealTokenOnLocalhost
and TestCreateHubClient_HubManagedAgentUsesRealTokenOnLocalhost, plus fixed
TestGetAuthInfo_DevAuthPreferredOverStaleAgentTokenOnLocalhost to explicitly
clear SCION_AGENT_ID (a latent hermeticity gap the fix exposed: the test
suite happened to always run outside any agent context before, so nothing
noticed SCION_AGENT_ID wasn't cleared like the other env vars).
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request ensures that hub-managed agents (where SCION_AGENT_ID is set) use their freshly minted scion-token when communicating with a localhost Hub, rather than being incorrectly overridden by dev auth. This resolves a bug where agents on workstation-mode Hubs received 401 Unauthorized errors on self-only endpoints. The feedback recommends handling errors explicitly with t.Fatal(err) instead of ignoring them with _ = in the new test setup, and resolving or removing the <TBD> placeholder issue URL in the code comments.
| _ = os.MkdirAll(scionDir, 0700) | ||
| // Real per-agent token, freshly issued by the broker for this Hub. | ||
| _ = os.WriteFile(filepath.Join(scionDir, "scion-token"), []byte("fresh-agent-jwt"), 0600) |
There was a problem hiding this comment.
In test setup, ignoring errors with _ = can lead to silent failures or confusing test errors if the directory cannot be created or the token file cannot be written. It is highly recommended to handle these errors using t.Fatal(err) to fail the test immediately with a clear error message.
| _ = os.MkdirAll(scionDir, 0700) | |
| // Real per-agent token, freshly issued by the broker for this Hub. | |
| _ = os.WriteFile(filepath.Join(scionDir, "scion-token"), []byte("fresh-agent-jwt"), 0600) | |
| if err := os.MkdirAll(scionDir, 0700); err != nil { | |
| t.Fatal(err) | |
| } | |
| // Real per-agent token, freshly issued by the broker for this Hub. | |
| if err := os.WriteFile(filepath.Join(scionDir, "scion-token"), []byte("fresh-agent-jwt"), 0600); err != nil { | |
| t.Fatal(err) | |
| } |
| // minted for *this* Hub, not stale, and some Hub endpoints (e.g. an agent's own outbound | ||
| // message to a user) require the real per-agent identity that only that token carries — | ||
| // dev auth resolves to a superuser/dev identity, not any specific agent, so it 401s on | ||
| // self-only endpoints. See https://github.com/GoogleCloudPlatform/scion/issues/<TBD>. |
There was a problem hiding this comment.
|
Thanks for the contribution - this looks correct, can you click through and sign the CLA? |
Summary
createHubClient()inpkg/hubsync/sync.go(and a duplicated copy,getHubClient(), incmd/hub.go, plus the display-onlygetAuthInfo())prefers dev auth over a non-dev
scion-tokenwhenever the Hub endpoint islocalhost, specifically to avoid a stale agent token left over on a
developer's machine from a previous remote hub connection.
That exception fires unconditionally — including inside a container the
Runtime Broker itself started. Workstation mode (Hub + Broker + Dashboard on
one machine, per the docs) always runs the Hub on
localhost, so everyhub-managed agent in workstation mode hits this branch. Inside that
container the
scion-tokenis freshly minted for this exact Hub, not staleat all — but it gets silently swapped for dev auth anyway.
Dev auth resolves to a generic/dev identity, not a specific agent. Any Hub
endpoint that requires the caller to be a particular agent breaks under
it — concretely,
POST /api/v1/agents/{id}/outbound-message(used byscion message user:<x>, an agent's only way to reach a human's inboxdirectly) 401s with "Agents can only send outbound messages as themselves",
surfaced to the CLI user as the generic
authentication failed, login to hub with 'scion hub auth login'.Net effect: no hub-managed agent running against a workstation-mode Hub
could ever message a user directly via
scion message— a full featuregap for any agent/harness that doesn't have its own out-of-band relay to
the message broker (discovered running an OpenCode-harnessed agent, which
has no such native relay, alongside a Claude Code agent that does).
Fix
Gate the localhost dev-auth-preference exception behind
!config.IsHubManagedAgent()— an existing helper keyed offSCION_AGENT_ID, which the Runtime Broker sets when it starts an agentcontainer — in both
createHubClientandgetHubClient, and apply thesame condition in
getAuthInfososcion hub auth statusreportsaccurately from inside an agent container too.
The original developer-workstation scenario this exception was written for
is untouched: it only ever applied when
SCION_AGENT_IDisn't set, whichis true for any normal CLI invocation outside an agent container.
Test plan
go build ./...— cleango vet ./cmd/... ./pkg/hubsync/... ./pkg/config/...— cleango test ./cmd/... ./pkg/hubsync/...— all passingTestGetAuthInfo_HubManagedAgentUsesRealTokenOnLocalhostandTestCreateHubClient_HubManagedAgentUsesRealTokenOnLocalhost—regression coverage for the fixed case (agent token used, not dev
auth, when
SCION_AGENT_IDis set on a localhost endpoint)TestGetAuthInfo_DevAuthPreferredOverStaleAgentTokenOnLocalhostto explicitly clear
SCION_AGENT_ID— a latent test-hermeticity gapthe fix exposed (the existing suite happened to always run outside
any agent context, so nothing had previously needed this cleared)
container that previously 401'd running
scion message user:<name> --channel telegram "..."from inside itself