From f058ea3bfbe3f6604f9e1776bb8cd917258c6a7b Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 17:50:24 +0200 Subject: [PATCH 1/4] chore: enable gofmt linter in golangci config --- .golangci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 65bdec2..0dd4ee9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -34,11 +34,16 @@ linters: - unconvert # unnecessary type conversions # --- Testing --- - usetesting # modern test helpers (t.Context, etc.) + # --- Formatting --- + - gofmt # canonical gofmt formatting (applies to test files too) linters-settings: gocognit: min-complexity: 40 + gofmt: + simplify: true + gocritic: enabled-tags: - diagnostic From 4674de5f0578a6eeee2ad4ceeabbea13e43e39ad Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 17:51:04 +0200 Subject: [PATCH 2/4] style: gofmt existing files --- cmd/configure.go | 2 +- cmd/configure_test.go | 1 - cmd/env.go | 4 +-- cmd/favorites_test.go | 6 ++-- cmd/helpers.go | 4 +-- cmd/integration_test.go | 4 +-- cmd/interfaces.go | 1 - cmd/output_types.go | 10 +++---- cmd/request_submit.go | 8 +++--- cmd/request_test.go | 42 ++++++++++++++-------------- cmd/revoke_test.go | 2 +- cmd/root.go | 12 ++++---- cmd/status_test.go | 17 ++++++----- cmd/test_mocks.go | 1 - internal/sca/logging_client_test.go | 8 +++--- internal/sca/models/eligibility.go | 4 +-- internal/ui/request_selector_test.go | 2 +- internal/workflows/models/request.go | 8 +++--- internal/workflows/service_test.go | 25 ++++++++--------- 19 files changed, 78 insertions(+), 83 deletions(-) diff --git a/cmd/configure.go b/cmd/configure.go index 834d75f..f40c9d7 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -8,11 +8,11 @@ import ( "path/filepath" "strings" + survey "github.com/Iilun/survey/v2" "github.com/aaearon/grant-cli/internal/config" "github.com/cyberark/idsec-sdk-golang/pkg/models" authmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/auth" "github.com/cyberark/idsec-sdk-golang/pkg/profiles" - survey "github.com/Iilun/survey/v2" "github.com/spf13/cobra" ) diff --git a/cmd/configure_test.go b/cmd/configure_test.go index c73f7e8..233a42d 100644 --- a/cmd/configure_test.go +++ b/cmd/configure_test.go @@ -373,4 +373,3 @@ func TestValidateTenantURL(t *testing.T) { }) } } - diff --git a/cmd/env.go b/cmd/env.go index 3d77206..2843c17 100644 --- a/cmd/env.go +++ b/cmd/env.go @@ -106,9 +106,9 @@ func runEnvWithDeps( if isJSONOutput() { return writeJSON(cmd.OutOrStdout(), awsCredentialOutput{ - AccessKeyID: awsCreds.AccessKeyID, + AccessKeyID: awsCreds.AccessKeyID, SecretAccessKey: awsCreds.SecretAccessKey, - SessionToken: awsCreds.SessionToken, + SessionToken: awsCreds.SessionToken, }) } diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go index 1f84714..6028f84 100644 --- a/cmd/favorites_test.go +++ b/cmd/favorites_test.go @@ -814,9 +814,9 @@ func TestFavoritesAddInteractiveMode(t *testing.T) { cfg := config.DefaultConfig() _ = config.Save(cfg, path) }, - eligLister: nil, - selector: nil, - args: []string{"--target", "sub-123", "--role", "Contributor"}, + eligLister: nil, + selector: nil, + args: []string{"--target", "sub-123", "--role", "Contributor"}, wantContain: []string{"name is required"}, wantErr: true, }, diff --git a/cmd/helpers.go b/cmd/helpers.go index 05e0ae1..78c3af3 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -14,8 +14,8 @@ import ( type statusData struct { sessions *scamodels.SessionsResponse nameMap map[string]string - groupNameMap map[string]string // groupID -> groupName - remainingMap map[string]time.Duration // sessionID -> remaining time + groupNameMap map[string]string // groupID -> groupName + remainingMap map[string]time.Duration // sessionID -> remaining time } // fetchStatusData fires sessions and all-CSP eligibility calls concurrently, diff --git a/cmd/integration_test.go b/cmd/integration_test.go index 299ed13..ad96343 100644 --- a/cmd/integration_test.go +++ b/cmd/integration_test.go @@ -259,8 +259,8 @@ func TestIntegration_InvalidCommand(t *testing.T) { // Should show an error or help message if !strings.Contains(outputStr, "unknown command") && - !strings.Contains(outputStr, "Error:") && - !strings.Contains(outputStr, "Usage:") { + !strings.Contains(outputStr, "Error:") && + !strings.Contains(outputStr, "Usage:") { t.Errorf("Expected error for invalid command, got:\n%s", outputStr) } } diff --git a/cmd/interfaces.go b/cmd/interfaces.go index 4d8677e..448af8f 100644 --- a/cmd/interfaces.go +++ b/cmd/interfaces.go @@ -100,4 +100,3 @@ type accessRequestService interface { CancelRequest(ctx context.Context, requestID string, reason *string) (*wfmodels.AccessRequest, error) FinalizeRequest(ctx context.Context, requestID string, result string, reason *string) (*wfmodels.AccessRequest, error) } - diff --git a/cmd/output_types.go b/cmd/output_types.go index 91dff4f..77fea39 100644 --- a/cmd/output_types.go +++ b/cmd/output_types.go @@ -2,11 +2,11 @@ package cmd // cloudElevationOutput is the JSON representation of a cloud elevation result. type cloudElevationOutput struct { - Type string `json:"type"` - Provider string `json:"provider"` - SessionID string `json:"sessionId"` - Target string `json:"target"` - Role string `json:"role"` + Type string `json:"type"` + Provider string `json:"provider"` + SessionID string `json:"sessionId"` + Target string `json:"target"` + Role string `json:"role"` Credentials *awsCredentialOutput `json:"credentials,omitempty"` } diff --git a/cmd/request_submit.go b/cmd/request_submit.go index 095807b..b73ffbe 100644 --- a/cmd/request_submit.go +++ b/cmd/request_submit.go @@ -77,10 +77,10 @@ type submitFields struct { // submitWorkspace holds deduplicated workspace info derived from eligibility. type submitWorkspace struct { - WorkspaceID string - WorkspaceName string - WorkspaceType models.WorkspaceType - CSP models.CSP + WorkspaceID string + WorkspaceName string + WorkspaceType models.WorkspaceType + CSP models.CSP OrganizationID string } diff --git a/cmd/request_test.go b/cmd/request_test.go index 34d5338..749e87a 100644 --- a/cmd/request_test.go +++ b/cmd/request_test.go @@ -26,8 +26,8 @@ func TestRequestListCommand(t *testing.T) { svc: &mockAccessRequestService{ listItems: []wfmodels.AccessRequest{ { - RequestID: "req-1", - RequestState: wfmodels.RequestStatePending, + RequestID: "req-1", + RequestState: wfmodels.RequestStatePending, RequestResult: wfmodels.RequestResultUnknown, RequestDetails: map[string]interface{}{ "workspaceName": "Azure Subscription", @@ -48,13 +48,13 @@ func TestRequestListCommand(t *testing.T) { svc: &mockAccessRequestService{ listItems: []wfmodels.AccessRequest{ { - RequestID: "req-1", - RequestState: wfmodels.RequestStatePending, + RequestID: "req-1", + RequestState: wfmodels.RequestStatePending, RequestResult: wfmodels.RequestResultUnknown, - CreatedBy: "user@test.com", - CreatedAt: "t", - UpdatedBy: "SYSTEM", - UpdatedAt: "t", + CreatedBy: "user@test.com", + CreatedAt: "t", + UpdatedBy: "SYSTEM", + UpdatedAt: "t", }, }, listTotalCount: 1, @@ -112,9 +112,9 @@ func TestRequestGetCommand(t *testing.T) { name: "get request text", svc: &mockAccessRequestService{ getResult: &wfmodels.AccessRequest{ - RequestID: "req-1", - RequestState: wfmodels.RequestStateFinished, - RequestResult: wfmodels.RequestResultApproved, + RequestID: "req-1", + RequestState: wfmodels.RequestStateFinished, + RequestResult: wfmodels.RequestResultApproved, TargetCategory: "CLOUD_CONSOLE", RequestDetails: map[string]interface{}{ "workspaceName": "Azure Sub", @@ -136,13 +136,13 @@ func TestRequestGetCommand(t *testing.T) { name: "get request JSON", svc: &mockAccessRequestService{ getResult: &wfmodels.AccessRequest{ - RequestID: "req-1", - RequestState: wfmodels.RequestStateFinished, + RequestID: "req-1", + RequestState: wfmodels.RequestStateFinished, RequestResult: wfmodels.RequestResultApproved, - CreatedBy: "user@test.com", - CreatedAt: "t", - UpdatedBy: "SYSTEM", - UpdatedAt: "t", + CreatedBy: "user@test.com", + CreatedAt: "t", + UpdatedBy: "SYSTEM", + UpdatedAt: "t", }, }, args: []string{"get", "req-1", "--output", "json"}, @@ -328,8 +328,8 @@ func TestRequestListJSON(t *testing.T) { svc := &mockAccessRequestService{ listItems: []wfmodels.AccessRequest{ { - RequestID: "req-1", - RequestState: wfmodels.RequestStatePending, + RequestID: "req-1", + RequestState: wfmodels.RequestStatePending, RequestResult: wfmodels.RequestResultUnknown, RequestDetails: map[string]interface{}{ "priority": "High", @@ -370,8 +370,8 @@ func TestRequestListJSON(t *testing.T) { func TestValidateSubmitFields(t *testing.T) { tests := []struct { - name string - fields *submitFields + name string + fields *submitFields wantErr bool }{ {"valid", &submitFields{"need access", "High", "2026-04-21", "America/New_York", "09:00", "17:00"}, false}, diff --git a/cmd/revoke_test.go b/cmd/revoke_test.go index 64df31f..6295034 100644 --- a/cmd/revoke_test.go +++ b/cmd/revoke_test.go @@ -358,7 +358,7 @@ func TestRevokeCommand(t *testing.T) { setupLister: func() *mockSessionLister { return &mockSessionLister{sessions: activeSessions} }, - setupElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, setupRevoker: func() *mockSessionRevoker { return &mockSessionRevoker{} }, setupSelector: func() *mockSessionSelector { return &mockSessionSelector{selectErr: errors.New("prompt interrupted")} diff --git a/cmd/root.go b/cmd/root.go index 43cfaa0..5e0c244 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -132,10 +132,10 @@ var rootCmd = newRootCommand(runElevateProduction) // bootstrap memoization state — shared auth/profile across all service // bootstraps within a single process invocation so we authenticate exactly once. var ( - bootstrapOnce = new(sync.Once) - bootstrapISPAuthVal auth.IdsecAuth - bootstrapProfileVal *sdkmodels.IdsecProfile - errBootstrap error + bootstrapOnce = new(sync.Once) + bootstrapISPAuthVal auth.IdsecAuth + bootstrapProfileVal *sdkmodels.IdsecProfile + errBootstrap error ) // bootstrapImpl is the function that performs the profile load + authentication. @@ -893,9 +893,9 @@ func writeElevationJSON(cmd *cobra.Command, cloudRes *elevationResult, groupRes return fmt.Errorf("failed to parse access credentials: %w", err) } out.Credentials = &awsCredentialOutput{ - AccessKeyID: awsCreds.AccessKeyID, + AccessKeyID: awsCreds.AccessKeyID, SecretAccessKey: awsCreds.SecretAccessKey, - SessionToken: awsCreds.SessionToken, + SessionToken: awsCreds.SessionToken, } } diff --git a/cmd/status_test.go b/cmd/status_test.go index 375d95c..ba7e918 100644 --- a/cmd/status_test.go +++ b/cmd/status_test.go @@ -19,14 +19,14 @@ func TestStatusCommand(t *testing.T) { expiresIn := commonmodels.IdsecRFC3339Time(now.Add(1 * time.Hour)) tests := []struct { - name string - setupAuth func() *mockAuthLoader - setupSvc func() *mockSessionLister - setupEligibility func() *mockEligibilityLister - provider string - wantContain []string - wantNotContain []string - wantErr bool + name string + setupAuth func() *mockAuthLoader + setupSvc func() *mockSessionLister + setupEligibility func() *mockEligibilityLister + provider string + wantContain []string + wantNotContain []string + wantErr bool }{ { name: "not authenticated", @@ -658,7 +658,6 @@ func TestStatusCommandIntegration(t *testing.T) { } } - func TestStatusCommand_CachedEligibility(t *testing.T) { now := time.Now() expiresIn := commonmodels.IdsecRFC3339Time(now.Add(1 * time.Hour)) diff --git a/cmd/test_mocks.go b/cmd/test_mocks.go index 5e01aa3..aa6b636 100644 --- a/cmd/test_mocks.go +++ b/cmd/test_mocks.go @@ -301,4 +301,3 @@ func (c *countingEligibilityLister) CallCount(csp models.CSP) int { defer c.mu.Unlock() return c.counts[csp] } - diff --git a/internal/sca/logging_client_test.go b/internal/sca/logging_client_test.go index 146a6a1..0a30447 100644 --- a/internal/sca/logging_client_test.go +++ b/internal/sca/logging_client_test.go @@ -209,10 +209,10 @@ func TestLoggingClient_DebugLogsHeaders(t *testing.T) { func TestRedactHeaders(t *testing.T) { tests := []struct { - name string - headers http.Header - wantHas string - wantNot string + name string + headers http.Header + wantHas string + wantNot string }{ { name: "redacts Authorization", diff --git a/internal/sca/models/eligibility.go b/internal/sca/models/eligibility.go index 957bbf4..3b60b90 100644 --- a/internal/sca/models/eligibility.go +++ b/internal/sca/models/eligibility.go @@ -65,6 +65,6 @@ func (t *EligibleTarget) UnmarshalJSON(data []byte) error { // EligibilityResponse is the response from GET /api/access/{CSP}/eligibility. type EligibilityResponse struct { Response []EligibleTarget `json:"response"` - NextToken *string `json:"nextToken"` - Total int `json:"total"` + NextToken *string `json:"nextToken"` + Total int `json:"total"` } diff --git a/internal/ui/request_selector_test.go b/internal/ui/request_selector_test.go index 9495809..a4dbc6f 100644 --- a/internal/ui/request_selector_test.go +++ b/internal/ui/request_selector_test.go @@ -83,7 +83,7 @@ func TestBuildRequestOptions_SortedCorrectlyWithOffsets(t *testing.T) { // String sort would put +02:00 after Z; time sort must put 09:30Z first. reqs := []wfmodels.AccessRequest{ {RequestID: "offset", CreatedAt: "2026-04-20T10:00:00+02:00"}, // 08:00 UTC - {RequestID: "utc", CreatedAt: "2026-04-20T09:30:00Z"}, // 09:30 UTC (more recent) + {RequestID: "utc", CreatedAt: "2026-04-20T09:30:00Z"}, // 09:30 UTC (more recent) } _, sorted := BuildRequestOptions(reqs) if sorted[0].RequestID != "utc" { diff --git a/internal/workflows/models/request.go b/internal/workflows/models/request.go index f7dc12d..15ab41a 100644 --- a/internal/workflows/models/request.go +++ b/internal/workflows/models/request.go @@ -43,10 +43,10 @@ type AccessRequest struct { // Entity represents a user or approver identity. type Entity struct { - EntityID string `json:"entityId"` - EntityName string `json:"entityName"` - EntityDisplayName string `json:"entityDisplayName,omitempty"` - EntityEmail string `json:"entityEmail,omitempty"` + EntityID string `json:"entityId"` + EntityName string `json:"entityName"` + EntityDisplayName string `json:"entityDisplayName,omitempty"` + EntityEmail string `json:"entityEmail,omitempty"` EntityDirectorySource *DirectorySource `json:"entityDirectorySource,omitempty"` } diff --git a/internal/workflows/service_test.go b/internal/workflows/service_test.go index f49b324..c696005 100644 --- a/internal/workflows/service_test.go +++ b/internal/workflows/service_test.go @@ -149,7 +149,6 @@ func TestListRequests_MaxPagesExceeded(t *testing.T) { } } - func TestListRequests_WithFilters(t *testing.T) { mock := &mockHTTPClient{ getFn: func(_ context.Context, _ string, params interface{}) (*http.Response, error) { @@ -191,13 +190,13 @@ func TestListRequests_WithFilters(t *testing.T) { func TestGetRequest(t *testing.T) { expected := models.AccessRequest{ - RequestID: "8a45155d-0273-4bc8-8d45-9fe3f4d4de6d", - RequestState: models.RequestStateFinished, + RequestID: "8a45155d-0273-4bc8-8d45-9fe3f4d4de6d", + RequestState: models.RequestStateFinished, RequestResult: models.RequestResultApproved, - CreatedBy: "user@test", - CreatedAt: "2025-08-12T09:41:00", - UpdatedBy: "SYSTEM", - UpdatedAt: "2025-08-12T09:42:31", + CreatedBy: "user@test", + CreatedAt: "2025-08-12T09:41:00", + UpdatedBy: "SYSTEM", + UpdatedAt: "2025-08-12T09:42:31", } mock := &mockHTTPClient{ @@ -233,13 +232,13 @@ func TestSubmitRequest(t *testing.T) { t.Errorf("targetCategory: got %q", req.TargetCategory) } return jsonResponse(200, models.AccessRequest{ - RequestID: "new-id", - RequestState: models.RequestStateStarting, + RequestID: "new-id", + RequestState: models.RequestStateStarting, RequestResult: models.RequestResultUnknown, - CreatedBy: "user@test", - CreatedAt: "2025-08-12T09:41:00", - UpdatedBy: "SYSTEM", - UpdatedAt: "2025-08-12T09:41:00", + CreatedBy: "user@test", + CreatedAt: "2025-08-12T09:41:00", + UpdatedBy: "SYSTEM", + UpdatedAt: "2025-08-12T09:41:00", }), nil }, } From f054d4e41acc4803a601f29d2758afe62f3439c3 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 17:51:30 +0200 Subject: [PATCH 3/4] docs: record gofmt linter in CLAUDE.md lint section --- CLAUDE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c0d02d8..03dc69d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -138,8 +138,9 @@ Custom `SCAAccessService` follows SDK conventions: ## 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 +- 20 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, gofmt (`simplify: true`) +- Test files excluded from gosec, gocognit, bodyclose — `gofmt` has no exclusion, formatting is universal +- Run `gofmt -s -w .` before committing; `gofumpt` was rejected because the codebase is not gofumpt-clean - `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) From a648e4ce7b1d858dcaab0adf8feaaf80882b2c8c Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 17:43:38 +0200 Subject: [PATCH 4/4] fix: report the real SDK profile directory in grant configure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `grant configure` printed `~/.idsec_profiles/grant` in both its Long help and its "Profile saved to" success message. That path has been wrong since the idsec-sdk-golang upgrade shipped in v0.7.0: the SDK's profiles.GetProfilesFolder() resolves IDSEC_PROFILES_FOLDER, falling back to $HOME/.idsec/profiles. The hand-rolled resolution is replaced by a direct call to profiles.GetProfilesFolder(). It is deliberately NOT "improved" to use os.UserHomeDir(): the SDK reads os.Getenv("HOME"), which on Windows is often unset and yields a relative .idsec/profiles under the CWD. configure must print exactly the path the loader will later read — diverging would be worse than the stale string. The Windows caveat is documented in CLAUDE.md. Tests (written first, confirmed failing): - TestRunConfigurePrintsProfilePath — table-driven over the full resolution matrix: custom IDSEC_PROFILES_FOLDER, override unset, override set-but-empty (must fall back to HOME — the SDK only takes it when non-empty), empty HOME (relative path, the Windows case) and relative HOME. Each case also asserts the printed path equals profiles.GetProfilesFolder()/grant, pinning the write path to what login/root read, and that the legacy path never appears. - TestConfigureLongHelpHasNoLegacyPath — guards the help text. CHANGELOG: the released [0.7.0] section is edited to add the migration note, a deliberate exception to Keep-a-Changelog immutability — the directory move was a user-visible side effect of that release's SDK upgrade and was never recorded, so users upgrading from v0.6.x have no way to learn about it from an [Unreleased] entry. The documented migration uses `mv -n` so a user who already re-ran `grant configure` cannot clobber their current profile with the stale one; `-n` is supported by both GNU and BSD/macOS mv. The help-text fix itself is under [Unreleased]/Fixed. --- CHANGELOG.md | 5 ++ CLAUDE.md | 3 +- cmd/configure.go | 12 ++-- cmd/configure_test.go | 133 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af6023f..464e29f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,15 @@ All notable changes to this project will be documented in this file. - Rebranded user-facing references from CyberArk to Idira (formerly CyberArk) in the README and CLI help/prompt text, following the Palo Alto Networks rebrand. No functional or API changes; SDK import paths (`github.com/cyberark/idsec-sdk-golang`), `*.cyberark.cloud` URLs, and environment variables are unchanged. +### Fixed + +- `grant configure` no longer reports a stale SDK profile location. The help text and the "Profile saved to" success message printed `~/.idsec_profiles/grant`, which has been wrong since the SDK upgrade in v0.7.0. The path is now resolved with the SDK's own `profiles.GetProfilesFolder()`, so it always matches the directory the profile loader reads (`~/.idsec/profiles/grant` by default, or `IDSEC_PROFILES_FOLDER` when set). + ## [0.7.0] - 2026-04-21 ### Changed +- **Profile location moved.** As a side effect of the SDK upgrade, the SDK profile directory changed from `~/.idsec_profiles/` to `~/.idsec/profiles/` (override with `IDSEC_PROFILES_FOLDER`). Users upgrading from v0.6.x or earlier must either re-run `grant configure` (recommended) or move the old file into place. If a profile already exists at the new location it is the current one and must win, so use the no-clobber form (`-n` is supported by both GNU and BSD/macOS `mv`): `mkdir -p ~/.idsec/profiles && mv -n ~/.idsec_profiles/grant ~/.idsec/profiles/grant`. - Upgraded `github.com/cyberark/idsec-sdk-golang` from v0.1.14 to v0.2.3. `isp.FromISPAuth` now takes a retry-strategy argument; we pass `nil` to preserve v0.1.14 behavior (which itself defaulted to nil internally). No new direct Go module dependencies; indirect dep set is smaller - `--output json` is now a pure serialisation flag; it no longer forces non-interactive mode. Interactive pickers and prompts (e.g. `grant request get -o json` with no ID, `grant request submit -o json` without `--target`/`--role`) work in a TTY, writing prompts to stderr and JSON to stdout. diff --git a/CLAUDE.md b/CLAUDE.md index 03dc69d..1d4d597 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,7 +129,8 @@ Custom `SCAAccessService` follows SDK conventions: ## Config - App config: `~/.grant/config.yaml` -- SDK profile: `~/.idsec_profiles/grant` +- SDK profile: `~/.idsec/profiles/grant` (default; override via `IDSEC_PROFILES_FOLDER`) +- Always resolve the profile directory with `profiles.GetProfilesFolder()` (SDK) — never hand-roll it. The SDK reads `os.Getenv("HOME")`, not `os.UserHomeDir()`; on Windows `HOME` is frequently unset, so it resolves to a **relative** `.idsec/profiles` under the process CWD. Any code that prints or computes the profile path must agree with the loader, so reproduce the SDK's behavior rather than "correcting" it ## Authentication - Use the `/grant-login` skill when you need to authenticate to the grant CLI (e.g., before manual testing) diff --git a/cmd/configure.go b/cmd/configure.go index f40c9d7..ad2b639 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/url" - "os" "path/filepath" "strings" @@ -24,7 +23,7 @@ func NewConfigureCommand() *cobra.Command { Long: `Configure grant by providing your Idira username and optional Identity URL. This command creates two configuration files: -- SDK profile at ~/.idsec_profiles/grant +- SDK profile at ~/.idsec/profiles/grant (override with IDSEC_PROFILES_FOLDER) - App config at ~/.grant/config.yaml The Identity URL is optional — the SDK can auto-discover it from your username. @@ -110,12 +109,9 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st return fmt.Errorf("failed to save profile: %w", err) } - // Get profile directory for success message - profileDir := os.Getenv("IDSEC_PROFILES_FOLDER") - if profileDir == "" { - home, _ := os.UserHomeDir() - profileDir = filepath.Join(home, ".idsec_profiles") - } + // Get profile directory for success message. Must use the SDK's own resolver + // so the printed path matches what the profile loader will later read. + profileDir := profiles.GetProfilesFolder() profilePath := filepath.Join(profileDir, "grant") // Create app config diff --git a/cmd/configure_test.go b/cmd/configure_test.go index 233a42d..1da67e9 100644 --- a/cmd/configure_test.go +++ b/cmd/configure_test.go @@ -9,6 +9,7 @@ import ( "github.com/aaearon/grant-cli/internal/config" "github.com/cyberark/idsec-sdk-golang/pkg/models" authmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/auth" + "github.com/cyberark/idsec-sdk-golang/pkg/profiles" ) func TestConfigureCommand(t *testing.T) { @@ -373,3 +374,135 @@ func TestValidateTenantURL(t *testing.T) { }) } } + +// How the IDSEC_PROFILES_FOLDER override is set up for a test case. The SDK +// only honors the override when it is set AND non-empty, so "unset" and +// "empty" must be distinguishable. +const ( + folderUnset = "unset" + folderEmpty = "empty" + folderCustom = "custom" +) + +// TestRunConfigurePrintsProfilePath verifies the success message reports the +// same directory the SDK profile loader actually uses (profiles.GetProfilesFolder). +// +// These cases deliberately mirror the SDK's own resolution — including its use +// of os.Getenv("HOME") rather than os.UserHomeDir(), which yields a relative +// path when HOME is empty (the common Windows case). configure must print +// whatever the loader will later read, warts included. +// +// Not parallel: t.Setenv mutates process-global state. +func TestRunConfigurePrintsProfilePath(t *testing.T) { + tests := []struct { + name string + homeTempDir bool // use t.TempDir() as HOME + home string // literal HOME value when homeTempDir is false + folderMode string + wantContainsFn func(profilesFolder, home string) string + }{ + { + name: "honors IDSEC_PROFILES_FOLDER", + homeTempDir: true, + folderMode: folderCustom, + wantContainsFn: func(profilesFolder, home string) string { + return filepath.Join(profilesFolder, "grant") + }, + }, + { + name: "default under HOME when override unset", + homeTempDir: true, + folderMode: folderUnset, + wantContainsFn: func(profilesFolder, home string) string { + return filepath.Join(home, ".idsec", "profiles", "grant") + }, + }, + { + name: "empty IDSEC_PROFILES_FOLDER falls back to HOME", + homeTempDir: true, + folderMode: folderEmpty, + wantContainsFn: func(profilesFolder, home string) string { + return filepath.Join(home, ".idsec", "profiles", "grant") + }, + }, + { + name: "empty HOME yields a relative path", + home: "", + folderMode: folderUnset, + wantContainsFn: func(profilesFolder, home string) string { + return filepath.Join(".idsec", "profiles", "grant") + }, + }, + { + name: "relative HOME is preserved", + home: filepath.Join("relative", "home"), + folderMode: folderUnset, + wantContainsFn: func(profilesFolder, home string) string { + return filepath.Join(home, ".idsec", "profiles", "grant") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + home := tt.home + if tt.homeTempDir { + home = t.TempDir() + } + t.Setenv("HOME", home) + + // t.Setenv always restores the original value on cleanup, so the + // unset case is safe to express as Setenv followed by Unsetenv. + t.Setenv("IDSEC_PROFILES_FOLDER", "") + profilesFolder := "" + switch tt.folderMode { + case folderCustom: + profilesFolder = t.TempDir() + t.Setenv("IDSEC_PROFILES_FOLDER", profilesFolder) + case folderEmpty: + // already set to "" above + case folderUnset: + if err := os.Unsetenv("IDSEC_PROFILES_FOLDER"); err != nil { + t.Fatalf("failed to unset IDSEC_PROFILES_FOLDER: %v", err) + } + default: + t.Fatalf("unknown folderMode %q", tt.folderMode) + } + + cfgPath := filepath.Join(t.TempDir(), "config.yaml") + t.Setenv("GRANT_CONFIG", cfgPath) + + saver := &mockProfileSaver{ + saveFunc: func(p *models.IdsecProfile) error { return nil }, + } + cmd := NewConfigureCommandWithDeps(saver, "https://example.cyberark.cloud", "test.user@example.com") + + output, err := executeCommand(cmd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The printed path must equal what the SDK loader resolves, so the + // path configure reports is the path login will later read. + wantDir := profiles.GetProfilesFolder() + if !strings.Contains(output, filepath.Join(wantDir, "grant")) { + t.Errorf("output does not match profiles.GetProfilesFolder() %q\ngot:\n%s", wantDir, output) + } + + want := tt.wantContainsFn(profilesFolder, home) + if !strings.Contains(output, want) { + t.Errorf("output missing %q\ngot:\n%s", want, output) + } + if strings.Contains(output, ".idsec_profiles") { + t.Errorf("output contains legacy path %q\ngot:\n%s", ".idsec_profiles", output) + } + }) + } +} + +func TestConfigureLongHelpHasNoLegacyPath(t *testing.T) { + long := NewConfigureCommand().Long + if strings.Contains(long, ".idsec_profiles") { + t.Errorf("configure Long help still mentions legacy path .idsec_profiles:\n%s", long) + } +}