Skip to content

[go] allow opt-in Tailscale ACLs for admin/view access#503

Open
patrickod wants to merge 2 commits into
mainfrom
patrickod/tailscale-acls
Open

[go] allow opt-in Tailscale ACLs for admin/view access#503
patrickod wants to merge 2 commits into
mainfrom
patrickod/tailscale-acls

Conversation

@patrickod

Copy link
Copy Markdown
Collaborator

Extend the Tailscale serve integration to optionally consume Tailscale capability grants and use these to create admin and view roles for accessing the web UI with the latter being restricted from changing device settings.

LAN access is unaffected and retains admin privileges to all endpoints.

Copilot AI review requested due to automatic review settings May 8, 2026 19:49
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown

🤖 Version Bump Advisory

Warnings

Radar version unchanged - consider bumping version in Makefile

New Features

ℹ️ New API handlers (2 handler(s)) - MINOR bump suggested
ℹ️ New CLI flags (1 flag(s)) - MINOR bump suggested


📖 See CHANGELOG.md for detailed guidelines.

This is an automated advisory. Review the detected changes and update versions accordingly.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an opt-in authorisation layer for Tailscale-served HTTP access, using Tailscale capability grants to distinguish view (read-only) vs admin access, while keeping LAN/loopback behaviour unchanged by default.

Changes:

  • Add a Tailscale peer-capability lookup surface (with short-TTL caching) to resolve a peer’s view/admin grants.
  • Add an API-layer auth gate + route classifier that applies to the entire HTTP mux (default-deny; view allowlists).
  • Wire the feature behind a new -ts-cap-enforcement=off|on flag and document the operational model.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
internal/tailscale/peercaps.go Implements peer capability lookup and short-TTL caches for peer identity and local prefixes.
internal/tailscale/peercaps_test.go Unit tests for capability parsing and caching behaviours.
internal/tailscale/manager.go Extends LocalClient with WhoIs and adds caches to the Manager.
internal/tailscale/manager_test.go Updates test fake client to implement WhoIs.
internal/api/server.go Adds route classification (view/admin/allowlist) and wraps the server handler with the auth gate.
internal/api/auth.go New middleware implementing trust model, enforcement modes, and 403 JSON responses.
internal/api/auth_test.go Unit + integration tests covering source classification, grant matrix, failure modes, and default-deny semantics.
docs/platform/operations/tailscale-remote-access.md Updates ops documentation for capability grants, enforcement modes, and troubleshooting.
cmd/radar/radar.go Adds -ts-cap-enforcement flag and wires SetAuthGate into server startup.

Comment thread internal/api/server.go
Comment thread internal/api/server.go
Comment thread internal/api/auth.go Outdated
Comment thread internal/api/auth.go Outdated
Comment thread internal/tailscale/peercaps.go Outdated
Comment thread docs/platform/operations/tailscale-remote-access.md
@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.69767% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/api/auth.go 86.36% 7 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

@patrickod patrickod force-pushed the patrickod/tailscale-acls branch from 234f34f to 3639adc Compare May 8, 2026 20:07
patrickod and others added 2 commits May 15, 2026 12:06
Extend the Tailscale serve integration to optionally consume Tailscale
capability grants and use these to create admin and view roles for
accessing the web UI with the latter being restricted from changing
device settings.

LAN access is unaffected and retains admin privileges to all endpoints.
Five fixes from the CoPilot review on #503:

1. Allowlist bare /app and view-gate bare /api/reports.  The auth
   wrapper runs before the ServeMux trailing-slash redirect, so a
   request to /app or /api/reports would hit default-deny CapAdmin
   and 403 view-only peers instead of redirecting to the
   trailing-slash sibling.

2. Narrow isPeerNotFound to match only "no match for IP".  The
   previous substring set ("not found", "404") caught generic
   transport errors and reported them as ErrPeerNotFound, which made
   the api layer fail closed (403) on a transient lookup blip
   instead of failing open as documented.

3. Remove the ineffective LocalTailnetPrefixes fallback in
   isTailnetIP.  The method returned /32 and /128 prefixes for
   *this* node's own Tailscale IPs, so the fallback loop could only
   ever match the node's own address, never a remote peer.  The
   method, the prefix cache, and the related fakePeerAuth wiring are
   removed; isTailnetIP and classifySource are simplified to drop
   the now-unused ctx/gate parameters.

4. Log "auth: capability enforcement armed (mode=on)" at SetAuthGate
   when EnforcementOn is the effective mode.  The troubleshooting
   doc instructed operators to grep journald for this line but no
   such line was emitted.

Test updates: route-classifier tests now cover bare /app and
/api/reports, peercaps tests assert that transient errors containing
"not found"/"404" are not misclassified, and the now-defunct
TestLocalTailnetPrefixes_Cached is removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants