Skip to content

feat(ui): structured editor for ACL rules, tags and groups - #608

Open
albedev wants to merge 6 commits into
tale:mainfrom
albedev:feat/acl-visual-editor
Open

feat(ui): structured editor for ACL rules, tags and groups#608
albedev wants to merge 6 commits into
tale:mainfrom
albedev:feat/acl-visual-editor

Conversation

@albedev

@albedev albedev commented Aug 17, 2026

Copy link
Copy Markdown

Closes #607.

The Access Control page is a raw HuJSON editor today. This adds a structured
editor in front of it for the parts that change most often, and surfaces tags
and groups on the pages where the objects they apply to already live.

Nothing is sent to Headscale until Save is pressed, and the structured
editors write into the same buffer the file editor uses — so the file editor,
the diff view and the save button all keep operating on a single source of
truth.

Access Control page

Two tabs in front of Edit file:

  • Rulesacls, ssh and hosts as editable lists. Sources and
    destinations are built from chips and suggested from the groups, tags, hosts
    and Headscale users that actually exist in the tailnet. A destination entered
    without a port gets :* appended, since Headscale rejects one that has none.
  • Tags & Groups — CRUD over groups and tagOwners, showing which machines
    currently carry each tag.

A policy that fails to parse falls back to a notice pointing at the file editor
instead of breaking the page. A policy containing comments warns that the
structured editors will drop them — the file editor stays the way to keep them.

Users page

Group membership lives in the policy, which made it invisible from the page
where users are managed. The groups a user belongs to are now shown under their
name, and the row menu gains Edit groups. Editing requires write_policy on
top of write_users; if the policy cannot be read the UI simply does not appear.

Machines page

A tag assigned to a node but missing from tagOwners is accepted by Headscale
yet will never match a rule. The tag dialog now flags those and links to Access
Control.

Policy model

app/utils/acl-policy.ts parses the policy into a typed model and serializes it
back close to a hand-written policy: rules on one line, key order preserved,
empty sections omitted, so the diff shows only what changed. Unknown top-level
keys (autoApprovers, nodeAttrs, ...) are round-tripped untouched.
stripJsonCommentsAndTrailingCommas is exported from node-info.ts and reused
rather than duplicated.

Verification

  • pnpm run typecheck, pnpm run lint, pnpm run format clean
  • pnpm run test:unit — 216 passing, 26 of them new for the policy model
    (parsing, round-trip, group membership, port defaulting, validation)
  • Exercised by hand against Headscale 0.29.0 in database policy mode: creating
    and editing rules, tags and groups from the UI, editing a user's groups from
    the Users page, and confirming the resulting policy with headscale policy get

Overlap with existing work

This overlaps with #548 / #549 / #550 and sits next to #603 (which is about
visualizing reachability rather than editing). I would rather not step on
anyone's toes: happy to close this in favour of that work, split it into smaller
PRs, or rebase on top of whichever direction you prefer.

🤖 Generated with Claude Code

@albedev
albedev requested a review from tale as a code owner August 17, 2026 22:16
@github-actions github-actions Bot added the Docs Improvements or additions to documentation label Aug 17, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The policy model and editors are well-built, but three edge cases should be fixed before merge: the undeclared-tag warning fires as a false positive whenever the policy can't be read, group edits from the Users page silently drop policy comments, and "Edit groups" isn't gated on policy writability so it fails silently in file mode.

Reviewed changes

  • ACL policy model (app/utils/acl-policy.ts) — typed parsePolicy/serializePolicy over the HuJSON policy with round-tripping of unknown top-level keys, :* port defaulting (IPv6-aware), and name validation.
  • Rules / Tags & Groups editors (app/routes/acls/) — structured editors and dialogs that write into the same codePolicy buffer as the file editor, keeping the diff view and Save on a single source of truth.
  • Users page (app/routes/users/) — group membership shown under each user, plus an "Edit groups" dialog and update_user_groups action.
  • Machines page (app/routes/machines/) — tags not declared under tagOwners are flagged in the tag dialog.
  • Docs & tests — new docs/features/acls.md and 26 new unit tests for the policy model.

ℹ️ Nitpicks

  • serializePolicy always emits groupstagOwnershostsaclsssh, so "key order preserved" only holds within a section: a hand-written policy that lists sections in a different order gets reshuffled on the first structured edit, producing a noisy diff that undercuts the stated "diff shows only what changed" goal.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Comment thread app/routes/machines/dialogs/tags.tsx
Comment thread app/routes/users/user-actions.ts Outdated
Comment thread app/routes/users/overview.tsx Outdated
@albedev

albedev commented Aug 17, 2026

Copy link
Copy Markdown
Author

Thanks — all three findings were real, and the nitpick was right too. Fixed in
4976aa5.

Undeclared-tag warning on an unreadable policy. Correct, and the diagnosis is
exact: extractTagOwnerTags returns [] for both "no tags declared" and "no
policy", so the policyTags === undefined guard was dead code and every tag got
flagged whenever api.policy.get() rejected. Went with the first suggested
approach — the machine loaders now pass
policy === undefined ? undefined : extractTagOwnerTags(policy) — rather than
changing extractTagOwnerTags, so sortAssignableTags keeps its current
contract. A policy that reads fine but declares nothing is still ""[], so
a genuinely undeclared tag is still flagged.

"Edit groups" not gated on writability. Also correct, including the detail
that the dialog closes on failure. The users loader now reads updatedAt from
the policy it already fetches and gates canEditGroups on
updatedAt !== null, reusing the same signal as acl-loader.ts. Belt and
braces on the action too: update_user_groups now catches the write, maps
update is disabled to a 403 with a message naming policy.mode, and returns
any other failure as { error } so the dialog renders it instead of closing.

Comments dropped from the Users page. Right, the asymmetry was
unintentional. The loader now surfaces parsePolicy(policy).hasComments and the
group dialog shows the same notice as the Access Control tab before you save.

Nitpick — section order. Fair, and it undercut the stated goal. parsePolicy
now records the top-level key order and serializePolicy emits in that order,
appending any section that did not exist before. So a policy written as
sshhostsgroups stays that way after a structured edit. Two tests
cover it.

218 unit tests passing, typecheck and lint clean.

One thing worth flagging for a human reviewer rather than the bot: this PR
overlaps with #548 / #549 / #550 and sits next to #603. @tale — happy to close
this, split it, or rebase onto whichever of those you want to take forward.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

The three edge cases flagged in the prior review are all addressed cleanly, along with the top-level key-ordering nitpick. I verified the two Headscale-facing assumptions against v0.29.0 source: GetPolicy returns a nil updatedAt in file mode (so updatedAt !== null correctly detects database mode), and SetPolicy in file mode returns ErrPolicyUpdateIsDisabled ("update is disabled for modes other than 'database'"), which the new catch block matches.

Reviewed changes

  • Fixed the undeclared-tag false positive — both machine loaders now pass policyTags as undefined when the policy could not be read, so the tag dialog no longer flags every tag when api.policy.get() rejects.
  • Gated "Edit groups" on policy writabilitycanEditGroups now also requires updatedAt !== null, and update_user_groups catches api.policy.set failures, returning a friendly 403 in file mode instead of failing silently.
  • Warned before dropping policy commentspolicyHasComments is surfaced from the users loader and shown as a notice in the group-edit dialog before a save rewrites the policy.
  • Preserved top-level key orderparsePolicy records keyOrder and serializePolicy re-emits sections in that order, appending newly created sections at the end so the diff only shows real changes.
  • Added tests — two new unit tests cover section-order preservation and appending a new section.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@albedev
albedev force-pushed the feat/acl-visual-editor branch from 4976aa5 to 3ddc8cf Compare August 17, 2026 22:35
@tale

tale commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Fix your merge conflicts and I'll take a look

albedev and others added 6 commits August 19, 2026 11:20
Headscale stores the policy as an opaque HuJSON string. Parse it into a
typed model that the UI can edit, and serialize it back in a shape that
stays close to a hand-written policy: rules on a single line, key order
preserved, empty sections omitted.

Top-level keys Headplane does not model (autoApprovers, nodeAttrs, ...)
are round-tripped untouched so editing never silently drops them.

Also exposes helpers the editors need: the source/destination catalog,
group membership lookups, name validation, and `withDefaultPort`, which
appends `:*` to a destination that has no port spec since Headscale
rejects those.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Access Control page gains two tabs in front of the file editor:

- Rules renders `acls`, `ssh` and `hosts` as editable lists. Sources and
  destinations are built from chips, suggested from the groups, tags,
  hosts and Headscale users that actually exist in the tailnet. A
  destination typed without a port gets `:*` appended.
- Tags & Groups manages `groups` and `tagOwners`, showing which machines
  currently carry each tag.

Both write into the same buffer the file editor uses, so the diff view
and the save button keep working on a single source of truth and nothing
reaches Headscale until Save is pressed. A policy that fails to parse
falls back to a notice pointing at the file editor, and a policy with
comments warns that the structured editors will drop them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Group membership is stored in the ACL policy, which made it invisible
from the page where users are actually managed. Show the groups a user
belongs to under their name, and add an "Edit groups" entry to the row
menu that rewrites the `groups` section of the policy.

Editing requires `write_policy` on top of `write_users`, and the loader
treats the policy as optional: an unreadable one just hides the UI
instead of breaking the page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Headscale accepts any forced tag on a node, but a tag that is missing
from `tagOwners` will never match a rule, which is easy to miss. Mark
those tags in the machine tag dialog and point at the Access Control
page where they can be declared.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- The undeclared-tag warning fired for every tag whenever the policy could
  not be read at all: `extractTagOwnerTags` returns `[]` for a missing
  policy, so the `undefined` guard in the tag dialog was dead code. The
  machine loaders now pass `undefined` when `api.policy.get()` rejects, so
  the warning only appears for a policy that was actually read.

- "Edit groups" was offered whenever the role allowed it, ignoring whether
  the policy is writable at all. In `file` mode Headscale rejects the write
  with "update is disabled", and the failure never reached the operator.
  The users loader now gates on the same `updatedAt !== null` signal the
  Access Control page uses, and the action returns a readable error instead
  of throwing.

- Editing groups from the Users page rewrites the whole policy, which drops
  HuJSON comments, but only the Access Control page warned about it. The
  group dialog now shows the same notice.

- `serializePolicy` emitted sections in a fixed order, so a policy whose
  sections were written in a different order was reshuffled on the first
  structured edit. The parsed key order is now preserved, with new sections
  appended after it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@albedev
albedev force-pushed the feat/acl-visual-editor branch from 3ddc8cf to 4a0199e Compare August 19, 2026 09:22
@albedev

albedev commented Aug 19, 2026

Copy link
Copy Markdown
Author

Rebased onto main, conflicts resolved.

The only real overlap was #554 (key expiry toggle), which threads a supportsDisablingKeyExpiry prop through the same four files this PR threads policyTags through: machines/overview.tsx, machines/machine.tsx, components/machine-row.tsx and components/menu.tsx. Three merged cleanly; machine.tsx needed a manual resolution in the loaderData destructuring, where both props are now kept side by side. No behaviour changed on either side.

Verified locally: typecheck, lint and build clean, test:unit green (228 tests), and the diff against the pre-rebase branch is exactly the four commits from main. CI is green here too.

Ready for review whenever you have a moment.

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

Labels

Docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Structured editing for ACL rules, tags and groups

2 participants