From 9b16e2f8e34e489372358982eed6ea5c5da4fa0d Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 22:23:46 +0300 Subject: [PATCH 1/3] fix(httperr): do not leak the raw URL via a wrapped url.Error in TransportError.Error --- httperr/transport_error.go | 15 ++++++++++++--- httperr/transport_error_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/httperr/transport_error.go b/httperr/transport_error.go index 5c998ea..5f9dc33 100644 --- a/httperr/transport_error.go +++ b/httperr/transport_error.go @@ -9,6 +9,7 @@ import ( "fmt" "net" "net/http" + "net/url" "github.com/dexpace/go-sdk/redact" ) @@ -28,12 +29,20 @@ type TransportError struct { Err error } -// Error implements error. +// Error implements error. When the underlying cause is a *url.Error (as produced +// by net/http), its message — which embeds the full, unredacted URL — is replaced +// by the inner cause, so a query secret in the URL never reaches the error string. +// The redacted URL is reported via the Method/URL fields instead. func (e *TransportError) Error() string { + cause := e.Err + var ue *url.Error + if errors.As(e.Err, &ue) { + cause = ue.Err + } if e.URL == "" { - return fmt.Sprintf("transport error: %v", e.Err) + return fmt.Sprintf("transport error: %v", cause) } - return fmt.Sprintf("transport error: %s %s: %v", e.Method, e.URL, e.Err) + return fmt.Sprintf("transport error: %s %s: %v", e.Method, e.URL, cause) } // Unwrap returns the underlying cause so errors.Is/As reach through it. diff --git a/httperr/transport_error_test.go b/httperr/transport_error_test.go index 6249b80..70155e6 100644 --- a/httperr/transport_error_test.go +++ b/httperr/transport_error_test.go @@ -8,6 +8,7 @@ import ( "errors" "net/http" "net/url" + "strings" "testing" "github.com/dexpace/go-sdk/httperr" @@ -71,6 +72,35 @@ func TestFromErrorWrapsTransportFailure(t *testing.T) { } } +func TestTransportErrorDoesNotLeakURLInMessage(t *testing.T) { + t.Parallel() + + // net/http surfaces a *url.Error whose Error() embeds the full raw URL, + // including query secrets. TransportError.Error() must not reproduce it. + cause := &url.Error{ + Op: "Get", + URL: "https://api.example.test/things?token=SECRET", + Err: errors.New("dial tcp: connection refused"), + } + req := &http.Request{ + Method: http.MethodGet, + URL: &url.URL{Scheme: "https", Host: "api.example.test", Path: "/things", RawQuery: "token=SECRET"}, + } + te := httperr.FromError(cause, req) + + msg := te.Error() + if strings.Contains(msg, "SECRET") { + t.Fatalf("Error() leaked the query secret: %q", msg) + } + if !strings.Contains(msg, "connection refused") { + t.Fatalf("Error() should include the underlying cause: %q", msg) + } + // Unwrap must remain lossless: the original cause is still reachable. + if !errors.Is(te, cause) { + t.Fatal("Unwrap must still reach the original *url.Error cause") + } +} + func TestTransportErrorTimeout(t *testing.T) { t.Parallel() From 187ae530f37258db14990337d955888f3e1b02cc Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 22:23:54 +0300 Subject: [PATCH 2/3] feat(jsonl): add WithMaxBytes to bound stream reads --- jsonl/jsonl.go | 28 +++++++++++++++++++++++++++- jsonl/jsonl_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/jsonl/jsonl.go b/jsonl/jsonl.go index cddef96..20f62dd 100644 --- a/jsonl/jsonl.go +++ b/jsonl/jsonl.go @@ -10,12 +10,38 @@ import ( "iter" ) +// Option configures [Decode]. +type Option func(*config) + +type config struct { + maxBytes int64 +} + +// WithMaxBytes bounds the total number of bytes Decode reads from the stream, so +// an untrusted source cannot force unbounded memory use. A value <= 0 (the +// default) leaves the stream unbounded. When the limit is reached mid-value, the +// truncated value surfaces a decode error. +func WithMaxBytes(n int64) Option { + return func(c *config) { c.maxBytes = n } +} + // Decode reads a stream of JSON values from r and yields each decoded into a T. // Values may be separated by any JSON whitespace (newlines for NDJSON / JSON // Lines, or none). Iteration ends at end of stream; a decode error is delivered // as the second iteration value, after which iteration stops. The iterator is // single-pass. -func Decode[T any](r io.Reader) iter.Seq2[T, error] { +// +// Decode does not bound the size of an individual JSON value by default; for an +// untrusted stream, pass [WithMaxBytes] (or wrap r in an io.LimitReader) so a +// hostile value cannot exhaust memory. +func Decode[T any](r io.Reader, opts ...Option) iter.Seq2[T, error] { + var cfg config + for _, opt := range opts { + opt(&cfg) + } + if cfg.maxBytes > 0 { + r = io.LimitReader(r, cfg.maxBytes) + } return func(yield func(T, error) bool) { dec := json.NewDecoder(r) for { diff --git a/jsonl/jsonl_test.go b/jsonl/jsonl_test.go index 62cbb82..ef74fde 100644 --- a/jsonl/jsonl_test.go +++ b/jsonl/jsonl_test.go @@ -115,3 +115,39 @@ func TestDecodeEarlyBreak(t *testing.T) { t.Fatalf("consumed %d values, want 1 after break", count) } } + +func TestDecodeWithMaxBytes(t *testing.T) { + t.Parallel() + + // Two small values fit under the cap. + var got []rec + for v, err := range jsonl.Decode[rec](strings.NewReader("{\"n\":1}\n{\"n\":2}\n"), jsonl.WithMaxBytes(64)) { + if err != nil { + t.Fatalf("unexpected error under cap: %v", err) + } + got = append(got, v) + } + if len(got) != 2 { + t.Fatalf("got %d values under cap, want 2", len(got)) + } +} + +func TestDecodeMaxBytesTruncatesOversized(t *testing.T) { + t.Parallel() + + // A single value larger than the cap: the read is bounded, so decoding the + // truncated input yields an error rather than reading unbounded memory. + big := "{\"s\":\"" + strings.Repeat("a", 200) + "\"}" + var got int + var gotErr error + for _, err := range jsonl.Decode[map[string]any](strings.NewReader(big), jsonl.WithMaxBytes(32)) { + if err != nil { + gotErr = err + break + } + got++ + } + if gotErr == nil { + t.Fatalf("expected an error when a value exceeds the byte cap (got %d values)", got) + } +} From 112a46b2155a2f0cc41701218ebc8cd07bc5bcdc Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 22:25:11 +0300 Subject: [PATCH 3/3] docs: correct pipeline-order diagrams, layout tree, HTTPS-only note, and option grouping --- CLAUDE.md | 10 ++++++---- README.md | 19 ++++++++++++------- client.go | 2 +- pagination/strategies.go | 3 +++ 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index df11a1c..cec9272 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,6 +84,7 @@ go-sdk/ ├── pipeline/ # Policy, PolicyFunc, Transporter, Request, Pipeline ├── transport/ # default net/http Transporter ├── retry/ # retry policy (backoff + jitter + Retry-After) +├── idempotency/ # idempotency-key policy (default-on for POST) ├── auth/ # TokenCredential, BearerTokenPolicy, StaticToken ├── logging/ # slog request/response policy ├── httperr/ # ResponseError + FromResponse @@ -125,7 +126,7 @@ terminating in a `Transporter`: 2. **`transport`** — wraps an `*http.Client` (cloned `http.DefaultTransport` with larger idle-conn limits) to satisfy `Transporter`. 3. **Policies** — `retry`, `auth`, `logging`, each a `Policy`. Order is set by - `dexpace.New`: `user-agent → idempotency → retry → auth → date → [tracing] → [metrics] → logging → custom → transport`. + `dexpace.New`: `[errors] → user-agent → idempotency → retry → auth → [date] → [tracing] → [metrics] → logging → custom → transport`. 4. **Value layer** — `mediatype`, `header`, `httperr`, `pagination`: small, stdlib-only helpers over `net/http`. @@ -135,9 +136,10 @@ terminating in a `Transporter`: automatically for `bytes.Reader`/`bytes.Buffer`/`strings.Reader` bodies. A streaming body (`io.Reader` with no `GetBody`) is **not** replayable — rewind returns an error and retries fail. Buffer such bodies before sending. -- **`BearerTokenPolicy` is HTTPS-only.** It returns `auth.ErrInsecureTransport` - for a non-`https` URL rather than leaking a token. Tests must use `https://` - URLs (a stub transporter never dials). +- **The credential policies are HTTPS-only.** `BearerTokenPolicy`, + `BasicAuthPolicy`, and `APIKeyPolicy` all return `auth.ErrInsecureTransport` + for a non-`https` URL rather than leaking a credential. Tests must use + `https://` URLs (a stub transporter never dials). - **Policy order changes semantics.** Retry is outside auth, so a 401-triggered token refresh requires the auth policy to be inside retry (it is, by default). Moving logging outside retry collapses per-attempt logs into one. diff --git a/README.md b/README.md index 7cb9e27..84ca65c 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ standard library. `dexpace.New` assembles policies outermost-first: ``` -user-agent → idempotency → retry → auth → date → [tracing] → [metrics] → logging → custom → transport +[errors] → client-identity → idempotency → retry → auth → [date] → [tracing] → [metrics] → logging → custom → transport ``` Retry wraps the inner policies, so auth re-runs (and may refresh its token) on @@ -96,21 +96,26 @@ wire a backend): via the instrumentation `Tracer` SPI and injects a W3C `traceparent` header. - `WithMetrics(meter)` — installs a metrics policy recording request duration and in-flight requests via the instrumentation `Meter` SPI. +- `WithRedactionAllowlist(params...)` — preserves the listed query-param values in + redacted URLs (logs and traces); all other query values are redacted by default. + +URLs are redacted by default across logs, traces, and errors: userinfo is +stripped and query values are redacted unless allowlisted with +`WithRedactionAllowlist`. + +### Authentication and configuration + +- `WithCredential(cred, scopes...)` — authenticates requests with bearer tokens + from a `TokenCredential` (HTTPS-only, cached). - `WithTokenCache(cache)` — shares a bearer-token cache (an `auth.TokenCache`, in-memory by default) across clients so a cached token is reused. - `WithBasicAuth(username, password)` — authenticates requests with HTTP Basic auth (HTTPS-only). - `WithAPIKey(header, key)` — sets an API-key header on every request (HTTPS-only). -- `WithRedactionAllowlist(params...)` — preserves the listed query-param values in - redacted URLs (logs and traces); all other query values are redacted by default. - `WithConfig(cfg)` — sources defaults from `DEXPACE_*` environment variables — `DEXPACE_USER_AGENT`, `DEXPACE_MAX_RETRIES` (0 or negative disables retries), `DEXPACE_RETRY_BASE_DELAY`, `DEXPACE_HTTP_TIMEOUT` (default transport only) — for settings not set explicitly; explicit options always win. -URLs are redacted by default across logs, traces, and errors: userinfo is -stripped and query values are redacted unless allowlisted with -`WithRedactionAllowlist`. - ## Requirements Go **1.26+**. The module targets modern idioms: generics, range-over-func diff --git a/client.go b/client.go index 358092a..0fae87b 100644 --- a/client.go +++ b/client.go @@ -31,7 +31,7 @@ type Client struct { // New assembles a Client. Built-in policies are placed in stage order, outermost // first: // -// client-identity → idempotency → retry → auth → date → [tracing] → [metrics] → logging → transport +// [errors] → client-identity → idempotency → retry → auth → [date] → [tracing] → [metrics] → logging → transport // // When WithErrors is supplied, an errors stage is prepended as the outermost // policy, mapping the final result to the typed error model. diff --git a/pagination/strategies.go b/pagination/strategies.go index d7de9ee..79cca59 100644 --- a/pagination/strategies.go +++ b/pagination/strategies.go @@ -41,6 +41,9 @@ func NewPageNumber[T any](startPage int, fetch func(ctx context.Context, page in // NewLinkHeader returns a Pager that follows RFC 8288 Link headers. fetch is // called with the next URL (empty for the first page) and returns the page's // items and the HTTP response whose Link header carries the next URL. +// +// The next URL is taken from the server-controlled Link header, so fetch should +// validate or trust the URL's host before dialing it to avoid SSRF. func NewLinkHeader[T any](fetch func(ctx context.Context, url string) ([]T, *http.Response, error), opts ...Option) *Pager[T] { tokenFetch := func(ctx context.Context, token string) (Page[T], error) { items, resp, err := fetch(ctx, token)