From bdc005e518bc253e6c528bff92cd72c701477a5a Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 11:01:45 +0000 Subject: [PATCH] cli: validate offline policy set like policy check --- cmd/headscale/cli/policy.go | 56 ++++++++--------- cmd/headscale/cli/root_test.go | 110 +++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 28 deletions(-) diff --git a/cmd/headscale/cli/policy.go b/cmd/headscale/cli/policy.go index ae69cc8a9..6c34b2d7d 100644 --- a/cmd/headscale/cli/policy.go +++ b/cmd/headscale/cli/policy.go @@ -12,7 +12,6 @@ import ( "github.com/juanfont/headscale/hscontrol/policy" "github.com/juanfont/headscale/hscontrol/types" "github.com/spf13/cobra" - "tailscale.com/types/views" ) const ( @@ -37,6 +36,31 @@ func bypassDatabase() (*db.HSDatabase, error) { return d, nil } +// validateOfflinePolicy checks policyBytes against the database's users and +// nodes as a server-side policy write would. +func validateOfflinePolicy(d *db.HSDatabase, policyBytes []byte) error { + users, err := d.ListUsers(nil) + if err != nil { + return fmt.Errorf("loading users for policy validation: %w", err) + } + + nodes, err := d.ListNodes() + if err != nil { + return fmt.Errorf("loading nodes for policy validation: %w", err) + } + + // SetPolicy enforces user references, tests, and sshTests for writes; + // the constructor permits stored policies whose tests no longer pass. + pm, err := policy.NewPolicyManager(policyBytes, users, nodes.ViewSlice()) + if err != nil { + return fmt.Errorf("parsing policy file: %w", err) + } + + _, err = pm.SetPolicy(policyBytes) + + return err +} + // openBypassDB confirms the destructive bypass action and opens the database // directly. The caller is responsible for closing the returned handle. func openBypassDB(cmd *cobra.Command) (*db.HSDatabase, error) { @@ -139,14 +163,9 @@ var setPolicy = &cobra.Command{ } defer d.Close() - users, err := d.ListUsers(nil) + err = validateOfflinePolicy(d, policyBytes) if err != nil { - return fmt.Errorf("loading users for policy validation: %w", err) - } - - _, err = policy.NewPolicyManager(policyBytes, users, views.Slice[types.NodeView]{}) - if err != nil { - return fmt.Errorf("parsing policy file: %w", err) + return err } _, err = d.SetPolicy(string(policyBytes)) @@ -204,26 +223,7 @@ var checkPolicy = &cobra.Command{ } defer d.Close() - users, err := d.ListUsers(nil) - if err != nil { - return fmt.Errorf("loading users: %w", err) - } - - nodes, err := d.ListNodes() - if err != nil { - return fmt.Errorf("loading nodes: %w", err) - } - - // [policy.NewPolicyManager] validates structure and user references - // but intentionally skips test evaluation (boot path). - // [policy.PolicyManager.SetPolicy] is the user-write boundary and is what runs the - // tests and sshTests blocks. - pm, err := policy.NewPolicyManager(policyBytes, users, nodes.ViewSlice()) - if err != nil { - return fmt.Errorf("parsing policy file: %w", err) - } - - _, err = pm.SetPolicy(policyBytes) + err = validateOfflinePolicy(d, policyBytes) if err != nil { return err } diff --git a/cmd/headscale/cli/root_test.go b/cmd/headscale/cli/root_test.go index fb1fefd11..dc3185ed4 100644 --- a/cmd/headscale/cli/root_test.go +++ b/cmd/headscale/cli/root_test.go @@ -1,7 +1,16 @@ package cli import ( + "fmt" + "os" + "path/filepath" "testing" + + "github.com/juanfont/headscale/hscontrol/db" + "github.com/juanfont/headscale/hscontrol/types" + "github.com/spf13/cobra" + "github.com/spf13/viper" + "github.com/stretchr/testify/require" ) const ( @@ -306,3 +315,104 @@ func TestIsPreReleaseVersion(t *testing.T) { }) } } + +func TestOfflineSetPolicyRunsTests(t *testing.T) { + for _, tt := range []struct { + name string + policy string + invalid bool + }{ + { + name: "passing packet test uses database nodes", + policy: `{ + "acls": [{"action":"accept","src":["alice@"],"dst":["bob@:22"]}], + "tests": [{"src":"alice@","accept":["bob@:22"]}] +}`, + }, + { + name: "failing packet test preserves stored policy", + policy: `{ + "acls": [], + "tests": [{"src":"alice@","accept":["bob@:22"]}] +}`, + invalid: true, + }, + { + name: "failing SSH test preserves stored policy", + policy: `{ + "acls": [{"action":"accept","src":["*"],"dst":["*:*"]}], + "ssh": [{"action":"accept","src":["alice@"],"dst":["autogroup:member"],"users":["root"]}], + "sshTests": [{"src":"bob@","dst":["autogroup:member"],"accept":["root"]}] +}`, + invalid: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + dir := t.TempDir() + configPath := filepath.Join(dir, "config.yaml") + config := fmt.Sprintf(`server_url: https://headscale.example.com +noise: + private_key_path: noise_private.key +prefixes: + v4: 100.64.0.0/10 +database: + type: sqlite3 + sqlite: + path: %q +policy: + mode: database +dns: + magic_dns: false + override_local_dns: false +`, filepath.Join(dir, "headscale.db")) + require.NoError(t, os.WriteFile(configPath, []byte(config), 0o600)) + require.NoError(t, types.LoadConfig(configPath, true)) + + cfg, err := types.LoadServerConfig() + require.NoError(t, err) + + database, err := db.NewHeadscaleDatabase(cfg) + require.NoError(t, err) + + t.Cleanup(func() { _ = database.Close() }) + + for _, name := range []string{"alice", "bob"} { + user := database.CreateUserForTest(name) + database.CreateRegisteredNodeForTest(user, name+"-node") + } + + const oldPolicy = `{"acls":[{"action":"accept","src":["*"],"dst":["*:*"]}]}` + + before, err := database.SetPolicy(oldPolicy) + require.NoError(t, err) + + path := filepath.Join(dir, "policy.hujson") + require.NoError(t, os.WriteFile(path, []byte(tt.policy), 0o600)) + + cmd := &cobra.Command{} + cmd.Flags().String("file", path, "") + cmd.Flags().Bool(bypassFlag, true, "") + cmd.Flags().Bool("force", true, "") + + err = setPolicy.RunE(cmd, nil) + if tt.invalid { + require.ErrorContains(t, err, "test(s) failed") + } else { + require.NoError(t, err) + } + + after, err := database.GetPolicy() + require.NoError(t, err) + + if tt.invalid { + require.Equal(t, before.ID, after.ID) + require.JSONEq(t, oldPolicy, after.Data) + } else { + require.JSONEq(t, tt.policy, after.Data) + } + }) + } +}