From 7547dcb74ea4639231a479cd2b20e01a2a52b59c Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Thu, 5 Dec 2024 10:02:03 -0500 Subject: [PATCH] bugfix: orbit linux zenity progress windows (#24280) --- .github/workflows/fleet-and-orbit.yml | 9 +--- orbit/changes/24212-fix-zenity-progress | 1 + orbit/pkg/dialog/dialog.go | 2 +- orbit/pkg/execuser/execuser.go | 10 ++++ orbit/pkg/execuser/execuser_darwin.go | 4 ++ orbit/pkg/execuser/execuser_linux.go | 29 ++++++++++++ orbit/pkg/execuser/execuser_windows.go | 5 ++ orbit/pkg/luks/luks_linux.go | 30 +++++++----- orbit/pkg/zenity/zenity.go | 52 +++++++-------------- orbit/pkg/zenity/zenity_test.go | 62 ++++--------------------- tools/dialog/main.go | 25 +++++----- 11 files changed, 110 insertions(+), 119 deletions(-) create mode 100644 orbit/changes/24212-fix-zenity-progress diff --git a/.github/workflows/fleet-and-orbit.yml b/.github/workflows/fleet-and-orbit.yml index 22be676c78..2dcdd6e87e 100644 --- a/.github/workflows/fleet-and-orbit.yml +++ b/.github/workflows/fleet-and-orbit.yml @@ -213,13 +213,8 @@ jobs: # Here we generate the Fleet Desktop and osqueryd targets for # macOS which can only be generated from a macOS host. build-macos-targets: - # Set macOS version to '12' (current equivalent to macos-latest) for - # building the binary. This ensures compatibility with macOS version 13 and - # later, avoiding runtime errors on systems using macOS 13 or newer. - # - # Note: Update this version to '13' once GitHub marks macOS 13 as stable - # or if we revise our minimum supported macOS version. - runs-on: macos-12 + # Set macOS version to '13' for building the binary as Fleet's minimum supported macOS version. + runs-on: macos-13 steps: - name: Harden Runner uses: step-security/harden-runner@63c24ba6bd7ba022e95695ff85de572c04a18142 # v2.7.0 diff --git a/orbit/changes/24212-fix-zenity-progress b/orbit/changes/24212-fix-zenity-progress new file mode 100644 index 0000000000..8c105a4a6c --- /dev/null +++ b/orbit/changes/24212-fix-zenity-progress @@ -0,0 +1 @@ +* fixed issue where the linux encryption progress window in zenity was not displaying properly \ No newline at end of file diff --git a/orbit/pkg/dialog/dialog.go b/orbit/pkg/dialog/dialog.go index 362c77b0b6..5a1244a155 100644 --- a/orbit/pkg/dialog/dialog.go +++ b/orbit/pkg/dialog/dialog.go @@ -26,7 +26,7 @@ type Dialog interface { ShowInfo(ctx context.Context, opts InfoOptions) error // Progress displays a dialog that shows progress. It waits until the // context is cancelled. - ShowProgress(ctx context.Context, opts ProgressOptions) error + ShowProgress(opts ProgressOptions) (cancelFunc func() error, err error) } // EntryOptions represents options for a dialog that accepts end user input. diff --git a/orbit/pkg/execuser/execuser.go b/orbit/pkg/execuser/execuser.go index e598bdc2aa..9a6222b121 100644 --- a/orbit/pkg/execuser/execuser.go +++ b/orbit/pkg/execuser/execuser.go @@ -2,6 +2,8 @@ // SYSTEM service on Windows) as the current login user. package execuser +import "io" + type eopts struct { env [][2]string args [][2]string @@ -49,3 +51,11 @@ func RunWithOutput(path string, opts ...Option) (output []byte, exitCode int, er } return runWithOutput(path, o) } + +func RunWithStdin(path string, opts ...Option) (io.WriteCloser, error) { + var o eopts + for _, fn := range opts { + fn(&o) + } + return runWithStdin(path, o) +} diff --git a/orbit/pkg/execuser/execuser_darwin.go b/orbit/pkg/execuser/execuser_darwin.go index 6641b40604..78286ee5db 100644 --- a/orbit/pkg/execuser/execuser_darwin.go +++ b/orbit/pkg/execuser/execuser_darwin.go @@ -54,3 +54,7 @@ func run(path string, opts eopts) (lastLogs string, err error) { func runWithOutput(path string, opts eopts) (output []byte, exitCode int, err error) { return nil, 0, errors.New("not implemented") } + +func runWithStdin(path string, opts eopts) (io.WriteCloser, error) { + return nil, errors.New("not implemented") +} diff --git a/orbit/pkg/execuser/execuser_linux.go b/orbit/pkg/execuser/execuser_linux.go index 5ce487c23c..cbcb222b13 100644 --- a/orbit/pkg/execuser/execuser_linux.go +++ b/orbit/pkg/execuser/execuser_linux.go @@ -79,6 +79,35 @@ func runWithOutput(path string, opts eopts) (output []byte, exitCode int, err er return output, exitCode, nil } +func runWithStdin(path string, opts eopts) (io.WriteCloser, error) { + args, err := getUserAndDisplayArgs(path, opts) + if err != nil { + return nil, fmt.Errorf("get args: %w", err) + } + + args = append(args, path) + + if len(opts.args) > 0 { + for _, arg := range opts.args { + args = append(args, arg[0], arg[1]) + } + } + + cmd := exec.Command("sudo", args...) + log.Printf("cmd=%s", cmd.String()) + + stdin, err := cmd.StdinPipe() + if err != nil { + return nil, fmt.Errorf("stdin pipe: %w", err) + } + + if err := cmd.Start(); err != nil { + return nil, fmt.Errorf("open path %q: %w", path, err) + } + + return stdin, nil +} + func getUserAndDisplayArgs(path string, opts eopts) ([]string, error) { user, err := getLoginUID() if err != nil { diff --git a/orbit/pkg/execuser/execuser_windows.go b/orbit/pkg/execuser/execuser_windows.go index 9cf7e9d338..7dd329409e 100644 --- a/orbit/pkg/execuser/execuser_windows.go +++ b/orbit/pkg/execuser/execuser_windows.go @@ -8,6 +8,7 @@ package execuser import ( "errors" "fmt" + "io" "os" "unsafe" @@ -121,6 +122,10 @@ func runWithOutput(path string, opts eopts) (output []byte, exitCode int, err er return nil, 0, errors.New("not implemented") } +func runWithStdin(path string, opts eopts) (io.WriteCloser, error) { + return nil, errors.New("not implemented") +} + // getCurrentUserSessionId will attempt to resolve // the session ID of the user currently active on // the system. diff --git a/orbit/pkg/luks/luks_linux.go b/orbit/pkg/luks/luks_linux.go index c45cb74ebf..9bea2759fb 100644 --- a/orbit/pkg/luks/luks_linux.go +++ b/orbit/pkg/luks/luks_linux.go @@ -67,7 +67,7 @@ func (lr *LuksRunner) Run(oc *fleet.OrbitConfig) error { if err := removeKeySlot(ctx, devicePath, *keyslot); err != nil { log.Error().Err(err).Msgf("failed to remove key slot %d", *keyslot) } - return fmt.Errorf("Failed to get salt for key slot: %w", err) + response.Err = fmt.Sprintf("Failed to get salt for key slot: %s", err) } response.Salt = salt } @@ -118,13 +118,18 @@ func (lr *LuksRunner) getEscrowKey(ctx context.Context, devicePath string) ([]by return nil, nil, nil } - err = lr.notifier.ShowProgress(ctx, dialog.ProgressOptions{ + cancelProgress, err := lr.notifier.ShowProgress(dialog.ProgressOptions{ Title: infoTitle, Text: "Validating passphrase...", }) if err != nil { log.Error().Err(err).Msg("failed to show progress dialog") } + defer func() { + if err := cancelProgress(); err != nil { + log.Debug().Err(err).Msg("failed to cancel progress dialog") + } + }() // Validate the passphrase for { @@ -147,23 +152,26 @@ func (lr *LuksRunner) getEscrowKey(ctx context.Context, devicePath string) ([]by return nil, nil, nil } - err = lr.notifier.ShowProgress(ctx, dialog.ProgressOptions{ - Title: infoTitle, - Text: "Validating passphrase...", - }) - if err != nil { - log.Error().Err(err).Msg("failed to show progress dialog after retry") - } } - err = lr.notifier.ShowProgress(ctx, dialog.ProgressOptions{ + if err := cancelProgress(); err != nil { + log.Error().Err(err).Msg("failed to cancel progress dialog") + } + + cancelProgress, err = lr.notifier.ShowProgress(dialog.ProgressOptions{ Title: infoTitle, - Text: "Key escrow in progress...", + Text: "Escrowing key...", }) if err != nil { log.Error().Err(err).Msg("failed to show progress dialog") } + defer func() { + if err := cancelProgress(); err != nil { + log.Error().Err(err).Msg("failed to cancel progress dialog") + } + }() + escrowPassphrase, err := generateRandomPassphrase() if err != nil { return nil, nil, fmt.Errorf("Failed to generate random passphrase: %w", err) diff --git a/orbit/pkg/zenity/zenity.go b/orbit/pkg/zenity/zenity.go index 2d2989f9d8..d8dca21944 100644 --- a/orbit/pkg/zenity/zenity.go +++ b/orbit/pkg/zenity/zenity.go @@ -7,9 +7,7 @@ import ( "github.com/fleetdm/fleet/v4/orbit/pkg/dialog" "github.com/fleetdm/fleet/v4/orbit/pkg/execuser" - "github.com/fleetdm/fleet/v4/orbit/pkg/platform" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" - "github.com/rs/zerolog/log" ) const zenityProcessName = "zenity" @@ -18,26 +16,21 @@ type Zenity struct { // cmdWithOutput can be set in tests to mock execution of the dialog. cmdWithOutput func(ctx context.Context, args ...string) ([]byte, int, error) // cmdWithWait can be set in tests to mock execution of the dialog. - cmdWithWait func(ctx context.Context, args ...string) error - // killZenityFunc can be set in tests to mock killing the zenity process. - killZenityFunc func() + cmdWithCancel func(args ...string) (func() error, error) } // New creates a new Zenity dialog instance for zenity v4 on Linux. // Zenity implements the Dialog interface. func New() *Zenity { return &Zenity{ - cmdWithOutput: execCmdWithOutput, - cmdWithWait: execCmdWithWait, - killZenityFunc: killZenityProcesses, + cmdWithOutput: execCmdWithOutput, + cmdWithCancel: execCmdWithCancel, } } // ShowEntry displays an dialog that accepts end user input. It returns the entered // text or errors ErrCanceled, ErrTimeout, or ErrUnknown. func (z *Zenity) ShowEntry(ctx context.Context, opts dialog.EntryOptions) ([]byte, error) { - z.killZenityFunc() - args := []string{"--entry"} if opts.Title != "" { args = append(args, fmt.Sprintf("--title=%s", opts.Title)) @@ -69,8 +62,6 @@ func (z *Zenity) ShowEntry(ctx context.Context, opts dialog.EntryOptions) ([]byt // ShowInfo displays an information dialog. It returns errors ErrTimeout or ErrUnknown. func (z *Zenity) ShowInfo(ctx context.Context, opts dialog.InfoOptions) error { - z.killZenityFunc() - args := []string{"--info"} if opts.Title != "" { args = append(args, fmt.Sprintf("--title=%s", opts.Title)) @@ -95,18 +86,9 @@ func (z *Zenity) ShowInfo(ctx context.Context, opts dialog.InfoOptions) error { return nil } -// ShowProgress starts a Zenity progress dialog with the given options. -// This function is designed to block until the provided context is canceled. -// It is intended to be used within a separate goroutine to avoid blocking -// the main execution flow. -// -// If the context is already canceled, the function will return immediately. -// -// Use this function for cases where a progress dialog is needed to run -// alongside other operations, with explicit cancellation or termination. -func (z *Zenity) ShowProgress(ctx context.Context, opts dialog.ProgressOptions) error { - z.killZenityFunc() - +// ShowProgress starts a Zenity pulsating progress dialog with the given options. +// It returns a cancel function that can be used to cancel the dialog. +func (z *Zenity) ShowProgress(opts dialog.ProgressOptions) (func() error, error) { args := []string{"--progress"} if opts.Title != "" { args = append(args, fmt.Sprintf("--title=%s", opts.Title)) @@ -121,12 +103,15 @@ func (z *Zenity) ShowProgress(ctx context.Context, opts dialog.ProgressOptions) // --no-cancel disables the cancel button args = append(args, "--no-cancel") - err := z.cmdWithWait(ctx, args...) + // --auto-close automatically closes the dialog when stdin is closed + args = append(args, "--auto-close") + + cancel, err := z.cmdWithCancel(args...) if err != nil { - return ctxerr.Wrap(ctx, dialog.ErrUnknown, err.Error()) + return nil, fmt.Errorf("failed to start progress dialog: %w", err) } - return nil + return cancel, nil } func execCmdWithOutput(ctx context.Context, args ...string) ([]byte, int, error) { @@ -143,19 +128,16 @@ func execCmdWithOutput(ctx context.Context, args ...string) ([]byte, int, error) return output, exitCode, err } -func execCmdWithWait(ctx context.Context, args ...string) error { +func execCmdWithCancel(args ...string) (func() error, error) { var opts []execuser.Option for _, arg := range args { opts = append(opts, execuser.WithArg(arg, "")) // Using empty value for positional args } - _, err := execuser.Run(zenityProcessName, opts...) - return err -} - -func killZenityProcesses() { - _, err := platform.KillAllProcessByName(zenityProcessName) + stdin, err := execuser.RunWithStdin(zenityProcessName, opts...) if err != nil { - log.Warn().Err(err).Msg("failed to kill zenity process") + return nil, err } + + return stdin.Close, err } diff --git a/orbit/pkg/zenity/zenity_test.go b/orbit/pkg/zenity/zenity_test.go index f7b2337f8c..5d29d087bd 100644 --- a/orbit/pkg/zenity/zenity_test.go +++ b/orbit/pkg/zenity/zenity_test.go @@ -15,7 +15,6 @@ type mockExecCmd struct { output []byte exitCode int capturedArgs []string - waitDuration time.Duration } // MockCommandContext simulates exec.CommandContext and captures arguments @@ -29,17 +28,10 @@ func (m *mockExecCmd) runWithOutput(ctx context.Context, args ...string) ([]byte return m.output, m.exitCode, nil } -func (m *mockExecCmd) runWithWait(ctx context.Context, args ...string) error { +func (m *mockExecCmd) runWithStdin(args ...string) (func() error, error) { m.capturedArgs = append(m.capturedArgs, args...) - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(m.waitDuration): - - } - - return nil + return nil, nil } func TestShowEntryArgs(t *testing.T) { @@ -76,8 +68,7 @@ func TestShowEntryArgs(t *testing.T) { output: []byte("some output"), } z := &Zenity{ - cmdWithOutput: mock.runWithOutput, - killZenityFunc: func() {}, + cmdWithOutput: mock.runWithOutput, } output, err := z.ShowEntry(ctx, tt.opts) assert.NoError(t, err) @@ -118,8 +109,7 @@ func TestShowEntryError(t *testing.T) { exitCode: tt.exitCode, } z := &Zenity{ - cmdWithOutput: mock.runWithOutput, - killZenityFunc: func() {}, + cmdWithOutput: mock.runWithOutput, } output, err := z.ShowEntry(ctx, dialog.EntryOptions{}) require.ErrorIs(t, err, tt.expectedErr) @@ -135,8 +125,7 @@ func TestShowEntrySuccess(t *testing.T) { output: []byte("some output"), } z := &Zenity{ - cmdWithOutput: mock.runWithOutput, - killZenityFunc: func() {}, + cmdWithOutput: mock.runWithOutput, } output, err := z.ShowEntry(ctx, dialog.EntryOptions{}) assert.NoError(t, err) @@ -171,8 +160,7 @@ func TestShowInfoArgs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { mock := &mockExecCmd{} z := &Zenity{ - cmdWithOutput: mock.runWithOutput, - killZenityFunc: func() {}, + cmdWithOutput: mock.runWithOutput, } err := z.ShowInfo(ctx, tt.opts) assert.NoError(t, err) @@ -207,8 +195,7 @@ func TestShowInfoError(t *testing.T) { exitCode: tt.exitCode, } z := &Zenity{ - cmdWithOutput: mock.runWithOutput, - killZenityFunc: func() {}, + cmdWithOutput: mock.runWithOutput, } err := z.ShowInfo(ctx, dialog.InfoOptions{}) require.ErrorIs(t, err, tt.expectedErr) @@ -217,8 +204,6 @@ func TestShowInfoError(t *testing.T) { } func TestProgressArgs(t *testing.T) { - ctx := context.Background() - testCases := []struct { name string opts dialog.ProgressOptions @@ -230,7 +215,7 @@ func TestProgressArgs(t *testing.T) { Title: "A Title", Text: "Some text", }, - expectedArgs: []string{"--progress", "--title=A Title", "--text=Some text", "--pulsate", "--no-cancel"}, + expectedArgs: []string{"--progress", "--title=A Title", "--text=Some text", "--pulsate", "--no-cancel", "--auto-close"}, }, } @@ -238,38 +223,11 @@ func TestProgressArgs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { mock := &mockExecCmd{} z := &Zenity{ - cmdWithWait: mock.runWithWait, - killZenityFunc: func() {}, + cmdWithCancel: mock.runWithStdin, } - err := z.ShowProgress(ctx, tt.opts) + _, err := z.ShowProgress(tt.opts) assert.NoError(t, err) assert.Equal(t, tt.expectedArgs, mock.capturedArgs) }) } } - -func TestProgressKillOnCancel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - - mock := &mockExecCmd{ - waitDuration: 5 * time.Second, - } - z := &Zenity{ - cmdWithWait: mock.runWithWait, - killZenityFunc: func() {}, - } - - done := make(chan struct{}) - start := time.Now() - - go func() { - _ = z.ShowProgress(ctx, dialog.ProgressOptions{}) - close(done) - }() - - time.Sleep(100 * time.Millisecond) - cancel() - <-done - - assert.True(t, time.Since(start) < 5*time.Second) -} diff --git a/tools/dialog/main.go b/tools/dialog/main.go index 23e46da66c..036aadc369 100644 --- a/tools/dialog/main.go +++ b/tools/dialog/main.go @@ -27,21 +27,20 @@ func main() { panic(err) } - ctx, cancelProgress := context.WithCancel(context.Background()) - - go func() { - err := prompt.ShowProgress(ctx, dialog.ProgressOptions{ - Title: "Zenity Test Progress Title", - Text: "Zenity Test Progress Text", - }) - if err != nil { - fmt.Println("Err ShowProgress") - panic(err) - } - }() + cancelProgress, err := prompt.ShowProgress(dialog.ProgressOptions{ + Title: "Zenity Test Progress Title", + Text: "Zenity Test Progress Text", + }) + if err != nil { + fmt.Println("Err ShowProgress") + panic(err) + } time.Sleep(2 * time.Second) - cancelProgress() + if err := cancelProgress(); err != nil { + fmt.Println("Err cancelProgress") + panic(err) + } err = prompt.ShowInfo(ctx, dialog.InfoOptions{ Title: "Zenity Test Info Title",