Skip to content

Commit 9d7c2c0

Browse files
drewmalindrewmalin
committed
skip sudo when needed, ask for password when not
1 parent 981923f commit 9d7c2c0

9 files changed

Lines changed: 92 additions & 44 deletions

File tree

‎pkg/cmd/deregister/deregister.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ type deregisterDeps struct {
4848
platform externalnode.PlatformChecker
4949
prompter terminal.Selector
5050
confirmer terminal.Confirmer
51+
gater sudo.Gater
5152
netbird register.NetBirdManager
5253
nodeClients externalnode.NodeClientFactory
5354
registrationStore register.RegistrationStore
@@ -59,6 +60,7 @@ func defaultDeregisterDeps() deregisterDeps {
5960
platform: register.LinuxPlatform{},
6061
prompter: register.TerminalPrompter{},
6162
confirmer: register.TerminalPrompter{},
63+
gater: sudo.Default,
6264
netbird: register.Netbird{},
6365
nodeClients: register.DefaultNodeClientFactory{},
6466
registrationStore: register.NewFileRegistrationStore(),
@@ -100,7 +102,7 @@ func runDeregister(ctx context.Context, t *terminal.Terminal, s DeregisterStore,
100102
return fmt.Errorf("brev deregister is only supported on Linux")
101103
}
102104

103-
if err := sudo.Gate(t, deps.confirmer, "Device deregistration", skipConfirm); err != nil {
105+
if err := deps.gater.Gate(t, deps.confirmer, "Device deregistration", skipConfirm); err != nil {
104106
return fmt.Errorf("sudo issue: %w", err)
105107
}
106108

‎pkg/cmd/deregister/deregister_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/brevdev/brev-cli/pkg/cmd/register"
1515
"github.com/brevdev/brev-cli/pkg/entity"
1616
"github.com/brevdev/brev-cli/pkg/externalnode"
17+
"github.com/brevdev/brev-cli/pkg/sudo"
1718
"github.com/brevdev/brev-cli/pkg/terminal"
1819
)
1920

@@ -136,6 +137,7 @@ func testDeregisterDeps(t *testing.T, svc *fakeNodeService, regStore register.Re
136137
return ""
137138
}},
138139
confirmer: mockConfirmer{confirm: true},
140+
gater: sudo.CachedGater{},
139141
netbird: &mockNetBirdManager{},
140142
nodeClients: mockNodeClientFactory{serverURL: server.URL},
141143
registrationStore: regStore,

‎pkg/cmd/grantssh/grantssh_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,18 @@ func Test_runGrantSSH_HappyPath(t *testing.T) {
237237

238238
var gotReq *nodev1.GrantNodeSSHAccessRequest
239239
svc := &fakeNodeService{
240+
listNodesFn: func(_ *nodev1.ListNodesRequest) (*nodev1.ListNodesResponse, error) {
241+
// Return node with SshAccess so interactive flow gets Linux user options.
242+
return &nodev1.ListNodesResponse{
243+
Items: []*nodev1.ExternalNode{
244+
{
245+
ExternalNodeId: "unode_abc",
246+
Name: "My Spark",
247+
SshAccess: []*nodev1.SSHAccess{{UserId: "user_1", LinuxUser: "ubuntu"}},
248+
},
249+
},
250+
}, nil
251+
},
240252
grantSSHFn: func(req *nodev1.GrantNodeSSHAccessRequest) (*nodev1.GrantNodeSSHAccessResponse, error) {
241253
gotReq = req
242254
return &nodev1.GrantNodeSSHAccessResponse{}, nil

‎pkg/cmd/register/linux_user.go‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎pkg/cmd/register/register.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ type registerDeps struct {
5555
platform externalnode.PlatformChecker
5656
prompter terminal.Confirmer
5757
selector terminal.Selector
58+
gater sudo.Gater
5859
netbird NetBirdManager
5960
setupRunner SetupRunner
6061
nodeClients externalnode.NodeClientFactory
@@ -68,6 +69,7 @@ func defaultRegisterDeps() registerDeps {
6869
platform: LinuxPlatform{},
6970
prompter: p,
7071
selector: p,
72+
gater: sudo.Default,
7173
netbird: Netbird{},
7274
setupRunner: ShellSetupRunner{},
7375
nodeClients: DefaultNodeClientFactory{},
@@ -143,11 +145,11 @@ func runRegister(ctx context.Context, t *terminal.Terminal, s RegisterStore, opt
143145
if !deps.platform.IsCompatible() {
144146
return breverrors.New("brev register is only supported on Linux")
145147
}
146-
if opts.interactive {
147-
if err := sudo.Gate(t, deps.prompter, "Device registration", opts.skipConfirm); err != nil {
148-
return fmt.Errorf("sudo issue: %w", err)
149-
}
150-
} else {
148+
// Always gate on sudo; skip confirmation prompt when non-interactive or --approve.
149+
if err := deps.gater.Gate(t, deps.prompter, "Device registration", !opts.interactive || opts.skipConfirm); err != nil {
150+
return fmt.Errorf("sudo issue: %w", err)
151+
}
152+
if !opts.interactive {
151153
if opts.name == "" || opts.orgName == "" {
152154
return fmt.Errorf("in non-interactive mode --name and --org are required")
153155
}

‎pkg/cmd/register/register_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/brevdev/brev-cli/pkg/entity"
1515
"github.com/brevdev/brev-cli/pkg/externalnode"
16+
"github.com/brevdev/brev-cli/pkg/sudo"
1617
"github.com/brevdev/brev-cli/pkg/terminal"
1718
)
1819

@@ -172,6 +173,7 @@ func testRegisterDeps(t *testing.T, svc *fakeNodeService, regStore RegistrationS
172173
platform: mockPlatform{compatible: true},
173174
prompter: mockConfirmer{confirm: true},
174175
selector: mockSelector{},
176+
gater: sudo.CachedGater{},
175177
netbird: mockNetBirdManager{},
176178
setupRunner: &mockSetupRunner{},
177179
nodeClients: mockNodeClientFactory{serverURL: server.URL},

‎pkg/cmd/upgrade/upgrade.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type upgradeDeps struct {
4343
detector Detector
4444
upgrader Upgrader
4545
confirmer terminal.Confirmer
46+
gater sudo.Gater
4647
skillInstaller SkillInstaller
4748
}
4849

@@ -51,6 +52,7 @@ func defaultUpgradeDeps() upgradeDeps {
5152
detector: SystemDetector{},
5253
upgrader: SystemUpgrader{},
5354
confirmer: register.TerminalPrompter{},
55+
gater: sudo.Default,
5456
skillInstaller: defaultSkillInstaller{},
5557
}
5658
}
@@ -147,7 +149,7 @@ func upgradeViaDirect(t *terminal.Terminal, deps upgradeDeps) (bool, error) {
147149
t.Vprint("This will download the latest release and install it to /usr/local/bin/brev")
148150
t.Vprint("")
149151

150-
if err := sudo.Gate(t, deps.confirmer, "Upgrade", false); err != nil {
152+
if err := deps.gater.Gate(t, deps.confirmer, "Upgrade", false); err != nil {
151153
return false, fmt.Errorf("sudo issue: %w", err)
152154
}
153155

‎pkg/cmd/upgrade/upgrade_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/brevdev/brev-cli/pkg/cmd/version"
88
"github.com/brevdev/brev-cli/pkg/store"
9+
"github.com/brevdev/brev-cli/pkg/sudo"
910
"github.com/brevdev/brev-cli/pkg/terminal"
1011
)
1112

@@ -64,6 +65,7 @@ func Test_runUpgrade_AlreadyUpToDate(t *testing.T) {
6465
detector: mockDetector{method: InstallMethodBrew},
6566
upgrader: upgrader,
6667
confirmer: mockConfirmer{confirm: true},
68+
gater: sudo.CachedGater{},
6769
skillInstaller: skill,
6870
}
6971

@@ -92,6 +94,7 @@ func Test_runUpgrade_BrewPath(t *testing.T) {
9294
detector: mockDetector{method: InstallMethodBrew},
9395
upgrader: upgrader,
9496
confirmer: mockConfirmer{confirm: true},
97+
gater: sudo.CachedGater{},
9598
skillInstaller: skill,
9699
}
97100

@@ -123,6 +126,7 @@ func Test_runUpgrade_DirectPath(t *testing.T) {
123126
detector: mockDetector{method: InstallMethodDirect},
124127
upgrader: upgrader,
125128
confirmer: mockConfirmer{confirm: true},
129+
gater: sudo.CachedGater{},
126130
skillInstaller: skill,
127131
}
128132

@@ -154,6 +158,7 @@ func Test_runUpgrade_UserCancels(t *testing.T) {
154158
detector: mockDetector{method: InstallMethodBrew},
155159
upgrader: upgrader,
156160
confirmer: mockConfirmer{confirm: false},
161+
gater: sudo.CachedGater{},
157162
skillInstaller: skill,
158163
}
159164

@@ -176,6 +181,7 @@ func Test_runUpgrade_VersionCheckFails(t *testing.T) {
176181
detector: mockDetector{method: InstallMethodBrew},
177182
upgrader: &mockUpgrader{},
178183
confirmer: mockConfirmer{confirm: true},
184+
gater: sudo.CachedGater{},
179185
skillInstaller: &mockSkillInstaller{},
180186
}
181187

@@ -198,6 +204,7 @@ func Test_runUpgrade_UpgraderFails(t *testing.T) {
198204
detector: mockDetector{method: InstallMethodBrew},
199205
upgrader: upgrader,
200206
confirmer: mockConfirmer{confirm: true},
207+
gater: sudo.CachedGater{},
201208
skillInstaller: skill,
202209
}
203210

@@ -223,6 +230,7 @@ func Test_runUpgrade_SkillInstallFailure_DoesNotFailUpgrade(t *testing.T) {
223230
detector: mockDetector{method: InstallMethodBrew},
224231
upgrader: upgrader,
225232
confirmer: mockConfirmer{confirm: true},
233+
gater: sudo.CachedGater{},
226234
skillInstaller: skill,
227235
}
228236

‎pkg/sudo/sudo.go‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Package sudo provides utilities for checking and gating on sudo access.
21
package sudo
32

43
import (
@@ -21,27 +20,20 @@ const (
2120
StatusUncached Status = 2
2221
)
2322

24-
// Check returns the current sudo status.
25-
func Check() Status {
26-
if os.Getuid() == 0 {
27-
return StatusRoot
28-
}
29-
if exec.Command("sudo", "-n", "true").Run() == nil { //nolint:gosec // intentional sudo probe
30-
return StatusCached
31-
}
32-
return StatusUncached
23+
// Gater gates execution on sudo availability; callers can use Default or inject a stub in tests.
24+
type Gater interface {
25+
Gate(t *terminal.Terminal, confirmer terminal.Confirmer, reason string, assumeYes bool) error
3326
}
3427

35-
// Gate checks sudo status, informs the user, and asks for confirmation before
36-
// proceeding. Returns nil if the user confirms (or is already root). Returns
37-
// an error if the user declines. When assumeYes is true, the confirmation prompt is skipped.
38-
func Gate(t *terminal.Terminal, confirmer terminal.Confirmer, reason string, assumeYes bool) error {
39-
status := Check()
40-
if status == StatusRoot {
41-
return nil
42-
}
28+
// Default is the Gater used by commands. Tests may replace it with a stub.
29+
var Default Gater = &systemGater{}
4330

44-
if assumeYes {
31+
// systemGater implements Gater using the real sudo check and optional password prompt.
32+
type systemGater struct{}
33+
34+
func (g *systemGater) Gate(t *terminal.Terminal, confirmer terminal.Confirmer, reason string, assumeYes bool) error {
35+
status := check()
36+
if status == StatusRoot {
4537
return nil
4638
}
4739

@@ -54,9 +46,52 @@ func Gate(t *terminal.Terminal, confirmer terminal.Confirmer, reason string, ass
5446
}
5547
t.Vprint("")
5648

57-
if !confirmer.ConfirmYesNo(fmt.Sprintf("%s requires sudo. Continue?", reason)) {
49+
if !assumeYes && !confirmer.ConfirmYesNo(fmt.Sprintf("%s requires sudo. Continue?", reason)) {
5850
return fmt.Errorf("%s canceled by user", reason)
5951
}
6052

53+
if status == StatusUncached {
54+
if exec.Command("sudo", "-n", "-v").Run() != nil { //nolint:gosec // intentional sudo -n -v
55+
if isTTY(os.Stdin) {
56+
cmd := exec.Command("sudo", "-v") //nolint:gosec // intentional sudo -v
57+
cmd.Stdin = os.Stdin
58+
cmd.Stdout = os.Stdout
59+
cmd.Stderr = os.Stderr
60+
if err := cmd.Run(); err != nil {
61+
return fmt.Errorf("sudo authentication failed: %w", err)
62+
}
63+
}
64+
}
65+
}
66+
6167
return nil
6268
}
69+
70+
// check returns the current sudo status.
71+
func check() Status {
72+
if os.Getuid() == 0 {
73+
return StatusRoot
74+
}
75+
if exec.Command("sudo", "-n", "true").Run() == nil { //nolint:gosec // intentional sudo probe
76+
return StatusCached
77+
}
78+
return StatusUncached
79+
}
80+
81+
func isTTY(f *os.File) bool {
82+
if f == nil {
83+
return false
84+
}
85+
info, err := f.Stat()
86+
if err != nil {
87+
return false
88+
}
89+
return (info.Mode() & os.ModeCharDevice) != 0
90+
}
91+
92+
93+
// CachedGater is a Gater that always acts as if sudo is cached (no prompt, no refresh). For tests.
94+
type CachedGater struct{}
95+
96+
func (CachedGater) Gate(*terminal.Terminal, terminal.Confirmer, string, bool) error { return nil }
97+

0 commit comments

Comments
 (0)