diff --git a/CLAUDE.md b/CLAUDE.md index a95cf08..d40ff70 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,6 +64,12 @@ Custom `SCAAccessService` follows SDK conventions: - `--refresh` bypasses eligibility cache on `grant` and `grant env` - `fetchEligibility()` and `resolveTargetCSP()` in `cmd/root.go` — shared by root, env, and favorites +## TTY Detection +- `internal/ui/tty.go` — `IsTerminalFunc` (overridable), `IsInteractive()`, `ErrNotInteractive` +- All interactive prompts (`SelectTarget`, `SelectSessions`, `ConfirmRevocation`, `SelectGroup`, `uiUnifiedSelector.SelectItem`, `surveyNamePrompter.PromptName`) fail fast with `ErrNotInteractive` when stdin is not a TTY +- Error messages suggest the appropriate non-interactive flag (e.g., `--target/--role`, `--all`, `--yes`, `--group`, `--favorite`) +- `go-isatty` v0.0.20 is a direct dependency (promoted from indirect via survey) + ## Cache - Eligibility responses cached in `~/.grant/cache/` as JSON files (e.g., `eligibility_azure.json`, `groups_eligibility_azure.json`) - Default TTL: 4 hours, configurable via `cache_ttl` in `~/.grant/config.yaml` (Go duration syntax: `2h`, `30m`) diff --git a/cmd/favorites.go b/cmd/favorites.go index d94f592..8ba4d39 100644 --- a/cmd/favorites.go +++ b/cmd/favorites.go @@ -8,6 +8,7 @@ import ( survey "github.com/Iilun/survey/v2" "github.com/aaearon/grant-cli/internal/config" + "github.com/aaearon/grant-cli/internal/ui" "github.com/spf13/cobra" ) @@ -95,6 +96,10 @@ func newFavoritesAddCommand() *cobra.Command { type surveyNamePrompter struct{} func (s *surveyNamePrompter) PromptName() (string, error) { + if !ui.IsInteractive() { + return "", fmt.Errorf("%w; provide the name as an argument", ui.ErrNotInteractive) + } + var name string if err := survey.AskOne(&survey.Input{ Message: "Favorite name:", diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go index 06fc181..2fee498 100644 --- a/cmd/favorites_test.go +++ b/cmd/favorites_test.go @@ -11,6 +11,7 @@ import ( "github.com/aaearon/grant-cli/internal/cache" "github.com/aaearon/grant-cli/internal/config" "github.com/aaearon/grant-cli/internal/sca/models" + "github.com/aaearon/grant-cli/internal/ui" ) func TestFavoritesListCommand(t *testing.T) { @@ -1183,3 +1184,21 @@ func TestFavoritesListWithGroupFavorites(t *testing.T) { }) } } + +func TestSurveyNamePrompter_NonTTY(t *testing.T) { + original := ui.IsTerminalFunc + defer func() { ui.IsTerminalFunc = original }() + ui.IsTerminalFunc = func(fd uintptr) bool { return false } + + prompter := &surveyNamePrompter{} + _, err := prompter.PromptName() + if err == nil { + t.Fatal("expected error for non-TTY") + } + if !errors.Is(err, ui.ErrNotInteractive) { + t.Errorf("expected ErrNotInteractive, got: %v", err) + } + if !strings.Contains(err.Error(), "argument") { + t.Errorf("error should suggest providing name as argument, got: %v", err) + } +} diff --git a/cmd/root.go b/cmd/root.go index 5868fed..ad93846 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -830,6 +830,10 @@ func (s *uiSelector) SelectTarget(targets []models.EligibleTarget) (*models.Elig type uiUnifiedSelector struct{} func (s *uiUnifiedSelector) SelectItem(items []selectionItem) (*selectionItem, error) { + if !ui.IsInteractive() { + return nil, fmt.Errorf("%w; use --target/--role, --group, or --favorite flags for non-interactive mode", ui.ErrNotInteractive) + } + if len(items) == 0 { return nil, errors.New("no eligible targets or groups available") } diff --git a/cmd/root_test.go b/cmd/root_test.go index d364f95..a75ed42 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -6,6 +6,8 @@ import ( "strings" "testing" + "github.com/aaearon/grant-cli/internal/sca/models" + "github.com/aaearon/grant-cli/internal/ui" "github.com/cyberark/idsec-sdk-golang/pkg/config" "github.com/spf13/cobra" ) @@ -219,3 +221,30 @@ func TestExecuteHintOutput(t *testing.T) { }) } } + +func TestUnifiedSelector_NonTTY(t *testing.T) { + original := ui.IsTerminalFunc + defer func() { ui.IsTerminalFunc = original }() + ui.IsTerminalFunc = func(fd uintptr) bool { return false } + + selector := &uiUnifiedSelector{} + items := []selectionItem{ + {kind: selectionCloud, cloud: &models.EligibleTarget{ + WorkspaceName: "Sub A", + WorkspaceType: models.WorkspaceTypeSubscription, + RoleInfo: models.RoleInfo{Name: "Owner"}, + }}, + } + + _, err := selector.SelectItem(items) + if err == nil { + t.Fatal("expected error for non-TTY") + } + if !errors.Is(err, ui.ErrNotInteractive) { + t.Errorf("expected ErrNotInteractive, got: %v", err) + } + errMsg := err.Error() + if !strings.Contains(errMsg, "--target") || !strings.Contains(errMsg, "--group") || !strings.Contains(errMsg, "--favorite") { + t.Errorf("error should mention --target/--role, --group, and --favorite, got: %v", err) + } +} diff --git a/go.mod b/go.mod index f82b37d..3880ea3 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/Iilun/survey/v2 v2.5.3 github.com/blang/semver v3.5.1+incompatible github.com/cyberark/idsec-sdk-golang v0.1.14 + github.com/mattn/go-isatty v0.0.20 github.com/rhysd/go-github-selfupdate v1.2.3 github.com/spf13/cobra v1.9.1 gopkg.in/yaml.v3 v3.0.1 @@ -36,7 +37,6 @@ require ( github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/mattn/go-colorable v0.1.14 // indirect - github.com/mattn/go-isatty v0.0.20 // indirect github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect github.com/mtibben/percent v0.2.1 // indirect github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go index 7a60a7d..8087dba 100644 --- a/internal/ui/group_selector.go +++ b/internal/ui/group_selector.go @@ -47,6 +47,10 @@ func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (* // It sorts a copy of the groups so that FindGroupByDisplay searches the same // ordered slice the user saw, avoiding wrong-group selection on display collisions. func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) { + if !IsInteractive() { + return nil, fmt.Errorf("%w; use --group flag for non-interactive mode", ErrNotInteractive) + } + if len(groups) == 0 { return nil, errors.New("no eligible groups available") } diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go index 5597e80..609e9dd 100644 --- a/internal/ui/group_selector_test.go +++ b/internal/ui/group_selector_test.go @@ -1,6 +1,8 @@ package ui import ( + "errors" + "strings" "testing" "github.com/aaearon/grant-cli/internal/sca/models" @@ -196,3 +198,25 @@ func TestFindGroupByDisplay(t *testing.T) { }) } } + +func TestSelectGroup_NonTTY(t *testing.T) { + t.Parallel() + original := IsTerminalFunc + defer func() { IsTerminalFunc = original }() + IsTerminalFunc = func(fd uintptr) bool { return false } + + groups := []models.GroupsEligibleTarget{ + {DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + } + + _, err := SelectGroup(groups) + if err == nil { + t.Fatal("expected error for non-TTY") + } + if !errors.Is(err, ErrNotInteractive) { + t.Errorf("expected ErrNotInteractive, got: %v", err) + } + if !strings.Contains(err.Error(), "--group") { + t.Errorf("error should mention --group, got: %v", err) + } +} diff --git a/internal/ui/selector.go b/internal/ui/selector.go index 8d1518d..287bef4 100644 --- a/internal/ui/selector.go +++ b/internal/ui/selector.go @@ -65,6 +65,10 @@ func FindTargetByDisplay(targets []models.EligibleTarget, display string) (*mode // SelectTarget presents an interactive selector for choosing a target. func SelectTarget(targets []models.EligibleTarget) (*models.EligibleTarget, error) { + if !IsInteractive() { + return nil, fmt.Errorf("%w; use --target and --role flags for non-interactive mode", ErrNotInteractive) + } + if len(targets) == 0 { return nil, errors.New("no eligible targets available") } diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go index 75beaed..f973f43 100644 --- a/internal/ui/selector_test.go +++ b/internal/ui/selector_test.go @@ -1,6 +1,8 @@ package ui import ( + "errors" + "strings" "testing" "github.com/aaearon/grant-cli/internal/sca/models" @@ -185,6 +187,28 @@ func TestBuildOptions(t *testing.T) { } } +func TestSelectTarget_NonTTY(t *testing.T) { + t.Parallel() + original := IsTerminalFunc + defer func() { IsTerminalFunc = original }() + IsTerminalFunc = func(fd uintptr) bool { return false } + + targets := []models.EligibleTarget{ + {WorkspaceName: "Sub A", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{Name: "Owner"}}, + } + + _, err := SelectTarget(targets) + if err == nil { + t.Fatal("expected error for non-TTY") + } + if !errors.Is(err, ErrNotInteractive) { + t.Errorf("expected ErrNotInteractive, got: %v", err) + } + if !strings.Contains(err.Error(), "--target") { + t.Errorf("error should mention --target, got: %v", err) + } +} + func TestFindTargetByDisplay(t *testing.T) { t.Parallel() targets := []models.EligibleTarget{ diff --git a/internal/ui/session_selector.go b/internal/ui/session_selector.go index 78ae22c..9f2876d 100644 --- a/internal/ui/session_selector.go +++ b/internal/ui/session_selector.go @@ -62,6 +62,10 @@ func FindSessionByDisplay(sessions []models.SessionInfo, nameMap map[string]stri // SelectSessions presents a multi-select prompt for choosing sessions to revoke. func SelectSessions(sessions []models.SessionInfo, nameMap map[string]string) ([]models.SessionInfo, error) { + if !IsInteractive() { + return nil, fmt.Errorf("%w; use --all or provide session IDs as arguments", ErrNotInteractive) + } + if len(sessions) == 0 { return nil, errors.New("no sessions available") } @@ -96,6 +100,10 @@ func SelectSessions(sessions []models.SessionInfo, nameMap map[string]string) ([ // ConfirmRevocation prompts the user to confirm session revocation. func ConfirmRevocation(count int) (bool, error) { + if !IsInteractive() { + return false, fmt.Errorf("%w; use --yes to skip confirmation", ErrNotInteractive) + } + var confirmed bool prompt := &survey.Confirm{ Message: fmt.Sprintf("Revoke %d session(s)?", count), diff --git a/internal/ui/session_selector_test.go b/internal/ui/session_selector_test.go index 0b97444..f8cb65a 100644 --- a/internal/ui/session_selector_test.go +++ b/internal/ui/session_selector_test.go @@ -1,6 +1,8 @@ package ui import ( + "errors" + "strings" "testing" "github.com/aaearon/grant-cli/internal/sca/models" @@ -165,3 +167,43 @@ func TestFindSessionByDisplay(t *testing.T) { } }) } + +func TestSelectSessions_NonTTY(t *testing.T) { + t.Parallel() + original := IsTerminalFunc + defer func() { IsTerminalFunc = original }() + IsTerminalFunc = func(fd uintptr) bool { return false } + + sessions := []models.SessionInfo{ + {SessionID: "s1", RoleID: "Admin", WorkspaceID: "ws-a", SessionDuration: 3600}, + } + + _, err := SelectSessions(sessions, nil) + if err == nil { + t.Fatal("expected error for non-TTY") + } + if !errors.Is(err, ErrNotInteractive) { + t.Errorf("expected ErrNotInteractive, got: %v", err) + } + if !strings.Contains(err.Error(), "--all") { + t.Errorf("error should mention --all, got: %v", err) + } +} + +func TestConfirmRevocation_NonTTY(t *testing.T) { + t.Parallel() + original := IsTerminalFunc + defer func() { IsTerminalFunc = original }() + IsTerminalFunc = func(fd uintptr) bool { return false } + + _, err := ConfirmRevocation(3) + if err == nil { + t.Fatal("expected error for non-TTY") + } + if !errors.Is(err, ErrNotInteractive) { + t.Errorf("expected ErrNotInteractive, got: %v", err) + } + if !strings.Contains(err.Error(), "--yes") { + t.Errorf("error should mention --yes, got: %v", err) + } +} diff --git a/internal/ui/tty.go b/internal/ui/tty.go new file mode 100644 index 0000000..e0df838 --- /dev/null +++ b/internal/ui/tty.go @@ -0,0 +1,23 @@ +package ui + +import ( + "errors" + "os" + + "github.com/mattn/go-isatty" +) + +// IsTerminalFunc checks whether the given file descriptor is a terminal. +// It is a variable so tests can override it. +var IsTerminalFunc = func(fd uintptr) bool { + return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) +} + +// IsInteractive reports whether stdin is connected to a terminal. +func IsInteractive() bool { + return IsTerminalFunc(os.Stdin.Fd()) +} + +// ErrNotInteractive is returned when an interactive prompt is attempted +// without a terminal attached to stdin. +var ErrNotInteractive = errors.New("interactive selection requires a terminal") diff --git a/internal/ui/tty_test.go b/internal/ui/tty_test.go new file mode 100644 index 0000000..5c28b87 --- /dev/null +++ b/internal/ui/tty_test.go @@ -0,0 +1,27 @@ +package ui + +import "testing" + +func TestIsInteractive_WhenTerminal(t *testing.T) { + t.Parallel() + original := IsTerminalFunc + defer func() { IsTerminalFunc = original }() + + IsTerminalFunc = func(fd uintptr) bool { return true } + + if !IsInteractive() { + t.Error("IsInteractive() = false, want true when terminal") + } +} + +func TestIsInteractive_WhenNotTerminal(t *testing.T) { + t.Parallel() + original := IsTerminalFunc + defer func() { IsTerminalFunc = original }() + + IsTerminalFunc = func(fd uintptr) bool { return false } + + if IsInteractive() { + t.Error("IsInteractive() = true, want false when not terminal") + } +}