From 2adf97690f021c74b0959ae1d19b63aeb66105cf Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 17:43:59 +0200 Subject: [PATCH 1/3] chore: disable SDK transient retry on SCA and UAR clients idsec-sdk-golang v0.8.1 enables automatic transient retry by default (3 retries, 500ms base, 10s cap). Two paths can replay non-idempotent POSTs: - the 429 branch (idsec_client.go:865-885) has no HTTP method filter - the transport-error branch (:835-849 via isRetryableTransportError :1244-1283) retries bare io.EOF / io.ErrUnexpectedEOF / "eof" / "server closed idle connection" for ANY method including POST (:1252-1266); only connection-reset, broken-pipe and GOAWAY are gated behind isIdempotentMethod (:1269-1281) An EOF can surface after the server already processed the request, so that path could double-elevate or file a duplicate access request in an approver's queue. The SDK offers no per-request opt-out and toggling client-wide state per call is race-prone (grant fans out concurrently across CSPs), so retry is disabled client-wide on both services via the new internal/sdkclient.DisableTransientRetry helper. SCA elevate and group-elevate are just as non-idempotent as UAR submit. --- CHANGELOG.md | 1 + CLAUDE.md | 10 +++++ internal/sca/service.go | 7 ++++ internal/sdkclient/retry.go | 31 ++++++++++++++ internal/sdkclient/retry_test.go | 72 ++++++++++++++++++++++++++++++++ internal/workflows/service.go | 7 ++++ 6 files changed, 128 insertions(+) create mode 100644 internal/sdkclient/retry.go create mode 100644 internal/sdkclient/retry_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ccd5d5..acebd07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ All notable changes to this project will be documented in this file. - 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. - No new modules enter the compiled binary: `go list -e -deps` reports 38 → 38 external modules (0 delta) and 309 → 310 packages, the single addition being the SDK's own `pkg/common/pvwa`. `go list -m all` grows 110 → 124 selected modules, but none of the additions are in the linked graph. Binary size 13,529,380 → 13,652,260 bytes (+0.91%). +- 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 - Reduced `govulncheck` findings from 34 vulnerabilities across 4 modules to 26 (25 of which are Go standard library issues fixed in go1.25.x patch releases, plus the unmaintained `golang.org/x/crypto/openpgp` pulled in by `rhysd/go-github-selfupdate`) ## [0.7.0] - 2026-04-21 diff --git a/CLAUDE.md b/CLAUDE.md index c0d02d8..b73fad5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,16 @@ 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, 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` +- Non-idempotent POSTs protected: `internal/sca/service.go` elevate, sessions/revoke, ondemand roles, group elevate; `internal/workflows/service.go` submit, cancel, finalize + ## Testing - TDD: write `_test.go` before `.go` for every package - Table-driven tests diff --git a/internal/sca/service.go b/internal/sca/service.go index 378e8ed..42d0c60 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,12 @@ 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 / revoke POSTs below. + 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..ea0a8dd --- /dev/null +++ b/internal/sdkclient/retry.go @@ -0,0 +1,31 @@ +// 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 attempts, 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/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 From 1399e8f9e87a9cf3ae09967825b3dbe0b8a9ebf5 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 17:52:05 +0200 Subject: [PATCH 2/3] test: cover retry policy at the service constructors Codex review of 6a4d4b1: the helper was only tested in isolation, so deleting DisableTransientRetry from either constructor would not have failed any test. Both constructors are now driven end-to-end without credentials: auth.IdsecAuthBase.Token is an exported field, so an IdsecISPAuth can be seeded with an unsigned fake JWT carrying the subdomain/platform_domain claims that resolveServiceURL reads. The constructed client's exported BaseURL is then repointed at an httptest server that always returns 429, and the test asserts exactly one inbound request (plus X-API-Version on the SCA client). Verified by mutation: stubbing out either DisableTransientRetry call makes the matching test fail with 4 requests. Also corrects two doc inaccuracies: the SDK does 3 retries on top of the initial attempt (4 requests total, matching the measured count), and the on-demand roles POST is read-shaped role discovery rather than a mutation, so CLAUDE.md now splits the POST list by actual replay risk. --- CLAUDE.md | 7 +- internal/sca/retry_policy_test.go | 96 +++++++++++++++++++++++++ internal/sdkclient/retry.go | 7 +- internal/workflows/retry_policy_test.go | 92 ++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 5 deletions(-) create mode 100644 internal/sca/retry_policy_test.go create mode 100644 internal/workflows/retry_policy_test.go diff --git a/CLAUDE.md b/CLAUDE.md index b73fad5..bf1077b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,14 +61,17 @@ Custom `SCAAccessService` follows SDK conventions: - **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, 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`) +- 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` -- Non-idempotent POSTs protected: `internal/sca/service.go` elevate, sessions/revoke, ondemand roles, group elevate; `internal/workflows/service.go` submit, cancel, finalize +- 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 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/sdkclient/retry.go b/internal/sdkclient/retry.go index ea0a8dd..e210041 100644 --- a/internal/sdkclient/retry.go +++ b/internal/sdkclient/retry.go @@ -6,9 +6,10 @@ 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 attempts, 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: +// 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, 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) + } +} From b8b5b5a8e6684b7fb172ce9a48643b9e5a3c04f2 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 13 Aug 2026 21:52:04 +0200 Subject: [PATCH 3/3] docs: correct retry comment about which SCA POSTs are non-idempotent sessions/revoke is effectively idempotent (CLAUDE.md already says so); only elevate and group-elevate genuinely cannot be replayed. Behaviour unchanged. --- internal/sca/service.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/sca/service.go b/internal/sca/service.go index 42d0c60..ffe8c58 100644 --- a/internal/sca/service.go +++ b/internal/sca/service.go @@ -65,7 +65,9 @@ func NewSCAAccessService(authenticators ...auth.IdsecAuth) (*SCAAccessService, e // 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 / revoke POSTs below. + // 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