starred/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-01-11 20:00:28 +01:00
Code Issues 98 Packages Projects Releases 10 Wiki Activity

[PR #2850] [CLOSED] Filter exit node visibility through ACL policy #2893

New Issue
Closed
opened 2025-12-29 04:19:32 +01:00 by adam · 0 comments
adam commented 2025-12-29 04:19:32 +01:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/2850
Author: @Copilot
Created: 11/1/2025
Status: ❌ Closed

Base: main ← Head: copilot/investigate-headscale-issue-2788

📝 Commits (4)

  • a55cdc2 Initial plan
  • e010702 Filter exit routes through ACL policy to fix issue #2788
  • 4fa1f4b Add integration test for exit node ACL visibility (issue #2788)
  • 1f4b645 Refactor: Extract route filtering logic into helper function

📊 Changes

4 files changed (+182 additions, -9 deletions)

View changed files

📝 hscontrol/mapper/builder.go (+30 -6)
📝 hscontrol/mapper/tail.go (+1 -1)
📝 hscontrol/mapper/tail_test.go (+0 -2)
📝 integration/route_test.go (+151 -0)

📄 Description

Exit nodes were visible to all peers regardless of ACL policy. This regression allowed nodes without autogroup:internet:* in their ACL destinations to see exit nodes in their peer list, differing from Tailscale SaaS behavior.

Root cause:
Exit routes (0.0.0.0/0, ::/0) were unconditionally appended to AllowedIPs in tailNode(), bypassing the policy.ReduceRoutes() filtering applied to primary subnet routes.

Changes:

  • hscontrol/mapper/builder.go: Extracted buildRouteFilterFunc() helper that filters both primary and exit routes through policy.ReduceRoutes(). Applied to WithSelfNode() and buildTailPeers().

  • hscontrol/mapper/tail.go: Removed unconditional node.ExitRoutes() append since filtered routes now include exit routes when permitted.

  • hscontrol/mapper/tail_test.go: Updated test expectations to exclude exit routes when no policy grants access.

  • integration/route_test.go: Added TestExitNodeVisibilityWithACL verifying exit nodes are invisible without autogroup:internet ACL rule.

Example ACL requiring explicit exit node access:

{
  "acls": [
    {"action": "accept", "src": ["mobile"], "dst": ["server:80"]},
    {"action": "accept", "src": ["mobile"], "dst": ["autogroup:internet:*"]}
  ]
}

Without the second rule, mobile cannot see exit nodes despite them being advertised and approved.

Breaking change: Networks expecting all nodes to see exit nodes must add autogroup:internet:* to appropriate ACL destination rules.

Fixes #2788

Original prompt

Investigate https://github.com/juanfont/headscale/issues/2788

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/juanfont/headscale/pull/2850 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 11/1/2025 **Status:** ❌ Closed **Base:** `main` ← **Head:** `copilot/investigate-headscale-issue-2788` --- ### 📝 Commits (4) - [`a55cdc2`](https://github.com/juanfont/headscale/commit/a55cdc2636436afb3b2371bf1f3ac04c921d349a) Initial plan - [`e010702`](https://github.com/juanfont/headscale/commit/e0107024e894a4fef74076fa73729fe013018b9f) Filter exit routes through ACL policy to fix issue #2788 - [`4fa1f4b`](https://github.com/juanfont/headscale/commit/4fa1f4baa3d80e77febec614ab9c2501ae328441) Add integration test for exit node ACL visibility (issue #2788) - [`1f4b645`](https://github.com/juanfont/headscale/commit/1f4b645d5bb2413fbfeb139478e96b9c45b2da29) Refactor: Extract route filtering logic into helper function ### 📊 Changes **4 files changed** (+182 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `hscontrol/mapper/builder.go` (+30 -6) 📝 `hscontrol/mapper/tail.go` (+1 -1) 📝 `hscontrol/mapper/tail_test.go` (+0 -2) 📝 `integration/route_test.go` (+151 -0) </details> ### 📄 Description Exit nodes were visible to all peers regardless of ACL policy. This regression allowed nodes without `autogroup:internet:*` in their ACL destinations to see exit nodes in their peer list, differing from Tailscale SaaS behavior. **Root cause:** Exit routes (0.0.0.0/0, ::/0) were unconditionally appended to `AllowedIPs` in `tailNode()`, bypassing the `policy.ReduceRoutes()` filtering applied to primary subnet routes. **Changes:** - **hscontrol/mapper/builder.go**: Extracted `buildRouteFilterFunc()` helper that filters both primary and exit routes through `policy.ReduceRoutes()`. Applied to `WithSelfNode()` and `buildTailPeers()`. - **hscontrol/mapper/tail.go**: Removed unconditional `node.ExitRoutes()` append since filtered routes now include exit routes when permitted. - **hscontrol/mapper/tail_test.go**: Updated test expectations to exclude exit routes when no policy grants access. - **integration/route_test.go**: Added `TestExitNodeVisibilityWithACL` verifying exit nodes are invisible without `autogroup:internet` ACL rule. **Example ACL requiring explicit exit node access:** ```json { "acls": [ {"action": "accept", "src": ["mobile"], "dst": ["server:80"]}, {"action": "accept", "src": ["mobile"], "dst": ["autogroup:internet:*"]} ] } ``` Without the second rule, `mobile` cannot see exit nodes despite them being advertised and approved. **Breaking change:** Networks expecting all nodes to see exit nodes must add `autogroup:internet:*` to appropriate ACL destination rules. Fixes #2788 <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > Investigate https://github.com/juanfont/headscale/issues/2788 </details> <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-29 04:19:32 +01:00
adam closed this issue 2025-12-29 04:19:32 +01:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
CLI
DERP
DNS
Nix
OIDC
SSH
bug
database
documentation
duplicate
enhancement
faq
good first issue
grants
help wanted
might-come
needs design doc
needs investigation
no-stale-bot
out of scope
performance
policy 📝
pull-request
Mirrored from GitHub Pull Request
 question
regression
routes
stale
tags
tailscale-feature-gap
well described ❤️
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#2893
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?