From edb2be1a0126aecab98c1579dc5ce12ea4942708 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Thu, 26 Mar 2026 16:43:42 +0000 Subject: [PATCH] cmd/tailscale: improve `tailscale lock` error message if no keys Previously, running `add/remove/revoke-keys` without passing any keys would fail with an unhelpful error: ```console $ tailscale lock revoke-keys generation of recovery AUM failed: sending generate-recovery-aum: 500 Internal Server Error: no provided key is currently trusted ``` or ```console $ tailscale lock revoke-keys generation of recovery AUM failed: sending generate-recovery-aum: 500 Internal Server Error: network-lock is not active ``` Now they fail with a more useful error: ```console $ tailscale lock revoke-keys missing argument, expected one or more tailnet lock keys ``` Fixes #19130 Change-Id: I9d81fe2f5b92a335854e71cbc6928e7e77e537e3 Signed-off-by: Alex Chan --- cmd/tailscale/cli/network-lock.go | 38 ++++++++++++++------------ tstest/integration/integration_test.go | 32 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/cmd/tailscale/cli/network-lock.go b/cmd/tailscale/cli/network-lock.go index 9ec0e1d7f..f741f450d 100644 --- a/cmd/tailscale/cli/network-lock.go +++ b/cmd/tailscale/cli/network-lock.go @@ -305,9 +305,7 @@ func runNetworkLockStatus(ctx context.Context, args []string) error { Name: "add", ShortUsage: "tailscale lock add ...", ShortHelp: "Add one or more trusted signing keys to tailnet lock", - Exec: func(ctx context.Context, args []string) error { - return runNetworkLockModify(ctx, args, nil) - }, + Exec: runNetworkLockAdd, } var nlRemoveArgs struct { @@ -331,6 +329,9 @@ func runNetworkLockRemove(ctx context.Context, args []string) error { if err != nil { return err } + if len(removeKeys) == 0 { + return fmt.Errorf("missing argument, expected one or more tailnet lock keys") + } st, err := localClient.NetworkLockStatus(ctx) if err != nil { return fixTailscaledConnectError(err) @@ -445,7 +446,15 @@ func parseNLArgs(args []string, parseKeys, parseDisablements bool) (keys []tka.K return keys, disablements, nil } -func runNetworkLockModify(ctx context.Context, addArgs, removeArgs []string) error { +func runNetworkLockAdd(ctx context.Context, addArgs []string) error { + addKeys, _, err := parseNLArgs(addArgs, true, false) + if err != nil { + return err + } + if len(addKeys) == 0 { + return fmt.Errorf("missing argument, expected one or more tailnet lock keys") + } + st, err := localClient.NetworkLockStatus(ctx) if err != nil { return fixTailscaledConnectError(err) @@ -454,16 +463,7 @@ func runNetworkLockModify(ctx context.Context, addArgs, removeArgs []string) err return errors.New("tailnet lock is not enabled") } - addKeys, _, err := parseNLArgs(addArgs, true, false) - if err != nil { - return err - } - removeKeys, _, err := parseNLArgs(removeArgs, true, false) - if err != nil { - return err - } - - if err := localClient.NetworkLockModify(ctx, addKeys, removeKeys); err != nil { + if err := localClient.NetworkLockModify(ctx, addKeys, nil); err != nil { return err } return nil @@ -819,13 +819,17 @@ func wrapAuthKey(ctx context.Context, keyStr string, status *ipnstate.Status) er func runNetworkLockRevokeKeys(ctx context.Context, args []string) error { // First step in the process if !nlRevokeKeysArgs.cosign && !nlRevokeKeysArgs.finish { - removeKeys, _, err := parseNLArgs(args, true, false) + revokeKeys, _, err := parseNLArgs(args, true, false) if err != nil { return err } - keyIDs := make([]tkatype.KeyID, len(removeKeys)) - for i, k := range removeKeys { + if len(revokeKeys) == 0 { + return fmt.Errorf("missing argument, expected one or more tailnet lock keys") + } + + keyIDs := make([]tkatype.KeyID, len(revokeKeys)) + for i, k := range revokeKeys { keyIDs[i], err = k.ID() if err != nil { return fmt.Errorf("generating keyID: %v", err) diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 19f9fa159..74c9c745b 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -2374,6 +2374,38 @@ func TestTailnetLock(t *testing.T) { t.Fatalf("ping node3 -> signing1: expected success, got err: %v", err) } }) + + // If you run `tailscale lock (add|remove|revoke-keys)` but don't pass any keys, + // we print a helpful error message. + // + // Regression test for tailscale/tailscale#19130 + t.Run("no-keys-is-error", func(t *testing.T) { + for _, verb := range []string{"add", "remove", "revoke-keys"} { + t.Run(verb, func(t *testing.T) { + tstest.Shard(t) + t.Parallel() + + env := NewTestEnv(t) + n1 := NewTestNode(t, env) + d1 := n1.StartDaemon() + defer d1.MustCleanShutdown(t) + + n1.MustUp() + n1.AwaitRunning() + + revokeCmd := n1.Tailscale("lock", verb) + out, err := revokeCmd.CombinedOutput() + if err == nil { + t.Fatal("expected command to fail, but succeeded") + } + want := "missing argument" + got := string(out) + if !strings.Contains(string(out), want) { + t.Fatalf("expected output to contain %q, got %q", want, got) + } + }) + } + }) } func TestNodeWithBadStateFile(t *testing.T) {