policy/v2: keep the live policy when SetPolicy fails to compile

SetPolicy assigned the new policy before compiling it, so a compile that
failed partway left the rejected filter live while the stored policy stayed
old.
This commit is contained in:
Kristoffer Dalby
2026-09-28 14:03:57 +00:00
parent 3417e4cb77
commit 9ce160ed5b
2 changed files with 61 additions and 1 deletions
+16 -1
View File
@@ -577,9 +577,24 @@ func (pm *PolicyManager) SetPolicy(polB []byte) (bool, error) {
Int("tests.count", len(pol.Tests)).
Msg("Policy parsed successfully")
prev := pm.pol
pm.pol = pol
return pm.updateLocked()
changed, err := pm.updateLocked()
if err != nil {
// updateLocked stops partway, so the rejected policy's filter may
// already be live; recompile the previous one.
pm.pol = prev
_, rerr := pm.updateLocked()
if rerr != nil {
log.Error().Err(rerr).Msg("restoring previous policy after rejected SetPolicy")
}
return false, err
}
return changed, nil
}
// Filter returns the current filter rules for the entire tailnet and the associated matchers.
+45
View File
@@ -85,6 +85,44 @@ func TestPolicyManager(t *testing.T) {
}
}
// TestSetPolicyRejectedKeepsLiveFilter pins that a policy SetPolicy rejects
// does not take effect. A nodeAttrs target naming no user passes validation
// but fails to compile after the filter is already compiled; keeping that
// filter would run the rejected policy while the stored one is unchanged.
func TestSetPolicyRejectedKeepsLiveFilter(t *testing.T) {
// IDs are assigned after construction so the test also builds where
// types.User embeds gorm.Model.
users := types.Users{{Name: "user1"}, {Name: "user2"}}
users[0].ID, users[1].ID = 1, 2
nodes := types.Nodes{
node("n1", "100.64.0.1", "fd7a:115c:a1e0::1", users[0]),
node("n2", "100.64.0.2", "fd7a:115c:a1e0::2", users[1]),
}
nodes[0].ID, nodes[1].ID = 1, 2
pm, err := NewPolicyManager([]byte(`{
"acls": [{"action": "accept", "src": ["user1@"], "dst": ["user1@:*"]}]
}`), users, nodes.ViewSlice())
require.NoError(t, err)
before, _ := pm.Filter()
beforeRules, err := pm.FilterForNode(nodes[1].View())
require.NoError(t, err)
_, err = pm.SetPolicy([]byte(`{
"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}],
"nodeAttrs": [{"target": ["ghost@"], "attr": ["randomize-client-port"]}]
}`))
require.Error(t, err)
after, _ := pm.Filter()
require.Equal(t, before, after, "a rejected policy must not replace the live filter")
afterRules, err := pm.FilterForNode(nodes[1].View())
require.NoError(t, err)
require.Equal(t, beforeRules, afterRules)
}
func TestInvalidateAutogroupSelfCache(t *testing.T) {
users := types.Users{
{ID: 1, Name: "user1", Email: "user1@headscale.net"},
@@ -2420,6 +2458,13 @@ func TestValidateUserReferences_AllSites(t *testing.T) {
"tagOwners": {"tag:ssh": ["alice@"]},
"acls": [{"action":"accept","src":["*"],"dst":["*:*"]}],
"ssh": [{"action":"accept","src":["dup@"],"dst":["tag:ssh"],"users":["root"]}]
}`,
},
{
name: "nodeAttrs.target",
pol: `{
"acls": [{"action":"accept","src":["*"],"dst":["*:*"]}],
"nodeAttrs": [{"target":["dup@"],"attr":["randomize-client-port"]}]
}`,
},
{