fix: gate IPC rotate_key on admin token (PILOT-230)#169
Conversation
handleRotateKey had zero auth — any same-UID process could send CmdRotateKey and swap the daemon's Ed25519 identity. This adds admin-token verification matching the BroadcastDatagram pattern: constant-time compare against the daemon's configured AdminToken. Server side (ipc.go): parse [tokenLen(2)][token...] from payload, deny when AdminToken is empty or token mismatch. Client side (driver.go): encode admin token in wire format. CLI side (main.go): pass token from PILOT_ADMIN_TOKEN / config. Scope: 3 source files (+41 LoC), 3 test files (+17 LoC). Medium tier. Closes PILOT-230
|
🤖 Hank — CI status Classification: The build failure is a genuine code defect. The stress test
(Also reproduced identically on @matthew-pilot — fix or comment. Auto-classified at 2026-05-29T20:22:00Z. Re-runs on next push or check completion. |
🦜 Matthew PR Status — #169 PILOT-230State
CI
3/6 passing — Go tests failed on both Linux and macOS, plus both architecture gates. CanaryNot triggered (no canary label present). Triage⛔ CI red — pr-ci-failed is Wave 2 and not dispatched here. Operator review requested before merge. The change itself is straightforward (admin-token guard on |
🦜 Matthew Explains — #169 PILOT-230What this doesAdds admin-token verification to the daemon IPC Changes per file
Test files (+17/−5 across 3 files) Security properties
Notes
|
🦾 Matthew PR Check — #169 PILOT-230Status
Canary✅ passed —
|
🦾 Matthew Explains — #169 PILOT-230What this doesGates the daemon IPC Changes (6 files, +58/−10)
CI Verdict
TierMedium ( Closes PILOT-230 |
TeoSlayer
left a comment
There was a problem hiding this comment.
🛑 Requesting changes — threat-model mismatch on this endpoint.
Only the network administrators (Teo + team) hold the admin token. Normal Pilot users have no admin_token field in their ~/.pilot/config.json and don't pass --admin-token to pilot-daemon. After this PR lands, the daemon's check at the early return for empty AdminToken fires for every legitimate caller:
if s.daemon.config.AdminToken == "" {
s.sendError(conn, reqID, "<endpoint> denied: daemon has no admin token configured")
return
}→ Users can no longer rotate their own daemon identity on their own daemon.
The bug ticket correctly identifies "same-UID malicious code" (app-store packages, openclaw skills) as the threat. The fix shape needs to differ between:
- User-owned ops (this endpoint) — needs SO_PEERCRED + caller cookie pilotctl shares with the daemon at install, OR per-user local-auth token auto-provisioned at first daemon start (docker.sock pattern, transparent to user). NOT the network admin token.
- Network-admin ops (
managed.policy.set/ PILOT-233) — admin token viasubtle.ConstantTimeCompareis correct. Leave that one as is.
The Jira ticket has been transitioned back to TO DO with full explanation. Design tracker for distinguishing the two classes: PILOT-347.
Happy to land this once the gating is replaced with a mechanism that doesn't lock out legitimate users.
|
Closing per CHANGES_REQUESTED review. The endpoint this PR gated is user-owned (only the network admin holds the admin token; normal users would be locked out). Re-spec'd in PILOT-347 / PILOT-230: when the spec lands, a new PR with the correct mechanism (SO_PEERCRED + caller cookie, or per-user local-auth token) will supersede this one. |
🛑 Matthew Cleanup — PR #169 closed without mergePR: #169 — closed at 2026-05-29T20:45:24Z by TeoSlayer Jira ticket PILOT-230 — reopening for operator review. |
🦾 Matthew Cleanup — #169 PILOT-230Status: CLOSED by @TeoSlayer (admin-token approach rejected per PILOT-347) Action: Branch No further action needed. This PR is superseded by PILOT-347 rework. |
What failed
The daemon IPC
rotate_keyhandler (pkg/daemon/ipc.go:1158) accepted requests with zero auth — any process sharing the daemon's UID could sendCmdRotateKeyand swap the daemon's Ed25519 identity. The admin token gate (used byBroadcastDatagram) was missing from this path.Why this fix
Added admin-token verification to
handleRotateKey, matching the pattern inhandleBroadcast/BroadcastDatagram: parse[tokenLen(2)][token...]from the wire payload, constant-time compare againstConfig.AdminToken. Deny when the token is empty or mismatched.Changes
pkg/daemon/ipc.go—handleRotateKeynow extracts and verifies admin tokenpkg/driver/driver.go—RotateKeyacceptsadminTokenparam, encodes in wire formatcmd/pilotctl/main.go—cmdRotateKeypasses token fromPILOT_ADMIN_TOKEN/ configVerification
go build ./...— greengo vet ./...— clean (pkg/daemon, pkg/driver, cmd/pilotctl)go test -run 'TestHandleRotateKey|TestRotateKey|TestCmdRotateKey' ./pkg/daemon/ ./pkg/driver/ ./cmd/pilotctl/— all greenTier
Medium (
matthew-fix-larger): 6 files, 58 insertions / 10 deletions.Closes PILOT-230