- Add nolint:errchkjson comments to test json.Marshal calls
- Add nolint:contextcheck comments to intentional context patterns
- Add nolint:containedctx comment to mapSession struct (context stored for session lifecycle)
- Add nolint:embeddedstructfieldcheck to User struct (intentional formatting)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add missing switch cases for AutoGroup (NonRoot) in policy/v2/types.go
- Add missing switch cases for Protocol (IPv6ICMP, FC, IPInIP) in policy/v2/types.go
- Add missing switch cases for StateUpdateType in types/common.go
- Fix unchecked type assertion in db/text_serialiser.go
- Fix duration multiplication in types/config.go by using GetInt64
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add explicit handling for unchecked error returns:
- Add _, _ = prefix to intentionally ignored w.Write calls in debug handlers
- Add _, _ = prefix to rand.Read, fmt.Scanln, io.Copy, and WriteString calls
- Add _ = prefix to AddNode calls in tests where error is not relevant
- Wrap defer cleanup calls in closures to properly ignore errors
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add nolint:err113 comments to integration test infrastructure code where
dynamic errors are appropriate for debugging test failures. These errors
include context-specific information like container names, hostnames,
and test states that help diagnose issues.
Files updated:
- derp_verify_endpoint_test.go: DERP verification errors
- helpers.go: command execution and user lookup errors
- hsic/hsic.go: database export errors
- scenario.go: network and HTTP client validation errors
- tsic/tsic.go: container status and peer validation errors
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add sentinel errors and use %w wrapping for dynamic errors in:
- cmd/headscale/cli/mockoidc.go: errMockOidcUsersNotDefined
- cmd/headscale/cli/users.go: errFlagRequired, errMultipleUsersMatch
- cmd/hi/docker.go: ErrMemoryLimitViolations
- cmd/hi/stats.go: ErrStatsCollectionAlreadyStarted
Add nolint:err113 comments for test infrastructure errors that include
dynamic context for debugging:
- hscontrol/auth_test.go: extractRegistrationIDFromAuthURL
- hscontrol/state/node_store_test.go: concurrent test error reporting
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add sentinel errors and use %w wrapping for all dynamic errors in the
policy v2 package. This includes:
- types.go: ~35 new sentinel errors for validation messages
- utils.go: 8 new sentinel errors for port parsing
- utils_test.go: Updated to use sentinel errors
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Simplify globalResolvers() and splitResolvers() by removing unused
warn variable and using structured logging directly
- Add nolint:unused comments to deprecator methods (warnWithAlias,
warnNoAlias, warn) that are kept for future configuration deprecation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add nolint:unused comments to intentionally unused code that may be
useful for future debugging or is kept for backward compatibility:
- Test helper functions in integration tests
- Debug helpers in mapper tests
- Error variables that may be used later
- Functions marked for future use
Also remove actually unused constants and error variables.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix godoclint, exptostd, gocritic, inamedparam, ineffassign,
and embeddedstructfieldcheck lint issues:
- Fix documentation comments to start with symbol name
- Replace golang.org/x/exp/maps and slices with stdlib equivalents
- Simplify redundant closures (unlambda)
- Flatten nested conditionals (elseif)
- Use type switch with assignment (typeSwitchVar)
- Add parameter names to interface method signatures
- Fix ineffectual assignments
- Add empty line after embedded struct fields
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Run golangci-lint --fix and manually correct the resulting compilation
errors where := was incorrectly used instead of = for already-declared
variables.
Also fix lint issues:
- Use static errors instead of dynamic errors.New() (err113)
- Define constants for magic numbers (mnd)
- Use http.NewRequestWithContext instead of http.NewRequest (noctx)
Errors should not start capitalised and they should not contain the word error
or state that they "failed" as we already know it is an error
Signed-off-by: Kristoffer Dalby <kristoffer@dalby.cc>
Go style recommends that log messages and error strings should not be
capitalized (unless beginning with proper nouns or acronyms) and should
not end with punctuation.
This change normalizes all zerolog .Msg() and .Msgf() calls to start
with lowercase letters, following Go conventions and making logs more
consistent across the codebase.
Replace raw string field names with zf constants in state.go and
db/node.go for consistent, type-safe logging.
state.go changes:
- User creation, hostinfo validation, node registration
- Tag processing during reauth (processReauthTags)
- Auth path and PreAuthKey handling
- Route auto-approval and MapRequest processing
db/node.go changes:
- RegisterNodeForTest logging
- Invalid hostname replacement logging
Add sub-logger patterns to worker(), AddNode(), RemoveNode() and
multiChannelNodeConn to eliminate repeated field calls. Use zf.*
constants for consistent field naming.
Changes in batcher_lockfree.go:
- Add wlog sub-logger in worker() with worker.id context
- Add log field to multiChannelNodeConn struct
- Initialize mc.log with node.id in newMultiChannelNodeConn()
- Add nlog sub-loggers in AddNode() and RemoveNode()
- Update all connection methods to use mc.log
Changes in batcher.go:
- Use zf.NodeID and zf.Reason in handleNodeChange()
Replace manual field extraction with EmbedObject for node logging
in gRPC handlers. Use zf.* constants for consistent field naming.
Changes:
- RegisterNode: use EmbedObject(node), zf.RegistrationKey, etc.
- SetTags: use EmbedObject(node)
- ExpireNode: use EmbedObject(node), zf.ExpiresAt
- RenameNode: use EmbedObject(node), zf.NewName
- SetApprovedRoutes: use zf.NodeID
Add sub-logger pattern to SetRoutes() to eliminate repeated node.id
field calls. Replace raw strings with zf.* constants throughout
the primary routes code for consistent field naming.
Changes:
- Add nlog sub-logger in SetRoutes() with node.id context
- Replace "prefix" with zf.Prefix
- Replace "changed" with zf.Changes
- Replace "newState" with zf.NewState
- Replace "finalState" with zf.FinalState
Replace the helper functions (logf, infof, tracef, errf) with a
zerolog sub-logger initialized in newMapSession(). The sub-logger
is pre-populated with session context (component, node, omitPeers,
stream) eliminating repeated field calls throughout the code.
Changes:
- Add log field to mapSession struct
- Initialize sub-logger with EmbedObject(node) and request context
- Remove logf/infof/tracef/errf helper functions
- Update all callers to use m.log.Level().Caller()... pattern
- Update noise.go to use sess.log instead of sess.tracef
This reduces code by ~20 lines and eliminates ~15 repeated field
calls per log statement.
Add a lint rule to enforce use of zf.* constants for zerolog field
names instead of inline string literals. This catches at lint time
any new code that doesn't follow the convention.
The rule matches common zerolog field methods (Str, Int, Bool, etc.)
and flags any usage with a string literal first argument.
Add hscontrol/util/zlog package with:
- zf subpackage: field name constants for compile-time safety
- SafeHostinfo: wrapper that redacts device fingerprinting data
- SafeMapRequest: wrapper that redacts client endpoints
The zf (zerolog fields) subpackage provides short constant names
(e.g., zf.NodeID instead of inline "node.id" strings) ensuring
consistent field naming across all log statements.
Security considerations:
- SafeHostinfo never logs: OSVersion, DeviceModel, DistroName
- SafeMapRequest only logs endpoint counts, not actual IPs
Update integration test expectations to match current policy behavior:
1. IPProto defaults include all four protocols (TCP, UDP, ICMPv4,
ICMPv6) for port-range ACL rules, not just TCP and UDP.
2. Filter rules with identical SrcIPs and IPProto are now merged
into a single rule with combined DstPorts, so the subnet router
receives one filter rule instead of two.
Updates #3036
According to Tailscale SaaS behavior, autogroup:internet is handled
by exit node routing via AllowedIPs, not by packet filtering. ACL
rules with autogroup:internet as destination should produce no
filter rules for any node.
Previously, Headscale expanded autogroup:internet to public CIDR
ranges and distributed filters to exit nodes (because 0.0.0.0/0
"covers" internet destinations). This was incorrect.
Add detection for AutoGroupInternet in filter compilation to skip
filter generation for this autogroup. Update test expectations
accordingly.
Fix two compatibility issues discovered in Tailscale SaaS testing:
1. Wildcard DstPorts format: Headscale was expanding wildcard
destinations to CGNAT ranges (100.64.0.0/10, fd7a:115c:a1e0::/48)
while Tailscale uses {IP: "*"} directly. Add detection for
wildcard (Asterix) alias type in filter compilation to use the
correct format.
2. proto:icmp handling: The "icmp" protocol name was returning both
ICMPv4 (1) and ICMPv6 (58), but Tailscale only returns ICMPv4.
Users should use "ipv6-icmp" or protocol number 58 explicitly
for IPv6 ICMP.
Update all test expectations accordingly. This significantly reduces
test file line count by replacing duplicated CGNAT range patterns
with single wildcard entries.
Update test expectations across policy tests to expect merged
FilterRule entries instead of separate ones. Tests now expect:
- Single FilterRule with combined DstPorts for same source
- Reduced matcher counts for exit node tests
Updates #3036
Tailscale merges multiple ACL rules into fewer FilterRule entries
when they have identical SrcIPs and IPProto, combining their DstPorts
arrays. This change implements the same behavior in Headscale.
Add mergeFilterRules() which uses O(n) hash map lookup to merge rules
with identical keys. DstPorts are NOT deduplicated to match Tailscale
behavior.
Also fix DestsIsTheInternet() to handle merged filter rules where
TheInternet is combined with other destinations - now uses superset
check instead of equality check.
Updates #3036
Change Asterix.Resolve() to use Tailscale's CGNAT range (100.64.0.0/10)
and ULA range (fd7a:115c:a1e0::/48) instead of all IPs (0.0.0.0/0 and
::/0).
This better matches Tailscale's security model where wildcard (*) means
"any node in the tailnet" rather than literally "any IP address on the
internet".
Updates #3036
Updates #3036
Tailscale validates that autogroup:self destinations in ACL rules can
only be used when ALL sources are users, groups, autogroup:member, or
wildcard (*). Previously, Headscale only performed this validation for
SSH rules.
Add validateACLSrcDstCombination() to enforce that tags, autogroup:tagged,
hosts, and raw IPs cannot be used as sources with autogroup:self
destinations. Invalid policies like `tag:client → autogroup:self:*` are
now rejected at validation time, matching Tailscale behavior.
Wildcard (*) is allowed because autogroup:self evaluation narrows it
per-node to only the node's own IPs.
Updates #3036
When ACL rules don't specify a protocol, Headscale now defaults to
[TCP, UDP, ICMP, ICMPv6] instead of just [TCP, UDP], matching
Tailscale's behavior.
Also export protocol number constants (ProtocolTCP, ProtocolUDP, etc.)
for use in external test packages, renaming the string protocol
constants to ProtoNameTCP, ProtoNameUDP, etc. to avoid conflicts.
This resolves 78 ICMP-related TODOs in the Tailscale compatibility
tests, reducing the total from 165 to 87.
Updates #3036
Add extensive test coverage verifying Headscale's ACL policy behavior
matches Tailscale's coordination server. Tests cover:
- Source/destination resolution for users, groups, tags, hosts, IPs
- autogroup:member, autogroup:tagged, autogroup:self behavior
- Filter rule deduplication and merging semantics
- Multi-rule interaction patterns
- Error case validation
Key behavioral differences documented:
- Headscale creates separate filter entries per ACL rule; Tailscale
merges rules with identical sources
- Headscale deduplicates Dsts within a rule; Tailscale does not
- Headscale does not validate autogroup:self source restrictions for
ACL rules (only SSH rules); Tailscale rejects invalid sources
Tests are based on real Tailscale coordination server responses
captured from a test environment with 5 nodes (1 user-owned, 4 tagged).
Updates #3036
Skip autogroup:self destination processing for tagged nodes since they
can never match autogroup:self (which only applies to user-owned nodes).
Also reorder the IsTagged() check to short-circuit before accessing
User() to avoid potential nil pointer access on tagged nodes.
Updates #3036
