mirror of
https://github.com/juanfont/headscale.git
synced 2026-09-25 17:54:54 +09:00
policy: ignore '#' metadata fields across the whole policy
The filter lived in ACL.UnmarshalJSON, so grants, ssh and nodeAttrs still hit RejectUnknownMembers. Strip the members in the HuJSON AST instead, at the single decode entrypoint. Grant "app" payloads are left untouched. Fixes #3479
This commit is contained in:
committed by
Kristoffer Dalby
parent
932b8174f6
commit
5401edb6a8
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user