From 992b93f20bb77c3de7779b583e04b6cafbdb5764 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Fri, 17 May 2024 10:43:17 -0700 Subject: [PATCH 01/10] HCPPS-1620 ReadSecret: Listen for canceled context and quit when prompted for read --- internal/pkg/iostreams/io.go | 11 ++++++++++- main.go | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 9fd28cad..52ee06bd 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -156,7 +156,16 @@ func (s *system) ReadSecret() ([]byte, error) { return nil, fmt.Errorf("prompting is disabled") } - return term.ReadPassword(int(s.in.Fd())) + go func() { + select { + case <-s.ctx.Done(): + fmt.Fprintf(s.Err(), "\n%v\n", context.Cause(s.ctx)) + + os.Exit(1) + } + }() + + return term.ReadPassword(int(s.in.Fd())) } func (s *system) PromptConfirm(prompt string) (confirmed bool, err error) { diff --git a/main.go b/main.go index de27f0ad..ebbf1e40 100644 --- a/main.go +++ b/main.go @@ -34,7 +34,7 @@ func main() { func realMain() int { args := os.Args[1:] - // Listen for interupts + // Listen for interrupts shutdownCtx, shutdown := context.WithCancelCause(context.Background()) defer shutdown(nil) go func() { From 449e7332bece29c40be01989fac1876a074b771e Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Fri, 17 May 2024 10:58:57 -0700 Subject: [PATCH 02/10] HCPPS-1620 Formatting --- internal/pkg/iostreams/io.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 52ee06bd..7af2a190 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -156,16 +156,16 @@ func (s *system) ReadSecret() ([]byte, error) { return nil, fmt.Errorf("prompting is disabled") } - go func() { - select { - case <-s.ctx.Done(): - fmt.Fprintf(s.Err(), "\n%v\n", context.Cause(s.ctx)) + go func() { + select { + case <-s.ctx.Done(): + fmt.Fprintf(s.Err(), "\n%v\n", context.Cause(s.ctx)) - os.Exit(1) - } - }() + os.Exit(1) + } + }() - return term.ReadPassword(int(s.in.Fd())) + return term.ReadPassword(int(s.in.Fd())) } func (s *system) PromptConfirm(prompt string) (confirmed bool, err error) { From d9be61f777e59e22d5bb7803f6de0013773161e4 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Fri, 17 May 2024 11:03:32 -0700 Subject: [PATCH 03/10] HCPPS-1620 staticcheck revision --- internal/pkg/iostreams/io.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 7af2a190..cd49ff4e 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -157,12 +157,10 @@ func (s *system) ReadSecret() ([]byte, error) { } go func() { - select { - case <-s.ctx.Done(): - fmt.Fprintf(s.Err(), "\n%v\n", context.Cause(s.ctx)) + <-s.ctx.Done() + fmt.Fprintf(s.Err(), "\n%v\n", context.Cause(s.ctx)) - os.Exit(1) - } + os.Exit(1) }() return term.ReadPassword(int(s.in.Fd())) From 8cf1d860dbaa6fb22641b395385fac6eac12e2c1 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Fri, 17 May 2024 15:38:16 -0700 Subject: [PATCH 04/10] HCPPS-1620 Refactor state handling in ReadSecret --- internal/pkg/iostreams/io.go | 42 ++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index cd49ff4e..1561b2d8 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -12,7 +12,9 @@ import ( "fmt" "io" "os" + "os/signal" "strings" + "syscall" "github.com/muesli/termenv" "golang.org/x/term" @@ -156,14 +158,46 @@ func (s *system) ReadSecret() ([]byte, error) { return nil, fmt.Errorf("prompting is disabled") } + fd := int(s.in.Fd()) + + // Store and restore the terminal status on interruptions to + // avoid that the terminal remains in the password state + // This is necessary as for https://github.com/golang/go/issues/31180 + oldState, err := term.GetState(fd) + if err != nil { + return make([]byte, 0), err + } + + type Buffer struct { + Buffer []byte + Error error + } + errorChannel := make(chan Buffer, 1) + + // SIGINT and SIGTERM restore the terminal, otherwise the no-echo mode would remain intact + interruptChannel := make(chan os.Signal, 1) + signal.Notify(interruptChannel, syscall.SIGINT, syscall.SIGTERM) + defer func() { + signal.Stop(interruptChannel) + close(interruptChannel) + }() go func() { - <-s.ctx.Done() - fmt.Fprintf(s.Err(), "\n%v\n", context.Cause(s.ctx)) + for range interruptChannel { + if oldState != nil { + _ = term.Restore(fd, oldState) + } + errorChannel <- Buffer{Buffer: make([]byte, 0), Error: ErrInterrupt} + } + }() - os.Exit(1) + go func() { + buf, err := term.ReadPassword(fd) + errorChannel <- Buffer{Buffer: buf, Error: err} }() - return term.ReadPassword(int(s.in.Fd())) + buf := <-errorChannel + + return buf.Buffer, buf.Error } func (s *system) PromptConfirm(prompt string) (confirmed bool, err error) { From ee3c2c96aa2d771e74c3c7da332a2a1a694c74f4 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Fri, 17 May 2024 15:41:45 -0700 Subject: [PATCH 05/10] HCPPS-1620 Add missing error var --- internal/pkg/iostreams/io.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 1561b2d8..6407a547 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -9,6 +9,7 @@ package iostreams import ( "bufio" "context" + "errors" "fmt" "io" "os" @@ -20,6 +21,8 @@ import ( "golang.org/x/term" ) +var ErrInterrupt = errors.New("interrupted") + // IOStreams is an interface for interacting with IO and general terminal // output. Commands should not directly interact with os.Stdout/Stderr/Stdin but // utilize the passed IOStreams. From fc96e0a07462136a99c3624856330296f717a39f Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Mon, 20 May 2024 09:35:50 -0700 Subject: [PATCH 06/10] Update internal/pkg/iostreams/io.go Co-authored-by: Alex Dadgar --- internal/pkg/iostreams/io.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 6407a547..4700c1bd 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -168,7 +168,7 @@ func (s *system) ReadSecret() ([]byte, error) { // This is necessary as for https://github.com/golang/go/issues/31180 oldState, err := term.GetState(fd) if err != nil { - return make([]byte, 0), err + return nil, err } type Buffer struct { From 32649504818ead7ad9ab6bc5899e407b54d5c591 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Mon, 20 May 2024 09:45:01 -0700 Subject: [PATCH 07/10] HCPPS-1620 Listen to context rather than duplicate signal channel --- internal/pkg/iostreams/io.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 6407a547..91a0d214 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -13,9 +13,7 @@ import ( "fmt" "io" "os" - "os/signal" "strings" - "syscall" "github.com/muesli/termenv" "golang.org/x/term" @@ -177,20 +175,13 @@ func (s *system) ReadSecret() ([]byte, error) { } errorChannel := make(chan Buffer, 1) - // SIGINT and SIGTERM restore the terminal, otherwise the no-echo mode would remain intact - interruptChannel := make(chan os.Signal, 1) - signal.Notify(interruptChannel, syscall.SIGINT, syscall.SIGTERM) - defer func() { - signal.Stop(interruptChannel) - close(interruptChannel) - }() + // Cancelled context restores the terminal, otherwise the no-echo mode would remain intact go func() { - for range interruptChannel { - if oldState != nil { - _ = term.Restore(fd, oldState) - } - errorChannel <- Buffer{Buffer: make([]byte, 0), Error: ErrInterrupt} + <-s.ctx.Done() + if oldState != nil { + _ = term.Restore(fd, oldState) } + errorChannel <- Buffer{Buffer: make([]byte, 0), Error: ErrInterrupt} }() go func() { From 81cc02d35aa44f3fedab851dcde514d48b396d13 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Mon, 20 May 2024 10:17:40 -0700 Subject: [PATCH 08/10] Update internal/pkg/iostreams/io.go Co-authored-by: Alex Dadgar --- internal/pkg/iostreams/io.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 93d5a90a..9a442da2 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -174,14 +174,21 @@ func (s *system) ReadSecret() ([]byte, error) { Error error } errorChannel := make(chan Buffer, 1) + doneChannel := make(chan struct{}) + defer close(doneChannel) // Cancelled context restores the terminal, otherwise the no-echo mode would remain intact go func() { - <-s.ctx.Done() + select { + case <-doneChannel: + return + case <-s.ctx.Done(): + } + if oldState != nil { _ = term.Restore(fd, oldState) } - errorChannel <- Buffer{Buffer: make([]byte, 0), Error: ErrInterrupt} + errorChannel <- Buffer{Buffer: make([]byte, 0), Error: context.Cause(s.ctx)} }() go func() { From 212f1eb0624970a86a2a9545fc646c9e03bc704f Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Mon, 20 May 2024 10:17:48 -0700 Subject: [PATCH 09/10] Update internal/pkg/iostreams/io.go --- internal/pkg/iostreams/io.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 9a442da2..060c1a31 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -19,7 +19,6 @@ import ( "golang.org/x/term" ) -var ErrInterrupt = errors.New("interrupted") // IOStreams is an interface for interacting with IO and general terminal // output. Commands should not directly interact with os.Stdout/Stderr/Stdin but From 0eae269d6f90829b5f8c885e952d55ec02ed4963 Mon Sep 17 00:00:00 2001 From: Jason Pilz Date: Mon, 20 May 2024 10:19:50 -0700 Subject: [PATCH 10/10] HCPPS-1620 Linting --- internal/pkg/iostreams/io.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/pkg/iostreams/io.go b/internal/pkg/iostreams/io.go index 060c1a31..620b34f4 100644 --- a/internal/pkg/iostreams/io.go +++ b/internal/pkg/iostreams/io.go @@ -9,7 +9,6 @@ package iostreams import ( "bufio" "context" - "errors" "fmt" "io" "os" @@ -19,7 +18,6 @@ import ( "golang.org/x/term" ) - // IOStreams is an interface for interacting with IO and general terminal // output. Commands should not directly interact with os.Stdout/Stderr/Stdin but // utilize the passed IOStreams. @@ -178,12 +176,12 @@ func (s *system) ReadSecret() ([]byte, error) { // Cancelled context restores the terminal, otherwise the no-echo mode would remain intact go func() { - select { + select { case <-doneChannel: return case <-s.ctx.Done(): } - + if oldState != nil { _ = term.Restore(fd, oldState) }