- Language: Go 1.25+
- Module:
github.com/aaearon/grant-cli - Dependencies:
github.com/cyberark/idsec-sdk-golangis the primary dependency; zero-new-Go-module-deps is a goal, not an absolute rule. Documented exception:github.com/minio/selfupdate(+ its one transitiveaead.dev/minisign) forgrant update, adopted to remove the abandonedrhysd/go-github-selfupdateand advisory GO-2026-5932. It was a net reduction in every dependency measure — the exception cost nothing. Measure withgo list -deps/go list -m allif a current figure is needed; do not record one here
import (
"github.com/cyberark/idsec-sdk-golang/pkg/auth"
"github.com/cyberark/idsec-sdk-golang/pkg/common"
"github.com/cyberark/idsec-sdk-golang/pkg/common/isp"
"github.com/cyberark/idsec-sdk-golang/pkg/models"
"github.com/cyberark/idsec-sdk-golang/pkg/services"
)| Type | Package | Purpose |
|---|---|---|
auth.IdsecAuth |
pkg/auth |
Auth interface |
auth.IdsecISPAuth |
pkg/auth |
ISP authenticator — NewIdsecISPAuth(isServiceUser bool) |
isp.IdsecISPServiceClient |
pkg/common/isp |
HTTP client with auth headers |
services.IdsecService |
pkg/services |
Service interface |
services.IdsecBaseService |
pkg/services |
Base service with auth resolution |
models.IdsecProfile |
pkg/models |
Profile storage |
Custom SCAAccessService follows SDK conventions:
- Embed
services.IdsecService+*services.IdsecBaseService - Create client via
isp.FromISPAuth(ispAuth, "sca", ".", "", refreshCallback) - Set
X-API-Version: 2.0header on all requests httpClientinterface for DI/testing- The service slug (
"sca"/"uar") is whatisp.FromISPAuthresolves into the live host, and the retry/header tests overwriteclient.BaseURLbefore issuing a request — so it is pinned separately, by asserting the constructedBaseURLagainst the fake-JWT tenant (TestNewSCAAccessService_UsesSCAServiceSlug,TestNewAccessRequestService_UsesUARServiceSlug) before any swap. Assert it before mutatingBaseURL, never after - Wire contracts are asserted on what is sent: the
mockHTTPClientin both packages recordsgotRoute/gotBody/gotParamsbefore dispatching. Assert the exact route and the full body contents — "non-nil body" lets a nil payload through, and a canned response tells you nothing about the request
- Base URL:
https://{subdomain}.sca.{platform_domain}/api - Endpoints:
GET /api/access/{CSP}/eligibility— list eligible targets (AZURE,AWS,GCP)POST /api/access/elevate— request JIT elevation (AWS responses includeaccessCredentialsJSON string)GET /api/access/sessions— list active sessionsPOST /api/access/sessions/revoke— revoke sessions by ID (request:sessionIds[], response:SessionRevocationInfo[])sessionIdsis capped atmaxItems: 100per request, socmd/revoke_batch.gochunks the requested set into sequential ≤100-ID calls and aggregates; a mid-sequence batch error keeps the outcomes already collectedrevocationStatus: the spec enum is onlySUCCESSFULLY_REVOKEDandREVOCATION_IN_PROGRESS. The live API also returns the undocumentedREVOCATION_NOT_APPLICABLE(in neither the spec nor the SDK), so the status set is open andClassifyRevocationStatus(internal/sca/models/revoke.go) fails closed — anything unrecognized, including"", isOutcomeUnknownand counts as a failure. Match is exact; case variants are unknown- Outcomes are reconciled against the requested session IDs, never the returned rows (
cmd/revoke_reconcile.go): a requested session with no row isunknown, duplicate rows resolve worst-outcome-wins, and rows for unrequested or empty IDs satisfy nothing - Exit policy (deliberate, not blanket "fail closed"): exit 0 when every requested session was accepted, which includes
in_progress— a documented async success state, so failing on it would make legitimate revocations look broken. Exit 1 onnot_applicable, unrecognized statuses and missing rows. So exit 0 does not prove access is gone; only that nothing was refused or unaccounted for. State it that precisely in docs —ClassifyRevocationStatusfails closed, the command does not fail on in-progress outcome(revoked/in_progress/not_applicable/unknown) is the single classification field in the JSON. Do not add derived booleans (accepted,complete,revoked) beside it: two representations of one concept drift apart and disagree. Callers switch onoutcome- Never attribute a cause for
REVOCATION_NOT_APPLICABLE(e.g. an AWS/STS story). Observing the status does not prove the reason, and grant supports Azure/AWS/GCP plus group sessions. Render provider-neutral text and keep the raw token - Observed live (2026-08-18, one tenant): AWS sessions return
REVOCATION_NOT_APPLICABLEand stay active —sts:GetCallerIdentitystill succeeds after the revoke. Azure roles and Entra groups returnREVOCATION_IN_PROGRESSand disappear within ~25s. Cause: AWS exposes no session-scoped revocation primitive (STS tokens cannot be invalidated, andaws:TokenIssueTimedeny / permission removal are role-scoped, so they would hit every concurrent session on that role). Azure/Entra elevations are role assignments, which are individually removable. Sogrant revokecannot contain leaked AWS credentials — use the IAM console's Revoke sessions, or wait out expiry - The provider-neutral rule above still stands for user-facing text: this note records what was measured, it is not licence to print an AWS story at runtime
GET /api/access/{CSP}/eligibility/groups— list eligible Entra ID groups (response:groupId/groupName/directoryId)POST /api/access/elevate/groups— request group membership elevation (response wrapped inresponsekey, same as cloud elevation)
- Headers:
Authorization: Bearer {jwt},X-API-Version: 2.0,Content-Type: application/json
- Base URL:
https://{subdomain}.uar.{platform_domain}/api - Package:
internal/workflows/—AccessRequestService(mirrors SCA service pattern with ISP client for "uar" service) - Models:
internal/workflows/models/—AccessRequest,RequestState,RequestResult,SubmitAccessRequest,CancelAccessRequest,FinalizeAccessRequest,RequestFormResponse - Endpoints:
GET /api/workflows/request-forms— get form structure for target category + request typeGET /api/workflows/requests— list access requests (offset/limit pagination, filter/sort/freeText)GET /api/workflows/requests/{requestId}— get single request detailsPOST /api/workflows/requests— submit new access requestPOST /api/workflows/requests/{requestId}/cancel— cancel an open requestPOST /api/workflows/requests/{requestId}/finalize— approve or reject a request
- Pagination: offset/limit (not nextToken);
ListRequestsfetches all pages automatically - DI interfaces:
accessRequestServiceincmd/interfaces.go - Target category:
CLOUD_CONSOLE(hardcoded for v1) - Headers:
Authorization: Bearer {jwt},Content-Type: application/json
- idsec-sdk-golang v0.8.1 added automatic transient retry, enabled by default: 3 retries on top of the initial attempt (up to 4 requests), 500ms base wait, 10s cap (
pkg/common/idsec_client.go:53-60,:375-377). Backoff jitter can exceed the cap by up to ~50% (:1331-1349) - Two retry paths, neither safe for non-idempotent POSTs:
- 429 (
:865-885) — no HTTP method filter at all; honorsRetry-After(clamped to max wait), else exponential backoff - Transport errors (
:835-849viaisRetryableTransportError:1244-1283) — bareio.EOF/io.ErrUnexpectedEOF/"eof"/"server closed idle connection"are retried for any method including POST (:1252-1266); only connection-reset/broken-pipe/GOAWAY are gated behindisIdempotentMethod(:1269-1281). An EOF can surface after the server already processed the request, so this path can replay a mutation
- 429 (
- grant disables transient retry on BOTH the SCA and UAR/workflows clients via
internal/sdkclient.DisableTransientRetry(callsSetTransientRetry(0, 0, 0),:1187-1197), invoked right afterisp.FromISPAuthininternal/sca/service.goandinternal/workflows/service.go - Why client-wide rather than per-mutation: the SDK has no per-request opt-out, and toggling client state around individual calls is race-prone (grant fans out concurrently across CSPs). SCA
elevate/elevate/groupsare as non-idempotent as UARsubmit isp.IdsecISPServiceClientembeds*common.IdsecClient, so the helper takesclient.IdsecClient- POSTs protected, by actual replay risk:
- Mutating —
internal/sca/service.goelevate, group elevate (a duplicate creates a second session);internal/workflows/service.gosubmit (a duplicate is visible in an approver's queue), cancel, finalize - Effectively idempotent / read-shaped, covered incidentally —
internal/sca/service.gosessions/revoke (re-revoking is a no-op) and on-demand roles (POSTused only for role discovery)
- Mutating —
- Tests:
internal/sdkclient/retry_test.gocovers the helper;internal/sca/retry_policy_test.goandinternal/workflows/retry_policy_test.godrive the real constructors with a fake-JWT ISP token, then repointclient.BaseURLat anhttptest429 server and assert exactly one inbound request. Removing aDisableTransientRetrycall makes them fail with 4
- TDD: write
_test.gobefore.gofor every package - Table-driven tests
httptest.NewServerfor service mockshttpClientinterface for DI- Test files co-located as
_test.go - Mock capture convention (
cmd/test_mocks_test.go, precedentmockSessionRevoker): record the arguments in the method body before dispatching to anyxxxFunccallback, keep a history slice plus alastX()accessor (a history is what answers "called exactly once?"), defensively copy slices/maps and pointer-to-struct args, and guard against a nil request. An optional*stringargument is flattened toreason string+reasonSet boolso a test can tellnilfrom"". There is exactly one mock per interface — an arg-blind sibling silently opts every future test out of capture - No mutex on those histories: the only mocks reached from more than one goroutine are the eligibility listers, via the fan-outs in
fetchEligibility/fetchGroupsEligibility(cmd/root.go),resolveAndElevateUnifiedPath(cmd/root.go) andfetchAllTargets/fetchAllGroups(cmd/helpers.go), and those mocks are stateless readers.Elevate,ElevateGroupsand all fiveaccessRequestServicemethods are called from strictly sequential paths.make test-raceis what keeps that honest. Cite the function name, not a line number — this reasoning has already been invalidated once by an unrelated insertion above it - Test scaffolding in
cmdlives in_test.gofiles sotestingnever enters the production build:cmd/test_helpers_test.go(executeCommand,executeCommandStreams,executeWithHint,withInteractiveTTY) andcmd/test_mocks_test.go(every shared mock). Both used to be production files. Neither linkedtestingin at the time, so the moves were preventive, not remedial — butwithInteractiveTTYwould have been the first helper to pull it in, and the mock file is the larger and faster-growing of the two.go list -deps . | grep -c '^testing$'must stay0 - Output contracts: each machine-facing document has exactly ONE whole-object test comparing the emitted JSON against an inline literal with
assertJSONEqual(cmd/test_helpers_test.go) — nine of them: cloud elevation, group elevation,envcredentials,list,status,revoke,favorites list, the access-request object (shared byrequest get/submit/cancel/approve/reject) and therequest listenvelope. Optional fields additionally get a whole-object case exercising the ABSENT state, so dropping anomitemptyfails. It is deliberately brittle against added fields — these documents are a compatibility surface, and a new field should force a conscious review rather than pass silently. Inline literals, not golden files: the repo has notestdatamachinery and the objects are small. Keep focused tests for conditional and optional fields instead of converting every behavioral test into a whole-object one - Fixture values must be distinct and self-describing (
ws-name,ws-id,role-name,role-id,grp-id,dir-id,AKIA-fixture, …). A swap mutation — target with role, secretAccessKey with sessionToken, groupId with directoryId — is undetectable when both sides hold"test". Same reason two same-named groups in different directories are the fixture for the favoritesDirectoryIDtests: a unique name makes the directory ID non-load-bearing - Any test whose behavior depends on interactivity MUST set it explicitly with
withInteractiveTTY:go testhappens to run with a non-TTY stdin, but that is an accident of the harness, not an assertion - Tests that swap a package-level var (e.g.
ui.IsTerminalFunc,recordSessionTimestamp,bootstrapImpl) MUST NOT callt.Parallel()—-raceflags concurrent access to the global. Mark them with a// Not parallel: mutates the package-global X.comment. This is why thecmdpackage tests are all serial.
spf13/cobrafor CLI frameworkIilun/survey/v2for interactive promptsgrant env— AWS only; performs elevation, outputs onlyexportstatements (no human text); usage:eval $(grant env --provider aws); supports--refresh. Non-AWS targets are rejected byrequireAWSTarget(passed as thepreElevatehook toresolveAndElevate) before any elevation is issued, so no session is created. It fails closed — an unresolved CSP is rejected too; theAccessCredentials == nilcheck remains as a fallbackgrant list— list eligible targets and groups without triggering elevation; supports--provider,--groups,--refresh,--output json; used by LLMs to discover available targets programmaticallygrant revoke— revoke sessions: direct (grant revoke <id>),--all, or interactive multi-select;--yesskips confirmationgrant request— manage access requests through approval workflow; subcommands:submit,list,get,cancel,approve,rejectgrant request submit— submit on-demand access request; workspace selector uses SCA eligibility (deduplicated to unique workspaces); after workspace selection, interactive role selector fetches roles via SCA on-demand endpoints (GET/api/cloud/resources/ondemandforazure_ad/aws, POST/api/cloud/cloud-roles/ondemandforazure_resource); shows summary + confirmation before submitting; flags:--target,--role-id,--role,--provider,--reason,--priority,--date,--timezone,--from,--to,--yes,--refresh- Interactive role selection supports
DIRECTORY,ACCOUNT(AWS),MANAGEMENT_GROUP,SUBSCRIPTION,RESOURCE_GROUP, andRESOURCEworkspaces (azure_resource scopes use naive 2-level ancestors; custom roles scoped to intermediate management groups may be missing — fall back to--role-id) - On-demand role cache:
~/.grant/cache/ondemand_roles_<platform>_<sha256(workspaceID)>.json(4h TTL);--refreshongrant request submitbypasses the cache
- Interactive role selection supports
grant request list— list access requests; flags:--state,--result,--priority,--role(CREATOR/APPROVER),--search,--sort,--descgrant request get [id]— get full request details; omitting<id>in a TTY opens a fuzzy-filterable picker of all your requestsgrant request cancel [id]— cancel an open request; optional--reason. Omitting<id>in a TTY opens a picker scoped to STARTING/RUNNING/PENDING requests you created (role=CREATOR)grant request approve [id]/grant request reject [id]— finalize a request; optional--reason. Omitting<id>in a TTY opens a picker scoped to PENDING requests assigned to you (role=APPROVER)- Request picker:
internal/ui/request_selector.gomirrors the role-selector Format/Build/Select quartet;resolveRequestIDFnincmd/request_picker.gois injectable for tests. Non-TTY invocation without<id>returnsErrNotInteractivewith a hint to rungrant request list grant update— self-update binary via GitHub Releases; guards against dev builds. Implemented ininternal/selfupdate/:- Discovery:
GET https://api.github.com/repos/aaearon/grant-cli/releases/latest(apiBaseURLfield injectable for tests) - Version compare: in-house SemVer 2.0.0 parser (
ParseVersion/CompareVersions). Handles pre-release and build metadata (GoReleaser can emit both) with SemVer precedence: build metadata ignored for ordering, pre-release sorts before its release. A leadingv/Vis tolerated; leading zeroes are rejected - Asset selection:
grant-cli_<version>_<goos>_<goarch>.tar.gz(.zipon windows) — must stay in sync with.goreleaser.yaml - Integrity: SHA-256 of the archive checked against the release's
checksums.txt(GNU*filenamebinary marker tolerated). Trust model:checksums.txtcomes from the same origin as the archive, so it defends against corrupted/tampered downloads in transit, not against a compromised GitHub account or release pipeline. Signature verification would be needed for that. Note the checksum covers the archive, not the extracted binary — hence the independent size checks below - Extraction:
archive/tar+compress/gzip/archive/zip. Rejects absolute, drive-absolute (C:\), UNC and..paths (gosec G305); accepts only a singlegrant/grant.exeat the archive root (nested entries and duplicate candidates are errors). Size cap is 128 MiB (maxDownloadBytes), enforced byreadCapped, which probes one byte past the cap — a bareio.LimitReaderreports a successful short read and would silently install a truncated binary (gosec G110).maxDownloadBytesis a var only so tests can shrink it — mutate it exclusively through thewithMaxDownloadBytes(t, n)helper (restores viat.Cleanup), and never callt.Parallel()in a test that does - Zero-length binaries are refused twice, in
extractBinaryand again atapplyBinaryTo: the checksum covers the archive, not the extracted bytes, so an empty payload verifies against itself and would replace a working binary with nothing. Backstops, not the fix — they do not catch a non-regular entry carrying non-empty bytes (tar.TypeContand vendor types'A'..'Z'are not header-only), which is why the type guards below must never be removed in favour of them - Path-guard order in
checkArchivePathis load-bearing: the UNC (//) arm must precedepath.IsAbs, becausepath.Cleancollapses//host/share/xto/host/share/xand the absolute arm would otherwise make the UNC arm dead code - Non-regular entries are rejected symmetrically and deliberately so: tar filters on
hdr.Typeflag != tar.TypeReg, zip onf.Mode()&fs.ModeType != 0.IsDir()alone accepted afs.ModeSymlinkentry namedgrant.exe, extracting the link-target string as the binary. The two formats must reject the same shapes - The tar/zip declared-size asymmetry is intentional: tar checks every entry because reaching the next header inflates the current one, while
zip.NewReaderreads only the central directory and never opens a skipped entry. Neither cap is an aggregate one — nothing bounds total inflated bytes or entry count, so a tar bomb can burn CPU insideextractBinary— butverifyChecksumruns first, so reaching it requires control ofchecksums.txt, which the trust model above already excludes internal/selfupdate/fuzz_test.gocarries fuzz targets forcheckArchivePathand both extractors; run them by hand (never in CI) and commit anytestdata/fuzz/entry a real failure produces, as a permanent regression seed- Apply:
github.com/minio/selfupdatev0.6.0 owns the staged-file write, the two-rename swap including the Windows path, and rollback — do not hand-roll this. grant adds thefsyncof the staged file (minio does not sync) plus a best-effort directory sync. Seams:applyWithOptions,prepareFn,commitFn,syncStagedFileFnininternal/selfupdate/apply.go;syncStagedFileFnis a test seam only, and the realfsyncfailure is exercised on Unix alone via a FIFO staged path (//go:build !windows— no portable Windows equivalent exists) - Atomicity, precisely: each rename is atomic, so the installed binary is never partially written. The pair is not: a kill between the two renames, or a failed second rename whose rollback also fails, leaves the binary path absent with
.grant.old/.grant.newbeside it.InterruptedUpdate()detects that state andrecoveryHint()prints themvcommand that fixes it. Do not describe this as fully atomic selfUpdaterincmd/interfaces.gois defined over grant-owned types:UpdateSelf(ctx, current string) (newVersion string, updated bool, err error)- End-to-end apply test:
internal/selfupdate/e2e_test.go, build tagselfupdate_e2e, run withgo test -tags=selfupdate_e2e ./internal/selfupdate/. Everything else inapply_test.goswaps inert byte blobs; this compiles two real fixture binaries (a dependency-free module built with-ldflags -X main.version=..., so no network), executes one, and replaces it throughapplyBinaryTo/applyWithOptionswhile a process is still running from that image. That running child is what makes the Windows path real — a running.execannot be deleted, only renamed.heldProcess.alive()asserts the child survived the swap so the held cases cannot silently degrade into the idle cases. Covers success, rollback after a failed second rename (the restored file must still execute), and debris - Windows leaves
.grant.oldbehind, by design:minio.CommitBinarycannotos.Removethe backup while a process still runs from that image, so it callsSetFileAttributesW(FILE_ATTRIBUTE_HIDDEN)and leaves it. It does not accumulate — the nextCommitBinaryremoves the old path before renaming. The e2e test asserts exactly that rather than demanding a debris-free directory on Windows. The staged.grant.newmust never survive on either platform
- Discovery:
--groupsflag on root command shows only Entra ID groups in the interactive selector--group/-gflag on root command for direct group membership elevation (grant --group "Cloud Admins")- Root command unified selector shows both cloud roles and Entra ID groups; groups use
/eligibility/groupsand/elevate/groupsAPI endpoints - Multi-CSP: omitting
--providerfetches eligibility from all supported CSPs (supportedCSPsincmd/root.go— Azure, AWS, GCP) and merges results; a CSP that errors is skipped - GCP:
list/elevate/status/revokeonly. Workspace typesPROJECT/FOLDER/GCP_ORGANIZATION; noaccessCredentialsin the API spec, sogrant envstays AWS-only andgrant request submitrejects GCP (rejectGCPWorkspace, applied after target resolution so--role-idcannot skip it). Untested against a live GCP tenant --refreshbypasses eligibility cache ongrantandgrant envfetchEligibility()andresolveTargetCSP()incmd/root.go— shared by root, env, and favorites
internal/ui/tty.go—IsTerminalFunc(overridable),IsInteractive(),ErrNotInteractive- All interactive prompts (
SelectTarget,SelectSessions,ConfirmRevocation,SelectGroup,uiUnifiedSelector.SelectItem,surveyNamePrompter.PromptName) fail fast withErrNotInteractivewhen stdin is not a TTY - Error messages suggest the appropriate non-interactive flag (e.g.,
--target/--role,--all,--yes,--group,--favorite) go-isattyis a direct dependency (promoted from indirect via survey); seego.modfor the pinned version
--output/-opersistent flag on root command:text(default) orjson- Validated in
PersistentPreRunE.--output jsonis a pure serialisation flag; it does not affect interactivity — interactive prompts still run in a TTY and write to stderr while JSON goes to stdout cmd/output.go—outputFormatvar,isJSONOutput(),writeJSON(w, data)cmd/output_types.go— JSON structs:cloudElevationOutput,groupElevationJSON,sessionOutput,statusOutput,revocationOutput,favoriteOutput,awsCredentialOutput,accessRequestOutput,accessRequestListOutput- All commands support JSON: root elevation,
env,status,revoke,favorites list,request list,request get,request submit,request cancel,request approve,request reject config.Favoritehas bothyaml:"..."andjson:"..."struct tags
- Eligibility responses cached in
~/.grant/cache/as JSON files (e.g.,eligibility_azure.json,groups_eligibility_azure.json) - Default TTL: 4 hours, configurable via
cache_ttlin~/.grant/config.yaml(Go duration syntax:2h,30m) config.ParseCacheTTLreturns(time.Duration, error). Absent means "use the default"; any explicitly supplied value that cannot serve as a TTL — unparseable, zero or negative — is an error. Treating those two the same way is the point: silently defaultinggarbagewhile rejecting0swould validate one field by two opposite rules.config.Loadvalidates it so a bad value surfaces at load, not when some command happens to build a cache.buildCachedLister(cmd/root.go) therefore returns an error too — its bad-TTL arm is reachable only for aConfigassembled in memory. Both rejection messages must name a remedy: the unparseable arm names the expected syntax (must be a positive Go duration such as 4h or 30m) and still wraps thetime.ParseDurationerror with%w; the non-positive arm names removing the setting, since0sused to work as an accidental kill-switch and--refreshdoes not exist on every cache-consuming command (status,revoke,favorites add) — nor would it help, becauseconfig.Loadrejects the value before any flag is read. Neither may point atgrant configure(see Config)--refreshflag ongrantandgrant envbypasses cache reads but still writes fresh datainternal/cache/cache.go— genericStorewithGet[T]/Set[T], injectable clock for testinginternal/cache/cached_eligibility.go—CachedEligibilityListerdecorator implementingeligibilityLister+groupsEligibilityListerinternal/cache/session_tracker.go—RecordSession,SessionTimestamps,CleanupSessionsfor tracking elevation timestamps insession_timestamps.jsonsessionTimestampRetention(24h) is local retention for the remaining-time display only — not a session lifetime, not a session limit, not an access-control boundary. Dropping a timestamp only costs grant the ability to show how long a session has left.SessionTimestampsfilters on it;CleanupSessionsdoes not read it at all — that filters purely onactiveIDsmembership
buildCachedLister()incmd/root.go— shared factory used by all commands (root, env, status, revoke, favorites add)- Commands without
--refresh(status, revoke, favorites add) always passrefresh: false— they use eligibility for display only - Cache failures (read/write) silently fall through to the live API
cmd/session_tracking.go—recordSessionTimestampvar (injectable for tests), called after elevation in root and env commands
--verbose/-vglobal flag wired viaPersistentPreRunEincmd/root.go- Calls
config.EnableVerboseLogging("INFO")(setsIDSEC_LOG_LEVEL=INFO) orconfig.DisableVerboseLogging()(setsIDSEC_LOG_LEVEL=CRITICAL) cmdLoggerinterface incmd/verbose.go—Info(msg string, v ...interface{}), satisfied by*common.IdsecLoggerlogpackage-level var incmd/verbose.go— all commands uselog.Info(...)for verbose output; tests swap withspyLoggerloggingClientininternal/sca/logging_client.godecorateshttpClient, logging method/route/status/duration at INFO, response headers at DEBUG with Authorization redactionNewSCAAccessService()wraps ISP client withloggingClientusingcommon.GetLogger("grant", -1)(dynamic level from env)NewSCAAccessServiceWithClient()(test constructor) does not wrap — tests don't need loggingExecute()prints"Hint: re-run with --verbose for more details"on error when verbose is off- Users can set
IDSEC_LOG_LEVEL=DEBUGenv var for deeper SDK output
- App config:
~/.grant/config.yaml - SDK profile:
~/.idsec/profiles/grant(default; override viaIDSEC_PROFILES_FOLDER) runConfiguremerges onto the existing config (cmd/configure.go): it callsconfig.Loadand overwrites only what it owns (profile), so favorites,default_providerandcache_ttlsurvive a re-run (TestConfigure_PreservesExistingConfigOnRerun).Loadtreats an absent file as success returningDefaultConfig(), so a first run merges onto defaults (TestConfigure_FirstRunMergesOntoDefaults)- The rebuild-from-defaults fallback is the no-lockout property and only fires when
Loadfails: configure must stay usable when the on-disk config is unloadable, so it warns on stderr and writes a freshDefaultConfig()over the top (TestConfigure_UnloadableConfigIsRebuiltFromDefaults). Do not make configure fail on an unloadable config — that locks the user out of the one command that can repair it. On that path the old file cannot be read, so its favorites are genuinely lost; thereforegrant configuremust still never be advertised as the remedy for a bad config value. The user-facing remedy is to edit the file, whose path every load error names - Always resolve the profile directory with
profiles.GetProfilesFolder()(SDK) — never hand-roll it. The SDK readsos.Getenv("HOME"), notos.UserHomeDir(); on WindowsHOMEis frequently unset, so it resolves to a relative.idsec/profilesunder 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
- The SDK stores the auth token via
pkg/common/keyring.GetKeyring(enforceBasic bool)(idsec_keyring.go:117-131) picks: basic (file) keyring if Docker or its ownisWSL()orIDSEC_BASIC_KEYRING != ""orenforceBasic; else the OS keyring on windows/darwin, and on linux the OS-provided (D-Bus/libsecret) keyring wheneverDBUS_SESSION_BUS_ADDRESSis non-empty; else basic - SDK bug (v0.8.1):
isWSL()(:90-95) matches"Microsoft"case-sensitively against/proc/version. Modern WSL2 reports-microsoft-standard-WSL2, so it returns false. Under WSLgDBUS_SESSION_BUS_ADDRESSis set, the D-Bus keyring is chosen, and the call can block forever against an unresponsivegnome-keyring-daemon - The SDK's fallbacks cannot save you. Both the OS-keyring wrapper and
SaveToken(:154-171) fall back to the basic keyring only when the underlying call returns an error. A hang is not an error, so the fallback is unreachable. The hang is also not write-only —LoadToken/GetPasswordcan wedge before anything is ever written IDSEC_BASIC_KEYRINGsemantics: the SDK testsos.Getenv(...) != "", so any non-empty value forces the file keyring,0andfalseincluded. Empty/unset does not itself force it — the otherGetKeyringconditions (Docker, SDK-detected WSL,enforceBasic) still apply- grant's override:
internal/keyringenvdetects WSL and setsIDSEC_BASIC_KEYRING=1. A non-empty existing value is preserved; an explicitly empty value is overwritten on WSL, because empty is precisely the dangerous setting. No-op off linux (theGOOSguard short-circuits before any filesystem access) - Signals, OR'd, in the order the diagnostic reports them:
/run/WSLexists →/proc/sys/fs/binfmt_misc/WSLInteropexists →/proc/sys/kernel/osreleasecontainsmicrosoft/wsl→/proc/versioncontainsmicrosoft→ non-emptyWSL_DISTRO_NAME/WSL_INTEROP. All string matching is case-insensitive - Why the token sets differ:
osreleaseis short and structured, sowslis safe there — systemd matchesMicrosoft/WSLon it insrc/basic/virt.c. (npmis-wslis not a precedent for thewsltoken: it matches onlymicrosoft, onos.release()then/proc/version, before falling back toWSLInterop//run/WSL, with everything gated behind!isInsideContainer(). It is a precedent for the markers.)/proc/versionis free-form and carries the kernel build user, build host and compiler banner, so a barewslthere would match a plain Linux box built on a host namedwsl-builder—microsoftonly /run/WSLis the strongest single signal, not the string matching: it survives custom kernels,sudo, systemd units and cron.WSL_DISTRO_NAMEis lost undersudo -i(microsoft/WSL#5914) and absent in systemd units (#9719), and custom WSL2 kernels may carry neither token (#6911). snapd abandoned string matching for this marker after Launchpad #1991823. WSL's init creates it inInteropServer::Create()(src/linux/init/util.cpp) from an unguarded call inConfigInitializeInstance()(src/linux/init/config.cpp), so it appears even when interop is disabledWSLInteropis a supplement only — but not for the reason previously recorded here. The old claim was "that binfmt entry exists only when interop is enabled". That is true on WSL1, where the per-distro registration is gated onConfig.InteropEnabled(src/linux/init/config.cpp:543-551); on WSL2 the entry is registered at VM level and is kernel-global, so it is instead vulnerable to being wiped or shadowed VM-wide- All five signals are deliberately redundant, and the redundancy is about visibility, not reliability. This is the point that stops someone simplifying this again — an attempt in
db73645was reverted in23ac108for exactly this reason. The five sources fail independently because they are reached by different mechanisms:- filesystem markers (
/run/WSL,WSLInterop) are namespace-local — a chroot or mount namespace with a clean/run, or withoutbinfmt_miscmounted, sees neither, no matter what init did - proc paths (
osrelease,/proc/version) are independently maskable — they are separate mount-visible paths, so one can be masked, omitted or replaced while the other is readable - env vars (
WSL_DISTRO_NAME,WSL_INTEROP) are the only ones inherited across chroot and mount-namespace boundaries, which is precisely where every marker above disappears
- filesystem markers (
- Content equivalence is not signal redundancy.
/proc/versionandosreleaseprovably cannot disagree:fs/proc/version.cdoesseq_printf(m, linux_proc_banner, utsname()->sysname, utsname()->release, utsname()->version), and/proc/sys/kernel/osreleaseisutsname()->release(kernel/utsname_sysctl.c,uts_kern_tableentryosrelease→init_uts_ns.name.release, resolved per-namespace byget_uts()); a UTS namespace change moves both together. Useful for reasoning about the values, but not grounds for removing either — identical content is no help when one path is not visible. Same shape of argument for the env vars: init setsWSL_DISTRO_NAMEand creates/run/WSLin the same unguarded function, which proves init made the marker, not that a descendant process can still see it - Error asymmetry drives the tuning: a false negative attempts the OS keyring under WSLg and hangs indefinitely with no error and no timeout (unrecoverable); a false positive is a file keyring on a plain Linux desktop (bounded security downgrade). Bias toward over-detection. A redundant check is the cheap side of that trade — do not trade it away for tidiness
- Rejected: container gating (
/run/WSLis container-safe since a container gets its own/runtmpfs). Note the old justification — "inside a containerDBUS_SESSION_BUS_ADDRESSis rarely set, so the SDK already picks basic" — is refuted: WSL init injectsDBUS_SESSION_BUS_ADDRESSexplicitly for systemd-backed launches (src/linux/init/init.cpp), and ordinarychrootpreserves it, so the SDK cannot be assumed to fall back on its own. Also rejected:WSLENV(user-configurable, often absent), shelling out tosystemd-detect-virt, and third-party libs (gookit/goutilhas the same case-sensitive"Microsoft"bug) - Applied in
executeWithKeyringOverride(cmd/root.go), called fromExecute()beforerootCmd.Execute()— the single deterministic entry point for the binary, ahead of every keyring access (cmd/login.go,cmd/root.go,cmd/logout.go). It fails closed: aSetenverror aborts before any command runs - The applied notice is stashed in
keyringEnvNoticeand emitted fromPersistentPreRunEinside theif verbosebranch, so it is verbose-only and unit-testable withspyLogger(gating onIDSEC_LOG_LEVELalone would be invisible to the spy) - Backend switch caveat: a token written to the OS keyring is invisible to the file keyring. Worst case the user re-runs
grant login
- Use the
/grant-loginskill when you need to authenticate to the grant CLI (e.g., before manual testing) - Skill definition:
.claude/skills/grant-login/SKILL.md - Requires
.envat project root withGRANT_PASSWORDandTOTP_SECRET
- Config:
.golangci.yml(golangci-lint v1 format) - Linters enabled (
disable-all: true, so this list is exhaustive): 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 —
gofmthas no exclusion, formatting is universal - Run
gofmt -s -w .before committing;gofumptwas rejected because the codebase is not gofumpt-clean revive/unused-parameterandrevive/exporteddisabled (Cobra signatures, established API names)- Use
errors.Newfor static error strings (perfsprint enforced);fmt.Errorfonly with%verbs - Use
t.Context()instead ofcontext.Background()in tests (usetesting enforced)
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 (golangci-lint)
make clean # Clean build artifacts-trimpathused in bothMakefileand.goreleaser.yamlfor reproducible buildsVERSION ?= dev(Makefile:2) is injected as-X ...cmd.version=$(VERSION)(Makefile:5-8), so a plainmake buildstampsversion=dev.runUpdaterefuses""/"dev"(cmd/update.go:38-39, "cannot update a dev build"), sogrant updatecan never succeed on a default local build- To exercise
grant updatelocally:make build VERSION=0.7.0, then./grant update. Use a version older than the latest release so an update is actually found .goreleaser.yamlusesCommitDate(not build date) andmod_timestampfor reproducibility
.github/workflows/ci.yml—testjob runs as a matrix overubuntu-latestandwindows-latestwithfail-fast: false- Windows runners have no GNU make, so that leg runs the equivalent Go commands directly (
go build -trimpath -o grant.exe .,go test -race ./... -v); Linux keepsmake build/make test-race. Keep the two legs in sync when Makefile targets change go test -raceworks on windows/amd64 because the runner image ships gcc (the race detector needs cgo)- The
Self-update end-to-endstep runs theselfupdate_e2e-tagged tests on both legs (noif:guard) — it is the only test that replaces a real running executable, and comparing the two platforms is the whole point. It builds its fixtures locally, so it needs no network. Keep it unguarded; guarding it to Linux would defeat its purpose Integration tests(go test -tags=integration ./cmd -shuffle=on) andTest with shuffled order(go test -shuffle=on -count=1 ./...) run unguarded on both legs. Integration needs no network and takes ~2s; shuffling is what catches order dependence in a suite that mutates package globals. The integration step is shuffled too because the untaggedcmd/*_test.gofiles compile into that binary as well — without it their order dependence is only ever shuffled in the untagged build.golangci.ymlsetsrun.build-tags: [integration, selfupdate_e2e]so both tagged files are linted. Side effect: withintegrationset,cmd/main_test.go(//go:build !integration) is excluded from linting- Lint (
golangci-lint-action) runs on Linux only — a second pass on Windows adds minutes and finds nothing new - Tests must be OS-portable. Never assert POSIX permission bits without a
runtime.GOOS == "windows"skip: Go synthesizes0666/0777for Windows files andos.Chmodthere only toggles the read-only attribute. Current skips:internal/config/config_test.go(TestLoadConfig_PermissionError,TestConfigDir_Error— chmod 0000 andHOME) andinternal/cache/cache_test.go(TestSet_FilePermissions) - Prefer a portable construction over a skip where one exists. To force a write failure, point at a path whose parent component is an existing regular file rather than a hardcoded
/dev/null/...path, which is an ordinary writable location on Windows.MkdirAllfails withENOTDIRon both platforms — no Windows error code is involved:os.MkdirAll(os/path.go) stats the parent itself and synthesizes&PathError{Op: "mkdir", Err: syscall.ENOTDIR}in platform-independent Go, which is also why assertingOp == "mkdir"is portable
Entries are short and concise. This applies to [Unreleased] and everything added from now on; already-released sections are published history and are not rewritten.
- One line per entry — a single sentence, ideally under ~120 characters.
- Say WHAT changed and, where it isn't obvious, the user-visible effect. Not the mechanism, not the root cause, not measurements, not
file:linereferences. - Rationale, evidence, benchmarks, dependency counts and advisory analysis go in the PR description. Durable architecture and policy go in CLAUDE.md.
- Keep the Keep-a-Changelog section headings:
Added/Changed/Fixed/Security. - A breaking or behaviour-changing entry may add a short second clause naming the impact. Brevity must never hide a behavioural break from someone skimming before an upgrade.
- Move
[Unreleased]entries inCHANGELOG.mdto a new[X.Y.Z] - YYYY-MM-DDsection (leave[Unreleased]header empty) - Commit:
docs: prepare CHANGELOG for vX.Y.Z release - Tag:
git tag vX.Y.Z - Push commit and tag:
git push origin main && git push origin vX.Y.Z - The
release.ymlGitHub Actions workflow triggers onv*tags and runs GoReleaser to build binaries and create the GitHub Release
- Feature branches, conventional commits
- Branch naming:
feat/,fix/,docs/
Commands follow Cobra best practices:
// Factory function for testability
func NewCommandName() *cobra.Command {
return &cobra.Command{
Use: "command-name",
Short: "Brief description",
Long: "Detailed description...",
RunE: func(cmd *cobra.Command, args []string) error {
return runCommandName(cmd, args)
},
}
}
// Separate run function for testability
func runCommandName(cmd *cobra.Command, args []string) error {
// Implementation
}
// Auto-register in init()
func init() {
rootCmd.AddCommand(NewCommandName())
}Commands declare their collaborators as interfaces in cmd/interfaces.go (authLoader, eligibilityLister, groupsEligibilityLister, elevateService, accessRequestService, selfUpdater, …). There are two ways to substitute them.
Preferred — New*WithDeps constructors. Every command has one: NewRootCommandWithDeps, NewEnvCommandWithDeps, NewStatusCommandWithDeps, NewRevokeCommandWithDeps, NewListCommandWithDeps, NewRequestCommandWithDeps, NewLogoutCommandWithDeps, NewUpdateCommandWithDeps, plus runFavoritesAddWithDeps. The production factory is a thin wrapper that bootstraps the real services and calls the same function. Tests build the command with mocks and never touch a global.
Package-var seams, for the few things a constructor cannot reach:
| Var | File | Purpose |
|---|---|---|
bootstrapImpl |
cmd/root.go |
Profile load + authenticate. Memoized by bootstrapISPAuth via sync.Once; clear it with resetBootstrapCache() |
recordSessionTimestamp |
cmd/session_tracking.go |
Elevation timestamp writer |
resolveRequestIDFn |
cmd/request_picker.go |
Interactive request picker |
submitPromptFn, confirmSubmitFn, resolveSubmitTargetFn, submitWorkspaceSelectorFn, resolveRoleFn |
cmd/request_submit.go |
request submit prompt and resolution steps |
log |
cmd/verbose.go |
Verbose logger; tests swap in spyLogger |
ui.IsTerminalFunc |
internal/ui/tty.go |
TTY detection |
There is no getAuth/getSCAService; those never existed. Every test that swaps one of these globals restores it via t.Cleanup, and no such test may call t.Parallel().
internal/testenv is a normal package imported only from _test.go files, so it never links into the binary. testenv.Run(m.Run) redirects eight variables under one temp root before m.Run, then restores them: HOME, USERPROFILE, XDG_CONFIG_HOME, IDSEC_PROFILES_FOLDER, GRANT_CONFIG, IDSEC_KEYRING_FOLDER, IDSEC_FILE_LOG_PATH and IDSEC_BASIC_KEYRING.
USERPROFILEis not optional: Go's Windowsos.UserHomeDirreadsUSERPROFILE, thenHOMEDRIVE+HOMEPATH, and neverHOME.HOMEalone leaves the Windows CI leg pointed at the real profile. The SDK profile loader, conversely, readsHOMEon every platform.GRANT_CONFIGdoes not cover the cache:cache.CacheDir()→config.ConfigDir()→os.UserHomeDir(). Before this existed the suite wrote the developer's real~/.grant/cache/session_timestamps.jsonon every run.IDSEC_KEYRING_FOLDERandIDSEC_FILE_LOG_PATHare absolute-path overrides that bypass theHOMEfallback entirely (pkg/common/keyring/idsec_basic_keyring.go,pkg/common/idsec_logger.go, whichMkdirAlls the log's parent). A value already exported in the developer's or CI environment sends those writes outside the sandbox no matter whatHOMEsays.IDSEC_BASIC_KEYRING=1is set because the OS keyring is a daemon, not a path, so no redirect can sandbox it: on a non-WSL Linux box withDBUS_SESSION_BUS_ADDRESSset,GetKeyringpicks the real libsecret store. Forcing the file backend puts keyring state into the sandboxed folder instead. If a future SDK stops honoring the variable, this containment is gone and nothing here detects it.XDG_CONFIG_HOMEis speculative/defensive — nothing in grant or the pinned SDK reads it (rg XDG_finds only testenv's own comment and tests). It is redirected because it is the conventional escape hatch and costs nothing.IDSEC_PROFILEandDEPLOY_ENVare unset, not redirected (unsetVars) — they select behavior, not a location, so the only safe state is absent.IDSEC_PROFILEpicks the SDK's default profile name;DEPLOY_ENVfeeds tenant-env resolution insideisp.FromISPAuth, which the sca/workflows retry-policy tests drive for real.Runcaptures set-vs-unset per variable and restores that exact state.testenvmust not importtesting;AssertSandboxedtherefore takes aTBinterface (Helper/Errorf) that*testing.Tsatisfies.AssertSandboxedchecks the configured destinations —config.ConfigDir,config.ConfigPath,cache.CacheDir,profiles.GetProfilesFolder, the SDK keyring folder and the SDK file-log path — plus thatIDSEC_BASIC_KEYRINGis non-empty. The last two resolvers are reimplemented in testenv rather than called, because the SDK constructor creates the directory as a side effect and an assertion must not write. It does not prove nothing was written outside the sandbox, and cannot see reads. A snapshot-diff gate was considered and rejected: a concurrently running realgrantfalse-positives with certainty, and size+mtime misses same-size rewrites.- Each
AssertSandboxedblock must be individually killable, which means asserting the failure count and which resolver failed, notlen(errs) > 0: any other assertion satisfies a bare "at least one", so the block under test could be deleted outright.GRANT_CONFIGpointed outside the sandbox isolatesconfig.ConfigPath(1 failure);HOME+USERPROFILEpointed outside isolatesconfig.ConfigDirandcache.CacheDir(2 failures, becauseCacheDirdelegates toConfigDir→os.UserHomeDir).recordingTBtherefore stores the formatted message — the resolver name is an argument, not part of the format string. - The redirect list is validated against an explicit literal in
testenv_test.go, and every entry additionally gets a hostile pre-existing value beforeRuninTestRun_OverridesPreExistingHostileValues. Ranging overredirectedVarsitself is the trap this replaced: dropping an entry merely checked one fewer thing, and four of the original five survived a drop-one mutation because withHOMEredirected their fallbacks already landed in-sandbox. A var's whole value is defending against a pre-existing value, so that is what the test must supply. Runrestores the environment and removes the sandbox from adefer, so a panic insidem.Run(a-racedetection, a stray panic) cannot leak the directory or leave the process redirected.sandboxRootis saved and restored rather than cleared, so a nestedRunhands the outer root back.TestMainlives incmd/main_test.go(//go:build !integration, becausecmd/integration_test.godeclares its own),internal/config/main_test.go,internal/cache/main_test.go,internal/sca/main_test.go,internal/workflows/main_test.goandinternal/sdkclient/main_test.go. Theconfig/cacheones are in the external test package (config_test/cache_test) becausetestenvimports those packages — an in-package test file importing it would be an import cycle.sca/workflows/sdkclientcan be in-package:testenvimportsinternal/sca/models, notinternal/sca. Those three earn aTestMainbecause their retry-policy tests drive the real service constructors.os.SetenvinTestMainruns beforem.Run, so it does not collide with thet.Parallel()sites ininternal/— unliket.Setenv, which the stdlib forbids in parallel tests.cmd/bootstrap_stub_test.go(untagged, so both build configurations get it) pointsbootstrapImplaterrTestBootstrapDisabled. Assert it witherrors.Is, never a barewantErr: true— otherwise a stray bootstrap attempt silently satisfies an unrelated case.recordSessionTimestampis deliberately left live: it is what proves the redirect works.executeCommand/executeCommandStreams/executeWithHintcallrestoreCommandGlobalsto put back both globals Cobra binds to persistent flags:outputFormat(--output) andverbose(--verbose). Without that a command built outside the root (e.g.NewEnvCommandWithDeps) inherits the previous test's values — an order dependencego test -shuffle=onexposes.outputFormatis the load-bearing half (a no-op restore fails 5 of 8 fixed shuffle seeds);verboseis latent only because pflag rewrites it at registration.
func TestCommand(t *testing.T) {
tests := []struct {
name string
args []string
flags map[string]string
wantErr bool
wantOutput string
}{
{name: "success case", args: []string{}, wantErr: false},
{name: "error case", args: []string{}, wantErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test implementation
})
}
}// test_mocks_test.go - shared mocks across tests
type mockAuthProvider struct {
authenticateFn func(*models.IdsecProfile) (*models.IdsecToken, error)
}
func (m *mockAuthProvider) Authenticate(p *models.IdsecProfile) (*models.IdsecToken, error) {
if m.authenticateFn != nil {
return m.authenticateFn(p)
}
return &models.IdsecToken{}, nil
}cmd/integration_test.go (//go:build integration) drives the compiled binary as a child process. Its TestMain runs inside testenv.Run and builds into a unique temp directory — never the shared ../grant-test, which two concurrent runs would fight over. GOCACHE/GOMODCACHE/GOPATH/GOENV are resolved before the HOME redirect and passed to the build, otherwise it starts from an empty module cache and needs the network. GOENV is in the list because the child go build finds its env file via os.UserConfigDir — $XDG_CONFIG_HOME/go/env on Linux (redirected to an empty sandbox) but %AppData%\go\env on Windows (not redirected at all), so without it the two CI legs read different files and any go env -w GOPROXY=…/GOFLAGS/GOPRIVATE is silently dropped on Linux.
Assertions are exact exit codes plus exact error text. Keyword soup (error|Error|failed|not found) is banned here: a panic satisfies it. runGrant fails the test outright if the child output contains a panic, and closes stdin so no prompt can block.
Commands use consistent error patterns:
// Return errors, don't print
func runCommand(cmd *cobra.Command, args []string) error {
if err := validate(args); err != nil {
return fmt.Errorf("validation failed: %w", err)
}
result, err := doWork()
if err != nil {
return fmt.Errorf("operation failed: %w", err)
}
cmd.Println("Success:", result)
return nil
}
// Cobra automatically prints errors from RunEUse cmd.OutOrStdout() for testability:
func runCommand(cmd *cobra.Command, args []string) error {
// Use cmd methods for output
fmt.Fprintln(cmd.OutOrStdout(), "Output message")
fmt.Fprintf(cmd.ErrOrStderr(), "Error: %s\n", err)
// NOT: fmt.Println("message")
}cmd.Flags().StringP("flag", "f", "default", "description")
cmd.Flags().StringVarP(&variable, "flag", "f", "default", "description")
// Mark flags as required
cmd.MarkFlagRequired("required-flag")
// Mutually exclusive flags (handled in RunE)
if cmd.Flags().Changed("flag1") && cmd.Flags().Changed("flag2") {
return errors.New("--flag1 and --flag2 are mutually exclusive")
}// Load config with GRANT_CONFIG override
cfg, err := config.Load()
if err != nil {
// Default config if not found
cfg = config.DefaultConfig()
}// Create ISP auth
ispAuth := auth.NewIdsecISPAuth(true) // cacheAuthentication=true
// Load profile and authenticate
profile, err := models.LoadProfile(cfg.Profile)
token, err := ispAuth.Authenticate(profile)
// Create SCA service
svc, err := sca.NewSCAAccessService(ispAuth)