Skip to content

Fix TLS verify passthrough, POST replay, repr token leak, and unbounded pagination#90

Open
splch wants to merge 2 commits into
mainfrom
security-hardening-transport-pagination
Open

Fix TLS verify passthrough, POST replay, repr token leak, and unbounded pagination#90
splch wants to merge 2 commits into
mainfrom
security-hardening-transport-pagination

Conversation

@splch

@splch splch commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes four security findings in the transport and pagination layers:

  • verify_ssl silently ignored (CWE-295): IonQClient injects a transport into the httpx clients, and httpx ignores its verify= argument entirely once a transport is supplied; the retry transport's default inner transports were built against the system trust store. build_transport() now constructs the socket-opening httpx.HTTPTransport / AsyncHTTPTransport with the caller's verify_ssl, so a pinned ssl.SSLContext, a private CA, or verify_ssl=False actually reaches the connections that open sockets (ionq_core/_transport.py, DualTransport).
  • POST replay on retryable statuses: the retry policy added POST to allowed_methods, so a 429/5xx received after the server committed (e.g. a load-balancer 503 post-commit) transparently re-sent create_job / clone_job / create_session / end_session, duplicating billable work. The transport now runs two RetryTransport policies: idempotent methods keep the existing status + exception retries, while POST retries only pre-send failures (ConnectError, ConnectTimeout, PoolTimeout) and is never re-sent after any response. This matches urllib3/requests semantics: connection-phase errors are retried for all methods because the request never left the client; received responses never trigger a re-send of a non-idempotent request.
  • API key in repr() after first request (CWE-532): get_httpx_client() / get_async_httpx_client() copy Authorization: apiKey <token> into _headers, which attrs included in __repr__ despite token being repr=False. Fixed with a generator post-hook (the same mechanism as the existing token redaction) so it survives regeneration; client.py is regenerated in this PR.
  • Unbounded pagination loop (CWE-835): iter_jobs / aiter_jobs / iter_session_jobs / aiter_session_jobs followed the server-controlled next cursor forever. They now raise IonQError when the server echoes back the cursor it was just sent, or when the page count exceeds the new max_pages argument (default 10,000; None disables the cap).

One supporting change: a post-hook normalizes client.py to end with a single newline. ruff-format excludes client.py, so the raw generator output ends with a blank line that pre-commit's end-of-file-fixer strips - without this, the generated.yml staleness check and pre-commit disagree about the same bytes. Note for a follow-up: ~50 generated files under ionq_core/models/ and ionq_core/api/ have the same latent EOF quirk; this PR only normalizes the file it already touches.

Test plan

  • New behavioral tests drive the retry stack over a fake transport: POST returning 503/429 is not re-sent (exactly one call), POST retries each of ConnectError / ConnectTimeout / PoolTimeout, POST does not retry ReadTimeout / RemoteProtocolError, GET keeps status and read-timeout retries, plus async variants and max_retries=0.
  • verify_ssl tests assert a caller's ssl.SSLContext is the exact object on the innermost sync and async connection pools (identity check), verify_ssl=False yields CERT_NONE, and the default yields CERT_REQUIRED - both at the build_transport() level and end-to-end through IonQClient(...).
  • Repr tests now cover the post-request state: repr(client) after get_httpx_client() / get_async_httpx_client() no longer contains the token (the pre-existing test only checked a freshly constructed client).
  • Pagination tests cover repeated-cursor detection, max_pages enforcement (sync/async, jobs and session jobs), and the max_pages=None opt-out.
  • uv run pytest: 275 passed, 100% branch coverage on hand-written code. uv run ruff check, uv run ruff format --check, uv run ty check ionq_core/, and pre-commit on the changed files all pass.
  • Regenerated with the vendored spec; the committed tree is byte-identical to fresh generator output, matching the generated.yml check.

Important

Most code in ionq_core/ is auto-generated and overwritten on regeneration.
See CONTRIBUTING.md for which files are safe to edit.

…ed pagination

- Thread verify_ssl through to the socket-opening transports; httpx
  ignores verify= once a transport is injected, so a pinned SSLContext or
  private CA previously fell back to the system trust store silently.
- Split the retry policy: idempotent methods keep status+exception
  retries, POST retries only pre-send connection failures and is never
  re-sent after a response, so a post-commit 429/5xx cannot duplicate a
  billable job or session (urllib3-style semantics).
- Redact the default-headers dict from Client/AuthenticatedClient repr
  via a generator post-hook; the auth header is copied into _headers on
  first use, defeating the existing token repr=False redaction.
- Bound cursor pagination: raise IonQError on a repeated cursor or when
  max_pages (default 10,000, None disables) is exceeded.
- Normalize generated client.py EOF in a post-hook so pre-commit's
  end-of-file-fixer and the generated-code staleness check agree.
@splch splch requested a review from a team as a code owner July 9, 2026 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant