diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..65bdec2 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,111 @@ +# golangci-lint v1 configuration for grant-cli +# golangci-lint v1.64.8 | Go 1.25+ +# https://golangci-lint.run/usage/configuration/ + +run: + timeout: 3m + tests: true + +linters: + disable-all: true + enable: + # --- Default linters --- + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - unused + # --- Bug detection --- + - bodyclose # unclosed HTTP response bodies + - errorlint # error wrapping correctness (Go 1.13+) + - noctx # HTTP requests without context.Context + # --- Security --- + - gosec # security issue detection + # --- Style & naming --- + - errname # sentinel error naming (ErrFoo) + - gocritic # diagnostic/style/performance checks + - misspell # common English typos + - revive # configurable Go linter (golint replacement) + # --- Complexity --- + - gocognit # cognitive complexity + # --- Performance --- + - perfsprint # fmt.Sprintf replaceable with concatenation + - unconvert # unnecessary type conversions + # --- Testing --- + - usetesting # modern test helpers (t.Context, etc.) + +linters-settings: + gocognit: + min-complexity: 40 + + gocritic: + enabled-tags: + - diagnostic + - style + - performance + disabled-checks: + - hugeParam # small structs in a CLI are fine + - rangeValCopy # same: small value copies are fine + + gosec: + excludes: + # G101 "hardcoded credentials" false-positives on test JWT tokens + - G101 + + revive: + severity: warning + enable-all-rules: false + rules: + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: empty-block + - name: error-naming + - name: error-return + - name: error-strings + - name: errorf + # exported: disabled because stuttering names (cache.CacheDir, + # config.ConfigDir, sca.SCAAccessService) are established API. + - name: exported + disabled: true + - name: increment-decrement + - name: indent-error-flow + - name: range + - name: receiver-naming + - name: redefines-builtin-id + - name: superfluous-else + - name: time-naming + - name: unexported-return + - name: unreachable-code + - name: var-declaration + - name: var-naming + # unused-parameter: disabled because Cobra RunE signatures require + # func(cmd *cobra.Command, args []string) and test mocks implement + # interfaces with necessarily unused parameters. + - name: unused-parameter + disabled: true + + misspell: + locale: US + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 + + exclude-rules: + # Test files: relax security checks (G306 WriteFile perms in temp dirs) + - path: _test\.go + linters: + - gosec + + # Test files: relax complexity checks (table-driven tests are long) + - path: _test\.go + linters: + - gocognit + + # Test files: bodyclose on mock HTTP responses + - path: _test\.go + linters: + - bodyclose diff --git a/.goreleaser.yaml b/.goreleaser.yaml index eaf4307..6563f14 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -2,11 +2,14 @@ version: 2 builds: - binary: grant + flags: + - -trimpath + mod_timestamp: "{{ .CommitTimestamp }}" ldflags: - -s -w - -X github.com/aaearon/grant-cli/cmd.version={{.Version}} - -X github.com/aaearon/grant-cli/cmd.commit={{.ShortCommit}} - - -X github.com/aaearon/grant-cli/cmd.buildDate={{.Date}} + - -X github.com/aaearon/grant-cli/cmd.buildDate={{.CommitDate}} goos: - linux - darwin diff --git a/CLAUDE.md b/CLAUDE.md index c025e39..a95cf08 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,15 +94,25 @@ Custom `SCAAccessService` follows SDK conventions: - Skill definition: `.claude/skills/grant-login/SKILL.md` - Requires `.env` at project root with `GRANT_PASSWORD` and `TOTP_SECRET` +## Lint +- Config: `.golangci.yml` (golangci-lint v1 format) +- 19 linters enabled: defaults (errcheck, gosimple, govet, ineffassign, staticcheck, unused) + bodyclose, errorlint, noctx, gosec (G101 excluded), errname, gocritic, misspell, revive, gocognit (threshold 40), perfsprint, unconvert, usetesting +- Test files excluded from gosec, gocognit, bodyclose +- `revive/unused-parameter` and `revive/exported` disabled (Cobra signatures, established API names) +- Use `errors.New` for static error strings (perfsprint enforced); `fmt.Errorf` only with `%` verbs +- Use `t.Context()` instead of `context.Background()` in tests (usetesting enforced) + ## Build ```bash -make build # Build binary with ldflags +make build # Build binary with -trimpath and ldflags make test # Run unit tests make test-integration # Run integration tests (builds binary) make test-all # Run all tests -make lint # Run linter +make lint # Run linter (golangci-lint) make clean # Clean build artifacts ``` +- `-trimpath` used in both `Makefile` and `.goreleaser.yaml` for reproducible builds +- `.goreleaser.yaml` uses `CommitDate` (not build date) and `mod_timestamp` for reproducibility ## Git - Feature branches, conventional commits diff --git a/Makefile b/Makefile index 6e7cd32..f64ce7d 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ LDFLAGS := -s -w \ .PHONY: build test test-race test-integration test-all test-coverage lint clean build: - go build -ldflags "$(LDFLAGS)" -o $(BINARY_NAME) . + go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY_NAME) . test: go test ./... -v diff --git a/cmd/configure.go b/cmd/configure.go index 0f9b563..bd521e3 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "net/url" "os" @@ -83,7 +84,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st } if strings.TrimSpace(username) == "" { - return fmt.Errorf("username is required") + return errors.New("username is required") } // Create SDK profile @@ -144,7 +145,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st // validateTenantURL validates that the tenant URL is a valid HTTPS URL func validateTenantURL(tenantURL string) error { if strings.TrimSpace(tenantURL) == "" { - return fmt.Errorf("invalid tenant URL: cannot be empty") + return errors.New("invalid tenant URL: cannot be empty") } u, err := url.Parse(tenantURL) @@ -153,11 +154,11 @@ func validateTenantURL(tenantURL string) error { } if u.Scheme != "https" { - return fmt.Errorf("invalid tenant URL: must use HTTPS scheme") + return errors.New("invalid tenant URL: must use HTTPS scheme") } if u.Host == "" { - return fmt.Errorf("invalid tenant URL: must have a host") + return errors.New("invalid tenant URL: must have a host") } return nil diff --git a/cmd/env.go b/cmd/env.go index 36e9864..36f49c9 100644 --- a/cmd/env.go +++ b/cmd/env.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "github.com/aaearon/grant-cli/internal/config" @@ -92,7 +93,7 @@ func runEnvWithDeps( } if res.result.AccessCredentials == nil { - return fmt.Errorf("no credentials returned; grant env is only supported for AWS elevations") + return errors.New("no credentials returned; grant env is only supported for AWS elevations") } awsCreds, err := models.ParseAWSCredentials(*res.result.AccessCredentials) diff --git a/cmd/favorites.go b/cmd/favorites.go index 629a482..d94f592 100644 --- a/cmd/favorites.go +++ b/cmd/favorites.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "fmt" "strings" @@ -111,7 +112,7 @@ func runFavoritesAddProduction(cmd *cobra.Command, args []string) error { // Validate: target and role must both be provided or both omitted if (target != "" && role == "") || (target == "" && role != "") { - return fmt.Errorf("both --target and --role must be provided") + return errors.New("both --target and --role must be provided") } favType, _ := cmd.Flags().GetString("type") @@ -150,36 +151,117 @@ func runFavoritesAddProduction(cmd *cobra.Command, args []string) error { return runFavoritesAddWithDeps(cmd, args, cachedLister, &uiUnifiedSelector{}, &surveyNamePrompter{}, cfg, cachedLister) } -// runFavoritesAddWithDeps contains the core logic for favorites add. -// When eligLister and sel are nil, it uses the non-interactive flag path. -// If preloadedCfg is non-nil, it is used instead of loading from disk. -func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligibilityLister, sel unifiedSelector, prompter namePrompter, preloadedCfg *config.Config, groupsElig groupsEligibilityLister) error { - // Read flags - provider, _ := cmd.Flags().GetString("provider") - target, _ := cmd.Flags().GetString("target") - role, _ := cmd.Flags().GetString("role") - favType, _ := cmd.Flags().GetString("type") - group, _ := cmd.Flags().GetString("group") +// favoritesAddFlags holds the parsed flags for the favorites add command. +type favoritesAddFlags struct { + provider string + target string + role string + favType string + group string +} - // Validate type flag - if favType != "" && favType != config.FavoriteTypeCloud && favType != config.FavoriteTypeGroups { - return fmt.Errorf("invalid --type %q: must be one of: cloud, groups", favType) +// parseFavoritesAddFlags reads and validates the flags for favorites add. +func parseFavoritesAddFlags(cmd *cobra.Command) (*favoritesAddFlags, error) { + f := &favoritesAddFlags{} + f.provider, _ = cmd.Flags().GetString("provider") + f.target, _ = cmd.Flags().GetString("target") + f.role, _ = cmd.Flags().GetString("role") + f.favType, _ = cmd.Flags().GetString("type") + f.group, _ = cmd.Flags().GetString("group") + + if f.favType != "" && f.favType != config.FavoriteTypeCloud && f.favType != config.FavoriteTypeGroups { + return nil, fmt.Errorf("invalid --type %q: must be one of: cloud, groups", f.favType) } - // Validate flag combinations - if favType == config.FavoriteTypeGroups { - if target != "" || role != "" { - return fmt.Errorf("--target and --role cannot be used with --type groups") + if f.favType == config.FavoriteTypeGroups { + if f.target != "" || f.role != "" { + return nil, errors.New("--target and --role cannot be used with --type groups") } } else { - if group != "" { - return fmt.Errorf("--group requires --type groups") + if f.group != "" { + return nil, errors.New("--group requires --type groups") } - if (target != "" && role == "") || (target == "" && role != "") { - return fmt.Errorf("both --target and --role must be provided") + if (f.target != "" && f.role == "") || (f.target == "" && f.role != "") { + return nil, errors.New("both --target and --role must be provided") } } + return f, nil +} + +// selectFavoriteInteractive presents a unified selector and returns the chosen favorite and name. +func selectFavoriteInteractive(provider string, eligLister eligibilityLister, groupsElig groupsEligibilityLister, sel unifiedSelector, prompter namePrompter, name string, cfg *config.Config) (config.Favorite, string, error) { + ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) + defer cancel() + + allTargets, err := fetchEligibility(ctx, eligLister, provider) + if err != nil { + return config.Favorite{}, "", err + } + + var items []selectionItem + for i := range allTargets { + items = append(items, selectionItem{kind: selectionCloud, cloud: &allTargets[i]}) + } + + if groupsElig != nil { + groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister) + if gErr == nil { + for i := range groups { + items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) + } + } + } + + if len(items) == 0 { + return config.Favorite{}, "", errors.New("no eligible targets or groups found") + } + + selected, err := sel.SelectItem(items) + if err != nil { + return config.Favorite{}, "", fmt.Errorf("selection failed: %w", err) + } + + var fav config.Favorite + switch selected.kind { + case selectionCloud: + resolveTargetCSP(selected.cloud, allTargets, provider) + if provider != "" { + fav.Provider = provider + } else { + fav.Provider = strings.ToLower(string(selected.cloud.CSP)) + } + fav.Target = selected.cloud.WorkspaceName + fav.Role = selected.cloud.RoleInfo.Name + case selectionGroup: + fav.Type = config.FavoriteTypeGroups + fav.Provider = "azure" + fav.Group = selected.group.GroupName + fav.DirectoryID = selected.group.DirectoryID + } + + if name == "" { + name, err = prompter.PromptName() + if err != nil { + return config.Favorite{}, "", fmt.Errorf("failed to read favorite name: %w", err) + } + if _, err := config.GetFavorite(cfg, name); err == nil { + return config.Favorite{}, "", fmt.Errorf("favorite %q already exists", name) + } + } + + return fav, name, nil +} + +// runFavoritesAddWithDeps contains the core logic for favorites add. +// When eligLister and sel are nil, it uses the non-interactive flag path. +// If preloadedCfg is non-nil, it is used instead of loading from disk. +func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligibilityLister, sel unifiedSelector, prompter namePrompter, preloadedCfg *config.Config, groupsElig groupsEligibilityLister) error { + f, err := parseFavoritesAddFlags(cmd) + if err != nil { + return err + } + // Determine name from arg var name string if len(args) > 0 { @@ -187,15 +269,15 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi } // Non-interactive mode requires name upfront - isNonInteractive := (target != "" && role != "") || (favType == config.FavoriteTypeGroups && group != "") + isNonInteractive := (f.target != "" && f.role != "") || (f.favType == config.FavoriteTypeGroups && f.group != "") if isNonInteractive { - log.Info("Non-interactive mode: target=%q role=%q provider=%q group=%q", target, role, provider, group) + log.Info("Non-interactive mode: target=%q role=%q provider=%q group=%q", f.target, f.role, f.provider, f.group) } if isNonInteractive && name == "" { - if favType == config.FavoriteTypeGroups { - return fmt.Errorf("name is required when using --group flag\n\nUsage:\n grant favorites add --type groups --group ") + if f.favType == config.FavoriteTypeGroups { + return errors.New("name is required when using --group flag\n\nUsage:\n grant favorites add --type groups --group ") } - return fmt.Errorf("name is required when using --target and --role flags\n\nUsage:\n grant favorites add --target --role ") + return errors.New("name is required when using --target and --role flags\n\nUsage:\n grant favorites add --target --role ") } // Load config (skip if pre-loaded) @@ -205,7 +287,6 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi cfg = preloadedCfg cfgPath, _ = config.ConfigPath() } else { - var err error cfg, cfgPath, err = config.LoadDefaultWithPath() if err != nil { return err @@ -220,80 +301,24 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi } // Groups flow - if favType == config.FavoriteTypeGroups { - return addGroupFavorite(cmd, name, group, cfg, cfgPath, groupsElig, eligLister, sel, prompter) + if f.favType == config.FavoriteTypeGroups { + return addGroupFavorite(cmd, name, f.group, cfg, cfgPath, groupsElig, eligLister, sel, prompter) } // Cloud flow var fav config.Favorite - if target != "" && role != "" { - // Non-interactive: target and role specified via flags - fav.Target = target - fav.Role = role - fav.Provider = provider + if f.target != "" && f.role != "" { + fav.Target = f.target + fav.Role = f.role + fav.Provider = f.provider if fav.Provider == "" { fav.Provider = cfg.DefaultProvider } } else { - // Interactive: unified selector showing cloud targets and groups - ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) - defer cancel() - - allTargets, err := fetchEligibility(ctx, eligLister, provider) + fav, name, err = selectFavoriteInteractive(f.provider, eligLister, groupsElig, sel, prompter, name, cfg) if err != nil { return err } - - var items []selectionItem - for i := range allTargets { - items = append(items, selectionItem{kind: selectionCloud, cloud: &allTargets[i]}) - } - - // Fetch groups eligibility (best-effort, enriched with directory names) - if groupsElig != nil { - groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister) - if gErr == nil { - for i := range groups { - items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) - } - } - } - - if len(items) == 0 { - return fmt.Errorf("no eligible targets or groups found") - } - - selected, err := sel.SelectItem(items) - if err != nil { - return fmt.Errorf("selection failed: %w", err) - } - - switch selected.kind { - case selectionCloud: - resolveTargetCSP(selected.cloud, allTargets, provider) - if provider != "" { - fav.Provider = provider - } else { - fav.Provider = strings.ToLower(string(selected.cloud.CSP)) - } - fav.Target = selected.cloud.WorkspaceName - fav.Role = selected.cloud.RoleInfo.Name - case selectionGroup: - fav.Type = config.FavoriteTypeGroups - fav.Provider = "azure" - fav.Group = selected.group.GroupName - fav.DirectoryID = selected.group.DirectoryID - } - - if name == "" { - name, err = prompter.PromptName() - if err != nil { - return fmt.Errorf("failed to read favorite name: %w", err) - } - if _, err := config.GetFavorite(cfg, name); err == nil { - return fmt.Errorf("favorite %q already exists", name) - } - } } log.Info("Saving favorite %q...", name) @@ -384,7 +409,7 @@ func newFavoritesRemoveCommand() *cobra.Command { Example: " grant favorites remove prod-admin", Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { - return fmt.Errorf("requires a favorite name\n\nUsage:\n grant favorites remove \n\nSee available favorites with 'grant favorites list'") + return errors.New("requires a favorite name\n\nUsage:\n grant favorites remove \n\nSee available favorites with 'grant favorites list'") } if len(args) > 1 { return fmt.Errorf("expected 1 favorite name, got %d", len(args)) diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go index 9769b76..06fc181 100644 --- a/cmd/favorites_test.go +++ b/cmd/favorites_test.go @@ -686,7 +686,7 @@ func TestFavoritesAddInteractiveMode(t *testing.T) { wantErr: true, }, { - name: "selector cancelled", + name: "selector canceled", setupConfig: func(path string) { cfg := config.DefaultConfig() _ = config.Save(cfg, path) @@ -700,7 +700,7 @@ func TestFavoritesAddInteractiveMode(t *testing.T) { }, }, selector: &mockUnifiedSelector{ - selectErr: errors.New("user cancelled"), + selectErr: errors.New("user canceled"), }, args: []string{"myfav"}, wantContain: []string{"selection failed"}, @@ -801,7 +801,7 @@ func TestFavoritesAddInteractiveMode(t *testing.T) { selector: &mockUnifiedSelector{ item: &selectionItem{kind: selectionCloud, cloud: &twoTargets[0]}, }, - namePrompter: &mockNamePrompter{promptErr: errors.New("user cancelled")}, + namePrompter: &mockNamePrompter{promptErr: errors.New("user canceled")}, args: []string{}, wantContain: []string{"failed to read favorite name"}, wantErr: true, diff --git a/cmd/helpers.go b/cmd/helpers.go index c3b7641..9f5a735 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -189,7 +189,7 @@ func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, se // findMatchingGroup finds a group by name (case-insensitive). // If directoryID is non-empty, only matches groups in that directory. -func findMatchingGroup(groups []scamodels.GroupsEligibleTarget, name string, directoryID string) *scamodels.GroupsEligibleTarget { +func findMatchingGroup(groups []scamodels.GroupsEligibleTarget, name, directoryID string) *scamodels.GroupsEligibleTarget { for i := range groups { if strings.EqualFold(groups[i].GroupName, name) { if directoryID != "" && groups[i].DirectoryID != directoryID { diff --git a/cmd/helpers_test.go b/cmd/helpers_test.go index 8950578..bb40abe 100644 --- a/cmd/helpers_test.go +++ b/cmd/helpers_test.go @@ -194,7 +194,7 @@ func TestFetchStatusData(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() + ctx := t.Context() if tt.name == "context cancellation - no hang" { var cancel context.CancelFunc ctx, cancel = context.WithCancel(ctx) @@ -238,7 +238,7 @@ func TestFetchStatusData_VerboseWarning(t *testing.T) { log = spy defer func() { log = oldLog }() - ctx := context.Background() + ctx := t.Context() sessionLister := &mockSessionLister{ sessions: &scamodels.SessionsResponse{ Response: []scamodels.SessionInfo{ @@ -409,7 +409,7 @@ func TestBuildWorkspaceNameMap(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() + ctx := t.Context() if tt.name == "context cancellation - no hang" { var cancel context.CancelFunc ctx, cancel = context.WithCancel(ctx) @@ -444,7 +444,7 @@ func TestBuildWorkspaceNameMap_VerboseWarning(t *testing.T) { log = spy defer func() { log = oldLog }() - ctx := context.Background() + ctx := t.Context() eligLister := &mockEligibilityLister{ listErr: errors.New("eligibility API unavailable"), } diff --git a/cmd/login.go b/cmd/login.go index 892b445..f09be8d 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "time" @@ -64,7 +65,7 @@ func runLogin(cmd *cobra.Command, auth authenticator) error { } if profile == nil { - return fmt.Errorf("profile not found after configuration") + return errors.New("profile not found after configuration") } } diff --git a/cmd/login_test.go b/cmd/login_test.go index 97eae9e..c12da94 100644 --- a/cmd/login_test.go +++ b/cmd/login_test.go @@ -55,16 +55,16 @@ func TestLoginCommand(t *testing.T) { wantErr: true, }, { - name: "authentication failure - user cancelled", + name: "authentication failure - user canceled", setupAuth: func() authenticator { return &mockAuthenticator{ token: nil, - authErr: errors.New("user cancelled"), + authErr: errors.New("user canceled"), } }, wantContain: []string{ "authentication failed", - "user cancelled", + "user canceled", }, wantErr: true, }, diff --git a/cmd/revoke.go b/cmd/revoke.go index 9ee1a58..414b94a 100644 --- a/cmd/revoke.go +++ b/cmd/revoke.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "fmt" "github.com/aaearon/grant-cli/internal/config" @@ -100,10 +101,10 @@ func runRevoke( // Validate mutual exclusivity if allFlag && len(args) > 0 { - return fmt.Errorf("--all cannot be used with session ID arguments") + return errors.New("--all cannot be used with session ID arguments") } if len(args) > 0 && provider != "" { - return fmt.Errorf("--provider cannot be used with session ID arguments") + return errors.New("--provider cannot be used with session ID arguments") } // Validate provider @@ -156,7 +157,7 @@ func runRevoke( return fmt.Errorf("confirmation failed: %w", err) } if !confirmed { - fmt.Fprintln(cmd.OutOrStdout(), "Revocation cancelled.") + fmt.Fprintln(cmd.OutOrStdout(), "Revocation canceled.") return nil } } @@ -180,7 +181,7 @@ func runRevoke( return fmt.Errorf("confirmation failed: %w", err) } if !confirmed { - fmt.Fprintln(cmd.OutOrStdout(), "Revocation cancelled.") + fmt.Fprintln(cmd.OutOrStdout(), "Revocation canceled.") return nil } } diff --git a/cmd/revoke_test.go b/cmd/revoke_test.go index 3fa8999..3688b18 100644 --- a/cmd/revoke_test.go +++ b/cmd/revoke_test.go @@ -229,7 +229,7 @@ func TestRevokeCommand(t *testing.T) { wantErr: false, }, { - name: "all mode - confirmation cancelled", + name: "all mode - confirmation canceled", args: []string{"--all"}, setupAuth: func() *mockAuthLoader { return &mockAuthLoader{ @@ -243,7 +243,7 @@ func TestRevokeCommand(t *testing.T) { setupRevoker: func() *mockSessionRevoker { return &mockSessionRevoker{} }, setupSelector: func() *mockSessionSelector { return &mockSessionSelector{} }, setupConfirm: func() *mockConfirmPrompter { return &mockConfirmPrompter{confirmed: false} }, - wantContain: []string{"Revocation cancelled"}, + wantContain: []string{"Revocation canceled"}, wantErr: false, }, { diff --git a/cmd/root.go b/cmd/root.go index d41ea2b..23b6b2f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,7 +1,9 @@ +// Package cmd implements the grant CLI commands. package cmd import ( "context" + "errors" "fmt" "os" "slices" @@ -265,7 +267,7 @@ func fetchEligibility(ctx context.Context, eligLister eligibilityLister, provide } } if len(all) == 0 { - return nil, fmt.Errorf("no eligible targets found, check your SCA policies") + return nil, errors.New("no eligible targets found, check your SCA policies") } return all, nil } @@ -369,7 +371,7 @@ func resolveAndElevate( // Validate direct mode flags if (targetName != "" && roleName == "") || (targetName == "" && roleName != "") { - return nil, fmt.Errorf("both --target and --role must be provided") + return nil, errors.New("both --target and --role must be provided") } provider = flags.provider @@ -421,7 +423,7 @@ func resolveAndElevate( // Check for errors in response if len(elevateResp.Response.Results) == 0 { - return nil, fmt.Errorf("elevation failed: no results returned") + return nil, errors.New("elevation failed: no results returned") } result := elevateResp.Response.Results[0] @@ -442,7 +444,7 @@ func fetchGroupsEligibility(ctx context.Context, groupsEligLister groupsEligibil return nil, fmt.Errorf("failed to fetch eligible groups: %w", err) } if len(eligResp.Response) == 0 { - return nil, fmt.Errorf("no eligible groups found, check your SCA policies") + return nil, errors.New("no eligible groups found, check your SCA policies") } // Resolve directory names from cloud eligibility (best-effort) @@ -456,6 +458,52 @@ func fetchGroupsEligibility(ctx context.Context, groupsEligLister groupsEligibil return eligResp.Response, nil } +// resolvedFlags holds the resolved state after processing favorites and flag defaults. +type resolvedFlags struct { + targetName string + roleName string + provider string + favDirectoryID string + isFavoriteMode bool + isGroupFavorite bool +} + +// resolveFavoriteFlags resolves favorite and direct flags into concrete values. +func resolveFavoriteFlags(flags *elevateFlags, cfg *config.Config) (*resolvedFlags, error) { + rf := &resolvedFlags{} + + if flags.favorite != "" { + rf.isFavoriteMode = true + fav, err := config.GetFavorite(cfg, flags.favorite) + if err != nil { + return nil, fmt.Errorf("favorite %q not found, run 'grant favorites list'", flags.favorite) + } + + if fav.ResolvedType() == config.FavoriteTypeGroups { + rf.isGroupFavorite = true + flags.group = fav.Group + rf.favDirectoryID = fav.DirectoryID + } else { + if flags.provider != "" && !strings.EqualFold(flags.provider, fav.Provider) { + return nil, fmt.Errorf("provider %q does not match favorite provider %q", flags.provider, fav.Provider) + } + rf.provider = fav.Provider + rf.targetName = fav.Target + rf.roleName = fav.Role + } + } else { + rf.targetName = flags.target + rf.roleName = flags.role + rf.provider = flags.provider + + if (rf.targetName != "" && rf.roleName == "") || (rf.targetName == "" && rf.roleName != "") { + return nil, errors.New("both --target and --role must be provided") + } + } + + return rf, nil +} + // resolveAndElevateUnified handles all elevation modes: cloud, group, and unified. // Returns (*elevationResult, nil, nil) for cloud or (nil, *groupElevationResult, nil) for group. func resolveAndElevateUnified( @@ -479,114 +527,94 @@ func resolveAndElevateUnified( return nil, nil, fmt.Errorf("not authenticated, run 'grant login' first: %w", err) } - // Determine execution mode - var targetName, roleName string - var isFavoriteMode bool - var provider string - var favDirectoryID string - var isGroupFavorite bool + rf, err := resolveFavoriteFlags(flags, cfg) + if err != nil { + return nil, nil, err + } - if flags.favorite != "" { - isFavoriteMode = true - fav, err := config.GetFavorite(cfg, flags.favorite) - if err != nil { - return nil, nil, fmt.Errorf("favorite %q not found, run 'grant favorites list'", flags.favorite) - } + // Dispatch to the appropriate elevation path + if flags.group != "" { + return resolveAndElevateDirectGroup(ctx, flags.group, rf.favDirectoryID, groupsEligLister, eligibilityLister, groupsElevator) + } + if flags.groups { + return resolveAndElevateGroupsFilter(ctx, groupsEligLister, eligibilityLister, selector, groupsElevator) + } + if rf.provider != "" || rf.isFavoriteMode || (rf.targetName != "" && rf.roleName != "") { + return resolveAndElevateCloudOnly(ctx, rf, eligibilityLister, elevateService, selector) + } + return resolveAndElevateUnifiedPath(ctx, eligibilityLister, groupsEligLister, selector, elevateService, groupsElevator) +} - if fav.ResolvedType() == config.FavoriteTypeGroups { - // Group favorite — set flags to use group path - isGroupFavorite = true - flags.group = fav.Group - favDirectoryID = fav.DirectoryID - } else { - // Cloud favorite - if flags.provider != "" && !strings.EqualFold(flags.provider, fav.Provider) { - return nil, nil, fmt.Errorf("provider %q does not match favorite provider %q", flags.provider, fav.Provider) - } - provider = fav.Provider - targetName = fav.Target - roleName = fav.Role - } - } else { - targetName = flags.target - roleName = flags.role - provider = flags.provider +// resolveAndElevateDirectGroup handles the --group flag or group favorite path. +func resolveAndElevateDirectGroup(ctx context.Context, groupName, favDirectoryID string, groupsEligLister groupsEligibilityLister, cloudEligLister eligibilityLister, groupsElevator groupsElevator) (*elevationResult, *groupElevationResult, error) { + groups, err := fetchGroupsEligibility(ctx, groupsEligLister, cloudEligLister) + if err != nil { + return nil, nil, err + } - if (targetName != "" && roleName == "") || (targetName == "" && roleName != "") { - return nil, nil, fmt.Errorf("both --target and --role must be provided") + selectedGroup := findMatchingGroup(groups, groupName, favDirectoryID) + if selectedGroup == nil { + if favDirectoryID != "" { + return nil, nil, fmt.Errorf("group %q not found in directory %q, run 'grant' to see available options", groupName, favDirectoryID) } + return nil, nil, fmt.Errorf("group %q not found, run 'grant' to see available options", groupName) } - // Group-only path (--group flag or group favorite) - if flags.group != "" { - groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister) - if err != nil { - return nil, nil, err - } + return elevateGroup(ctx, selectedGroup, groupsElevator) +} - selectedGroup := findMatchingGroup(groups, flags.group, favDirectoryID) - if selectedGroup == nil { - if favDirectoryID != "" { - return nil, nil, fmt.Errorf("group %q not found in directory %q, run 'grant' to see available options", flags.group, favDirectoryID) - } - return nil, nil, fmt.Errorf("group %q not found, run 'grant' to see available options", flags.group) - } +// resolveAndElevateGroupsFilter handles the --groups interactive filter path. +func resolveAndElevateGroupsFilter(ctx context.Context, groupsEligLister groupsEligibilityLister, cloudEligLister eligibilityLister, selector unifiedSelector, groupsElevator groupsElevator) (*elevationResult, *groupElevationResult, error) { + groups, err := fetchGroupsEligibility(ctx, groupsEligLister, cloudEligLister) + if err != nil { + return nil, nil, err + } - return elevateGroup(ctx, selectedGroup, groupsElevator) + var items []selectionItem + for i := range groups { + items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) } - // Groups-filter path (--groups flag, no --group) - if flags.groups { - groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister) - if err != nil { - return nil, nil, err - } + selected, err := selector.SelectItem(items) + if err != nil { + return nil, nil, fmt.Errorf("selection failed: %w", err) + } + return elevateGroup(ctx, selected.group, groupsElevator) +} + +// resolveAndElevateCloudOnly handles the cloud-only path (--provider, direct, or favorite). +func resolveAndElevateCloudOnly(ctx context.Context, rf *resolvedFlags, eligLister eligibilityLister, elevateService elevateService, selector unifiedSelector) (*elevationResult, *groupElevationResult, error) { + allTargets, err := fetchEligibility(ctx, eligLister, rf.provider) + if err != nil { + return nil, nil, err + } + + var selectedTarget *models.EligibleTarget + if rf.isFavoriteMode && !rf.isGroupFavorite || (rf.targetName != "" && rf.roleName != "") { + selectedTarget = findMatchingTarget(allTargets, rf.targetName, rf.roleName) + if selectedTarget == nil { + return nil, nil, fmt.Errorf("target %q or role %q not found, run 'grant' to see available options", rf.targetName, rf.roleName) + } + } else { var items []selectionItem - for i := range groups { - items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) + for i := range allTargets { + items = append(items, selectionItem{kind: selectionCloud, cloud: &allTargets[i]}) } selected, err := selector.SelectItem(items) if err != nil { return nil, nil, fmt.Errorf("selection failed: %w", err) } - - return elevateGroup(ctx, selected.group, groupsElevator) + selectedTarget = selected.cloud } - // Cloud-only path (--provider specified, or direct/favorite cloud mode) - if provider != "" || isFavoriteMode || (targetName != "" && roleName != "") { - allTargets, err := fetchEligibility(ctx, eligibilityLister, provider) - if err != nil { - return nil, nil, err - } - - var selectedTarget *models.EligibleTarget - if isFavoriteMode && !isGroupFavorite || (targetName != "" && roleName != "") { - selectedTarget = findMatchingTarget(allTargets, targetName, roleName) - if selectedTarget == nil { - return nil, nil, fmt.Errorf("target %q or role %q not found, run 'grant' to see available options", targetName, roleName) - } - } else { - // Interactive cloud-only - var items []selectionItem - for i := range allTargets { - items = append(items, selectionItem{kind: selectionCloud, cloud: &allTargets[i]}) - } - - selected, err := selector.SelectItem(items) - if err != nil { - return nil, nil, fmt.Errorf("selection failed: %w", err) - } - selectedTarget = selected.cloud - } - - resolveTargetCSP(selectedTarget, allTargets, provider) - return elevateCloud(ctx, selectedTarget, elevateService) - } + resolveTargetCSP(selectedTarget, allTargets, rf.provider) + return elevateCloud(ctx, selectedTarget, elevateService) +} - // Unified path (no filter flags) — fetch both cloud and groups in parallel +// resolveAndElevateUnifiedPath handles the unified path (no filter flags) with parallel fetch. +func resolveAndElevateUnifiedPath(ctx context.Context, eligLister eligibilityLister, groupsEligLister groupsEligibilityLister, selector unifiedSelector, elevateService elevateService, groupsElevator groupsElevator) (*elevationResult, *groupElevationResult, error) { type cloudResult struct { targets []models.EligibleTarget err error @@ -600,12 +628,12 @@ func resolveAndElevateUnified( groupsCh := make(chan groupsResult, 1) go func() { - targets, err := fetchEligibility(ctx, eligibilityLister, "") + targets, err := fetchEligibility(ctx, eligLister, "") cloudCh <- cloudResult{targets: targets, err: err} }() go func() { - groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister) + groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligLister) groupsCh <- groupsResult{groups: groups, err: err} }() @@ -625,7 +653,7 @@ func resolveAndElevateUnified( } if len(items) == 0 { - return nil, nil, fmt.Errorf("no eligible targets or groups found, check your SCA policies") + return nil, nil, errors.New("no eligible targets or groups found, check your SCA policies") } selected, err := selector.SelectItem(items) @@ -640,7 +668,7 @@ func resolveAndElevateUnified( case selectionGroup: return elevateGroup(ctx, selected.group, groupsElevator) default: - return nil, nil, fmt.Errorf("unexpected selection kind") + return nil, nil, errors.New("unexpected selection kind") } } @@ -663,7 +691,7 @@ func elevateCloud(ctx context.Context, target *models.EligibleTarget, elevateSer } if len(elevateResp.Response.Results) == 0 { - return nil, nil, fmt.Errorf("elevation failed: no results returned") + return nil, nil, errors.New("elevation failed: no results returned") } result := elevateResp.Response.Results[0] @@ -693,7 +721,7 @@ func elevateGroup(ctx context.Context, group *models.GroupsEligibleTarget, eleva } if len(elevateResp.Results) == 0 { - return nil, nil, fmt.Errorf("elevation failed: no results returned") + return nil, nil, errors.New("elevation failed: no results returned") } result := elevateResp.Results[0] @@ -731,7 +759,7 @@ func runElevateWithDeps( // Display group elevation result dirContext := "" if groupRes.group.DirectoryName != "" { - dirContext = fmt.Sprintf(" in %s", groupRes.group.DirectoryName) + dirContext = " in " + groupRes.group.DirectoryName } fmt.Fprintf(cmd.OutOrStdout(), "Elevated to group %s%s\n", groupRes.group.GroupName, dirContext) fmt.Fprintf(cmd.OutOrStdout(), " Session ID: %s\n", groupRes.result.SessionID) @@ -784,7 +812,7 @@ type uiUnifiedSelector struct{} func (s *uiUnifiedSelector) SelectItem(items []selectionItem) (*selectionItem, error) { if len(items) == 0 { - return nil, fmt.Errorf("no eligible targets or groups available") + return nil, errors.New("no eligible targets or groups available") } options, sorted := buildUnifiedOptions(items) diff --git a/cmd/root_elevate_test.go b/cmd/root_elevate_test.go index 2a9457b..8d25ddd 100644 --- a/cmd/root_elevate_test.go +++ b/cmd/root_elevate_test.go @@ -1079,7 +1079,7 @@ func TestFetchEligibility_SingleProviderOmitsCSPTag(t *testing.T) { }, } - targets, err := fetchEligibility(context.Background(), lister, "azure") + targets, err := fetchEligibility(t.Context(), lister, "azure") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1117,7 +1117,7 @@ func TestFetchEligibility_ConcurrentExecution(t *testing.T) { } start := time.Now() - targets, err := fetchEligibility(context.Background(), lister, "") + targets, err := fetchEligibility(t.Context(), lister, "") elapsed := time.Since(start) if err != nil { diff --git a/cmd/root_test.go b/cmd/root_test.go index bff80e1..d364f95 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,7 +1,7 @@ package cmd import ( - "fmt" + "errors" "os" "strings" "testing" @@ -145,7 +145,7 @@ func TestVerboseHintSuppressedForUnknownSubcommand(t *testing.T) { func TestVerboseHintShownForRuntimeErrors(t *testing.T) { root := newRootCommand(func(cmd *cobra.Command, args []string) error { - return fmt.Errorf("runtime failure") + return errors.New("runtime failure") }) root.SetArgs([]string{}) @@ -191,7 +191,7 @@ func TestExecuteHintOutput(t *testing.T) { name: "runtime error shows hint", setupCmd: func() *cobra.Command { return newRootCommand(func(cmd *cobra.Command, args []string) error { - return fmt.Errorf("something went wrong") + return errors.New("something went wrong") }) }, args: []string{}, diff --git a/cmd/selection.go b/cmd/selection.go index 1ec569f..2ad7d81 100644 --- a/cmd/selection.go +++ b/cmd/selection.go @@ -35,7 +35,7 @@ func formatSelectionItem(item selectionItem) string { case selectionCloud: return ui.FormatTargetOption(*item.cloud) case selectionGroup: - return fmt.Sprintf("%s (azure)", ui.FormatGroupOption(*item.group)) + return ui.FormatGroupOption(*item.group) + " (azure)" default: return "" } diff --git a/cmd/test_mocks.go b/cmd/test_mocks.go index 93aa336..3c45f4e 100644 --- a/cmd/test_mocks.go +++ b/cmd/test_mocks.go @@ -135,7 +135,7 @@ type mockAuthenticator struct { authErr error } -func (m *mockAuthenticator) Authenticate(profile *sdkmodels.IdsecProfile, authProfile *authmodels.IdsecAuthProfile, secret *authmodels.IdsecSecret, force bool, refreshAuth bool) (*authmodels.IdsecToken, error) { +func (m *mockAuthenticator) Authenticate(profile *sdkmodels.IdsecProfile, authProfile *authmodels.IdsecAuthProfile, secret *authmodels.IdsecSecret, force, refreshAuth bool) (*authmodels.IdsecToken, error) { if m.authenticateFunc != nil { return m.authenticateFunc(profile, authProfile, secret, force, refreshAuth) } diff --git a/cmd/update.go b/cmd/update.go index f3705d3..45d1cc2 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "strings" @@ -31,7 +32,7 @@ func NewUpdateCommandWithDeps(updater selfUpdater) *cobra.Command { func runUpdate(cmd *cobra.Command, updater selfUpdater) error { v := version if v == "" || v == "dev" { - return fmt.Errorf("cannot update a dev build; install a release build or download from GitHub Releases") + return errors.New("cannot update a dev build; install a release build or download from GitHub Releases") } log.Info("Current version: %s", v) @@ -48,7 +49,7 @@ func runUpdate(cmd *cobra.Command, updater selfUpdater) error { return fmt.Errorf("update failed: %w", err) } if rel == nil { - return fmt.Errorf("update check returned no release information") + return errors.New("update check returned no release information") } log.Info("Latest release: %s", rel.Version) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 3d696d3..3a3cdc4 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -1,3 +1,4 @@ +// Package cache provides file-based caching with TTL expiry. package cache import ( @@ -49,7 +50,7 @@ func Get[T any](s *Store, key string, dst *T) bool { // Set writes a value to the cache under key. Creates the directory if needed. func Set[T any](s *Store, key string, value T) error { - if err := os.MkdirAll(s.dir, 0700); err != nil { + if err := os.MkdirAll(s.dir, 0o700); err != nil { return err } @@ -63,7 +64,7 @@ func Set[T any](s *Store, key string, value T) error { return err } - return os.WriteFile(filepath.Join(s.dir, key+".json"), data, 0600) + return os.WriteFile(filepath.Join(s.dir, key+".json"), data, 0o600) } // Invalidate removes a cached entry by key. diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index 8345503..6866bef 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -65,7 +65,7 @@ func TestGet_CorruptJSON(t *testing.T) { // Write garbage to the cache file path := filepath.Join(dir, "corrupt.json") - if err := os.WriteFile(path, []byte("{not valid json"), 0600); err != nil { + if err := os.WriteFile(path, []byte("{not valid json"), 0o600); err != nil { t.Fatalf("failed to write corrupt file: %v", err) } @@ -104,7 +104,7 @@ func TestSet_FilePermissions(t *testing.T) { t.Fatalf("Stat() error = %v", err) } perm := info.Mode().Perm() - if perm != 0600 { + if perm != 0o600 { t.Errorf("file permissions = %o, want 0600", perm) } } diff --git a/internal/cache/cached_eligibility.go b/internal/cache/cached_eligibility.go index 3aaf22d..5127b49 100644 --- a/internal/cache/cached_eligibility.go +++ b/internal/cache/cached_eligibility.go @@ -2,7 +2,7 @@ package cache import ( "context" - "fmt" + "errors" "strings" "github.com/aaearon/grant-cli/internal/sca/models" @@ -93,7 +93,7 @@ func (c *CachedEligibilityLister) ListEligibility(ctx context.Context, csp model // ListGroupsEligibility checks the cache first, then falls through to the inner lister. func (c *CachedEligibilityLister) ListGroupsEligibility(ctx context.Context, csp models.CSP) (*models.GroupsEligibilityResponse, error) { if c.groupsInner == nil { - return nil, fmt.Errorf("groups eligibility listing not available") + return nil, errors.New("groups eligibility listing not available") } key := groupsEligibilityCacheKey(csp) diff --git a/internal/cache/cached_eligibility_test.go b/internal/cache/cached_eligibility_test.go index 13d7e47..be085a2 100644 --- a/internal/cache/cached_eligibility_test.go +++ b/internal/cache/cached_eligibility_test.go @@ -50,7 +50,7 @@ func TestCachedEligibilityLister_CacheHit(t *testing.T) { } cached := NewCachedEligibilityLister(inner, nil, store, false, nil) - ctx := context.Background() + ctx := t.Context() // First call — miss, calls inner resp1, err := cached.ListEligibility(ctx, models.CSPAzure) @@ -94,7 +94,7 @@ func TestCachedEligibilityLister_CacheMiss_Expired(t *testing.T) { // Write cache entry in the past pastStore := &Store{dir: dir, ttl: ttl, now: func() time.Time { return time.Now().Add(-2 * time.Hour) }} cached := NewCachedEligibilityLister(inner, nil, pastStore, false, nil) - ctx := context.Background() + ctx := t.Context() _, _ = cached.ListEligibility(ctx, models.CSPAzure) if inner.calls != 1 { @@ -128,7 +128,7 @@ func TestCachedEligibilityLister_RefreshBypass(t *testing.T) { // Pre-populate cache cached := NewCachedEligibilityLister(inner, nil, store, false, nil) - ctx := context.Background() + ctx := t.Context() _, _ = cached.ListEligibility(ctx, models.CSPAzure) if inner.calls != 1 { t.Fatalf("expected 1 call, got %d", inner.calls) @@ -153,7 +153,7 @@ func TestCachedEligibilityLister_APIError_NoCache(t *testing.T) { inner := &mockEligibilityLister{err: apiErr} cached := NewCachedEligibilityLister(inner, nil, store, false, nil) - ctx := context.Background() + ctx := t.Context() _, err := cached.ListEligibility(ctx, models.CSPAzure) if !errors.Is(err, apiErr) { @@ -180,7 +180,7 @@ func TestCachedEligibilityLister_CorruptCache_Fallthrough(t *testing.T) { } cached := NewCachedEligibilityLister(inner, nil, store, false, nil) - ctx := context.Background() + ctx := t.Context() resp, err := cached.ListEligibility(ctx, models.CSPAzure) if err != nil { @@ -208,7 +208,7 @@ func TestCachedGroupsEligibilityLister_CacheHit(t *testing.T) { } cached := NewCachedEligibilityLister(nil, inner, store, false, nil) - ctx := context.Background() + ctx := t.Context() // First call — miss resp1, err := cached.ListGroupsEligibility(ctx, models.CSPAzure) @@ -250,7 +250,7 @@ func TestCachedGroupsEligibilityLister_RefreshBypass(t *testing.T) { // Pre-populate cache cached := NewCachedEligibilityLister(nil, inner, store, false, nil) - ctx := context.Background() + ctx := t.Context() _, _ = cached.ListGroupsEligibility(ctx, models.CSPAzure) // With refresh=true, should bypass cache @@ -269,7 +269,7 @@ func TestCachedGroupsEligibilityLister_NilInner(t *testing.T) { store := NewStore(t.TempDir(), 4*time.Hour) cached := NewCachedEligibilityLister(nil, nil, store, false, nil) - ctx := context.Background() + ctx := t.Context() _, err := cached.ListGroupsEligibility(ctx, models.CSPAzure) if err == nil { @@ -289,7 +289,7 @@ func TestCachedEligibilityLister_DifferentCSPs_SeparateKeys(t *testing.T) { } cached := NewCachedEligibilityLister(inner, nil, store, false, nil) - ctx := context.Background() + ctx := t.Context() _, _ = cached.ListEligibility(ctx, models.CSPAzure) _, _ = cached.ListEligibility(ctx, models.CSPAWS) @@ -329,7 +329,7 @@ func TestCachedEligibilityLister_LogsHitAndMiss(t *testing.T) { } cached := NewCachedEligibilityLister(inner, nil, store, false, log) - ctx := context.Background() + ctx := t.Context() // First call — miss _, _ = cached.ListEligibility(ctx, models.CSPAzure) @@ -373,7 +373,7 @@ func TestCachedEligibilityLister_LogsRefreshBypass(t *testing.T) { } cached := NewCachedEligibilityLister(inner, nil, store, true, log) - ctx := context.Background() + ctx := t.Context() _, _ = cached.ListEligibility(ctx, models.CSPAzure) found := false @@ -389,5 +389,5 @@ func TestCachedEligibilityLister_LogsRefreshBypass(t *testing.T) { // writeCorruptCacheFile writes invalid JSON to a cache file. func writeCorruptCacheFile(dir, key string) error { - return os.WriteFile(dir+"/"+key+".json", []byte("{invalid json"), 0600) + return os.WriteFile(dir+"/"+key+".json", []byte("{invalid json"), 0o600) } diff --git a/internal/config/config.go b/internal/config/config.go index b6846e4..afb7a53 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,3 +1,4 @@ +// Package config manages grant application configuration. package config import ( @@ -71,7 +72,7 @@ func Load(path string) (*Config, error) { // Save writes a config to the given path, creating parent directories as needed. func Save(cfg *Config, path string) error { dir := filepath.Dir(path) - if err := os.MkdirAll(dir, 0700); err != nil { + if err := os.MkdirAll(dir, 0o700); err != nil { return err } @@ -80,7 +81,7 @@ func Save(cfg *Config, path string) error { return err } - return os.WriteFile(path, data, 0600) + return os.WriteFile(path, data, 0o600) } // LoadDefaultWithPath resolves the config path via ConfigPath() and loads the config. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 52fd135..ca5c11d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -45,7 +45,7 @@ favorites: target: account-456 role: ReadOnly `) - if err := os.WriteFile(path, content, 0644); err != nil { + if err := os.WriteFile(path, content, 0o644); err != nil { t.Fatalf("failed to write test file: %v", err) } @@ -180,13 +180,13 @@ func TestLoadConfig_PermissionError(t *testing.T) { path := filepath.Join(dir, "config.yaml") // Create a file, then make it unreadable - if err := os.WriteFile(path, []byte("profile: test"), 0644); err != nil { + if err := os.WriteFile(path, []byte("profile: test"), 0o644); err != nil { t.Fatalf("failed to write test file: %v", err) } - if err := os.Chmod(path, 0000); err != nil { + if err := os.Chmod(path, 0o000); err != nil { t.Fatalf("failed to chmod: %v", err) } - t.Cleanup(func() { _ = os.Chmod(path, 0644) }) + t.Cleanup(func() { _ = os.Chmod(path, 0o644) }) _, err := Load(path) if err == nil { @@ -325,7 +325,7 @@ favorites: target: sub-999 role: Reader `) - if err := os.WriteFile(path, content, 0644); err != nil { + if err := os.WriteFile(path, content, 0o644); err != nil { t.Fatalf("failed to write test file: %v", err) } @@ -377,7 +377,7 @@ favorites: target: sub-old role: Reader `) - if err := os.WriteFile(path, content, 0644); err != nil { + if err := os.WriteFile(path, content, 0o644); err != nil { t.Fatalf("failed to write test file: %v", err) } @@ -429,7 +429,7 @@ func TestLoadConfig_WithCacheTTL(t *testing.T) { default_provider: azure cache_ttl: 2h `) - if err := os.WriteFile(path, content, 0644); err != nil { + if err := os.WriteFile(path, content, 0o644); err != nil { t.Fatalf("failed to write test file: %v", err) } @@ -450,7 +450,7 @@ func TestLoadConfig_BackwardsCompat_NoCacheTTL(t *testing.T) { content := []byte(`profile: old-profile default_provider: azure `) - if err := os.WriteFile(path, content, 0644); err != nil { + if err := os.WriteFile(path, content, 0o644); err != nil { t.Fatalf("failed to write test file: %v", err) } diff --git a/internal/sca/logging_client_test.go b/internal/sca/logging_client_test.go index e40f711..146a6a1 100644 --- a/internal/sca/logging_client_test.go +++ b/internal/sca/logging_client_test.go @@ -1,7 +1,6 @@ package sca import ( - "context" "errors" "fmt" "net/http" @@ -63,7 +62,7 @@ func TestLoggingClient_Get(t *testing.T) { inner := &mockHTTPClient{getResponse: tt.resp, getError: tt.err} lc := newLoggingClient(inner, ml) - resp, err := lc.Get(context.Background(), tt.route, nil) + resp, err := lc.Get(t.Context(), tt.route, nil) if tt.err != nil { if err == nil { @@ -125,7 +124,7 @@ func TestLoggingClient_Post(t *testing.T) { inner := &mockHTTPClient{postResponse: tt.resp, postError: tt.err} lc := newLoggingClient(inner, ml) - resp, err := lc.Post(context.Background(), tt.route, nil) + resp, err := lc.Post(t.Context(), tt.route, nil) if tt.err != nil { if err == nil { @@ -161,7 +160,7 @@ func TestLoggingClient_LogsDuration(t *testing.T) { } lc := newLoggingClient(inner, ml) - _, _ = lc.Get(context.Background(), "/api/test", nil) + _, _ = lc.Get(t.Context(), "/api/test", nil) found := false for _, c := range ml.calls { @@ -185,7 +184,7 @@ func TestLoggingClient_DebugLogsHeaders(t *testing.T) { } lc := newLoggingClient(inner, ml) - _, _ = lc.Get(context.Background(), "/api/test", nil) + _, _ = lc.Get(t.Context(), "/api/test", nil) var debugCall *logCall for i, c := range ml.calls { diff --git a/internal/sca/models/credentials.go b/internal/sca/models/credentials.go index 0d52631..7af3ebd 100644 --- a/internal/sca/models/credentials.go +++ b/internal/sca/models/credentials.go @@ -2,6 +2,7 @@ package models import ( "encoding/json" + "errors" "fmt" ) @@ -15,14 +16,14 @@ type AWSCredentials struct { // ParseAWSCredentials parses an accessCredentials JSON string into AWSCredentials. func ParseAWSCredentials(s string) (*AWSCredentials, error) { if s == "" { - return nil, fmt.Errorf("empty credentials string") + return nil, errors.New("empty credentials string") } var creds AWSCredentials if err := json.Unmarshal([]byte(s), &creds); err != nil { return nil, fmt.Errorf("failed to parse AWS credentials: %w", err) } if creds.AccessKeyID == "" || creds.SecretAccessKey == "" || creds.SessionToken == "" { - return nil, fmt.Errorf("incomplete AWS credentials: missing required fields") + return nil, errors.New("incomplete AWS credentials: missing required fields") } return &creds, nil } diff --git a/internal/sca/models/eligibility.go b/internal/sca/models/eligibility.go index 2c5d882..957bbf4 100644 --- a/internal/sca/models/eligibility.go +++ b/internal/sca/models/eligibility.go @@ -1,3 +1,4 @@ +// Package models defines SCA API request and response types. package models import "encoding/json" diff --git a/internal/sca/models/session_test.go b/internal/sca/models/session_test.go index f9411fa..d6eb893 100644 --- a/internal/sca/models/session_test.go +++ b/internal/sca/models/session_test.go @@ -215,10 +215,8 @@ func TestSessionInfo_WithTarget(t *testing.T) { if session.Target.ID != tt.wantTargetID { t.Errorf("Target.ID = %q, want %q", session.Target.ID, tt.wantTargetID) } - } else { - if session.Target != nil { - t.Errorf("expected nil Target, got %+v", session.Target) - } + } else if session.Target != nil { + t.Errorf("expected nil Target, got %+v", session.Target) } if got := session.IsGroupSession(); got != tt.wantIsGroup { diff --git a/internal/sca/service.go b/internal/sca/service.go index 9e2c072..1d99e1b 100644 --- a/internal/sca/service.go +++ b/internal/sca/service.go @@ -1,8 +1,10 @@ +// Package sca provides the CyberArk SCA Access API client. package sca import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -49,7 +51,7 @@ func NewSCAAccessService(authenticators ...auth.IdsecAuth) (*SCAAccessService, e ispAuth, ok := ispAuthIface.(*auth.IdsecISPAuth) if !ok { - return nil, fmt.Errorf("authenticator is not *auth.IdsecISPAuth") + return nil, errors.New("authenticator is not *auth.IdsecISPAuth") } svc.ispAuth = ispAuth @@ -126,10 +128,10 @@ func (s *SCAAccessService) ListEligibility(ctx context.Context, csp models.CSP) // POST /api/access/elevate func (s *SCAAccessService) Elevate(ctx context.Context, req *models.ElevateRequest) (*models.ElevateResponse, error) { if req == nil { - return nil, fmt.Errorf("elevate request cannot be nil") + return nil, errors.New("elevate request cannot be nil") } if len(req.Targets) == 0 { - return nil, fmt.Errorf("elevate request must contain at least one target") + return nil, errors.New("elevate request must contain at least one target") } resp, err := s.httpClient.Post(ctx, "/api/access/elevate", req) @@ -154,10 +156,10 @@ func (s *SCAAccessService) Elevate(ctx context.Context, req *models.ElevateReque // POST /api/access/sessions/revoke func (s *SCAAccessService) RevokeSessions(ctx context.Context, req *models.RevokeRequest) (*models.RevokeResponse, error) { if req == nil { - return nil, fmt.Errorf("revoke request cannot be nil") + return nil, errors.New("revoke request cannot be nil") } if len(req.SessionIDs) == 0 { - return nil, fmt.Errorf("revoke request must contain at least one session ID") + return nil, errors.New("revoke request must contain at least one session ID") } resp, err := s.httpClient.Post(ctx, "/api/access/sessions/revoke", req) @@ -233,10 +235,10 @@ func (s *SCAAccessService) ListGroupsEligibility(ctx context.Context, csp models // POST /api/access/elevate/groups func (s *SCAAccessService) ElevateGroups(ctx context.Context, req *models.GroupsElevateRequest) (*models.GroupsElevateResponse, error) { if req == nil { - return nil, fmt.Errorf("groups elevate request cannot be nil") + return nil, errors.New("groups elevate request cannot be nil") } if len(req.Targets) == 0 { - return nil, fmt.Errorf("groups elevate request must contain at least one target") + return nil, errors.New("groups elevate request must contain at least one target") } resp, err := s.httpClient.Post(ctx, "/api/access/elevate/groups", req) diff --git a/internal/sca/service_test.go b/internal/sca/service_test.go index d5d45b5..c1abad8 100644 --- a/internal/sca/service_test.go +++ b/internal/sca/service_test.go @@ -60,7 +60,7 @@ func TestListEligibility_Success(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListEligibility(context.Background(), models.CSPAzure) + result, err := svc.ListEligibility(t.Context(), models.CSPAzure) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -92,7 +92,7 @@ func TestListEligibility_Empty(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListEligibility(context.Background(), models.CSPAzure) + result, err := svc.ListEligibility(t.Context(), models.CSPAzure) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -136,7 +136,7 @@ func TestListEligibility_WithPagination(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListEligibility(context.Background(), models.CSPAzure) + result, err := svc.ListEligibility(t.Context(), models.CSPAzure) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -161,7 +161,7 @@ func TestListEligibility_HTTPError(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - _, err := svc.ListEligibility(context.Background(), models.CSPAzure) + _, err := svc.ListEligibility(t.Context(), models.CSPAzure) if err == nil { t.Fatal("expected error for 401 response") @@ -208,7 +208,7 @@ func TestElevate_Success(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.Elevate(context.Background(), req) + result, err := svc.Elevate(t.Context(), req) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -270,7 +270,7 @@ func TestElevate_PartialSuccess(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.Elevate(context.Background(), req) + result, err := svc.Elevate(t.Context(), req) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -290,7 +290,7 @@ func TestElevate_NilRequest(t *testing.T) { mock := &mockHTTPClient{} svc := &SCAAccessService{httpClient: mock} - _, err := svc.Elevate(context.Background(), nil) + _, err := svc.Elevate(t.Context(), nil) if err == nil { t.Fatal("expected error for nil request") } @@ -309,7 +309,7 @@ func TestElevate_EmptyTargets(t *testing.T) { mock := &mockHTTPClient{} svc := &SCAAccessService{httpClient: mock} - _, err := svc.Elevate(context.Background(), req) + _, err := svc.Elevate(t.Context(), req) if err == nil { t.Fatal("expected error for empty targets") } @@ -337,7 +337,7 @@ func TestRevokeSessions_Success(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.RevokeSessions(context.Background(), &models.RevokeRequest{ + result, err := svc.RevokeSessions(t.Context(), &models.RevokeRequest{ SessionIDs: []string{"session-1"}, }) @@ -375,7 +375,7 @@ func TestRevokeSessions_Multiple(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.RevokeSessions(context.Background(), &models.RevokeRequest{ + result, err := svc.RevokeSessions(t.Context(), &models.RevokeRequest{ SessionIDs: []string{"session-1", "session-2"}, }) @@ -391,7 +391,7 @@ func TestRevokeSessions_NilRequest(t *testing.T) { mock := &mockHTTPClient{} svc := &SCAAccessService{httpClient: mock} - _, err := svc.RevokeSessions(context.Background(), nil) + _, err := svc.RevokeSessions(t.Context(), nil) if err == nil { t.Fatal("expected error for nil request") } @@ -404,7 +404,7 @@ func TestRevokeSessions_EmptySessionIDs(t *testing.T) { mock := &mockHTTPClient{} svc := &SCAAccessService{httpClient: mock} - _, err := svc.RevokeSessions(context.Background(), &models.RevokeRequest{ + _, err := svc.RevokeSessions(t.Context(), &models.RevokeRequest{ SessionIDs: []string{}, }) if err == nil { @@ -424,7 +424,7 @@ func TestRevokeSessions_HTTPError(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - _, err := svc.RevokeSessions(context.Background(), &models.RevokeRequest{ + _, err := svc.RevokeSessions(t.Context(), &models.RevokeRequest{ SessionIDs: []string{"session-1"}, }) @@ -461,7 +461,7 @@ func TestListSessions_Success(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListSessions(context.Background(), nil) + result, err := svc.ListSessions(t.Context(), nil) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -493,7 +493,7 @@ func TestListSessions_Empty(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListSessions(context.Background(), nil) + result, err := svc.ListSessions(t.Context(), nil) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -532,7 +532,7 @@ func TestListSessions_WithCSPFilter(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListSessions(context.Background(), &csp) + result, err := svc.ListSessions(t.Context(), &csp) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -566,7 +566,7 @@ func TestListGroupsEligibility_Success(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListGroupsEligibility(context.Background(), models.CSPAzure) + result, err := svc.ListGroupsEligibility(t.Context(), models.CSPAzure) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -597,7 +597,7 @@ func TestListGroupsEligibility_Empty(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ListGroupsEligibility(context.Background(), models.CSPAzure) + result, err := svc.ListGroupsEligibility(t.Context(), models.CSPAzure) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -616,7 +616,7 @@ func TestListGroupsEligibility_HTTPError(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - _, err := svc.ListGroupsEligibility(context.Background(), models.CSPAzure) + _, err := svc.ListGroupsEligibility(t.Context(), models.CSPAzure) if err == nil { t.Fatal("expected error for 401 response") @@ -658,7 +658,7 @@ func TestElevateGroups_Success(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ElevateGroups(context.Background(), req) + result, err := svc.ElevateGroups(t.Context(), req) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -710,7 +710,7 @@ func TestElevateGroups_WithError(t *testing.T) { } svc := &SCAAccessService{httpClient: mock} - result, err := svc.ElevateGroups(context.Background(), req) + result, err := svc.ElevateGroups(t.Context(), req) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -724,7 +724,7 @@ func TestElevateGroups_NilRequest(t *testing.T) { mock := &mockHTTPClient{} svc := &SCAAccessService{httpClient: mock} - _, err := svc.ElevateGroups(context.Background(), nil) + _, err := svc.ElevateGroups(t.Context(), nil) if err == nil { t.Fatal("expected error for nil request") } @@ -743,7 +743,7 @@ func TestElevateGroups_EmptyTargets(t *testing.T) { mock := &mockHTTPClient{} svc := &SCAAccessService{httpClient: mock} - _, err := svc.ElevateGroups(context.Background(), req) + _, err := svc.ElevateGroups(t.Context(), req) if err == nil { t.Fatal("expected error for empty targets") } diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go index b20d830..7a60a7d 100644 --- a/internal/ui/group_selector.go +++ b/internal/ui/group_selector.go @@ -1,6 +1,7 @@ package ui import ( + "errors" "fmt" "os" "sort" @@ -14,7 +15,7 @@ func FormatGroupOption(group models.GroupsEligibleTarget) string { if group.DirectoryName != "" { return fmt.Sprintf("Directory: %s / Group: %s", group.DirectoryName, group.GroupName) } - return fmt.Sprintf("Group: %s", group.GroupName) + return "Group: " + group.GroupName } // BuildGroupOptions builds a sorted list of display options from groups eligible targets. @@ -47,7 +48,7 @@ func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (* // ordered slice the user saw, avoiding wrong-group selection on display collisions. func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) { if len(groups) == 0 { - return nil, fmt.Errorf("no eligible groups available") + return nil, errors.New("no eligible groups available") } sorted := make([]models.GroupsEligibleTarget, len(groups)) diff --git a/internal/ui/selector.go b/internal/ui/selector.go index 7c49181..8d1518d 100644 --- a/internal/ui/selector.go +++ b/internal/ui/selector.go @@ -1,6 +1,8 @@ +// Package ui provides interactive selection prompts for the CLI. package ui import ( + "errors" "fmt" "os" "sort" @@ -64,7 +66,7 @@ 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 len(targets) == 0 { - return nil, fmt.Errorf("no eligible targets available") + return nil, errors.New("no eligible targets available") } options := BuildOptions(targets) diff --git a/internal/ui/session_selector.go b/internal/ui/session_selector.go index 02b9b46..78ae22c 100644 --- a/internal/ui/session_selector.go +++ b/internal/ui/session_selector.go @@ -1,6 +1,7 @@ package ui import ( + "errors" "fmt" "os" "sort" @@ -62,7 +63,7 @@ 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 len(sessions) == 0 { - return nil, fmt.Errorf("no sessions available") + return nil, errors.New("no sessions available") } options := BuildSessionOptions(sessions, nameMap) @@ -78,7 +79,7 @@ func SelectSessions(sessions []models.SessionInfo, nameMap map[string]string) ([ } if len(selected) == 0 { - return nil, fmt.Errorf("no sessions selected") + return nil, errors.New("no sessions selected") } var result []models.SessionInfo