diff --git a/AGENTS.md b/AGENTS.md index 7fc325c6..8f3428ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -275,6 +275,11 @@ Key reminders: `e = e.Str("k", v)`. Forgetting to reassign silently drops the field. - **Tests**: prefer `hscontrol/servertest/` for server-level tests that don't need Docker — faster than full integration tests. +- **View types in read paths**: response serializers must read through + `NodeView`/`UserView`/`PreAuthKeyView` accessors. `AsStruct()` clones the + whole record on every read — it is only for DB-write/merge clones and mutable + working copies, never to build an API response. `grep AsStruct hscontrol/api` + must come back empty. ## Gotchas diff --git a/hscontrol/api/v1/nodes.go b/hscontrol/api/v1/nodes.go index c105388f..dbc1adfc 100644 --- a/hscontrol/api/v1/nodes.go +++ b/hscontrol/api/v1/nodes.go @@ -223,7 +223,7 @@ func registerNodeReadOps(api huma.API, b Backend) { // Tags-as-identity: tagged nodes are presented as the special // TaggedDevices user. if node.IsTagged() { - user := userFromState(&types.TaggedDevices) + user := userFromView(types.TaggedDevices.View()) n.User = &user } @@ -583,74 +583,73 @@ func registerNodeAdminOps(api huma.API, b Backend) { }) } -// nodeFromView builds the Node response from a NodeView. SubnetRoutes is left -// empty; callers that serve routes set it explicitly. +// nodeFromView builds the Node response from a NodeView, reading through the +// view accessors. SubnetRoutes is left empty; callers that serve routes set it +// explicitly. func nodeFromView(view types.NodeView) Node { - node := view.AsStruct() - n := Node{ - ID: formatID(uint64(node.ID)), - MachineKey: node.MachineKey.String(), - NodeKey: node.NodeKey.String(), - DiscoKey: node.DiscoKey.String(), - IPAddresses: nonNilStrings(node.IPsAsString()), - Name: node.Hostname, - CreatedAt: node.CreatedAt, - RegisterMethod: registerMethodEnum(node.RegisterMethod), - GivenName: node.GivenName, - Online: node.IsOnline != nil && *node.IsOnline, - ApprovedRoutes: nonNilStrings(util.PrefixesToString(node.ApprovedRoutes)), - AvailableRoutes: nonNilStrings(util.PrefixesToString(node.AnnouncedRoutes())), + ID: view.StringID(), + MachineKey: view.MachineKey().String(), + NodeKey: view.NodeKey().String(), + DiscoKey: view.DiscoKey().String(), + IPAddresses: nonNilStrings(view.IPsAsString()), + Name: view.Hostname(), + CreatedAt: view.CreatedAt(), + RegisterMethod: registerMethodEnum(view.RegisterMethod()), + GivenName: view.GivenName(), + Online: view.IsOnline().Valid() && view.IsOnline().Get(), + ApprovedRoutes: nonNilStrings(util.PrefixesToString(view.ApprovedRoutes().AsSlice())), + AvailableRoutes: nonNilStrings(util.PrefixesToString(view.AnnouncedRoutes())), SubnetRoutes: []string{}, - Tags: nonNilStrings(node.Tags), + Tags: nonNilStrings(view.Tags().AsSlice()), } - if node.User != nil { - user := userFromState(node.User) + if view.User().Valid() { + user := userFromView(view.User()) n.User = &user } - if node.AuthKey != nil { - n.PreAuthKey = nodePreAuthKeyFromState(node.AuthKey) + if view.AuthKey().Valid() { + n.PreAuthKey = nodePreAuthKeyFromView(view.AuthKey()) } - if node.LastSeen != nil { - ls := *node.LastSeen + if view.LastSeen().Valid() { + ls := view.LastSeen().Get() n.LastSeen = &ls } - if node.Expiry != nil { - exp := *node.Expiry + if view.Expiry().Valid() { + exp := view.Expiry().Get() n.Expiry = &exp } return n } -// nodePreAuthKeyFromState builds the embedded NodePreAuthKey, masking the key to +// nodePreAuthKeyFromView builds the embedded NodePreAuthKey, masking the key to // its prefix (legacy plaintext keys are shown in full). -func nodePreAuthKeyFromState(key *types.PreAuthKey) *NodePreAuthKey { +func nodePreAuthKeyFromView(key types.PreAuthKeyView) *NodePreAuthKey { pak := &NodePreAuthKey{ - ID: formatID(key.ID), + ID: formatID(key.ID()), Key: maskedPreAuthKey(key), - Reusable: key.Reusable, - Ephemeral: key.Ephemeral, - Used: key.Used, - AclTags: nonNilStrings(key.Tags), + Reusable: key.Reusable(), + Ephemeral: key.Ephemeral(), + Used: key.Used(), + AclTags: nonNilStrings(key.Tags().AsSlice()), } - if key.User != nil { - user := userFromState(key.User) + if key.User().Valid() { + user := userFromView(key.User()) pak.User = &user } - if key.Expiration != nil { - exp := *key.Expiration + if key.Expiration().Valid() { + exp := key.Expiration().Get() pak.Expiration = &exp } - if key.CreatedAt != nil { - created := *key.CreatedAt + if key.CreatedAt().Valid() { + created := key.CreatedAt().Get() pak.CreatedAt = &created } diff --git a/hscontrol/api/v1/preauthkeys.go b/hscontrol/api/v1/preauthkeys.go index 6658262e..9afb0aa2 100644 --- a/hscontrol/api/v1/preauthkeys.go +++ b/hscontrol/api/v1/preauthkeys.go @@ -223,7 +223,7 @@ func preAuthKeyNewToResponse(key *types.PreAuthKeyNew) PreAuthKey { } if key.User != nil { - u := userFromState(key.User) + u := userFromView(key.User.View()) out.User = &u } @@ -243,7 +243,7 @@ func preAuthKeyNewToResponse(key *types.PreAuthKeyNew) PreAuthKey { func preAuthKeyToResponse(key *types.PreAuthKey) PreAuthKey { out := PreAuthKey{ ID: formatID(key.ID), - Key: maskedPreAuthKey(key), + Key: maskedPreAuthKey(key.View()), Reusable: key.Reusable, Ephemeral: key.Ephemeral, Used: key.Used, @@ -251,7 +251,7 @@ func preAuthKeyToResponse(key *types.PreAuthKey) PreAuthKey { } if key.User != nil { - u := userFromState(key.User) + u := userFromView(key.User.View()) out.User = &u } @@ -269,12 +269,12 @@ func preAuthKeyToResponse(key *types.PreAuthKey) PreAuthKey { // maskedPreAuthKey masks new keys (those with a stored prefix) so the secret is // never returned; legacy plaintext keys are returned in full for backwards // compatibility. -func maskedPreAuthKey(key *types.PreAuthKey) string { - if key.Prefix != "" { - return "hskey-auth-" + key.Prefix + "-***" +func maskedPreAuthKey(key types.PreAuthKeyView) string { + if key.Prefix() != "" { + return "hskey-auth-" + key.Prefix() + "-***" } - return key.Key + return key.Key() } // nonNilTags ensures aclTags serializes as [] rather than null, matching diff --git a/hscontrol/api/v1/types.go b/hscontrol/api/v1/types.go index 91943b77..e8ffbd87 100644 --- a/hscontrol/api/v1/types.go +++ b/hscontrol/api/v1/types.go @@ -29,23 +29,24 @@ type User struct { ProfilePicURL string `json:"profilePicUrl"` } -// userFromState converts a domain user into the v1 response shape: Name falls -// back to Username() (email/provider/id) when the stored Name is empty, so OIDC -// users display their email. -func userFromState(u *types.User) User { - name := u.Name +// userFromView converts a domain user into the v1 response shape, reading +// through the [types.UserView] accessors: Name falls back to Username() +// (email/provider/id) when the stored Name is empty, so OIDC users display +// their email. +func userFromView(u types.UserView) User { + name := u.Name() if name == "" { name = u.Username() } return User{ - ID: formatID(u.ID), + ID: formatID(u.ID()), Name: name, - CreatedAt: u.CreatedAt, - DisplayName: u.DisplayName, - Email: u.Email, - ProviderID: u.ProviderIdentifier.String, - Provider: u.Provider, - ProfilePicURL: u.ProfilePicURL, + CreatedAt: u.CreatedAt(), + DisplayName: u.DisplayName(), + Email: u.Email(), + ProviderID: u.ProviderIdentifier().String, + Provider: u.Provider(), + ProfilePicURL: u.ProfilePicURL(), } } diff --git a/hscontrol/api/v1/users.go b/hscontrol/api/v1/users.go index db8a0c5a..9468d67a 100644 --- a/hscontrol/api/v1/users.go +++ b/hscontrol/api/v1/users.go @@ -93,7 +93,7 @@ func registerUsers(api huma.API, b Backend) { b.Change(policyChanged) out := &userOutput{} - out.Body.User = userFromState(user) + out.Body.User = userFromView(user.View()) return out, nil }) @@ -129,7 +129,7 @@ func registerUsers(api huma.API, b Backend) { } out := &userOutput{} - out.Body.User = userFromState(newUser) + out.Body.User = userFromView(newUser.View()) return out, nil }) @@ -192,7 +192,7 @@ func registerUsers(api huma.API, b Backend) { out.Body.Users = make([]User, len(users)) for i := range users { - out.Body.Users[i] = userFromState(&users[i]) + out.Body.Users[i] = userFromView(users[i].View()) } return out, nil