diff --git a/CHANGELOG.md b/CHANGELOG.md index 972d0f5..c6e8d64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,15 +7,31 @@ All notable changes to this project will be documented in this file. ### Changed - 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. +- Upgraded dependencies: + - `github.com/cyberark/idsec-sdk-golang` v0.2.3 → v0.8.1 + - `github.com/spf13/cobra` v1.9.1 → v1.10.2 + - `github.com/spf13/pflag` v1.0.6 → v1.0.10 (indirect) + - `github.com/mattn/go-isatty` v0.0.20 → v0.0.24 + - `golang.org/x/crypto` v0.45.0 → v0.55.0 (indirect) + - `golang.org/x/net` v0.47.0 → v0.58.0 (indirect) + - `github.com/dvsekhvalnov/jose2go` v1.5.0 → v1.7.0 (indirect) + - `github.com/golang-jwt/jwt/v5` v5.2.2 → v5.3.1 (indirect) + - `golang.org/x/sys` v0.38.0 → v0.47.0 (indirect) + - `golang.org/x/term` v0.37.0 → v0.45.0 (indirect) + - `golang.org/x/text` v0.31.0 → v0.41.0 (indirect) +- SDK v0.8.1 enables automatic retries on HTTP 429 and transient transport errors by default. `grant` disables these on its SCA and UAR service clients (see the retry-policy change) because `Elevate` and `SubmitRequest` are non-idempotent POSTs with no idempotency header. +- SDK v0.8.1 changes 401 / re-authentication handling: the rejected response is drained, a token refresh is forced, a concurrently-refreshed token is adopted rather than duplicated, and cookie-decode errors that were previously silently ignored are now propagated. +- Disabled the SDK v0.8.1 automatic transient-retry on both the SCA and Access Requests (UAR) HTTP clients (`internal/sdkclient.DisableTransientRetry`). The SDK retries HTTP 429 with no method filter and bare `EOF` transport errors for any method including POST, which could replay non-idempotent requests (duplicate elevation or a duplicate access request in an approver's queue). Read paths lose automatic rate-limit retry; a 429 now surfaces as an error - `grant update` no longer depends on the abandoned `github.com/rhysd/go-github-selfupdate` (last commit Jan 2021). Release discovery, version comparison, asset selection, checksum verification and archive extraction are now implemented in-house in `internal/selfupdate/`; the binary swap (staged sibling file, the two-rename dance including the Windows path, and rollback) is delegated to `github.com/minio/selfupdate` v0.6.0, with grant adding an `fsync` of the staged file before the swap - **Replacement guarantees, stated precisely.** Each rename is individually atomic, so the installed binary is never a partially written file. The *pair* of renames is not atomic: if the process is killed between them, or if the second rename and the rollback both fail, the binary path is left absent with `.grant.old` and `.grant.new` beside it. grant now detects that state and prints the exact `mv` command that restores the previous binary -- Dependency graph shrinks. Build graph (`go list -deps`, the modules actually compiled in): 39 -> 33. `go.mod` require directives: 47 -> 39 (indirect 40 -> 33). Full module graph (`go list -m all`): 110 -> 95. The only genuinely new module paths are `github.com/minio/selfupdate` and `aead.dev/minisign`. Removed `blang/semver`, `rhysd/go-github-selfupdate`, `google/go-github/v30`, `google/go-querystring`, `golang.org/x/oauth2` (a Nov 2018 pseudo-version), `golang/protobuf`, `google.golang.org/appengine`, `tcnksm/go-gitconfig`, `inconshreveable/go-update` and `ulikunitz/xz` +- Dependency graph, after the selfupdate replacement and the SDK upgrade together. Build graph (`go list -deps`, the modules actually compiled in): 39 → 33. `go.mod` require directives: 47 → 39 (indirect 40 → 33). Full module graph (`go list -m all`): 110 → 109 — the selfupdate replacement removed 15 and SDK v0.8.1 adds 14 back, but none of the SDK's additions are in the linked graph. The only genuinely new module paths in the build graph are `github.com/minio/selfupdate` and `aead.dev/minisign`. Removed `blang/semver`, `rhysd/go-github-selfupdate`, `google/go-github/v30`, `google/go-querystring`, `golang.org/x/oauth2` (a Nov 2018 pseudo-version), `golang/protobuf`, `google.golang.org/appengine`, `tcnksm/go-gitconfig`, `inconshreveable/go-update` and `ulikunitz/xz` ### Security - `grant update` now verifies the downloaded archive's SHA-256 against the release's `checksums.txt` before replacing the binary. Previously no validation was performed at all (`selfupdate.DefaultUpdater()` ships a nil `Validator`). Note the trust model: `checksums.txt` is fetched from the same origin as the archive, so this protects against corrupted or tampered downloads in transit, not against a compromised GitHub account or release pipeline - Archive extraction now rejects oversized entries instead of silently truncating them at the 128 MiB cap, rejects drive-absolute (`C:\...`) and UNC archive paths alongside `..` traversal, and only accepts the binary at the archive root (never a nested entry, never two candidates) - Removes `golang.org/x/crypto/openpgp` from the build graph, clearing advisory GO-2026-5932 (unmaintained package, no fix available). Dropping `ulikunitz/xz` also clears GO-2025-3922 +- `govulncheck` findings drop from 34 vulnerabilities across 4 modules to 25, all of which are Go standard library issues fixed in go1.25.x patch releases. No third-party module in the build graph has a called vulnerability ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index a2287dd..a709068 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,19 @@ Custom `SCAAccessService` follows SDK conventions: - **Target category:** `CLOUD_CONSOLE` (hardcoded for v1) - **Headers:** `Authorization: Bearer {jwt}`, `Content-Type: application/json` +## SDK Retry Behavior +- 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; honors `Retry-After` (clamped to max wait), else exponential backoff + - **Transport errors** (`:835-849` via `isRetryableTransportError` `:1244-1283`) — bare `io.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 behind `isIdempotentMethod` (`:1269-1281`). An EOF can surface after the server already processed the request, so this path can replay a mutation +- **grant disables transient retry on BOTH the SCA and UAR/workflows clients** via `internal/sdkclient.DisableTransientRetry` (calls `SetTransientRetry(0, 0, 0)`, `:1187-1197`), invoked right after `isp.FromISPAuth` in `internal/sca/service.go` and `internal/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/groups` are as non-idempotent as UAR `submit` +- `isp.IdsecISPServiceClient` embeds `*common.IdsecClient`, so the helper takes `client.IdsecClient` +- POSTs protected, by actual replay risk: + - **Mutating** — `internal/sca/service.go` elevate, group elevate (a duplicate creates a second session); `internal/workflows/service.go` submit (a duplicate is visible in an approver's queue), cancel, finalize + - **Effectively idempotent / read-shaped, covered incidentally** — `internal/sca/service.go` sessions/revoke (re-revoking is a no-op) and on-demand roles (`POST` used only for role discovery) +- Tests: `internal/sdkclient/retry_test.go` covers the helper; `internal/sca/retry_policy_test.go` and `internal/workflows/retry_policy_test.go` drive the **real constructors** with a fake-JWT ISP token, then repoint `client.BaseURL` at an `httptest` 429 server and assert exactly one inbound request. Removing a `DisableTransientRetry` call makes them fail with 4 + ## Testing - TDD: write `_test.go` before `.go` for every package - Table-driven tests diff --git a/go.mod b/go.mod index 4c58b12..5d7a83a 100644 --- a/go.mod +++ b/go.mod @@ -4,10 +4,10 @@ go 1.25.0 require ( github.com/Iilun/survey/v2 v2.5.3 - github.com/cyberark/idsec-sdk-golang v0.2.3 - github.com/mattn/go-isatty v0.0.20 + github.com/cyberark/idsec-sdk-golang v0.8.1 + github.com/mattn/go-isatty v0.0.24 github.com/minio/selfupdate v0.6.0 - github.com/spf13/cobra v1.9.1 + github.com/spf13/cobra v1.10.2 gopkg.in/yaml.v3 v3.0.1 ) @@ -19,9 +19,9 @@ require ( github.com/PuerkitoBio/goquery v1.10.3 // indirect github.com/andybalholm/cascadia v1.3.3 // indirect github.com/danieljoos/wincred v1.2.2 // indirect - github.com/dvsekhvalnov/jose2go v1.5.0 // indirect + github.com/dvsekhvalnov/jose2go v1.7.0 // indirect github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect - github.com/golang-jwt/jwt/v5 v5.2.2 // indirect + github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/uuid v1.6.0 // indirect github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect github.com/iancoleman/strcase v0.3.0 // indirect @@ -32,17 +32,17 @@ require ( github.com/mattn/go-colorable v0.1.14 // indirect github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect github.com/mtibben/percent v0.2.1 // indirect - github.com/spf13/pflag v1.0.6 // indirect + github.com/spf13/pflag v1.0.10 // indirect github.com/toqueteos/webbrowser v1.2.0 // indirect golang.design/x/clipboard v0.7.0 // indirect - golang.org/x/crypto v0.45.0 // indirect + golang.org/x/crypto v0.55.0 // indirect golang.org/x/exp/shiny v0.0.0-20250408133849-7e4ce0ab07d0 // indirect golang.org/x/image v0.26.0 // indirect golang.org/x/mobile v0.0.0-20250408133729-978277e7eaf7 // indirect - golang.org/x/net v0.47.0 // indirect - golang.org/x/sys v0.38.0 // indirect - golang.org/x/term v0.37.0 // indirect - golang.org/x/text v0.31.0 // indirect + golang.org/x/net v0.58.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/term v0.45.0 // indirect + golang.org/x/text v0.41.0 // indirect gopkg.in/errgo.v1 v1.0.1 // indirect gopkg.in/retry.v1 v1.0.3 // indirect ) diff --git a/go.sum b/go.sum index 69af499..93ef7fc 100644 --- a/go.sum +++ b/go.sum @@ -18,22 +18,22 @@ github.com/andybalholm/cascadia v1.3.3/go.mod h1:xNd9bqTn98Ln4DwST8/nG+H0yuB8Hmg github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.17 h1:QeVUsEDNrLBW4tMgZHvxy18sKtr6VI492kBhUfhDJNI= github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/cyberark/idsec-sdk-golang v0.2.3 h1:UmSy9oOPfz4j1O7/fIHFOoQQST4xR1BkujlMR6QnB4Y= -github.com/cyberark/idsec-sdk-golang v0.2.3/go.mod h1:2nCmBsY0/brBJ5EySoZEX/RjArtKKKdcaap5e8sOgHQ= +github.com/cyberark/idsec-sdk-golang v0.8.1 h1:5VIv1r38QU9lqt+u7jb9aX0uPONPnzffnmCA0oDEUoc= +github.com/cyberark/idsec-sdk-golang v0.8.1/go.mod h1:MhrabeLOMO6GOb4TOLX1iPFpFUaTNErvsbjUwz4PbDk= github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0= github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dvsekhvalnov/jose2go v1.5.0 h1:3j8ya4Z4kMCwT5nXIKFSV84YS+HdqSSO0VsTQxaLAeM= -github.com/dvsekhvalnov/jose2go v1.5.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= +github.com/dvsekhvalnov/jose2go v1.7.0 h1:bnQc8+GMnidJZA8zc6lLEAb4xNrIqHwO+9TzqvtQZPo= +github.com/dvsekhvalnov/jose2go v1.7.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= github.com/frankban/quicktest v1.2.2/go.mod h1:Qh/WofXFeiAFII1aEBu529AtJo6Zg2VHscnEsbBnJ20= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= -github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= -github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= +github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= +github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/go-cmp v0.2.1-0.20190312032427-6f77996f0c42/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -63,8 +63,8 @@ github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVc github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= +github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQE9x6ikvDFZS2mDVS3drnohI= github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= @@ -79,10 +79,11 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/rogpeppe/clock v0.0.0-20190514195947-2896927a307a h1:3QH7VyOaaiUHNrA9Se4YQIRkDTCw1EJls9xTUCaCeRM= github.com/rogpeppe/clock v0.0.0-20190514195947-2896927a307a/go.mod h1:4r5QyqhjIWCcK8DO4KMclc5Iknq5qVBAlbYYzAbUScQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= -github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= -github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= -github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= +github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= +github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= +github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= @@ -93,6 +94,7 @@ github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9r github.com/toqueteos/webbrowser v1.2.0/go.mod h1:XWoZq4cyp9WeUeak7w7LXRUQf1F1ATJMir8RTqb4ayM= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.design/x/clipboard v0.6.3/go.mod h1:kqBSweBP0/im4SZGGjLrppH0D400Hnfo5WbFKSNK8N4= golang.design/x/clipboard v0.7.0 h1:4Je8M/ys9AJumVnl8m+rZnIvstSnYj1fvzqYrU3TXvo= golang.design/x/clipboard v0.7.0/go.mod h1:PQIvqYO9GP29yINEfsEn5zSQKAz3UgXmZKzDA6dnq2E= @@ -106,8 +108,8 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= -golang.org/x/crypto v0.45.0 h1:jMBrvKuj23MTlT0bQEOBcAE0mjg8mK9RXFhRH6nyF3Q= -golang.org/x/crypto v0.45.0/go.mod h1:XTGrrkGJve7CYK7J8PEww4aY7gM3qMCElcJQ8n8JdX4= +golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M= +golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56/go.mod h1:JhuoJpWY28nO4Vef9tZUw9qufEGTyX1+7lmHxV5q5G4= golang.org/x/exp/shiny v0.0.0-20250408133849-7e4ce0ab07d0 h1:tMSqXTK+AQdW3LpCbfatHSRPHeW6+2WuxaVQuHftn80= golang.org/x/exp/shiny v0.0.0-20250408133849-7e4ce0ab07d0/go.mod h1:ygj7T6vSGhhm/9yTpOQQNvuAUFziTH7RUiH74EoE2C8= @@ -140,8 +142,8 @@ golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= -golang.org/x/net v0.47.0 h1:Mx+4dIFzqraBXUugkia1OOvlD6LemFo1ALMHjrXDOhY= -golang.org/x/net v0.47.0/go.mod h1:/jNxtkgq5yWUGYkaZGqo27cfGZ1c5Nen03aYrrKpVRU= +golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To= +golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -163,14 +165,13 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= -golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -181,8 +182,8 @@ golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= -golang.org/x/term v0.37.0 h1:8EGAD0qCmHYZg6J17DvsMy9/wJ7/D/4pV/wfnld5lTU= -golang.org/x/term v0.37.0/go.mod h1:5pB4lxRNYYVZuTLmy8oR2BH8dflOR+IbTYFD8fi3254= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -194,8 +195,8 @@ golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= -golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM= -golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190312151545-0bb0c0a6e846/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= diff --git a/internal/sca/retry_policy_test.go b/internal/sca/retry_policy_test.go new file mode 100644 index 0000000..1193115 --- /dev/null +++ b/internal/sca/retry_policy_test.go @@ -0,0 +1,96 @@ +package sca + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/cyberark/idsec-sdk-golang/pkg/auth" + "github.com/cyberark/idsec-sdk-golang/pkg/common/isp" + authmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/auth" +) + +// fakeJWT builds an unsigned JWT carrying just the claims the SDK's +// resolveServiceURL reads (isp/idsec_isp_service_client.go:148-168). It is +// parsed with ParseUnverified, so no signature is required. +func fakeJWT(t *testing.T) string { + t.Helper() + enc := func(v interface{}) string { + b, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal jwt segment: %v", err) + } + return base64.RawURLEncoding.EncodeToString(b) + } + header := enc(map[string]string{"alg": "none", "typ": "JWT"}) + payload := enc(map[string]string{ + "subdomain": "testtenant", + "platform_domain": "example.test", + }) + return header + "." + payload + ".sig" +} + +// ispAuthWithToken returns an ISP authenticator pre-loaded with a fake token so +// isp.FromISPAuth succeeds without real credentials. +func ispAuthWithToken(t *testing.T) auth.IdsecAuth { + t.Helper() + a := auth.NewIdsecISPAuth(false) + concrete, ok := a.(*auth.IdsecISPAuth) + if !ok { + t.Fatalf("NewIdsecISPAuth returned %T, want *auth.IdsecISPAuth", a) + } + concrete.Token = &authmodels.IdsecToken{ + Token: fakeJWT(t), + TokenType: authmodels.JWT, + } + return a +} + +// TestNewSCAAccessServiceDisablesTransientRetry exercises the real constructor +// and asserts the policy landed on the client it publishes. This is the +// regression guard for the DisableTransientRetry call in NewSCAAccessService: +// deleting that call makes this test fail with 4 inbound requests. +func TestNewSCAAccessServiceDisablesTransientRetry(t *testing.T) { + var calls int32 + var gotAPIVersion string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + gotAPIVersion = r.Header.Get("X-API-Version") + w.Header().Set("Retry-After", "0") // keeps the test fast if retry is on + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + + svc, err := NewSCAAccessService(ispAuthWithToken(t)) + if err != nil { + t.Fatalf("NewSCAAccessService: %v", err) + } + + // Redirect the constructed client at the test server. The client is fully + // built by the constructor; only its base URL is swapped. + lc, ok := svc.httpClient.(*loggingClient) + if !ok { + t.Fatalf("httpClient is %T, want *loggingClient", svc.httpClient) + } + client, ok := lc.inner.(*isp.IdsecISPServiceClient) + if !ok { + t.Fatalf("inner client is %T, want *isp.IdsecISPServiceClient", lc.inner) + } + client.BaseURL = srv.URL + + resp, err := svc.httpClient.Post(t.Context(), "/api/access/elevate", map[string]string{"a": "b"}) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if got := atomic.LoadInt32(&calls); got != 1 { + t.Errorf("inbound requests = %d, want 1 (SDK transient retry must be disabled)", got) + } + if gotAPIVersion != "2.0" { + t.Errorf("X-API-Version = %q, want %q", gotAPIVersion, "2.0") + } +} diff --git a/internal/sca/service.go b/internal/sca/service.go index 378e8ed..ffe8c58 100644 --- a/internal/sca/service.go +++ b/internal/sca/service.go @@ -10,6 +10,7 @@ import ( "net/http" "github.com/aaearon/grant-cli/internal/sca/models" + "github.com/aaearon/grant-cli/internal/sdkclient" "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" @@ -61,6 +62,14 @@ func NewSCAAccessService(authenticators ...auth.IdsecAuth) (*SCAAccessService, e return nil, fmt.Errorf("failed to create ISP client: %w", err) } + // Disable the SDK's automatic transient retry. SDK v0.8.1 retries 429s with + // no method filter (idsec_client.go:865-885) and bare EOF transport errors + // for any method including POST (:1252-1266), which could replay the + // non-idempotent elevate / group-elevate POSTs below. The client-wide + // switch also covers sessions/revoke and on-demand role discovery, which + // are effectively idempotent and safe to retry either way. + sdkclient.DisableTransientRetry(client.IdsecClient) + // Set required API version header client.SetHeader("X-API-Version", "2.0") diff --git a/internal/sdkclient/retry.go b/internal/sdkclient/retry.go new file mode 100644 index 0000000..e210041 --- /dev/null +++ b/internal/sdkclient/retry.go @@ -0,0 +1,32 @@ +// Package sdkclient holds shared policy applied to idsec-sdk-golang HTTP +// clients before grant's services use them. +package sdkclient + +import "github.com/cyberark/idsec-sdk-golang/pkg/common" + +// DisableTransientRetry turns off the SDK's automatic transient-failure retry. +// +// idsec-sdk-golang v0.8.1 retries by default — 3 retries on top of the initial +// attempt, so up to 4 requests; 500ms base, 10s cap (idsec_client.go:53-60, +// :375-377) — on two paths, neither of which is safe for grant's +// non-idempotent POSTs: +// +// - HTTP 429 (idsec_client.go:865-885) — no HTTP method filter at all. +// - Transport errors (idsec_client.go:835-849 via isRetryableTransportError, +// :1244-1283) — a bare io.EOF / io.ErrUnexpectedEOF, or an error containing +// "eof" or "server closed idle connection", is retried for ANY method +// including POST (:1252-1266). Only connection-reset / broken-pipe / GOAWAY +// are gated behind isIdempotentMethod (:1269-1281). An EOF can surface +// after the server already processed the request, so that path can replay a +// mutation and produce a duplicate. +// +// The SDK exposes no per-request opt-out, and toggling client-wide state around +// individual calls is race-prone (grant fans out concurrently across CSPs), so +// grant disables transient retry on the whole client for both the SCA and +// UAR/workflows services. See idsec_client.go:1187-1197 for SetTransientRetry. +func DisableTransientRetry(c *common.IdsecClient) { + if c == nil { + return + } + c.SetTransientRetry(0, 0, 0) +} diff --git a/internal/sdkclient/retry_test.go b/internal/sdkclient/retry_test.go new file mode 100644 index 0000000..bbf1d67 --- /dev/null +++ b/internal/sdkclient/retry_test.go @@ -0,0 +1,72 @@ +package sdkclient + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/cyberark/idsec-sdk-golang/pkg/common" +) + +// TestDisableTransientRetry verifies that DisableTransientRetry stops the SDK +// from replaying a POST. The SDK's 429 branch (idsec_client.go:865-885) has no +// HTTP method filter, so without the opt-out a non-idempotent POST is sent +// defaultTransientRetryCount+1 times (idsec_client.go:53-60). +func TestDisableTransientRetry(t *testing.T) { + tests := []struct { + name string + disable bool + retryAfter string + wantRequests int32 + }{ + { + name: "default client replays a rate-limited POST", + disable: false, + retryAfter: "0", // keeps the test fast; delay parsing is SDK-tested + wantRequests: 4, // initial attempt + 3 transient retries + }, + { + name: "disabled client issues exactly one POST", + disable: true, + retryAfter: "1", + wantRequests: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var calls int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(&calls, 1) + w.Header().Set("Retry-After", tt.retryAfter) + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + + client := common.NewSimpleIdsecClient(srv.URL) + if tt.disable { + DisableTransientRetry(client) + } + + resp, err := client.Post(t.Context(), "/api/thing", map[string]string{"a": "b"}) + if err != nil { + t.Fatalf("Post returned error: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusTooManyRequests { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusTooManyRequests) + } + if got := atomic.LoadInt32(&calls); got != tt.wantRequests { + t.Errorf("inbound requests = %d, want %d", got, tt.wantRequests) + } + }) + } +} + +// TestDisableTransientRetryNilClient guards the call sites, which pass an +// embedded *common.IdsecClient that could in principle be nil. +func TestDisableTransientRetryNilClient(t *testing.T) { + DisableTransientRetry(nil) // must not panic +} diff --git a/internal/workflows/retry_policy_test.go b/internal/workflows/retry_policy_test.go new file mode 100644 index 0000000..53d2f41 --- /dev/null +++ b/internal/workflows/retry_policy_test.go @@ -0,0 +1,92 @@ +package workflows + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/cyberark/idsec-sdk-golang/pkg/auth" + "github.com/cyberark/idsec-sdk-golang/pkg/common/isp" + authmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/auth" +) + +// fakeJWT builds an unsigned JWT carrying just the claims the SDK's +// resolveServiceURL reads (isp/idsec_isp_service_client.go:148-168). It is +// parsed with ParseUnverified, so no signature is required. +func fakeJWT(t *testing.T) string { + t.Helper() + enc := func(v interface{}) string { + b, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshal jwt segment: %v", err) + } + return base64.RawURLEncoding.EncodeToString(b) + } + header := enc(map[string]string{"alg": "none", "typ": "JWT"}) + payload := enc(map[string]string{ + "subdomain": "testtenant", + "platform_domain": "example.test", + }) + return header + "." + payload + ".sig" +} + +// ispAuthWithToken returns an ISP authenticator pre-loaded with a fake token so +// isp.FromISPAuth succeeds without real credentials. +func ispAuthWithToken(t *testing.T) auth.IdsecAuth { + t.Helper() + a := auth.NewIdsecISPAuth(false) + concrete, ok := a.(*auth.IdsecISPAuth) + if !ok { + t.Fatalf("NewIdsecISPAuth returned %T, want *auth.IdsecISPAuth", a) + } + concrete.Token = &authmodels.IdsecToken{ + Token: fakeJWT(t), + TokenType: authmodels.JWT, + } + return a +} + +// TestNewAccessRequestServiceDisablesTransientRetry exercises the real +// constructor and asserts the policy landed on the client it publishes. +// Deleting the DisableTransientRetry call in NewAccessRequestService makes this +// test fail with 4 inbound requests — i.e. a duplicate access request in an +// approver's queue. +func TestNewAccessRequestServiceDisablesTransientRetry(t *testing.T) { + var calls int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(&calls, 1) + w.Header().Set("Retry-After", "0") // keeps the test fast if retry is on + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + + svc, err := NewAccessRequestService(ispAuthWithToken(t)) + if err != nil { + t.Fatalf("NewAccessRequestService: %v", err) + } + + // Redirect the constructed client at the test server. The client is fully + // built by the constructor; only its base URL is swapped. + lc, ok := svc.httpClient.(*loggingClient) + if !ok { + t.Fatalf("httpClient is %T, want *loggingClient", svc.httpClient) + } + client, ok := lc.inner.(*isp.IdsecISPServiceClient) + if !ok { + t.Fatalf("inner client is %T, want *isp.IdsecISPServiceClient", lc.inner) + } + client.BaseURL = srv.URL + + resp, err := svc.httpClient.Post(t.Context(), "/api/workflows/requests", map[string]string{"a": "b"}) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if got := atomic.LoadInt32(&calls); got != 1 { + t.Errorf("inbound requests = %d, want 1 (SDK transient retry must be disabled)", got) + } +} diff --git a/internal/workflows/service.go b/internal/workflows/service.go index 9c2c89a..1cceeb2 100644 --- a/internal/workflows/service.go +++ b/internal/workflows/service.go @@ -10,6 +10,7 @@ import ( "net/http" "strconv" + "github.com/aaearon/grant-cli/internal/sdkclient" "github.com/aaearon/grant-cli/internal/workflows/models" "github.com/cyberark/idsec-sdk-golang/pkg/auth" "github.com/cyberark/idsec-sdk-golang/pkg/common" @@ -57,6 +58,12 @@ func NewAccessRequestService(authenticators ...auth.IdsecAuth) (*AccessRequestSe return nil, fmt.Errorf("failed to create ISP client: %w", err) } + // Disable the SDK's automatic transient retry. SDK v0.8.1 retries 429s with + // no method filter (idsec_client.go:865-885) and bare EOF transport errors + // for any method including POST (:1252-1266); a replayed submit would put a + // duplicate access request in front of approvers. + sdkclient.DisableTransientRetry(client.IdsecClient) + svc.httpClient = newLoggingClient(client, common.GetLogger("grant", -1)) return svc, nil