镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

feat(acls): add HuJSON formatting and validation - #628

Merged
tale merged 3 commits into
tale:mainfrom
KnowItAllKing:feat/acl-hujson-format-validate
Oct 28, 2026
Merged

tale merged 3 commits into
tale:mainfrom
KnowItAllKing:feat/acl-hujson-format-validate

Conversation

@KnowItAllKing

Copy link
Copy Markdown
Contributor

Summary

  • add comment-preserving HuJSON formatting to the raw ACL editor
  • show live parse diagnostics and prevent invalid or empty saves
  • validate again server-side before sending policies to Headscale
  • document the editor behavior and add a Next changelog entry

Testing

  • pnpm run test:unit — 260 passed
  • pnpm run typecheck
  • pnpm run lint
  • pnpm run format
  • pnpm run build
  • manual ACL editor testing

Follow-up to #607 and #608.

@KnowItAllKing
KnowItAllKing requested a review from tale as a code owner September 3, 2026 21:04
@github-actions github-actions Bot added the Docs Improvements or additions to documentation label Sep 3, 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.

ℹ️ No blocking issues — two minor observations inline.

Reviewed changes

  • Comment-preserving formatting (app/utils/acl-policy.ts) — adds formatPolicy on top of jsonc-parser's format, and rewrites parsePolicy onto parseTree with allowTrailingComma: true. The move off scanHuJson-then-JSON.parse also fixes a latent bug where the trailing-comma regex corrupted ,}/,] sequences inside string values.
  • Live diagnostics (cm.client.tsx) — a CodeMirror linter plus a status bar surface validatePolicy errors inline, and a Format button reflows the buffer.
  • Save gating (overview.tsx, acl-action.ts) — the Save button disables on empty or invalid policy, and the action re-validates server-side before forwarding to Headscale.
  • Docs & changelog — docs/features/acls.md and a # Next changelog entry describe the new editor behavior.
  • Tests — unit coverage for formatPolicy, validatePolicy, the string-corruption regression, and the new action guard.

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

Comment thread app/utils/acl-policy.ts Resolved

@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.

Reviewed changes

  • Empty-policy neutrality (acl-policy.ts, cm.client.tsx) — validatePolicy no longer reports an empty policy as a syntax error, and the editor shows a neutral "No policy yet" state with a CircleDashed icon instead of a red error on first run. The Format button is disabled while empty.
  • Line-preserving formatting (acl-policy.ts) — formatPolicy switched to keepLines: true, so blank-line section groupings and inline arrays survive formatting while indentation and spacing are still normalized.
  • Nix hash bump (nix/package.nix) — the dependency hash now matches the jsonc-parser and @codemirror/lint additions from the earlier commit.
  • Tests — updated and extended to cover keepLines layout preservation, empty-policy neutrality, and non-formatting of empty input.

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

KnowItAllKing and others added 3 commits October 28, 2026 10:09
… as neutral

Formatting now keeps existing line breaks so blank-line groupings and inline
arrays in hand-written policies survive. An empty policy is no longer reported
as a syntax error; the editor shows a neutral "No policy yet" state instead.
@tale
tale force-pushed the feat/acl-hujson-format-validate branch from 5a910af to 42ef623 Compare October 28, 2026 21:23

@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.

Reviewed changes

  • Validation consolidated on parsePolicy (app/utils/acl-policy.ts, app/routes/acls/acl-action.ts, app/routes/acls/components/cm.client.tsx) — validatePolicy is removed; parsePolicy now carries diagnostics on failure, and the editor status bar, the linter and the save action all read from the one parse.
  • trimBracketPadding added (app/utils/acl-policy.ts) — post-processes the formatter output so single-line arrays render as ["a"] instead of [ "a" ]. Strings, comments and multi-line arrays are left untouched (verified against the jsonc-parser scanner and applyEdits behavior).
  • Humanized parse errors read more naturally — subsequent words are lowercased (Value Expected → Value expected) while the at line X, column Y suffix is kept.
  • Test cleanup — the now-redundant validatePolicy cases and the isolated acl-action unit test are dropped; the invalid-policy path stays covered by tests/integration/headplane/acls.test.ts.

Both prior findings from the initial review were already addressed and their threads resolved. typecheck, lint and pnpm test:unit (253 tests) pass at 42ef623.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@tale
tale merged commit c8c192d into tale:main Oct 28, 2026
8 checks passed
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.

2 participants