feat(BRE2-736): add ability to register device to Brev by patelspratik · Pull Request #295 · brevdev/brev-cli

patelspratik · 2026-02-25T23:26:05Z

Add ability to register/deregister device as External Node. Device should be a linux device (e.g. Spark).
Uses NetBird, not Cloudflare for Tunnel capability
Uses ExternalNode RPC Connect API in dev-plane, NOT brevapi
Add ability to grant SSH access to other users
Add ability to list external nodes
Extremely lightweight, no heartbeats

…nning of SSH sharing.

drewmalin · 2026-03-03T00:06:05Z

pkg/cmd/register/register.go

@@ -1,44 +1,320 @@
-// Package register provides the brev register command for DGX Spark registration
+// Package register provides the brev register command for device registration


drewmalin · 2026-03-03T00:09:39Z

pkg/cmd/register/register.go

+		return breverrors.WrapAndTrace(err)
+	}
+
+	u, _ := user.Current()


Can this not error?

Also, could we call this the osUser or something similar?

drewmalin · 2026-03-03T00:11:34Z

pkg/cmd/register/register.go

+// Confirmer prompts for yes/no confirmation.
+type Confirmer interface {
+	ConfirmYesNo(label string) bool
+}


I wonder if this + the Selector is pointing us to having more of a "Terminal" or "Prompter" interface, as I assume they would generally be satisfied by the same struct.

drewmalin · 2026-03-03T00:20:44Z

pkg/cmd/register/register.go

+	t.Vprint(t.Yellow("[Step 1/3] Installing NetBird..."))
+	if err := deps.netbird.Install(); err != nil {
+		return fmt.Errorf("NetBird installation failed: %w", err)
+	}
+	t.Vprint(t.Green("  NetBird installed successfully."))


For later discussion, but I do wonder about just saying "we are installing the tunnel" (or whatever). Maybe this will always be netbird, but it feels like a little bit of a behind the scenes detail.

drewmalin · 2026-03-03T00:22:52Z

pkg/cmd/register/register.go

+	if ci := node.GetConnectivityInfo(); ci != nil {
+		if cmd := ci.GetRegistrationCommand(); cmd != "" {
+			if err := deps.setupRunner.RunSetup(cmd); err != nil {
+				t.Vprintf("  Warning: setup command failed: %v\n", err)
+			}
+		}
+	}


Should we react to any of these else cases? ci == nil, cmd == ""?

drewmalin · 2026-03-03T00:32:30Z

pkg/cmd/ls/ls.go

+	if ci := n.GetConnectivityInfo(); ci != nil && ci.GetRegistrationCommand() != "" {
+		return "REGISTERED"
+	}
+	return "UNKNOWN"


We now have NetworkMemberStatus

drewmalin · 2026-03-03T00:33:58Z

pkg/cmd/register/netbird.go

+
+// runSetupCommand executes the setup command returned by the AddNode RPC.
+func runSetupCommand(script string) error {
+	cmd := exec.Command("bash", "-c", script) // #nosec G204


I wonder if we should minimally check to see that this command starts with netbird up

I don't mind that.

drewmalin · 2026-03-03T16:49:57Z

pkg/cmd/grantssh/grantssh.go

+// PlatformChecker checks whether the current platform is supported.
+type PlatformChecker interface {
+	IsCompatible() bool
+}


It looks like this same interface is defined several times -- I wonder if we should just have something more general-purpose in the register package here.

ya, I'm thinking of pulling out like an "interfaces.go" inside of one of these packages or something.

drewmalin · 2026-03-03T16:50:59Z

pkg/cmd/grantssh/grantssh.go

+// NodeClientFactory creates ConnectRPC ExternalNodeService clients.
+type NodeClientFactory interface {
+	NewNodeClient(provider register.TokenProvider, baseURL string) nodev1connect.ExternalNodeServiceClient
+}


Same as above -- do we need four different versions of these interfaces?

drewmalin · 2026-03-03T16:52:22Z

pkg/cmd/deregister/deregister.go

+// NetBirdUninstaller uninstalls the NetBird network agent.
+type NetBirdUninstaller interface {
+	Uninstall() error
+}


Could we just have a NetBirdManager (rather than an Installer and Uninstaller)? I don't know that we benefit from individual interfaces here.

ya, I was debating whether it was worth to have this separated out. No strong preference so less code is best code, will fix.

…emory auth store, added reconnect logic, renamed registration, tests

drewmalin · 2026-03-03T22:23:53Z

pkg/externalnode/types.go

+	case nodev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_DISCONNECTED:
+		return "Disconnected"
+	case nodev1.NetworkMemberStatus_NETWORK_MEMBER_STATUS_UNSPECIFIED:
+		return "Registered"


Probably not a bad idea, but "unspecified" is akin to an erroneous state, so this may be fine to put down as "Unknown"

drewmalin · 2026-03-03T22:39:25Z

pkg/cmd/register/register.go

+	if err != nil {
+		t.Vprintf("  %s\n", t.Yellow(fmt.Sprintf("Warning: could not fetch node status: %v", err)))
+	} else {
+		ci := resp.Msg.GetExternalNode().GetConnectivityInfo()


Possible nil deref here on GetExternalNode

drewmalin · 2026-03-03T22:41:01Z

pkg/cmd/register/register.go

+			t.Vprint("  Run 'brev deregister' first if you want to re-register.")
+			return nil
+		}
+		t.Vprintf("  Node status: %s\n", externalnode.FriendlyNetworkStatus(ci.GetStatus()))


Similar here as ci could be nil

drewmalin · 2026-03-03T22:42:59Z

pkg/cmd/register/sshkeys.go

+
+	// If the key exists but isn't tagged, replace it with the tagged version
+	// so that RemoveBrevAuthorizedKeys can find it later.
+	if strings.Contains(string(existing), pubKey) {


While this would be crazy if it led to an actual bug, I wonder if for safety we should do an exact match rather than a Contains

drewmalin · 2026-03-03T22:57:47Z

pkg/cmd/register/register.go

+		return nil, fmt.Errorf("brev register is only supported on Linux")
+	}
+
+	_, err := s.GetCurrentUser() // ensure active token


I wonder if we should unwind this a bit so that runRegister itself does:

check platform is compatible

check if registration is already complete

get current user

get current org

...

Right now it looks like:

check platform

get current user

get org

check if registration is already complete

get current user (again!)

..

ah ya didn't fix after my machinations

drewmalin · 2026-03-03T23:00:58Z

pkg/cmd/enablessh/enablessh.go

+
+// EnableSSH grants SSH access to the given node for the specified Brev user.
+// It is exported so that the register command can reuse it after registration.
+func EnableSSH(


As per this comment, it looks like the register command has its own (mostly identical) copy of this function. Did you intend to use this from there?

drewmalin · 2026-03-03T23:03:52Z

pkg/cmd/grantssh/grantssh.go

+	authKeysPath := filepath.Join(u.HomeDir, ".ssh", "authorized_keys")
+	existing, err := os.ReadFile(authKeysPath) // #nosec G304
+	if err != nil {
+		return fmt.Errorf("SSH has not been enabled on this device. Run 'brev enable-ssh' first.")


Could this be due to read permissions instead of the nonexistence of keys?

oh ya this is a bad error message, fixing

theFong · 2026-03-03T23:12:17Z

pkg/cmd/deregister/deregister.go

+
+	removeTunnel := deps.prompter.Select(
+		"Would you also like to remove the Brev tunnel?",
+		[]string{"Yes, remove Brev tunnel", "No, keep Brev tunnel installed"},


why would you want to keep the brev tunnel installed?

when it was called netbird it made more sense, I agree it now does not

theFong · 2026-03-03T23:33:12Z

pkg/cmd/register/hardware.go

+// unquote removes surrounding double quotes from a string.
+func unquote(s string) string {
+	s = strings.TrimSpace(s)
+	if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' {


can be simplified
return strings.TrimPrefix(strings.TrimSuffix(s, "), ")

oops missed this, will put it in the next

theFong · 2026-03-03T23:43:22Z

pkg/cmd/cmd.go

+	memAuthStore := store.NewMemoryAuthStore()
+	memAuthenticator := auth.StandardLogin("", "", nil)
+	memLoginAuth := auth.NewLoginAuth(memAuthStore, memAuthenticator)
+	memLoginAuth.WithShouldLogin(func() (bool, error) { return true, nil })


this is nice

drewmalin · 2026-03-04T00:06:21Z

pkg/cmd/register/register.go

-		return nil, breverrors.WrapAndTrace(err)
-	}
-
+func getOrgToRegisterFor(s RegisterStore) (*entity.Organization, error) {


This function name ends in a preposition and you should feel bad.

patelspratik force-pushed the BRE2-736-register branch 2 times, most recently from ac1ac18 to f41d40a Compare March 2, 2026 19:09

patelspratik marked this pull request as ready for review March 2, 2026 23:48

patelspratik requested a review from a team as a code owner March 2, 2026 23:48

feat(BRE2-736): registration of user owned linux device to Brev. Begi…

8f67b8c

…nning of SSH sharing.

patelspratik force-pushed the BRE2-736-register branch from 93c012b to 8f67b8c Compare March 3, 2026 00:06

brevdev deleted a comment from codecov bot Mar 3, 2026

claude review feedback

677f49e

drewmalin reviewed Mar 3, 2026

View reviewed changes

review feedback and more: some small interface changes, added an in m…

622a898

…emory auth store, added reconnect logic, renamed registration, tests

drewmalin reviewed Mar 3, 2026

View reviewed changes

Merge branch 'main' into BRE2-736-register

8239775

theFong reviewed Mar 3, 2026

View reviewed changes

review feedback

b627ee5

drewmalin reviewed Mar 4, 2026

View reviewed changes

drewmalin approved these changes Mar 4, 2026

View reviewed changes

patelspratik merged commit 88dc335 into main Mar 4, 2026
9 checks passed

patelspratik deleted the BRE2-736-register branch March 4, 2026 00:08

		@@ -1,44 +1,320 @@
		// Package register provides the brev register command for DGX Spark registration
		// Package register provides the brev register command for device registration

Conversation

patelspratik commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

patelspratik commented Feb 25, 2026 •

edited

Loading