fix(rpc): verify gRPC client UUID belongs to the agent secret owner

Auth.Check resolved client_uuid → server.ID via UUIDToID without
checking that the resolved server's UserID matched the user that the
secret was bound to. An agent presenting one user's secret could
target a different user's server UUID and impersonate it — poisoning
monitoring state, triggering alerts, or quietly receiving tasks the
real owner expected.

Add authorizeAgentForUUID: if the UUID is unknown we still allow new
registration bound to the secret owner; if the UUID is known but
points to someone else's server we reject. The rejection message
"client UUID does not belong to the agent secret owner" also helps
operators trace which user's secret has leaked.

Note: this changes runtime behaviour after batch-move/server — the
new owner must reconfigure agents with their own secret. That is the
correct contract; the previous behaviour was a cross-user reporting
bug.

Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
This commit is contained in:
naiba
2026-05-18 15:17:37 +00:00
co-authored by naiba/CloudCode
parent 280e34977f
commit 7c493e8f0b
2 changed files with 123 additions and 1 deletions
+34 -1
View File
@@ -2,6 +2,7 @@ package rpc
import (
"context"
"fmt"
"strings"
petname "github.com/dustinkirkland/golang-petname"
@@ -56,7 +57,10 @@ func (a *authHandler) Check(ctx context.Context) (uint64, error) {
return 0, status.Error(codes.Unauthenticated, "客户端 UUID 不合法")
}
clientID, hasID := singleton.ServerShared.UUIDToID(clientUUID)
clientID, hasID, err := authorizeAgentForUUID(userId, clientUUID)
if err != nil {
return 0, status.Error(codes.Unauthenticated, err.Error())
}
if !hasID {
s := model.Server{UUID: clientUUID, Name: petname.Generate(2, "-"), Common: model.Common{
UserID: userId,
@@ -73,3 +77,32 @@ func (a *authHandler) Check(ctx context.Context) (uint64, error) {
return clientID, nil
}
// authorizeAgentForUUID resolves a client UUID to the dashboard's internal
// server ID, ensuring the resolved server is actually owned by the agent
// secret's owner. Previously Check returned the resolved server ID without
// verifying ownership, allowing an agent that knew another user's server
// UUID to impersonate it (poisoning monitoring state, triggering alerts).
// hasID=false means the UUID is unknown and the caller may register it as
// a new server for the secret owner.
//
// The error path also doubles as a leak-detection signal for operators: if
// an agent persistently fails with "client UUID does not belong to the
// agent secret owner", it pins down which user's secret has been reused
// against a server they don't own.
func authorizeAgentForUUID(userId uint64, clientUUID string) (clientID uint64, hasID bool, err error) {
cid, found := singleton.ServerShared.UUIDToID(clientUUID)
if !found {
return 0, false, nil
}
server, _ := singleton.ServerShared.Get(cid)
if server == nil {
// Cache inconsistency: UUID maps to an ID, but no server record exists.
// Treat as unknown (registration path) rather than impersonation.
return 0, false, nil
}
if server.UserID != userId {
return 0, false, fmt.Errorf("client UUID does not belong to the agent secret owner")
}
return cid, true, nil
}
+89
View File
@@ -0,0 +1,89 @@
package rpc
import (
"testing"
"gorm.io/driver/sqlite"
"gorm.io/gorm"
"github.com/nezhahq/nezha/model"
"github.com/nezhahq/nezha/service/singleton"
)
// setupAuthAgentFixture seeds an in-memory DB and ServerShared with two
// servers belonging to different users so we can assert that a secret bound
// to user A cannot resolve a server UUID owned by user B.
func setupAuthAgentFixture(t *testing.T) func() {
t.Helper()
originalDB := singleton.DB
originalServerShared := singleton.ServerShared
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
if err != nil {
t.Fatalf("open db: %v", err)
}
if err := db.AutoMigrate(&model.Server{}); err != nil {
t.Fatalf("migrate: %v", err)
}
if err := db.Create(&model.Server{
Common: model.Common{ID: 1, UserID: 100},
UUID: "uuid-alice",
Name: "alice-srv",
}).Error; err != nil {
t.Fatalf("create alice: %v", err)
}
if err := db.Create(&model.Server{
Common: model.Common{ID: 2, UserID: 200},
UUID: "uuid-bob",
Name: "bob-srv",
}).Error; err != nil {
t.Fatalf("create bob: %v", err)
}
singleton.DB = db
singleton.ServerShared = singleton.NewServerClass()
return func() {
singleton.DB = originalDB
singleton.ServerShared = originalServerShared
}
}
func TestAuthorizeAgentForUUIDAcceptsOwnedServer(t *testing.T) {
defer setupAuthAgentFixture(t)()
cid, hasID, err := authorizeAgentForUUID(100, "uuid-alice")
if err != nil {
t.Fatalf("alice with her own server UUID must not error, got %v", err)
}
if !hasID || cid != 1 {
t.Fatalf("expected (cid=1, hasID=true), got (cid=%d, hasID=%v)", cid, hasID)
}
}
// Core regression: an agent presenting user A's secret but user B's server
// UUID must be rejected. Previously the code returned the resolved server ID
// without verifying the UserID matched the secret owner, allowing same-tenant
// (and worse — cross-tenant if UUID leaks) server impersonation.
func TestAuthorizeAgentForUUIDRejectsForeignServerUUID(t *testing.T) {
defer setupAuthAgentFixture(t)()
_, _, err := authorizeAgentForUUID(100, "uuid-bob") // alice's secret + bob's UUID
if err == nil {
t.Fatalf("UUID owned by another user must be rejected")
}
}
// An unknown UUID must NOT be treated as an impersonation attempt — it is
// the normal first-time registration path and the caller (Check) creates a
// new server bound to the secret owner.
func TestAuthorizeAgentForUUIDPermitsUnknownUUIDForRegistration(t *testing.T) {
defer setupAuthAgentFixture(t)()
cid, hasID, err := authorizeAgentForUUID(100, "uuid-never-seen-before")
if err != nil {
t.Fatalf("unknown UUID must be permitted for new registration, got %v", err)
}
if hasID {
t.Fatalf("hasID must be false for unknown UUID, got cid=%d", cid)
}
}