diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed98550..407d7003 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ keys remain all-access. - Fix tvOS setup instructions: install the VPN configuration before setting the coordination server URL [#3431](https://github.com/juanfont/headscale/pull/3431) - Map requests that only bump LastSeen, endpoints or DERP region no longer resend the whole node to every peer, and health probes that change nothing no longer write. Adds `headscale_mapper_changes_dropped_total` and `headscale_ha_health_updates_total` [#3417](https://github.com/juanfont/headscale/issues/3417) [#3450](https://github.com/juanfont/headscale/pull/3450) - Fix ACME renewal stopping permanently after a `badNonce` reply, because the error logging middleware drained the response body the acme client needs to detect it [#3461](https://github.com/juanfont/headscale/pull/3461) +- Fix `#`-prefixed metadata fields being rejected outside `acls`, so policy editors can store metadata in grants, SSH rules and `nodeAttrs` [#3481](https://github.com/juanfont/headscale/pull/3481) ## 0.29.3 (2026-07-29) diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index b6c0f725..a672e754 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -1831,48 +1831,6 @@ type ACL struct { Destinations []AliasWithPorts `json:"dst"` } -// UnmarshalJSON implements custom unmarshalling for [ACL] that ignores fields starting with '#'. -// headscale-admin uses # in some field names to add metadata, so we will ignore -// those to ensure it doesnt break. -// https://github.com/GoodiesHQ/headscale-admin/blob/214a44a9c15c92d2b42383f131b51df10c84017c/src/lib/common/acl.svelte.ts#L38 -func (a *ACL) UnmarshalJSON(b []byte) error { - // First unmarshal into a map to filter out comment fields - var raw map[string]any - if err := json.Unmarshal(b, &raw, policyJSONOpts...); err != nil { //nolint:noinlineerr - return err - } - - // Remove any fields that start with '#' - filtered := make(map[string]any) - - for key, value := range raw { - if !strings.HasPrefix(key, "#") { - filtered[key] = value - } - } - - // Marshal the filtered map back to JSON - filteredBytes, err := json.Marshal(filtered) - if err != nil { - return err - } - - // Create a type alias to avoid infinite recursion - type aclAlias ACL - - var temp aclAlias - - // Unmarshal into the temporary struct using the v2 JSON options - if err := json.Unmarshal(filteredBytes, &temp, policyJSONOpts...); err != nil { //nolint:noinlineerr - return err - } - - // Copy the result back to the original struct - *a = ACL(temp) - - return nil -} - type Grant struct { // TODO(kradalby): Validate grant src/dst according to ts docs Sources Aliases `json:"src"` @@ -3113,6 +3071,42 @@ func (u *SSHUser) UnmarshalJSON(b []byte) error { return nil } +// stripMetadataMembers removes every object member whose name starts with '#' +// from the policy AST. Policy editors such as headscale-admin store their own +// metadata in such members, and the policy decoder rejects unknown fields, so +// they are dropped before decoding instead of rejected. Deleting a member can +// leave a trailing comma behind; [hujson.Value.Standardize] removes it. +// https://github.com/GoodiesHQ/headscale-admin/blob/214a44a9c15c92d2b42383f131b51df10c84017c/src/lib/common/acl.svelte.ts#L38 +// +// Grant "app" values are opaque capability payloads handed to peers verbatim, +// not policy schema, so they are left untouched. +func stripMetadataMembers(v *hujson.Value) { + switch val := v.Value.(type) { + case *hujson.Object: + kept := val.Members[:0] + + for i := range val.Members { + name, isString := val.Members[i].Name.Value.(hujson.Literal) + if isString && strings.HasPrefix(name.String(), "#") { + continue + } + + if !isString || name.String() != "app" { + stripMetadataMembers(&val.Members[i].Value) + } + + kept = append(kept, val.Members[i]) + } + + val.Members = kept + + case *hujson.Array: + for i := range val.Elements { + stripMetadataMembers(&val.Elements[i]) + } + } +} + // unmarshalPolicy takes a byte slice and unmarshals it into a [Policy] struct. // In addition to unmarshalling, it will also validate the policy. // This is the only entrypoint of reading a policy from a file or other source. @@ -3128,6 +3122,7 @@ func unmarshalPolicy(b []byte) (*Policy, error) { return nil, fmt.Errorf("parsing HuJSON: %w", err) } + stripMetadataMembers(&ast) ast.Standardize() if err = json.Unmarshal(ast.Pack(), &policy, policyJSONOpts...); err != nil { //nolint:noinlineerr diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index c457c94a..bd8461d9 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -3,6 +3,7 @@ package v2 import ( "bytes" "encoding/json" + "fmt" "net/netip" "strings" "testing" @@ -4030,11 +4031,20 @@ func TestACL_UnmarshalJSON_WithCommentFields(t *testing.T) { }, } + // The entry is validated as part of a whole policy, the only path that + // filters '#' members, so groups and tags used below must be declared. + const wrapper = `{ + "groups": {"group:developers": ["user1@example.com"]}, + "tagOwners": { + "tag:client": ["user1@example.com"], + "tag:server": ["user1@example.com"] + }, + "acls": [%s] + }` + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var acl ACL - - err := json.Unmarshal([]byte(tt.input), &acl) + pol, err := unmarshalPolicy(fmt.Appendf(nil, wrapper, tt.input)) if tt.wantErr { assert.Error(t, err) @@ -4042,6 +4052,9 @@ func TestACL_UnmarshalJSON_WithCommentFields(t *testing.T) { } require.NoError(t, err) + require.Len(t, pol.ACLs, 1) + + acl := pol.ACLs[0] assert.Equal(t, tt.expected.Action, acl.Action) assert.Equal(t, tt.expected.Protocol, acl.Protocol) assert.Len(t, acl.Sources, len(tt.expected.Sources)) @@ -6267,3 +6280,124 @@ func TestValidateCapabilityName(t *testing.T) { }) } } + +// TestPolicyMetadataFields covers https://github.com/juanfont/headscale/issues/3479: +// the '#'-prefixed metadata escape hatch added for headscale-admin lives in +// [ACL.UnmarshalJSON] only, so every other policy object still hits +// RejectUnknownMembers and rejects the same metadata. +func TestPolicyMetadataFields(t *testing.T) { + tests := []struct { + name string + policy string + wantErr string + check func(t *testing.T, pol *Policy) + }{ + { + name: "acl", + policy: `{ + "acls": [{ + "#ha-meta": {"name": "web"}, + "action": "accept", + "src": ["*"], + "dst": ["*:80"] + }] + }`, + }, + { + name: "grant", + policy: `{ + "grants": [{ + "#ha-meta": {"name": "web"}, + "src": ["*"], + "dst": ["*"], + "ip": ["tcp:80"] + }] + }`, + }, + { + name: "ssh", + policy: `{ + "ssh": [{ + "#ha-meta": {"name": "admins"}, + "action": "accept", + "src": ["user1@headscale.net"], + "dst": ["autogroup:self"], + "users": ["root"] + }] + }`, + }, + { + name: "nodeattr", + policy: `{ + "nodeAttrs": [{ + "#ha-meta": {"name": "all"}, + "target": ["*"], + "attr": ["randomize-client-port"] + }] + }`, + }, + { + name: "toplevel", + policy: `{ + "#ha-meta": {"version": 1}, + "acls": [{"action": "accept", "src": ["*"], "dst": ["*:80"]}] + }`, + }, + { + name: "metadata as last member, after a comment", + policy: `{ + // a HuJSON comment + "acls": [{ + "action": "accept", + "src": ["*"], + "dst": ["*:80"], + "#ha-meta": {"name": "web"} + }], + "#ha-meta": {"version": 1} + }`, + }, + { + name: "unknown field is still rejected", + policy: `{ + "acls": [{"action": "accept", "src": ["*"], "dst": ["*:80"], "protocol": "tcp"}] + }`, + wantErr: `unknown field: "protocol"`, + }, + { + name: "app capability payload is left alone", + policy: `{ + "grants": [{ + "#ha-meta": {"name": "web"}, + "src": ["*"], + "dst": ["*"], + "app": {"example.com/cap/web": [{"#note": "kept", "domain": ["example.com"]}]} + }] + }`, + check: func(t *testing.T, pol *Policy) { + t.Helper() + + require.Len(t, pol.Grants, 1) + payload := pol.Grants[0].App["example.com/cap/web"] + require.Len(t, payload, 1) + assert.JSONEq(t, `{"#note": "kept", "domain": ["example.com"]}`, string(payload[0])) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pol, err := unmarshalPolicy([]byte(tt.policy)) + + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + + require.NoError(t, err) + + if tt.check != nil { + tt.check(t, pol) + } + }) + } +}