feat(BRE2-818): Standardize register, deregister, enablesssh (#325)

* interactive vs prompt mode

* save progress

* fix uninstall

* fmt

* consistent name printing

* consistent flag driven

* consolidate modes

* pattern for interacctive

* lint

* lint

* approve flag

* lint and bugfix

* fix test

* feat(BRE2-816): grant/revoke runnable from anywhere

* merge conflict

* fix enable-ssh port collision

* progress

* save progress

* remove caching of linux user

* grant revoke

* bugs

* make fmt lint

* more spacing

* consistent spinners

* skip sudo when needed, ask for password when not

* tests

* fix tests

---------

Co-authored-by: Pratik Patel <pratpatel@nvidia.com>

Author: drewmalin

pkg/cmd/deregister/deregister.go

@@ -48,6 +48,7 @@ type deregisterDeps struct {
 	platform          externalnode.PlatformChecker
 	prompter          terminal.Selector
 	confirmer         terminal.Confirmer
+	gater             sudo.Gater
 	netbird           register.NetBirdManager
 	nodeClients       externalnode.NodeClientFactory
 	registrationStore register.RegistrationStore
@@ -59,6 +60,7 @@ func defaultDeregisterDeps() deregisterDeps {
 		platform:          register.LinuxPlatform{},
 		prompter:          register.TerminalPrompter{},
 		confirmer:         register.TerminalPrompter{},
+		gater:             sudo.Default,
 		netbird:           register.Netbird{},
 		nodeClients:       register.DefaultNodeClientFactory{},
 		registrationStore: register.NewFileRegistrationStore(),
@@ -76,6 +78,8 @@ the Brev tunnel (network agent).`
 )
 
 func NewCmdDeregister(t *terminal.Terminal, store DeregisterStore) *cobra.Command {
+	var approveFlag bool
+
 	cmd := &cobra.Command{
 		Annotations:           map[string]string{"configuration": ""},
 		Use:                   "deregister",
@@ -84,65 +88,110 @@ func NewCmdDeregister(t *terminal.Terminal, store DeregisterStore) *cobra.Comman
 		Long:                  deregisterLong,
 		Example:               deregisterExample,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runDeregister(cmd.Context(), t, store, defaultDeregisterDeps())
+			return runDeregister(cmd.Context(), t, store, defaultDeregisterDeps(), approveFlag)
 		},
 	}
 
+	cmd.Flags().BoolVar(&approveFlag, "approve", false, "skip confirmation prompt (assume yes)")
+
 	return cmd
 }
 
-func runDeregister(ctx context.Context, t *terminal.Terminal, s DeregisterStore, deps deregisterDeps) error { //nolint:funlen // deregistration flow
+func runDeregister(ctx context.Context, t *terminal.Terminal, s DeregisterStore, deps deregisterDeps, skipConfirm bool) error { //nolint:funlen,gocyclo // deregistration flow
 	if !deps.platform.IsCompatible() {
 		return fmt.Errorf("brev deregister is only supported on Linux")
 	}
 
-	if err := sudo.Gate(t, deps.confirmer, "Device deregistration"); err != nil {
+	if err := deps.gater.Gate(t, deps.confirmer, "Device deregistration", skipConfirm); err != nil {
 		return fmt.Errorf("sudo issue: %w", err)
 	}
 
 	reg, err := deps.registrationStore.Load()
 	if err != nil {
+		return err //nolint:wrapcheck // do not present stack trace for this error
+	}
+
+	// Only prompt for login when there is a device to deregister.
+	if _, err := s.GetCurrentUser(); err != nil {
 		return breverrors.WrapAndTrace(err)
 	}
 
+	orgName := reg.OrgName
+	if orgName == "" {
+		orgName = "(unknown)"
+	}
+	osUser, _ := user.Current()
+	linuxUser := "(unknown)"
+	if osUser != nil {
+		linuxUser = osUser.Username
+	}
+
 	t.Vprint("")
-	t.Vprint(t.Green("Deregistering device"))
+	t.Vprint(t.White("══════════════════════════════════════════════════"))
+	t.Vprint(t.White("  Deregistering your device from Brev"))
+	t.Vprint(t.White("══════════════════════════════════════════════════"))
+	t.Vprint("")
+	if !skipConfirm {
+		t.Vprint(t.Green("  Please confirm before continuing:"))
+		t.Vprint("")
+	}
+	t.Vprintf("  %s %s\n", t.Green(fmt.Sprintf("%-14s", "Device:")), t.BoldBlue(reg.DisplayName+" ("+reg.ExternalNodeID+")"))
+	t.Vprintf("  %s %s\n", t.Green(fmt.Sprintf("%-14s", "Organization:")), t.BoldBlue(orgName+" ("+reg.OrgID+")"))
+	t.Vprintf("  %s %s\n", t.Green(fmt.Sprintf("%-14s", "Linux user:")), t.BoldBlue(linuxUser))
 	t.Vprint("")
-	t.Vprintf("  Node ID: %s\n", reg.ExternalNodeID)
-	t.Vprintf("  Name:    %s\n", reg.DisplayName)
+	t.Vprint(t.Yellow("  This will:"))
+	t.Vprint("    1. Remove this node from Brev")
+	t.Vprint("    2. Remove Brev SSH keys from this machine (if any)")
+	t.Vprint("    3. Uninstall the Brev tunnel")
+	t.Vprint("    4. Delete local registration data")
 	t.Vprint("")
 
-	confirm := deps.prompter.Select(
-		"Proceed with deregistration?",
-		[]string{"Yes, proceed", "No, cancel"},
-	)
-	if confirm != "Yes, proceed" {
-		t.Vprint("Deregistration canceled.")
-		return nil
+	if !skipConfirm {
+		confirm := deps.prompter.Select(
+			"Proceed with deregistration?",
+			[]string{"Yes, proceed", "No, cancel"},
+		)
+		if confirm != "Yes, proceed" {
+			t.Vprint("Deregistration canceled.")
+			return nil
+		}
 	}
 
-	t.Vprint("")
-	t.Vprint(t.Yellow("Removing node from Brev..."))
+	const clearLine = "\033[2K\n"
+
+	t.Vprint(t.Yellow("[Step 1/4] Removing node from Brev..."))
+	sp := t.NewSpinner()
+	sp.Suffix = " Deregistering device..."
+	sp.Start()
 	client := deps.nodeClients.NewNodeClient(s, config.GlobalConfig.GetBrevPublicAPIURL())
-	if _, err := client.RemoveNode(ctx, connect.NewRequest(&nodev1.RemoveNodeRequest{
+	_, err = client.RemoveNode(ctx, connect.NewRequest(&nodev1.RemoveNodeRequest{
 		ExternalNodeId: reg.ExternalNodeID,
-	})); err != nil {
+	}))
+	sp.FinalMSG = clearLine
+	sp.Stop()
+	if err != nil {
 		return fmt.Errorf("failed to deregister node: %w", err)
 	}
-	t.Vprint(t.Green("  Node removed from Brev."))
+	t.Vprintf("%s  Node removed from Brev.\n", t.Green("  ✓"))
 	t.Vprint("")
 
-	// Remove Brev SSH keys from authorized_keys.
-	osUser, err := user.Current()
-	if err != nil {
-		t.Vprintf("  Warning: could not determine current user for SSH key cleanup: %v\n", err)
+	t.Vprint(t.Yellow("[Step 2/4] Removing Brev SSH keys..."))
+	sp = t.NewSpinner()
+	sp.Suffix = " Deregistering device..."
+	sp.Start()
+	if osUser == nil {
+		sp.FinalMSG = clearLine
+		sp.Stop()
+		t.Vprintf("  %s\n", t.Yellow("Skipped: could not determine current user"))
 	} else {
 		removed, kerr := deps.sshKeys.RemoveBrevKeys(osUser)
+		sp.FinalMSG = clearLine
+		sp.Stop()
 		switch {
 		case kerr != nil:
-			t.Vprintf("  Warning: failed to remove Brev SSH keys: %v\n", kerr)
+			t.Vprintf("  %s\n", t.Yellow(fmt.Sprintf("Warning: failed to remove Brev SSH keys: %v", kerr)))
 		case len(removed) > 0:
-			t.Vprint(t.Green("  Brev SSH keys removed from authorized_keys:"))
+			t.Vprintf("%s  Brev SSH keys removed from authorized_keys:\n", t.Green("  ✓"))
 			for _, key := range removed {
 				t.Vprintf("    - %s\n", key)
 			}
@@ -152,21 +201,34 @@ func runDeregister(ctx context.Context, t *terminal.Terminal, s DeregisterStore,
 	}
 	t.Vprint("")
 
-	t.Vprint("Removing Brev tunnel...")
-	if err := deps.netbird.Uninstall(); err != nil {
-		t.Vprintf("  Warning: failed to remove Brev tunnel: %v\n", err)
+	t.Vprint(t.Yellow("[Step 3/4] Removing Brev tunnel..."))
+	sp = t.NewSpinner()
+	sp.Suffix = " Deregistering device..."
+	sp.Start()
+	err = deps.netbird.Uninstall()
+	sp.FinalMSG = clearLine
+	sp.Stop()
+	if err != nil {
+		t.Vprintf("  %s\n", t.Yellow(fmt.Sprintf("Warning: failed to remove Brev tunnel: %v", err)))
 	} else {
-		t.Vprint(t.Green("  Brev tunnel removed."))
+		t.Vprintf("%s  Brev tunnel removed.\n", t.Green("  ✓"))
 	}
 	t.Vprint("")
 
-	t.Vprint("Removing registration data...")
-	if err := deps.registrationStore.Delete(); err != nil {
-		t.Vprintf("  Warning: failed to remove local registration file: %v\n", err)
+	t.Vprint(t.Yellow("[Step 4/4] Removing registration data..."))
+	sp = t.NewSpinner()
+	sp.Suffix = " Deregistering device..."
+	sp.Start()
+	err = deps.registrationStore.Delete()
+	sp.FinalMSG = clearLine
+	sp.Stop()
+	if err != nil {
+		t.Vprintf("  %s\n", t.Yellow(fmt.Sprintf("Warning: failed to remove local registration file: %v", err)))
 		t.Vprint("  You can manually remove it with: rm /etc/brev/device_registration.json")
+	} else {
+		t.Vprintf("%s  Registration data removed.\n", t.Green("  ✓"))
 	}
-
-	t.Vprint(t.Green("Deregistration complete."))
+	t.Vprintf("%s  Deregistration complete.\n", t.Green("  ✓"))
 	t.Vprint("")
 
 	return nil

pkg/cmd/deregister/deregister_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/brevdev/brev-cli/pkg/cmd/register"
 	"github.com/brevdev/brev-cli/pkg/entity"
 	"github.com/brevdev/brev-cli/pkg/externalnode"
+	"github.com/brevdev/brev-cli/pkg/sudo"
 	"github.com/brevdev/brev-cli/pkg/terminal"
 )
 
@@ -136,6 +137,7 @@ func testDeregisterDeps(t *testing.T, svc *fakeNodeService, regStore register.Re
 			return ""
 		}},
 		confirmer:         mockConfirmer{confirm: true},
+		gater:             sudo.CachedGater{},
 		netbird:           &mockNetBirdManager{},
 		nodeClients:       mockNodeClientFactory{serverURL: server.URL},
 		registrationStore: regStore,
@@ -171,7 +173,7 @@ func Test_runDeregister_HappyPath(t *testing.T) {
 	defer server.Close()
 
 	term := terminal.New()
-	err := runDeregister(context.Background(), term, store, deps)
+	err := runDeregister(context.Background(), term, store, deps, false)
 	if err != nil {
 		t.Fatalf("runDeregister failed: %v", err)
 	}
@@ -214,7 +216,7 @@ func Test_runDeregister_UserCancels(t *testing.T) {
 	}}
 
 	term := terminal.New()
-	err := runDeregister(context.Background(), term, store, deps)
+	err := runDeregister(context.Background(), term, store, deps, false)
 	if err != nil {
 		t.Fatalf("expected nil error on cancel, got: %v", err)
 	}
@@ -243,7 +245,7 @@ func Test_runDeregister_NotRegistered(t *testing.T) {
 	defer server.Close()
 
 	term := terminal.New()
-	err := runDeregister(context.Background(), term, store, deps)
+	err := runDeregister(context.Background(), term, store, deps, false)
 	if err == nil {
 		t.Fatal("expected error when not registered")
 	}
@@ -274,7 +276,7 @@ func Test_runDeregister_RemoveNodeFails(t *testing.T) {
 	defer server.Close()
 
 	term := terminal.New()
-	err := runDeregister(context.Background(), term, store, deps)
+	err := runDeregister(context.Background(), term, store, deps, false)
 	if err == nil {
 		t.Fatal("expected error when RemoveNode fails")
 	}
@@ -316,7 +318,7 @@ func Test_runDeregister_AlwaysUninstallsNetbird(t *testing.T) {
 	deps.netbird = netbird
 
 	term := terminal.New()
-	err := runDeregister(context.Background(), term, store, deps)
+	err := runDeregister(context.Background(), term, store, deps, false)
 	if err != nil {
 		t.Fatalf("runDeregister failed: %v", err)
 	}
@@ -363,7 +365,7 @@ func Test_runDeregister_RemoveBrevKeysHandling(t *testing.T) {
 			deps.sshKeys = tt.sshKeys
 
 			term := terminal.New()
-			err := runDeregister(context.Background(), term, store, deps)
+			err := runDeregister(context.Background(), term, store, deps, false)
 			if err != nil {
 				t.Fatalf("runDeregister failed: %v", err)
 			}

pkg/cmd/enablessh/enablessh.go

@@ -8,7 +8,11 @@ import (
 	"os/exec"
 	"os/user"
 
+	nodev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1"
+	"connectrpc.com/connect"
+
 	"github.com/brevdev/brev-cli/pkg/cmd/register"
+	"github.com/brevdev/brev-cli/pkg/config"
 	"github.com/brevdev/brev-cli/pkg/entity"
 	breverrors "github.com/brevdev/brev-cli/pkg/errors"
 	"github.com/brevdev/brev-cli/pkg/externalnode"
@@ -83,10 +87,11 @@ func enableSSH(
 	reg *register.DeviceRegistration,
 	brevUser *entity.User,
 ) error {
-	u, err := user.Current()
+	linuxUser, err := user.Current()
 	if err != nil {
 		return fmt.Errorf("failed to determine current Linux user: %w", err)
 	}
+	linuxUsername := linuxUser.Username
 
 	checkSSHDaemon(t)
 
@@ -95,26 +100,58 @@ func enableSSH(
 	t.Vprint("")
 	t.Vprintf("  Node:       %s (%s)\n", reg.DisplayName, reg.ExternalNodeID)
 	t.Vprintf("  Brev user:  %s\n", brevUser.ID)
-	t.Vprintf("  Linux user: %s\n", u.Username)
+	t.Vprintf("  Linux user: %s\n", linuxUsername)
 	t.Vprint("")
 
-	port, err := register.PromptSSHPort(t)
+	// Check if the node already has an SSH port allocated (e.g. for another linux user)
+	port, err := existingSSHPort(ctx, deps, tokenProvider, reg)
 	if err != nil {
-		return fmt.Errorf("SSH port: %w", err)
+		t.Vprintf("  %s\n", t.Yellow(fmt.Sprintf("Warning: could not check for existing ports: %v", err)))
 	}
 
-	if err := register.OpenSSHPort(ctx, t, deps.nodeClients, tokenProvider, reg, port); err != nil {
-		return fmt.Errorf("enable SSH failed: %w", err)
+	if port != 0 {
+		t.Vprintf("  Using existing SSH port %d.\n", port)
+	} else {
+		t.Vprint("")
+		port, err = register.PromptSSHPort(t)
+		if err != nil {
+			return fmt.Errorf("SSH port: %w", err)
+		}
+
+		if err := register.OpenSSHPort(ctx, t, deps.nodeClients, tokenProvider, reg, port); err != nil {
+			return fmt.Errorf("enable SSH failed: %w", err)
+		}
 	}
 
-	if err := register.GrantSSHAccessToNode(ctx, t, deps.nodeClients, tokenProvider, reg, brevUser, u); err != nil {
+	if err := register.SetupAndRegisterNodeSSHAccess(ctx, t, deps.nodeClients, tokenProvider, reg, brevUser, linuxUsername); err != nil {
 		return fmt.Errorf("enable SSH failed: %w", err)
 	}
 
 	t.Vprint(t.Green(fmt.Sprintf("SSH access enabled. You can now SSH to this device via: brev shell %s", reg.DisplayName)))
 	return nil
 }
 
+// existingSSHPort calls GetNode and returns the PortNumber of an already-allocated
+// SSH port, or 0 if none exists
+func existingSSHPort(ctx context.Context, deps enableSSHDeps, tokenProvider externalnode.TokenProvider, reg *register.DeviceRegistration) (int32, error) {
+	client := deps.nodeClients.NewNodeClient(tokenProvider, config.GlobalConfig.GetBrevPublicAPIURL())
+	resp, err := client.GetNode(ctx, connect.NewRequest(&nodev1.GetNodeRequest{
+		ExternalNodeId: reg.ExternalNodeID,
+		OrganizationId: reg.OrgID,
+	}))
+	if err != nil {
+		return 0, fmt.Errorf("error retrieving node: %w", err)
+	}
+
+	for _, p := range resp.Msg.GetExternalNode().GetPorts() {
+		// TODO if we ever allow more than one SSH port, this should be modified
+		if p.GetProtocol() == nodev1.PortProtocol_PORT_PROTOCOL_SSH {
+			return p.GetPortNumber(), nil
+		}
+	}
+	return 0, nil
+}
+
 // checkSSHDaemon prints a warning if neither "ssh" nor "sshd" systemd services
 // appear to be active. It never returns an error — it is best-effort.
 func checkSSHDaemon(t *terminal.Terminal) {