Add TestTagsAuthKeyConvertToUserViaCLIRegister that reproduces the
exact panic from #3038: register a node with a tags-only PreAuthKey
(no user), force reauth with empty tags, then register via CLI with
a user. The mapper panics on node.Owner().Model().ID when User is nil.
The critical detail is using a tags-only PreAuthKey (User: nil). When
the key is created under a user, the node inherits the User pointer
from createAndSaveNewNode and the bug is masked.
Also add Owner() validity assertions to the existing unit test
TestTaggedNodeWithoutUserToDifferentUser to catch the nil pointer
at the unit test level.
Updates #3038
processReauthTags sets UserID when converting a tagged node to
user-owned, but does not set the User pointer. When the node was
registered with a tags-only PreAuthKey (User: nil), the in-memory
NodeStore cache holds a node with User=nil. The mapper's
generateUserProfiles then calls node.Owner().Model().ID, which
dereferences the nil pointer and panics.
Set node.User alongside node.UserID in processReauthTags. Also add
defensive nil checks in generateUserProfiles to gracefully handle
nodes with invalid owners rather than panicking.
Fixes#3038
These were thin wrappers around applyAuthNodeUpdate that only added
logging. Move the logging into applyAuthNodeUpdate and call it directly
from HandleNodeFromAuthPath.
This simplifies the code structure without changing behavior.
Updates #3038
Move tag validation before the UpdateNode callback in applyAuthNodeUpdate.
Previously, tag validation happened inside the callback, and the error
check occurred after UpdateNode had already committed changes to the
NodeStore. This left the NodeStore in an inconsistent state when tags
were rejected.
Now validation happens first, and UpdateNode is only called when we know
the operation will succeed. This follows the principle that UpdateNode
should only be called when we have all information and are ready to commit.
Also extract validateRequestTags as a reusable function and use it in
createAndSaveNewNode to deduplicate the tag validation logic.
Updates #3038
Updates #3048
Previously, expiry handling ran BEFORE processReauthTags(), using the
old tagged status to determine whether to set/clear expiry. This caused:
- Personal → Tagged: Expiry remained set (should be cleared to nil)
- Tagged → Personal: Expiry remained nil (should be set from client)
Move expiry handling after tag processing and handle all four transition
cases based on the new tagged status:
- Tagged → Personal: Set expiry from client request
- Personal → Tagged: Clear expiry (tagged nodes don't expire)
- Personal → Personal: Update expiry from client
- Tagged → Tagged: Keep existing nil expiry
Fixes#3048
Reorganize HandleNodeFromAuthPath (~300 lines) into a cleaner structure
with named conditions and extracted helper functions.
Changes:
- Add authNodeUpdateParams struct for shared update logic
- Extract applyAuthNodeUpdate for common reauth/convert operations
- Extract reauthExistingNode and convertTaggedNodeToUser handlers
- Extract createNewNodeFromAuth for new node creation
- Use named boolean conditions (nodeExistsForSameUser, existingNodeIsTagged,
existingNodeOwnedByOtherUser) instead of compound if conditions
- Create logger with common fields (registration_id, user.name, machine.key,
method) to reduce log statement verbosity
Updates #3038
When a node was registered with a tags-only PreAuthKey (no user
associated), the node had User=nil and UserID=nil. When attempting to
re-register this node to a different user via HandleNodeFromAuthPath,
two issues occurred:
1. The code called oldUser.Name() without checking if oldUser was valid,
causing a nil pointer dereference panic.
2. The existing node lookup logic didn't find the tagged node because it
searched by (machineKey, userID), but tagged nodes have no userID.
This caused a new node to be created instead of updating the existing
tagged node.
Fix this by restructuring HandleNodeFromAuthPath to:
1. First check if a node exists for the same user (existing behavior)
2. If not found, check if an existing TAGGED node exists with the same
machine key (regardless of userID)
3. If a tagged node exists, UPDATE it to convert from tagged to
user-owned (preserving the node ID)
4. Only create a new node if the existing node is user-owned by a
different user
This ensures consistent behavior between:
- personal → tagged → personal (same node, same owner)
- tagged (no user) → personal (same node, new owner)
Add a test that reproduces the panic and conversion scenario by:
1. Creating a tags-only PreAuthKey (no user)
2. Registering a node with that key
3. Re-registering the same machine to a different user
4. Verifying the node ID stays the same (conversion, not creation)
Fixes#3038
When a PreAuthKey is deleted, the database correctly sets auth_key_id
to NULL on referencing nodes via ON DELETE SET NULL. However, the
NodeStore (in-memory cache) retains the old AuthKeyID value.
When nodes send MapRequests (e.g., after tailscaled restart), GORM's
Updates() tries to persist the stale AuthKeyID, causing a foreign key
constraint error when trying to reference a deleted PreAuthKey.
Fix this by adding AuthKeyID and AuthKey to the Omit() call in all
three places where nodes are updated via GORM's Updates():
- persistNodeToDB (MapRequest processing)
- HandleNodeFromAuthPath (re-auth via web/OIDC)
- HandleNodeFromPreAuthKey (re-registration with preauth key)
This tells GORM to never touch the auth_key_id column or AuthKey
association during node updates, letting the database handle the
foreign key relationship correctly.
Added TestDeletedPreAuthKeyNotRecreatedOnNodeUpdate to verify that
deleted PreAuthKeys are not recreated when nodes send MapRequests.
Update integration tests to use valid SSH patterns:
- TestSSHOneUserToAll: use autogroup:member and autogroup:tagged
instead of wildcard destination
- TestSSHMultipleUsersAllToAll: use autogroup:self instead of
username destinations for group-to-user SSH access
Updates #3009
Updates #3010
Update unit tests to use valid SSH patterns that conform to Tailscale's
security model:
- Change group->user destinations to group->tag
- Change tag->user destinations to tag->tag
- Update expected error messages for new validation format
- Add proper tagged/untagged node setup in filter tests
Updates #3009
Updates #3010
Add validation for SSH source/destination combinations that enforces
Tailscale's security model:
- Tags/autogroup:tagged cannot SSH to user-owned devices
- autogroup:self destination requires source to contain only users/groups
- Username destinations require source to be that same single user only
- Wildcard (*) is no longer supported as SSH destination; use
autogroup:member or autogroup:tagged instead
The validateSSHSrcDstCombination() function is called during policy
validation to reject invalid configurations at load time.
Fixes#3009Fixes#3010
Refactor the RequestTags migration (202601121700-migrate-hostinfo-request-tags)
to use PolicyManager.NodeCanHaveTag() instead of reimplementing tag validation.
Changes:
- NewHeadscaleDatabase now accepts *types.Config to allow migrations
access to policy configuration
- Add loadPolicyBytes helper to load policy from file or DB based on config
- Add standalone GetPolicy(tx *gorm.DB) for use during migrations
- Replace custom tag validation logic with PolicyManager
Benefits:
- Full HuJSON parsing support (not just JSON)
- Proper group expansion via PolicyManager
- Support for nested tags and autogroups
- Works with both file and database policy modes
- Single source of truth for tag validation
Co-Authored-By: Shourya Gautam <shouryamgautam@gmail.com>
When autogroup:self was combined with other ACL rules (e.g., group:admin
-> *:*), tagged nodes became invisible to users who should have access.
The BuildPeerMap function had two code paths:
- Global filter path: used symmetric OR logic (if either can access, both
see each other)
- Autogroup:self path: used asymmetric logic (only add peer if that
specific direction has access)
This caused problems with one-way rules like admin -> tagged-server. The
admin could access the server, but since the server couldn't access the
admin, neither was added to the other's peer list.
Fix by using symmetric visibility in the autogroup:self path, matching
the global filter path behavior: if either node can access the other,
both should see each other as peers.
Credit: vdovhanych <vdovhanych@users.noreply.github.com>
Fixes#2990
Extend TestApiKeyCommand to test the new --id flag for expire and
delete commands, verifying that API keys can be managed by their
database ID in addition to the existing --prefix method.
Updates #2986
Add --id flag as an alternative to --prefix for expiring and
deleting API keys. This allows users to use the ID shown in
'headscale apikeys list' output, which is more convenient than
the prefix.
Either --id or --prefix must be provided; both flags are optional
but at least one is required.
Updates #2986
Update ExpireApiKey and DeleteApiKey handlers to accept either ID or
prefix for identifying the API key. Returns InvalidArgument error if
neither or both are provided.
Add tests for:
- Expire by ID
- Expire by prefix (backwards compatibility)
- Delete by ID
- Delete by prefix (backwards compatibility)
- Error when neither ID nor prefix provided
- Error when both ID and prefix provided
Updates #2986
Add id field to ExpireApiKeyRequest and DeleteApiKeyRequest messages.
This allows API keys to be expired or deleted by their database ID
in addition to the existing prefix-based lookup.
Updates #2986
Add GetAPIKeyByID method to the state layer, delegating to the existing
database layer function. This enables API key lookup by ID in addition
to the existing prefix-based lookup.
Updates #2986
Convert config loading tests from gopkg.in/check.v1 Suite-based testing
to standard Go tests with testify assert/require.
Changes:
- Remove Suite boilerplate (Test, Suite type, SetUpSuite, TearDownSuite)
- Convert TestConfigFileLoading and TestConfigLoading to standalone tests
- Replace check assertions with testify equivalents
Migrate all database tests from gopkg.in/check.v1 Suite-based testing
to standard Go tests with testify assert/require.
Changes:
- Remove empty Suite files (hscontrol/suite_test.go, hscontrol/mapper/suite_test.go)
- Convert hscontrol/db/suite_test.go to modern helpers only
- Convert 6 Suite test methods in node_test.go to standalone tests
- Convert 5 Suite test methods in api_key_test.go to standalone tests
- Fix stale global variable reference in db_test.go
The legacy TestListPeers Suite method was renamed to TestListPeersManyNodes
to avoid conflict with the existing modern TestListPeers function, as they
test different aspects (basic peer listing vs ID filtering).
Make DeleteUser call updatePolicyManagerUsers() to refresh the policy
manager's cached user list after user deletion. This ensures consistency
with CreateUser, UpdateUser, and RenameUser which all update the policy
manager.
Previously, DeleteUser only removed the user from the database without
updating the policy manager. This could leave stale user references in
the cached user list, potentially causing issues when policy is
re-evaluated.
The gRPC handler now uses the change returned from DeleteUser instead of
manually constructing change.UserRemoved().
Fixes#2967
Add DeleteUser method to ControlServer interface and implement it in
HeadscaleInContainer to enable testing user deletion scenarios.
Add two integration tests for issue #2967:
- TestACLGroupWithUnknownUser: tests that valid users can communicate
when a group references a non-existent user
- TestACLGroupAfterUserDeletion: tests connectivity after deleting a
user that was referenced in an ACL group
These tests currently pass but don't fully reproduce the reported issue
where deleted users break connectivity for the entire group.
Updates #2967
Replace the Tags column with an Owner column that displays:
- Tags (newline-separated) if the key has ACL tags
- User name if the key is associated with a user
- Dash (-) if neither is present
This aligns the CLI output with the tags-as-identity model where
preauthkeys can be created with either tags or user ownership.
- Rename TestTagsAuthKeyWithoutUserIgnoresAdvertisedTags to
TestTagsAuthKeyWithoutUserRejectsAdvertisedTags to reflect actual
behavior (PreAuthKey registrations reject advertised tags)
- Fix TestTagsAuthKeyWithoutUserInheritsTags to use ListNodes() without
user filter since tags-only nodes don't have a user association
Updates #2977
HandleNodeFromPreAuthKey assumed pak.User was always set, but
tags-only PreAuthKeys have nil User. This caused nil pointer
dereference when registering nodes with tags-only keys.
Also updates integration tests to use GetTags() instead of the
removed GetValidTags() method.
Updates #2977
When SetNodeTags changed a node's tags, the node's self view wasn't
updated. The bug manifested as: the first SetNodeTags call updates
the server but the client's self view doesn't update until a second
call with the same tag.
Root cause: Three issues combined to prevent self-updates:
1. SetNodeTags returned PolicyChange which doesn't set OriginNode,
so the mapper's self-update check failed.
2. The Change.Merge function didn't preserve OriginNode, so when
changes were batched together, OriginNode was lost.
3. generateMapResponse checked OriginNode only in buildFromChange(),
but PolicyChange uses RequiresRuntimePeerComputation which
bypasses that code path entirely and calls policyChangeResponse()
instead.
The fix addresses all three:
- state.go: Set OriginNode on the returned change
- change.go: Preserve OriginNode (and TargetNode) during merge
- batcher.go: Pass isSelfUpdate to policyChangeResponse so the
origin node gets both self info AND packet filters
- mapper.go: Add includeSelf parameter to policyChangeResponse
Fixes#2978
