From 8fcb5c8ace3f78e5c04781562c7cae9e159a7950 Mon Sep 17 00:00:00 2001 From: drifterza Date: Mon, 4 May 2026 14:00:34 +0200 Subject: [PATCH 1/2] feat(acl): add HuJSON policy parsing and manipulation utilities Add comment-json dependency for parsing HuJSON (JSON with comments and trailing commas) used by Headscale ACL policies. Introduces app/utils/acl-editor.ts with typed interfaces for ACL policy entities and pure functions for parsing, serializing, and mutating policy sections (rules, groups, hosts, tags, SSH rules). Comment metadata is preserved through mutations via comment-json's assign helper. Includes 45 unit tests covering parsing, round-trips, comment preservation, trailing comma handling, and all mutation functions. --- app/utils/acl-editor.ts | 133 +++++++++++ nix/package.nix | 2 +- package.json | 1 + pnpm-lock.yaml | 25 ++ tests/unit/utils/acl-editor.test.ts | 351 ++++++++++++++++++++++++++++ 5 files changed, 511 insertions(+), 1 deletion(-) create mode 100644 app/utils/acl-editor.ts create mode 100644 tests/unit/utils/acl-editor.test.ts diff --git a/app/utils/acl-editor.ts b/app/utils/acl-editor.ts new file mode 100644 index 00000000..909d1f29 --- /dev/null +++ b/app/utils/acl-editor.ts @@ -0,0 +1,133 @@ +import { assign, parse, stringify } from "comment-json"; + +export interface AclRule { + action: "accept"; + src: string[]; + dst: string[]; + proto?: string; +} + +export interface SshRule { + action: "accept" | "check"; + src: string[]; + dst: string[]; + users: string[]; + checkPeriod?: string; +} + +export interface AclPolicy { + acls?: AclRule[]; + groups?: Record; + hosts?: Record; + tagOwners?: Record; + ssh?: SshRule[]; + autoApprovers?: { routes?: Record; exitNode?: string[] }; + tests?: unknown[]; +} + +export function parsePolicy(raw: string): AclPolicy { + if (!raw.trim()) return {}; + return parse(raw) as AclPolicy; +} + +export function stringifyPolicy(policy: AclPolicy): string { + return stringify(policy, null, 2); +} + +// comment-json stores comments as Symbols which get lost in spread. +function patch(policy: AclPolicy, changes: Partial): AclPolicy { + return assign(assign({} as AclPolicy, policy), changes) as AclPolicy; +} + +// Generic array operations on a policy field +type ArrayField = "acls" | "ssh"; + +function appendTo( + policy: AclPolicy, + key: K, + item: NonNullable[number], +): AclPolicy { + return patch(policy, { + [key]: [...((policy[key] as unknown[]) ?? []), item], + } as Partial); +} + +function removeAt(policy: AclPolicy, key: K, index: number): AclPolicy { + const arr = [...((policy[key] as unknown[]) ?? [])]; + if (index < 0 || index >= arr.length) return policy; + arr.splice(index, 1); + return patch(policy, { [key]: arr } as Partial); +} + +function replaceAt( + policy: AclPolicy, + key: K, + index: number, + item: NonNullable[number], +): AclPolicy { + const arr = [...((policy[key] as unknown[]) ?? [])]; + if (index < 0 || index >= arr.length) return policy; + arr[index] = item; + return patch(policy, { [key]: arr } as Partial); +} + +// Generic record operations on a policy field +type RecordField = "groups" | "hosts" | "tagOwners"; + +function setEntry( + policy: AclPolicy, + key: K, + entryKey: string, + value: NonNullable[string], +): AclPolicy { + return patch(policy, { + [key]: { ...(policy[key] as Record), [entryKey]: value }, + } as Partial); +} + +function removeEntry( + policy: AclPolicy, + key: K, + entryKey: string, +): AclPolicy { + const map = { ...(policy[key] as Record) }; + delete map[entryKey]; + return patch(policy, { [key]: map } as Partial); +} + +// Prefix helpers +function groupKey(name: string) { + return name.startsWith("group:") ? name : `group:${name}`; +} + +function tagKey(name: string) { + return name.startsWith("tag:") ? name : `tag:${name}`; +} + +// ACL rules +export const addAclRule = (p: AclPolicy, rule: AclRule) => appendTo(p, "acls", rule); +export const removeAclRule = (p: AclPolicy, i: number) => removeAt(p, "acls", i); +export const updateAclRule = (p: AclPolicy, i: number, rule: AclRule) => + replaceAt(p, "acls", i, rule); + +// SSH rules +export const addSshRule = (p: AclPolicy, rule: SshRule) => appendTo(p, "ssh", rule); +export const removeSshRule = (p: AclPolicy, i: number) => removeAt(p, "ssh", i); +export const updateSshRule = (p: AclPolicy, i: number, rule: SshRule) => + replaceAt(p, "ssh", i, rule); + +// Groups +export const setGroup = (p: AclPolicy, name: string, members: string[]) => + setEntry(p, "groups", groupKey(name), members); +export const removeGroup = (p: AclPolicy, name: string) => removeEntry(p, "groups", groupKey(name)); + +// Hosts +export const setHost = (p: AclPolicy, name: string, addr: string) => + setEntry(p, "hosts", name, addr); +export const removeHost = (p: AclPolicy, name: string) => removeEntry(p, "hosts", name); + +// Tag owners +export const setTagOwner = (p: AclPolicy, tag: string, owners: string[]) => + setEntry(p, "tagOwners", tagKey(tag), owners); +export const removeTagOwner = (p: AclPolicy, tag: string) => + removeEntry(p, "tagOwners", tagKey(tag)); diff --git a/nix/package.nix b/nix/package.nix index 9a4774b5..67b719c9 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -33,7 +33,7 @@ in inherit (finalAttrs) pname version src; fetcherVersion = 3; pnpm = pnpm_10; - hash = "sha256-NGIeboj/2kXuWsmTVl1fv4LgU1VYRdO+qSnNLVuneC8="; + hash = "sha256-OTd6+KxPc0NZyPiof6DNAH+bZouSkKHN5YTPjL1ko1E="; }; buildPhase = '' diff --git a/package.json b/package.json index 058317bd..a778b382 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "@uiw/react-codemirror": "4.25.9", "arktype": "^2.2.0", "clsx": "^2.1.1", + "comment-json": "^5.0.0", "drizzle-orm": "1.0.0-beta.21", "isbot": "5.1.37", "jose": "6.2.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2a6a48a0..028783fe 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -59,6 +59,9 @@ importers: clsx: specifier: ^2.1.1 version: 2.1.1 + comment-json: + specifier: ^5.0.0 + version: 5.0.0 drizzle-orm: specifier: 1.0.0-beta.21 version: 1.0.0-beta.21(@libsql/client@0.17.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(@types/better-sqlite3@7.6.13)(@types/mssql@9.1.9(@azure/core-client@1.10.1))(arktype@2.2.0)(better-sqlite3@11.10.0)(mssql@11.0.1(@azure/core-client@1.10.1))(valibot@1.3.1(typescript@6.0.2)) @@ -2163,6 +2166,9 @@ packages: arktype@2.2.0: resolution: {integrity: sha512-t54MZ7ti5BhOEvzEkgKnWvqj+UbDfWig+DHr5I34xatymPusKLS0lQpNJd8M6DzmIto2QGszHfNKoFIT8tMCZQ==} + array-timsort@1.0.3: + resolution: {integrity: sha512-/+3GRL7dDAGEfM6TseQk/U+mi18TU2Ms9I3UlLdUMhz2hbvGNTKdj9xniwXfUqgYhHxRx0+8UnKkvlNwVU+cWQ==} + asn1@0.2.6: resolution: {integrity: sha512-ix/FxPn0MDjeyJ7i/yoHGFt/EX6LyNbxSEhPPXODPL+KB0VPk86UYfL0lMdy+KCnv+fmvIzySwaK5COwqVbWTQ==} @@ -2394,6 +2400,10 @@ packages: commander@2.20.3: resolution: {integrity: sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==} + comment-json@5.0.0: + resolution: {integrity: sha512-uiqLcOiVDJtBP8WGkZHEP+FZIhTzP1dxvn59EfoYUi9gqupjrBWVQkO2atDrbnKPwLeotFYDsuNb26uBMqB+hw==} + engines: {node: '>= 6'} + compress-commons@6.0.2: resolution: {integrity: sha512-6FqVXeETqWPoGcfzrXb37E50NP0LXT8kAMu5ooZayhWWdgEY4lBEEcbQNXtkuKQsGduxiIcI4gOTsxTmuq/bSg==} engines: {node: '>= 14'} @@ -2706,6 +2716,11 @@ packages: resolution: {integrity: sha512-WUj2qlxaQtO4g6Pq5c29GTcWGDyd8itL8zTlipgECz3JesAiiOKotd8JU6otB3PACgG6xkJUyVhboMS+bje/jA==} engines: {node: '>=6'} + esprima@4.0.1: + resolution: {integrity: sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==} + engines: {node: '>=4'} + hasBin: true + estree-walker@2.0.2: resolution: {integrity: sha512-Rfkk/Mp/DL7JVje3u18FxFujQlTNR2q6QfMSMB7AvCBx91NGj/ba3kCfza0f6dVDbw7YlRf/nDrn7pQrCCyQ/w==} @@ -3873,6 +3888,7 @@ packages: uuid@8.3.2: resolution: {integrity: sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==} + deprecated: uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028). hasBin: true valibot@1.3.1: @@ -5939,6 +5955,8 @@ snapshots: '@ark/util': 0.56.0 arkregex: 0.0.5 + array-timsort@1.0.3: {} + asn1@0.2.6: dependencies: safer-buffer: 2.1.2 @@ -6162,6 +6180,11 @@ snapshots: commander@2.20.3: optional: true + comment-json@5.0.0: + dependencies: + array-timsort: 1.0.3 + esprima: 4.0.1 + compress-commons@6.0.2: dependencies: crc-32: 1.2.2 @@ -6434,6 +6457,8 @@ snapshots: escalade@3.2.0: {} + esprima@4.0.1: {} + estree-walker@2.0.2: {} estree-walker@3.0.3: diff --git a/tests/unit/utils/acl-editor.test.ts b/tests/unit/utils/acl-editor.test.ts new file mode 100644 index 00000000..03d8c561 --- /dev/null +++ b/tests/unit/utils/acl-editor.test.ts @@ -0,0 +1,351 @@ +import { describe, expect, test } from "vitest"; + +import { + type AclPolicy, + type AclRule, + type SshRule, + addAclRule, + addSshRule, + parsePolicy, + removeAclRule, + removeGroup, + removeHost, + removeSshRule, + removeTagOwner, + setGroup, + setHost, + setTagOwner, + stringifyPolicy, + updateAclRule, + updateSshRule, +} from "~/utils/acl-editor"; + +const FULL = `{ + "acls": [ + {"action": "accept", "src": ["group:admin"], "dst": ["*:*"]}, + {"action": "accept", "src": ["group:dev"], "dst": ["tag:server:22"]} + ], + "groups": { + "group:admin": ["user1", "user2"], + "group:dev": ["user3"] + }, + "hosts": { + "server1": "100.64.0.1", + "server2": "100.64.0.2" + }, + "tagOwners": { + "tag:server": ["group:admin"], + "tag:ci": ["user3"] + }, + "ssh": [ + {"action": "accept", "src": ["group:admin"], "dst": ["tag:server"], "users": ["root"]} + ] +}`; + +const WITH_COMMENTS = `{ + // Main access rules + "acls": [ + // Allow admins everywhere + {"action": "accept", "src": ["group:admin"], "dst": ["*:*"]}, + ], + // Team groups + "groups": { + "group:admin": ["alice", "bob"], + }, +}`; + +const WITH_EXTRA_FIELDS = `{ + "acls": [ + {"action": "accept", "src": ["*"], "dst": ["*:*"]} + ], + "autoApprovers": { + "routes": {"10.0.0.0/8": ["group:admin"]}, + "exitNode": ["group:admin"] + }, + "tests": [ + {"src": "user1", "accept": ["100.64.0.1:80"]} + ] +}`; + +const rule = (o?: Partial): AclRule => ({ + action: "accept", + src: ["*"], + dst: ["*:*"], + ...o, +}); + +const ssh = (o?: Partial): SshRule => ({ + action: "accept", + src: ["group:admin"], + dst: ["tag:server"], + users: ["root"], + ...o, +}); + +describe("parsing", () => { + test("empty/whitespace input returns empty object", () => { + expect(parsePolicy("")).toEqual({}); + expect(parsePolicy(" ")).toEqual({}); + }); + + test("parses all policy sections", () => { + const p = parsePolicy(FULL); + expect(p.acls).toHaveLength(2); + expect(Object.keys(p.groups ?? {})).toEqual(["group:admin", "group:dev"]); + expect(Object.keys(p.hosts ?? {})).toEqual(["server1", "server2"]); + expect(Object.keys(p.tagOwners ?? {})).toEqual(["tag:server", "tag:ci"]); + expect(p.ssh).toHaveLength(1); + }); + + test("handles HuJSON (comments + trailing commas)", () => { + const p = parsePolicy(WITH_COMMENTS); + expect(p.acls).toHaveLength(1); + expect(p.groups?.["group:admin"]).toEqual(["alice", "bob"]); + }); + + test("rejects invalid input", () => { + expect(() => parsePolicy("{invalid}")).toThrow(); + }); + + test("stringify round-trips preserve data and section-level comments", () => { + const output = stringifyPolicy(parsePolicy(WITH_COMMENTS)); + expect(output).toContain("// Main access rules"); + expect(output).toContain("// Team groups"); + const reparsed = parsePolicy(output); + expect(reparsed.acls).toHaveLength(1); + expect(reparsed.groups?.["group:admin"]).toEqual(["alice", "bob"]); + }); +}); + +// Tests array generics (appendTo/removeAt/replaceAt) via acl + ssh wrappers +describe("array operations", () => { + test("append to empty and existing arrays", () => { + const fromEmpty = addAclRule({}, rule()); + expect(fromEmpty.acls).toHaveLength(1); + + const fromExisting = addAclRule(parsePolicy(FULL), rule({ src: ["group:ops"] })); + expect(fromExisting.acls).toHaveLength(3); + expect(fromExisting.acls?.[2].src).toEqual(["group:ops"]); + }); + + test("remove by index, out-of-bounds no-op, and last-element removal", () => { + const p = parsePolicy(FULL); + + const removed = removeAclRule(p, 0); + expect(removed.acls).toHaveLength(1); + expect(removed.acls?.[0].src).toEqual(["group:dev"]); + + // Out of bounds and undefined arrays return same ref + expect(removeAclRule(p, 99)).toBe(p); + expect(removeAclRule(p, -1)).toBe(p); + expect(removeAclRule({}, 0)).toEqual({}); + + // Removing last element leaves empty array + const single = parsePolicy(`{"acls": [{"action":"accept","src":["*"],"dst":["*:*"]}]}`); + expect(removeAclRule(single, 0).acls).toEqual([]); + }); + + test("replace at index", () => { + const p = parsePolicy(FULL); + const updated = updateAclRule(p, 1, rule({ src: ["group:ops"], dst: ["*:443"] })); + expect(updated.acls?.[1].src).toEqual(["group:ops"]); + expect(updated.acls?.[0].src).toEqual(["group:admin"]); // untouched + expect(updateAclRule(p, 99, rule())).toBe(p); // out of bounds + }); + + test("ssh rules use same generics", () => { + const added = addSshRule({}, ssh()); + expect(added.ssh).toHaveLength(1); + + const p = parsePolicy(FULL); + expect(removeSshRule(p, 0).ssh).toHaveLength(0); + + const updated = updateSshRule(p, 0, ssh({ action: "check", checkPeriod: "12h" })); + expect(updated.ssh?.[0].action).toBe("check"); + expect(updated.ssh?.[0].checkPeriod).toBe("12h"); + }); +}); + +// Tests record generics (setEntry/removeEntry) via groups/hosts/tags wrappers +describe("record operations", () => { + test("set new, overwrite existing, and auto-prefix", () => { + // Groups: new + overwrite + prefix + expect(setGroup({}, "ops", ["a"]).groups?.["group:ops"]).toEqual(["a"]); + const p = parsePolicy(FULL); + const updated = setGroup(p, "group:admin", ["newuser"]); + expect(updated.groups?.["group:admin"]).toEqual(["newuser"]); + expect(updated.groups?.["group:dev"]).toEqual(["user3"]); // sibling untouched + + // Hosts + expect(setHost({}, "web", "10.0.0.1").hosts?.web).toBe("10.0.0.1"); + + // Tags with auto-prefix + expect(setTagOwner({}, "web", ["group:ops"]).tagOwners?.["tag:web"]).toEqual(["group:ops"]); + }); + + test("remove existing, no-op for missing, and auto-prefix", () => { + const p = parsePolicy(FULL); + + const r = removeGroup(p, "dev"); // auto-prefixed + expect(r.groups?.["group:dev"]).toBeUndefined(); + expect(r.groups?.["group:admin"]).toBeDefined(); + + expect(removeHost(p, "server1").hosts?.server1).toBeUndefined(); + expect(removeTagOwner(p, "ci").tagOwners?.["tag:ci"]).toBeUndefined(); + + // No-op for missing keys + const noOp = removeGroup(p, "nonexistent"); + expect(Object.keys(noOp.groups ?? {})).toEqual(Object.keys(p.groups ?? {})); + }); +}); + +describe("immutability", () => { + test("no mutation function alters the source policy", () => { + const p = parsePolicy(FULL); + const snapshot = stringifyPolicy(p); + + addAclRule(p, rule()); + removeAclRule(p, 0); + updateAclRule(p, 0, rule({ src: ["changed"] })); + addSshRule(p, ssh()); + removeSshRule(p, 0); + updateSshRule(p, 0, ssh({ users: ["ubuntu"] })); + setGroup(p, "new", ["x"]); + removeGroup(p, "group:admin"); + setHost(p, "server1", "10.0.0.99"); + removeHost(p, "server1"); + setTagOwner(p, "tag:server", ["changed"]); + removeTagOwner(p, "tag:ci"); + + expect(stringifyPolicy(p)).toBe(snapshot); + }); +}); + +describe("comment preservation", () => { + test("section-level comments survive mutations on any field", () => { + const p = parsePolicy(WITH_COMMENTS); + const output = stringifyPolicy(setHost(p, "web", "10.0.0.5")); + expect(output).toContain("// Main access rules"); + expect(output).toContain("// Team groups"); + }); + + test("inline array element comments are lost on array mutation", () => { + // comment-json stores inline comments as Symbols on array elements. + // Spreading into a new array drops them — known tradeoff of immutability. + const p = parsePolicy(WITH_COMMENTS); + const output = stringifyPolicy(addAclRule(p, rule())); + expect(output).not.toContain("// Allow admins everywhere"); + expect(output).toContain("// Main access rules"); // section-level preserved + }); +}); + +describe("field isolation", () => { + test("autoApprovers and tests survive unrelated mutations", () => { + let p = parsePolicy(WITH_EXTRA_FIELDS); + p = addAclRule(p, rule({ src: ["group:ops"] })); + p = setGroup(p, "ops", ["admin1"]); + p = setHost(p, "web", "10.0.0.5"); + + const final = parsePolicy(stringifyPolicy(p)); + expect(final.autoApprovers?.routes?.["10.0.0.0/8"]).toEqual(["group:admin"]); + expect(final.autoApprovers?.exitNode).toEqual(["group:admin"]); + expect(final.tests).toHaveLength(1); + expect(final.acls).toHaveLength(2); + }); + + test("groups/hosts/tags/ssh survive acl removal", () => { + const r = removeAclRule(parsePolicy(FULL), 0); + expect(Object.keys(r.groups ?? {})).toEqual(["group:admin", "group:dev"]); + expect(Object.keys(r.hosts ?? {})).toEqual(["server1", "server2"]); + expect(r.ssh).toHaveLength(1); + }); +}); + +describe("error handling & edge cases", () => { + test("parsePolicy rejects invalid JSON but doesn't crash on non-object values", () => { + expect(() => parsePolicy("{invalid}")).toThrow(); + // comment-json wraps primitives in boxed objects, so just verify no crash + expect(() => parsePolicy('"just a string"')).not.toThrow(); + expect(() => parsePolicy("null")).not.toThrow(); + expect(() => parsePolicy("42")).not.toThrow(); + }); + + test("array ops on undefined fields default to empty", () => { + const empty: AclPolicy = {}; + // append creates the array + expect(addAclRule(empty, rule()).acls).toHaveLength(1); + expect(addSshRule(empty, ssh()).ssh).toHaveLength(1); + // remove/update on undefined returns same ref (no-op) + expect(removeAclRule(empty, 0)).toBe(empty); + expect(updateAclRule(empty, 0, rule())).toBe(empty); + expect(removeSshRule(empty, 0)).toBe(empty); + expect(updateSshRule(empty, 0, ssh())).toBe(empty); + }); + + test("record ops on undefined fields default to empty", () => { + const empty: AclPolicy = {}; + expect(setGroup(empty, "ops", ["a"]).groups?.["group:ops"]).toEqual(["a"]); + expect(setHost(empty, "web", "10.0.0.1").hosts?.web).toBe("10.0.0.1"); + expect(setTagOwner(empty, "web", ["a"]).tagOwners?.["tag:web"]).toEqual(["a"]); + // remove from undefined field doesn't crash + const r1 = removeGroup(empty, "ops"); + expect(r1.groups).toEqual({}); + const r2 = removeHost(empty, "web"); + expect(r2.hosts).toEqual({}); + const r3 = removeTagOwner(empty, "web"); + expect(r3.tagOwners).toEqual({}); + }); + + test("out-of-bounds array operations return same reference", () => { + const p = parsePolicy(FULL); + expect(removeAclRule(p, -1)).toBe(p); + expect(removeAclRule(p, 999)).toBe(p); + expect(updateAclRule(p, -1, rule())).toBe(p); + expect(updateAclRule(p, 999, rule())).toBe(p); + expect(removeSshRule(p, -1)).toBe(p); + expect(updateSshRule(p, 999, ssh())).toBe(p); + }); +}); + +describe("end-to-end workflows", () => { + test("build policy from scratch and round-trip", () => { + let p: AclPolicy = {}; + p = setGroup(p, "admin", ["alice", "bob"]); + p = setTagOwner(p, "server", ["group:admin"]); + p = setHost(p, "gateway", "100.64.0.1"); + p = addAclRule(p, rule({ src: ["group:admin"], dst: ["*:*"] })); + p = addSshRule(p, ssh()); + + const final = parsePolicy(stringifyPolicy(p)); + expect(final.groups?.["group:admin"]).toEqual(["alice", "bob"]); + expect(final.tagOwners?.["tag:server"]).toEqual(["group:admin"]); + expect(final.hosts?.gateway).toBe("100.64.0.1"); + expect(final.acls).toHaveLength(1); + expect(final.ssh).toHaveLength(1); + }); + + test("chained add/remove/update across sections", () => { + let p = parsePolicy(FULL); + p = addAclRule(p, rule({ src: ["group:temp"] })); + p = removeAclRule(p, 2); // remove the one we just added + p = updateAclRule(p, 0, rule({ src: ["group:ops"] })); + p = setGroup(p, "temp", ["user1"]); + p = removeGroup(p, "temp"); + + const final = parsePolicy(stringifyPolicy(p)); + expect(final.acls).toHaveLength(2); + expect(final.acls?.[0].src).toEqual(["group:ops"]); + expect(final.groups?.["group:temp"]).toBeUndefined(); + expect(final.groups?.["group:admin"]).toEqual(["user1", "user2"]); + }); + + test("optional fields (proto, checkPeriod) survive round-trips", () => { + let p: AclPolicy = {}; + p = addAclRule(p, rule({ proto: "udp", dst: ["*:53"] })); + p = addSshRule(p, ssh({ action: "check", checkPeriod: "24h" })); + + const final = parsePolicy(stringifyPolicy(p)); + expect(final.acls?.[0].proto).toBe("udp"); + expect(final.ssh?.[0].checkPeriod).toBe("24h"); + }); +}); From d0b6e263a934098b59526be4e660424d878bc412 Mon Sep 17 00:00:00 2001 From: drifterza Date: Mon, 4 May 2026 14:24:26 +0200 Subject: [PATCH 2/2] feat(acl): add visual editor component with generic section primitives --- app/routes/acls/components/visual-editor.tsx | 556 +++++++++++++++++++ app/routes/acls/overview.tsx | 14 +- app/utils/acl-editor.ts | 13 +- tests/integration/api/nodes.test.ts | 2 +- tests/unit/utils/acl-editor.test.ts | 288 +++++----- 5 files changed, 734 insertions(+), 139 deletions(-) create mode 100644 app/routes/acls/components/visual-editor.tsx diff --git a/app/routes/acls/components/visual-editor.tsx b/app/routes/acls/components/visual-editor.tsx new file mode 100644 index 00000000..2cfa4475 --- /dev/null +++ b/app/routes/acls/components/visual-editor.tsx @@ -0,0 +1,556 @@ +import { Pencil, Plus, Trash2 } from "lucide-react"; +import type { ReactNode } from "react"; +import { useState } from "react"; + +import Button from "~/components/button"; +import Chip from "~/components/chip"; +import Dialog, { DialogPanel } from "~/components/dialog"; +import Input from "~/components/input"; +import TableList from "~/components/table-list"; +import { + type AclPolicy, + type AclRule, + type SshRule, + addAclRule, + addSshRule, + groupKey, + parsePolicy, + removeAclRule, + removeGroup, + removeHost, + removeSshRule, + removeTagOwner, + setGroup, + setHost, + setTagOwner, + stringifyPolicy, + tagKey, + updateAclRule, + updateSshRule, +} from "~/utils/acl-editor"; + +function split(v: string): string[] { + return v + .split(",") + .map((s) => s.trim()) + .filter(Boolean); +} + +function join(v: string[]): string { + return v.join(", "); +} + +function Tags({ values }: { values: string[] }) { + return ( +
+ {values.map((v) => ( + + ))} +
+ ); +} + +type ArrayDialogState = + | { mode: "closed" } + | { mode: "add" } + | { mode: "edit"; index: number } + | { mode: "delete"; index: number }; + +interface ArraySectionProps { + title: string; + emptyText: string; + items: T[]; + renderRow: (item: T) => ReactNode; + formTitle: (editing: boolean) => string; + renderForm: (item: T | undefined) => ReactNode; + parseForm: (fd: FormData) => T; + onAdd: (item: T) => void; + onUpdate: (index: number, item: T) => void; + onRemove: (index: number) => void; + disabled?: boolean; +} + +function ArraySection({ + title, + emptyText, + items, + renderRow, + formTitle, + renderForm, + parseForm, + onAdd, + onUpdate, + onRemove, + disabled, +}: ArraySectionProps) { + const [dialog, setDialog] = useState({ mode: "closed" }); + const close = () => setDialog({ mode: "closed" }); + const editing = dialog.mode === "edit" ? items[dialog.index] : undefined; + + function handleSubmit(e: React.FormEvent) { + e.preventDefault(); + const item = parseForm(new FormData(e.currentTarget)); + if (dialog.mode === "add") onAdd(item); + else if (dialog.mode === "edit") onUpdate(dialog.index, item); + close(); + } + + function handleDelete(e: React.FormEvent) { + e.preventDefault(); + if (dialog.mode === "delete") onRemove(dialog.index); + close(); + } + + return ( +
+
+

{title}

+ +
+ + {items.length === 0 ? ( +

{emptyText}

+ ) : ( + + {items.map((item, i) => ( + + {renderRow(item)} +
+ + +
+ + ))} + + )} + + !open && close()} + > + +

{formTitle(dialog.mode === "edit")}

+ {renderForm(editing)} + + + + !open && close()}> + +

Remove {title.toLowerCase().replace(/s$/, "")}

+

+ This will remove the item from the policy. You can discard all changes to undo. +

+ + +
+ ); +} + +type RecordDialogState = + | { mode: "closed" } + | { mode: "add" } + | { mode: "edit"; key: string } + | { mode: "delete"; key: string }; + +interface RecordSectionProps { + title: string; + emptyText: string; + entries: [string, V][]; + renderRow: (key: string, value: V) => ReactNode; + formTitle: (editing: boolean) => string; + renderForm: (key: string | undefined, value: V | undefined) => ReactNode; + parseForm: (fd: FormData) => { key: string; value: V }; + onSet: (key: string, value: V) => void; + onRename: (oldKey: string, newKey: string, value: V) => void; + onRemove: (key: string) => void; + disabled?: boolean; +} + +function RecordSection({ + title, + emptyText, + entries, + renderRow, + formTitle, + renderForm, + parseForm, + onSet, + onRename, + onRemove, + disabled, +}: RecordSectionProps) { + const [dialog, setDialog] = useState({ mode: "closed" }); + const close = () => setDialog({ mode: "closed" }); + const editKey = dialog.mode === "edit" ? dialog.key : undefined; + const editValue = editKey ? entries.find(([k]) => k === editKey)?.[1] : undefined; + + function handleSubmit(e: React.FormEvent) { + e.preventDefault(); + const { key, value } = parseForm(new FormData(e.currentTarget)); + if (dialog.mode === "edit" && editKey && editKey !== key) { + onRename(editKey, key, value); + } else { + onSet(key, value); + } + close(); + } + + function handleDelete(e: React.FormEvent) { + e.preventDefault(); + if (dialog.mode === "delete") onRemove(dialog.key); + close(); + } + + return ( +
+
+

{title}

+ +
+ + {entries.length === 0 ? ( +

{emptyText}

+ ) : ( + + {entries.map(([key, value]) => ( + + {renderRow(key, value)} +
+ + +
+ + ))} + + )} + + !open && close()} + > + +

{formTitle(dialog.mode === "edit")}

+ {renderForm(editKey, editValue)} + + + + !open && close()}> + +

Remove {title.toLowerCase().replace(/s$/, "")}

+

+ This will remove {dialog.mode === "delete" ? dialog.key : ""} from the + policy. You can discard all changes to undo. +

+ + +
+ ); +} + +interface VisualEditorProps { + value: string; + onChange: (value: string) => void; + disabled?: boolean; +} + +export default function VisualEditor({ value, onChange, disabled }: VisualEditorProps) { + const policy = parsePolicy(value); + const emit = (p: AclPolicy) => onChange(stringifyPolicy(p)); + + return ( +
+ {/* ACL Rules */} + + title="ACL Rules" + emptyText="No ACL rules defined" + disabled={disabled} + items={policy.acls ?? []} + renderRow={(r) => ( +
+
+ + → + +
+ {r.proto && {r.proto}} +
+ )} + formTitle={(editing) => (editing ? "Edit Rule" : "Add Rule")} + renderForm={(item) => ( + <> + + + + + )} + parseForm={(fd) => ({ + action: "accept" as const, + src: split(fd.get("src") as string), + dst: split(fd.get("dst") as string), + ...((fd.get("proto") as string)?.trim() + ? { proto: (fd.get("proto") as string).trim() } + : {}), + })} + onAdd={(rule) => emit(addAclRule(policy, rule))} + onUpdate={(i, rule) => emit(updateAclRule(policy, i, rule))} + onRemove={(i) => emit(removeAclRule(policy, i))} + /> + + {/* Groups */} + + title="Groups" + emptyText="No groups defined" + disabled={disabled} + entries={Object.entries(policy.groups ?? {})} + renderRow={(key, members) => ( +
+ {key} + +
+ )} + formTitle={(editing) => (editing ? "Edit Group" : "Add Group")} + renderForm={(key, members) => ( + <> + + + + )} + parseForm={(fd) => ({ + key: groupKey((fd.get("name") as string).trim()), + value: split(fd.get("members") as string), + })} + onSet={(key, members) => emit(setGroup(policy, key, members))} + onRename={(oldKey, newKey, members) => + emit(setGroup(removeGroup(policy, oldKey), newKey, members)) + } + onRemove={(key) => emit(removeGroup(policy, key))} + /> + + {/* Hosts */} + + title="Hosts" + emptyText="No host aliases defined" + disabled={disabled} + entries={Object.entries(policy.hosts ?? {})} + renderRow={(name, addr) => ( +
+ {name} + → + {addr} +
+ )} + formTitle={(editing) => (editing ? "Edit Host" : "Add Host")} + renderForm={(key, addr) => ( + <> + + + + )} + parseForm={(fd) => ({ + key: (fd.get("name") as string).trim(), + value: (fd.get("address") as string).trim(), + })} + onSet={(name, addr) => emit(setHost(policy, name, addr))} + onRename={(oldKey, newKey, addr) => emit(setHost(removeHost(policy, oldKey), newKey, addr))} + onRemove={(name) => emit(removeHost(policy, name))} + /> + + {/* Tag Owners */} + + title="Tag Owners" + emptyText="No tag owners defined" + disabled={disabled} + entries={Object.entries(policy.tagOwners ?? {})} + renderRow={(tag, owners) => ( +
+ {tag} + +
+ )} + formTitle={(editing) => (editing ? "Edit Tag Owner" : "Add Tag Owner")} + renderForm={(key, owners) => ( + <> + + + + )} + parseForm={(fd) => ({ + key: tagKey((fd.get("tag") as string).trim()), + value: split(fd.get("owners") as string), + })} + onSet={(tag, owners) => emit(setTagOwner(policy, tag, owners))} + onRename={(oldKey, newKey, owners) => + emit(setTagOwner(removeTagOwner(policy, oldKey), newKey, owners)) + } + onRemove={(tag) => emit(removeTagOwner(policy, tag))} + /> + + {/* SSH Rules */} + + title="SSH Rules" + emptyText="No SSH rules defined" + disabled={disabled} + items={policy.ssh ?? []} + renderRow={(r) => ( +
+
+ + + → + +
+
+ as + + {r.checkPeriod && every {r.checkPeriod}} +
+
+ )} + formTitle={(editing) => (editing ? "Edit SSH Rule" : "Add SSH Rule")} + renderForm={(item) => ( + <> + + + + + + + )} + parseForm={(fd) => ({ + action: + (fd.get("action") as string) === "check" ? ("check" as const) : ("accept" as const), + src: split(fd.get("src") as string), + dst: split(fd.get("dst") as string), + users: split(fd.get("users") as string), + ...((fd.get("checkPeriod") as string)?.trim() + ? { checkPeriod: (fd.get("checkPeriod") as string).trim() } + : {}), + })} + onAdd={(rule) => emit(addSshRule(policy, rule))} + onUpdate={(i, rule) => emit(updateSshRule(policy, i, rule))} + onRemove={(i) => emit(removeSshRule(policy, i))} + /> +
+ ); +} diff --git a/app/routes/acls/overview.tsx b/app/routes/acls/overview.tsx index 51206f9f..650ff1e6 100644 --- a/app/routes/acls/overview.tsx +++ b/app/routes/acls/overview.tsx @@ -1,4 +1,4 @@ -import { AlertCircle, Construction, Eye, FlaskConical, Pencil } from "lucide-react"; +import { AlertCircle, Construction, Eye, FlaskConical, LayoutGrid, Pencil } from "lucide-react"; import { Suspense, lazy, useEffect, useState } from "react"; import { isRouteErrorResponse, useFetcher, useRevalidator } from "react-router"; @@ -23,6 +23,7 @@ const LazyEditor = lazy(() => const LazyDiffer = lazy(() => import("./components/cm.client").then((m) => ({ default: m.Differ })), ); +const LazyVisualEditor = lazy(() => import("./components/visual-editor")); export const loader = aclLoader; export const action = aclAction; @@ -100,6 +101,12 @@ export default function Page({ loaderData: { access, writable, policy } }: Route Preview changes + +
+ + Visual editor +
+
@@ -117,6 +124,11 @@ export default function Page({ loaderData: { access, writable, policy } }: Route + + }> + + +
diff --git a/app/utils/acl-editor.ts b/app/utils/acl-editor.ts index 909d1f29..c8b68b3a 100644 --- a/app/utils/acl-editor.ts +++ b/app/utils/acl-editor.ts @@ -34,12 +34,10 @@ export function stringifyPolicy(policy: AclPolicy): string { return stringify(policy, null, 2); } -// comment-json stores comments as Symbols which get lost in spread. function patch(policy: AclPolicy, changes: Partial): AclPolicy { return assign(assign({} as AclPolicy, policy), changes) as AclPolicy; } -// Generic array operations on a policy field type ArrayField = "acls" | "ssh"; function appendTo( @@ -71,7 +69,6 @@ function replaceAt( return patch(policy, { [key]: arr } as Partial); } -// Generic record operations on a policy field type RecordField = "groups" | "hosts" | "tagOwners"; function setEntry( @@ -95,38 +92,32 @@ function removeEntry( return patch(policy, { [key]: map } as Partial); } -// Prefix helpers -function groupKey(name: string) { +export function groupKey(name: string) { return name.startsWith("group:") ? name : `group:${name}`; } -function tagKey(name: string) { +export function tagKey(name: string) { return name.startsWith("tag:") ? name : `tag:${name}`; } -// ACL rules export const addAclRule = (p: AclPolicy, rule: AclRule) => appendTo(p, "acls", rule); export const removeAclRule = (p: AclPolicy, i: number) => removeAt(p, "acls", i); export const updateAclRule = (p: AclPolicy, i: number, rule: AclRule) => replaceAt(p, "acls", i, rule); -// SSH rules export const addSshRule = (p: AclPolicy, rule: SshRule) => appendTo(p, "ssh", rule); export const removeSshRule = (p: AclPolicy, i: number) => removeAt(p, "ssh", i); export const updateSshRule = (p: AclPolicy, i: number, rule: SshRule) => replaceAt(p, "ssh", i, rule); -// Groups export const setGroup = (p: AclPolicy, name: string, members: string[]) => setEntry(p, "groups", groupKey(name), members); export const removeGroup = (p: AclPolicy, name: string) => removeEntry(p, "groups", groupKey(name)); -// Hosts export const setHost = (p: AclPolicy, name: string, addr: string) => setEntry(p, "hosts", name, addr); export const removeHost = (p: AclPolicy, name: string) => removeEntry(p, "hosts", name); -// Tag owners export const setTagOwner = (p: AclPolicy, tag: string, owners: string[]) => setEntry(p, "tagOwners", tagKey(tag), owners); export const removeTagOwner = (p: AclPolicy, tag: string) => diff --git a/tests/integration/api/nodes.test.ts b/tests/integration/api/nodes.test.ts index 7565dd90..5bddf662 100644 --- a/tests/integration/api/nodes.test.ts +++ b/tests/integration/api/nodes.test.ts @@ -53,7 +53,7 @@ describe.sequential.for(HS_VERSIONS)("Headscale %s: Users", (version) => { await client.setNodeUser(workingNodeId, user.id); const reassignedNode = await client.getNode(workingNodeId); expect(reassignedNode).toBeDefined(); - expect(reassignedNode.user.name).toBe(user.name); + expect(reassignedNode.user?.name).toBe(user.name); }); test("nodes can be expired", async () => { diff --git a/tests/unit/utils/acl-editor.test.ts b/tests/unit/utils/acl-editor.test.ts index 03d8c561..7d70e266 100644 --- a/tests/unit/utils/acl-editor.test.ts +++ b/tests/unit/utils/acl-editor.test.ts @@ -6,6 +6,7 @@ import { type SshRule, addAclRule, addSshRule, + groupKey, parsePolicy, removeAclRule, removeGroup, @@ -16,6 +17,7 @@ import { setHost, setTagOwner, stringifyPolicy, + tagKey, updateAclRule, updateSshRule, } from "~/utils/acl-editor"; @@ -45,7 +47,6 @@ const FULL = `{ const WITH_COMMENTS = `{ // Main access rules "acls": [ - // Allow admins everywhere {"action": "accept", "src": ["group:admin"], "dst": ["*:*"]}, ], // Team groups @@ -82,8 +83,8 @@ const ssh = (o?: Partial): SshRule => ({ ...o, }); -describe("parsing", () => { - test("empty/whitespace input returns empty object", () => { +describe("parsePolicy", () => { + test("empty input returns empty object", () => { expect(parsePolicy("")).toEqual({}); expect(parsePolicy(" ")).toEqual({}); }); @@ -97,110 +98,158 @@ describe("parsing", () => { expect(p.ssh).toHaveLength(1); }); - test("handles HuJSON (comments + trailing commas)", () => { + test("handles HuJSON comments and trailing commas", () => { const p = parsePolicy(WITH_COMMENTS); expect(p.acls).toHaveLength(1); expect(p.groups?.["group:admin"]).toEqual(["alice", "bob"]); }); - test("rejects invalid input", () => { + test("parses policy with empty arrays", () => { + const p = parsePolicy(`{"acls": [], "ssh": []}`); + expect(p.acls).toEqual([]); + expect(p.ssh).toEqual([]); + }); + + test("throws on invalid JSON", () => { expect(() => parsePolicy("{invalid}")).toThrow(); }); +}); - test("stringify round-trips preserve data and section-level comments", () => { +describe("stringifyPolicy", () => { + test("round-trips preserve data", () => { + const final = parsePolicy(stringifyPolicy(parsePolicy(FULL))); + expect(final.acls).toHaveLength(2); + expect(final.groups?.["group:admin"]).toEqual(["user1", "user2"]); + expect(final.hosts?.server1).toBe("100.64.0.1"); + }); + + test("preserves section-level HuJSON comments", () => { const output = stringifyPolicy(parsePolicy(WITH_COMMENTS)); expect(output).toContain("// Main access rules"); expect(output).toContain("// Team groups"); - const reparsed = parsePolicy(output); - expect(reparsed.acls).toHaveLength(1); - expect(reparsed.groups?.["group:admin"]).toEqual(["alice", "bob"]); }); }); -// Tests array generics (appendTo/removeAt/replaceAt) via acl + ssh wrappers +describe("groupKey / tagKey", () => { + test("adds prefix when missing", () => { + expect(groupKey("admin")).toBe("group:admin"); + expect(tagKey("server")).toBe("tag:server"); + }); + + test("is idempotent when prefix already present", () => { + expect(groupKey("group:admin")).toBe("group:admin"); + expect(tagKey("tag:server")).toBe("tag:server"); + }); +}); + describe("array operations", () => { - test("append to empty and existing arrays", () => { - const fromEmpty = addAclRule({}, rule()); - expect(fromEmpty.acls).toHaveLength(1); + test("appends to empty and existing arrays", () => { + expect(addAclRule({}, rule()).acls).toHaveLength(1); - const fromExisting = addAclRule(parsePolicy(FULL), rule({ src: ["group:ops"] })); - expect(fromExisting.acls).toHaveLength(3); - expect(fromExisting.acls?.[2].src).toEqual(["group:ops"]); + const added = addAclRule(parsePolicy(FULL), rule({ src: ["group:ops"] })); + expect(added.acls).toHaveLength(3); + expect(added.acls?.[2].src).toEqual(["group:ops"]); }); - test("remove by index, out-of-bounds no-op, and last-element removal", () => { - const p = parsePolicy(FULL); + test("appends to an already-empty array field", () => { + const p = parsePolicy(`{"acls": []}`); + const added = addAclRule(p, rule()); + expect(added.acls).toHaveLength(1); + }); - const removed = removeAclRule(p, 0); + test("removes by index", () => { + const removed = removeAclRule(parsePolicy(FULL), 0); expect(removed.acls).toHaveLength(1); expect(removed.acls?.[0].src).toEqual(["group:dev"]); + }); - // Out of bounds and undefined arrays return same ref - expect(removeAclRule(p, 99)).toBe(p); - expect(removeAclRule(p, -1)).toBe(p); - expect(removeAclRule({}, 0)).toEqual({}); - - // Removing last element leaves empty array + test("removing last element leaves empty array", () => { const single = parsePolicy(`{"acls": [{"action":"accept","src":["*"],"dst":["*:*"]}]}`); expect(removeAclRule(single, 0).acls).toEqual([]); }); - test("replace at index", () => { - const p = parsePolicy(FULL); - const updated = updateAclRule(p, 1, rule({ src: ["group:ops"], dst: ["*:443"] })); + test("replaces at index without affecting siblings", () => { + const updated = updateAclRule(parsePolicy(FULL), 1, rule({ src: ["group:ops"] })); expect(updated.acls?.[1].src).toEqual(["group:ops"]); - expect(updated.acls?.[0].src).toEqual(["group:admin"]); // untouched - expect(updateAclRule(p, 99, rule())).toBe(p); // out of bounds + expect(updated.acls?.[0].src).toEqual(["group:admin"]); }); - test("ssh rules use same generics", () => { - const added = addSshRule({}, ssh()); - expect(added.ssh).toHaveLength(1); - + test("out-of-bounds and undefined arrays return same reference", () => { const p = parsePolicy(FULL); - expect(removeSshRule(p, 0).ssh).toHaveLength(0); + expect(removeAclRule(p, -1)).toBe(p); + expect(removeAclRule(p, 999)).toBe(p); + expect(updateAclRule(p, -1, rule())).toBe(p); + expect(updateAclRule(p, 999, rule())).toBe(p); + + const empty: AclPolicy = {}; + expect(removeAclRule(empty, 0)).toBe(empty); + expect(updateAclRule(empty, 0, rule())).toBe(empty); + }); + + test("ssh rules use the same mechanics", () => { + expect(addSshRule({}, ssh()).ssh).toHaveLength(1); + expect(removeSshRule(parsePolicy(FULL), 0).ssh).toHaveLength(0); - const updated = updateSshRule(p, 0, ssh({ action: "check", checkPeriod: "12h" })); + const updated = updateSshRule( + parsePolicy(FULL), + 0, + ssh({ action: "check", checkPeriod: "12h" }), + ); expect(updated.ssh?.[0].action).toBe("check"); expect(updated.ssh?.[0].checkPeriod).toBe("12h"); }); }); -// Tests record generics (setEntry/removeEntry) via groups/hosts/tags wrappers describe("record operations", () => { - test("set new, overwrite existing, and auto-prefix", () => { - // Groups: new + overwrite + prefix + test("sets new entries with auto-prefix", () => { expect(setGroup({}, "ops", ["a"]).groups?.["group:ops"]).toEqual(["a"]); - const p = parsePolicy(FULL); - const updated = setGroup(p, "group:admin", ["newuser"]); - expect(updated.groups?.["group:admin"]).toEqual(["newuser"]); - expect(updated.groups?.["group:dev"]).toEqual(["user3"]); // sibling untouched - - // Hosts expect(setHost({}, "web", "10.0.0.1").hosts?.web).toBe("10.0.0.1"); - - // Tags with auto-prefix expect(setTagOwner({}, "web", ["group:ops"]).tagOwners?.["tag:web"]).toEqual(["group:ops"]); }); - test("remove existing, no-op for missing, and auto-prefix", () => { - const p = parsePolicy(FULL); + test("overwrites existing entries without affecting siblings", () => { + const updated = setGroup(parsePolicy(FULL), "group:admin", ["newuser"]); + expect(updated.groups?.["group:admin"]).toEqual(["newuser"]); + expect(updated.groups?.["group:dev"]).toEqual(["user3"]); + }); - const r = removeGroup(p, "dev"); // auto-prefixed - expect(r.groups?.["group:dev"]).toBeUndefined(); - expect(r.groups?.["group:admin"]).toBeDefined(); + test("allows setting empty member lists", () => { + const p = setGroup({}, "empty", []); + expect(p.groups?.["group:empty"]).toEqual([]); + }); + test("removes entries with auto-prefix", () => { + const p = parsePolicy(FULL); + expect(removeGroup(p, "dev").groups?.["group:dev"]).toBeUndefined(); expect(removeHost(p, "server1").hosts?.server1).toBeUndefined(); expect(removeTagOwner(p, "ci").tagOwners?.["tag:ci"]).toBeUndefined(); + }); - // No-op for missing keys - const noOp = removeGroup(p, "nonexistent"); - expect(Object.keys(noOp.groups ?? {})).toEqual(Object.keys(p.groups ?? {})); + test("removing non-existent keys leaves record unchanged", () => { + const p = parsePolicy(FULL); + expect(Object.keys(removeGroup(p, "nonexistent").groups ?? {})).toEqual( + Object.keys(p.groups ?? {}), + ); + }); + + test("operations on undefined fields produce empty records", () => { + const empty: AclPolicy = {}; + expect(removeGroup(empty, "ops").groups).toEqual({}); + expect(removeHost(empty, "web").hosts).toEqual({}); + expect(removeTagOwner(empty, "web").tagOwners).toEqual({}); + }); + + test("rename via remove+set preserves siblings", () => { + const p = parsePolicy(FULL); + const renamed = setGroup(removeGroup(p, "group:dev"), "group:engineering", ["user3", "user4"]); + expect(renamed.groups?.["group:dev"]).toBeUndefined(); + expect(renamed.groups?.["group:engineering"]).toEqual(["user3", "user4"]); + expect(renamed.groups?.["group:admin"]).toEqual(["user1", "user2"]); }); }); describe("immutability", () => { - test("no mutation function alters the source policy", () => { + test("mutations never alter the source policy", () => { const p = parsePolicy(FULL); const snapshot = stringifyPolicy(p); @@ -221,93 +270,33 @@ describe("immutability", () => { }); }); -describe("comment preservation", () => { - test("section-level comments survive mutations on any field", () => { - const p = parsePolicy(WITH_COMMENTS); - const output = stringifyPolicy(setHost(p, "web", "10.0.0.5")); - expect(output).toContain("// Main access rules"); - expect(output).toContain("// Team groups"); - }); - - test("inline array element comments are lost on array mutation", () => { - // comment-json stores inline comments as Symbols on array elements. - // Spreading into a new array drops them — known tradeoff of immutability. - const p = parsePolicy(WITH_COMMENTS); - const output = stringifyPolicy(addAclRule(p, rule())); - expect(output).not.toContain("// Allow admins everywhere"); - expect(output).toContain("// Main access rules"); // section-level preserved - }); -}); - describe("field isolation", () => { - test("autoApprovers and tests survive unrelated mutations", () => { + test("unrecognized fields survive mutations", () => { let p = parsePolicy(WITH_EXTRA_FIELDS); p = addAclRule(p, rule({ src: ["group:ops"] })); p = setGroup(p, "ops", ["admin1"]); - p = setHost(p, "web", "10.0.0.5"); const final = parsePolicy(stringifyPolicy(p)); expect(final.autoApprovers?.routes?.["10.0.0.0/8"]).toEqual(["group:admin"]); expect(final.autoApprovers?.exitNode).toEqual(["group:admin"]); expect(final.tests).toHaveLength(1); - expect(final.acls).toHaveLength(2); }); - test("groups/hosts/tags/ssh survive acl removal", () => { + test("sibling sections survive targeted removal", () => { const r = removeAclRule(parsePolicy(FULL), 0); expect(Object.keys(r.groups ?? {})).toEqual(["group:admin", "group:dev"]); expect(Object.keys(r.hosts ?? {})).toEqual(["server1", "server2"]); expect(r.ssh).toHaveLength(1); }); -}); - -describe("error handling & edge cases", () => { - test("parsePolicy rejects invalid JSON but doesn't crash on non-object values", () => { - expect(() => parsePolicy("{invalid}")).toThrow(); - // comment-json wraps primitives in boxed objects, so just verify no crash - expect(() => parsePolicy('"just a string"')).not.toThrow(); - expect(() => parsePolicy("null")).not.toThrow(); - expect(() => parsePolicy("42")).not.toThrow(); - }); - - test("array ops on undefined fields default to empty", () => { - const empty: AclPolicy = {}; - // append creates the array - expect(addAclRule(empty, rule()).acls).toHaveLength(1); - expect(addSshRule(empty, ssh()).ssh).toHaveLength(1); - // remove/update on undefined returns same ref (no-op) - expect(removeAclRule(empty, 0)).toBe(empty); - expect(updateAclRule(empty, 0, rule())).toBe(empty); - expect(removeSshRule(empty, 0)).toBe(empty); - expect(updateSshRule(empty, 0, ssh())).toBe(empty); - }); - - test("record ops on undefined fields default to empty", () => { - const empty: AclPolicy = {}; - expect(setGroup(empty, "ops", ["a"]).groups?.["group:ops"]).toEqual(["a"]); - expect(setHost(empty, "web", "10.0.0.1").hosts?.web).toBe("10.0.0.1"); - expect(setTagOwner(empty, "web", ["a"]).tagOwners?.["tag:web"]).toEqual(["a"]); - // remove from undefined field doesn't crash - const r1 = removeGroup(empty, "ops"); - expect(r1.groups).toEqual({}); - const r2 = removeHost(empty, "web"); - expect(r2.hosts).toEqual({}); - const r3 = removeTagOwner(empty, "web"); - expect(r3.tagOwners).toEqual({}); - }); - test("out-of-bounds array operations return same reference", () => { - const p = parsePolicy(FULL); - expect(removeAclRule(p, -1)).toBe(p); - expect(removeAclRule(p, 999)).toBe(p); - expect(updateAclRule(p, -1, rule())).toBe(p); - expect(updateAclRule(p, 999, rule())).toBe(p); - expect(removeSshRule(p, -1)).toBe(p); - expect(updateSshRule(p, 999, ssh())).toBe(p); + test("section-level comments survive mutations on other fields", () => { + const output = stringifyPolicy(setHost(parsePolicy(WITH_COMMENTS), "web", "10.0.0.5")); + expect(output).toContain("// Main access rules"); + expect(output).toContain("// Team groups"); }); }); -describe("end-to-end workflows", () => { +describe("end-to-end", () => { test("build policy from scratch and round-trip", () => { let p: AclPolicy = {}; p = setGroup(p, "admin", ["alice", "bob"]); @@ -324,10 +313,10 @@ describe("end-to-end workflows", () => { expect(final.ssh).toHaveLength(1); }); - test("chained add/remove/update across sections", () => { + test("chained mutations across sections", () => { let p = parsePolicy(FULL); p = addAclRule(p, rule({ src: ["group:temp"] })); - p = removeAclRule(p, 2); // remove the one we just added + p = removeAclRule(p, 2); p = updateAclRule(p, 0, rule({ src: ["group:ops"] })); p = setGroup(p, "temp", ["user1"]); p = removeGroup(p, "temp"); @@ -339,7 +328,7 @@ describe("end-to-end workflows", () => { expect(final.groups?.["group:admin"]).toEqual(["user1", "user2"]); }); - test("optional fields (proto, checkPeriod) survive round-trips", () => { + test("optional fields survive round-trips", () => { let p: AclPolicy = {}; p = addAclRule(p, rule({ proto: "udp", dst: ["*:53"] })); p = addSshRule(p, ssh({ action: "check", checkPeriod: "24h" })); @@ -348,4 +337,51 @@ describe("end-to-end workflows", () => { expect(final.acls?.[0].proto).toBe("udp"); expect(final.ssh?.[0].checkPeriod).toBe("24h"); }); + + test("realistic policy with multiple mutation passes", () => { + let p: AclPolicy = {}; + + p = setGroup(p, "engineering", ["alice", "bob", "charlie"]); + p = setGroup(p, "ops", ["dave", "eve"]); + p = setGroup(p, "contractors", ["frank"]); + p = setHost(p, "prod-db", "100.64.1.10"); + p = setHost(p, "staging-db", "100.64.2.10"); + p = setHost(p, "monitoring", "100.64.3.1"); + p = setTagOwner(p, "production", ["group:ops"]); + p = setTagOwner(p, "staging", ["group:engineering", "group:ops"]); + p = addAclRule(p, rule({ src: ["group:ops"], dst: ["*:*"] })); + p = addAclRule(p, rule({ src: ["group:engineering"], dst: ["tag:staging:*"] })); + p = addAclRule(p, rule({ src: ["group:contractors"], dst: ["tag:staging:443"] })); + p = addSshRule( + p, + ssh({ src: ["group:ops"], dst: ["tag:production"], users: ["root", "ubuntu"] }), + ); + p = addSshRule( + p, + ssh({ + action: "check", + src: ["group:engineering"], + dst: ["tag:staging"], + users: ["ubuntu"], + checkPeriod: "12h", + }), + ); + + // Simulate ongoing edits: rename group, remove contractor access, add host + p = setGroup(removeGroup(p, "contractors"), "external", ["frank", "grace"]); + p = updateAclRule(p, 2, rule({ src: ["group:external"], dst: ["tag:staging:443"] })); + p = setHost(p, "cache", "100.64.3.5"); + p = removeHost(p, "monitoring"); + + const final = parsePolicy(stringifyPolicy(p)); + expect(Object.keys(final.groups ?? {})).toHaveLength(3); + expect(final.groups?.["group:external"]).toEqual(["frank", "grace"]); + expect(final.groups?.["group:contractors"]).toBeUndefined(); + expect(final.hosts?.cache).toBe("100.64.3.5"); + expect(final.hosts?.monitoring).toBeUndefined(); + expect(final.acls).toHaveLength(3); + expect(final.acls?.[2].src).toEqual(["group:external"]); + expect(final.ssh).toHaveLength(2); + expect(final.ssh?.[1].checkPeriod).toBe("12h"); + }); });