Fix security scan findings across the client and codegen pipeline by splch · Pull Request #96 · ionq/ionq-core-python

splch · 2026-08-25T20:29:18Z

Summary

Part 1 of the stack #96 -> #97 -> #98 -> #95; merge bottom-up (each PR targets the previous one's branch). Fixes all nine findings from the security scan. Where a fix lands in generated code, it lives in the generation pipeline (post-hooks/config), so regeneration preserves it and generated.yml stays byte-exact.

Path parameters equal to "", ".", or ".." now raise ValueError before a request is built; ".." previously survived quote() and collapsed a fixed URL segment under RFC 3986 normalization, e.g. GET /sessions/../jobs became the account-wide GET /jobs (CWE-23).
The Q-CTRL api_credentials field is excluded from attrs reprs, and AuthenticatedClient no longer writes Authorization into its repr-visible, caller-owned headers dict (CWE-532).
Retry-After is validated and clamped to at most 300 seconds; non-finite values (inf, nan, 1e309) are treated as absent (CWE-1284).
Error-response bodies are read via streaming with a 64 KiB decoded cap instead of an unbounded, transparently decompressed read() (CWE-409).
verify_ssl passed to IonQClient now reaches the connection-terminating transports on both sync and async paths; it was previously silently ignored because httpx disregards client-level verify when a custom transport is supplied (CWE-295).
Pagination aborts with IonQError when the server-supplied cursor is empty or repeats instead of looping forever (CWE-835).
The spec-drift workflow fetches from a pinned URL instead of one derived from the file under check, and writes nothing to GITHUB_ENV (CWE-807).
POST requests are no longer auto-retried: the API has no idempotency keys, so a replay after an ambiguous gateway 5xx could duplicate billable jobs (CWE-837). This is a deliberate behavior change, called out in the CHANGELOG.

Test plan

uv run pytest at this head: 288 passed with the 100% branch-coverage gate, warnings-as-errors. Every fix has dedicated tests: traversal rejection at unit and endpoint level, repr masking on both Q-CTRL models, Retry-After clamp table, body-cap stream accounting, TLS context asserted at the connection-pool level on both paths, cursor guard, header-dict hygiene, POST absent from the retry set.
uv run ruff check, uv run ruff format --check, uv run ty check ionq_core/: clean.
The documented regen command reproduces the committed tree byte-for-byte. zizmor on the changed workflow: no findings.

Important

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

Three fixes applied via post-generation hooks, mirroring the existing AuthenticatedClient.token repr hook, so regeneration preserves them: - Path parameters are routed through the new ionq_core._url.quote_path_param, which raises ValueError for "", ".", and ".." before a request is built. urllib.parse.quote never encodes dots, so ".." survived into the URL and RFC 3986 normalization (applied by httpx and any server) deleted the preceding fixed segment, e.g. GET /sessions/../jobs became the unscoped account-wide GET /jobs. (CWE-23) - AuthenticatedClient no longer writes the Authorization value into self._headers when the httpx clients are built. That dict is included in the attrs repr (defeating token's repr=False after first use) and is the very dict the caller passed as headers=, so the key bled into any other client sharing it. The header is merged into a method-local dict handed straight to httpx instead. (CWE-532) - QctrlQaoaJobCreationPayloadExternalSettings.api_credentials (a Q-CTRL API key) is excluded from the attrs repr, so logging or echoing a job payload cannot disclose it. to_dict() and the wire format are unchanged. (CWE-532)

- Clamp Retry-After to [0, 300] seconds and discard non-finite values ("inf", "nan", "1e309") before exposing RateLimitError.retry_after, so a forged header cannot drive callers that sleep on the documented attribute into an unbounded wait or an OverflowError. (CWE-1284) - Read at most 64 KiB (decoded) of an error-response body via streaming instead of response.read(). httpx transparently applies the server-chosen Content-Encoding, so the unbounded read let a small gzip-bombed error body allocate memory proportional to its decompressed size. (CWE-409) - Stop retrying POSTs: the API has no idempotency keys, so replaying create_job/create_session/end_session after an ambiguous gateway 5xx could duplicate billable work. Idempotent methods retry as before. (CWE-837) - Apply verify_ssl to the transports that terminate connections. httpx ignores client-level verify whenever a custom transport is supplied, so every verify_ssl value passed to IonQClient (False, CA bundle path, or a pinned ssl.SSLContext) was silently discarded on both the sync and async paths. (CWE-295) - Abort pagination with IonQError when the server-supplied next cursor is empty or repeats. The cursor was the loop's only exit condition, so a hostile server could keep iter_jobs and friends issuing authenticated requests forever while the consumer blocked in next(). (CWE-835)

The workflow derived its fetch target from servers[0].url inside the very openapi.json it exists to check, so a tampered vendored spec could point the drift comparison at a mirror serving an identical copy and suppress its own detection; the jq-derived value was also written unsanitized to GITHUB_ENV. The URL is now a workflow-defined constant (matching CONTRIBUTING.md) and nothing is written to GITHUB_ENV. (CWE-807) Also documents all fixes in CHANGELOG and updates CONTRIBUTING's description of the post-generation hooks.

natestemen

All seems reasonable to me. I'll approve for that reason with the caveat that I am not a security expert (i'm not even a security ... anything!).

natestemen · 2026-08-25T23:29:17Z

 from http import HTTPStatus
 from typing import Any, cast
-from urllib.parse import quote
+from ..._url import quote_path_param


What's the deal with the files where this is the only change? Doesn't it imply that both quote and quote_path_param are not used since there are no other chagnes in the file?

natestemen · 2026-08-26T21:02:46Z

+    segment = str(value)
+    if segment in ("", ".", ".."):
+        raise ValueError(f"Invalid URL path parameter {segment!r}: it would escape its path segment")
+    return quote(segment, safe="")


Why is / not safe?

The idea here seems to be that when constructing a URL from client supplied data, a common expectation might be that the path structure of the URL (i.e. the sequence of slashes) should not be controlled by the client data.

I don't know whether this is an issue in practice for our own APIs or whether I really find this solution compelling. For external facing services, an attacker can also just construct the URL and call it themselves. For internal services we should be very careful about including client controlled data in URLs and this monkeypatching of quote only sweeps the larger issue under the rug.

hodgestar-ionq

Partial review.

hodgestar-ionq · 2026-08-26T13:21:22Z

+    env:
+      # Pinned (keep in sync with CONTRIBUTING.md); never derived from openapi.json,
+      # so a tampered vendored spec cannot point the check at a mirror that hides it.
+      SPEC_URL: https://api.ionq.co/v0.4/api-docs


What attack is this defending against? openapi.json is part of this repository. If an attack can edit the repository, nothing in the repository can defend against it.

I don't mind the simplification, but on the face of it this doesn't seem like a security issue.

hodgestar-ionq · 2026-08-27T11:26:51Z

+`ErrorRaisingTransport` converts HTTP error responses and connection failures
+into structured `IonQError` exceptions; `build_transport` assembles the default
+stack used by `IonQClient`. Idempotent methods are retried on status codes 429,
+500, 502, 503, and 520-529 with exponential backoff (factor 0.5, jitter 0.5,
+max 60s); POST is never retried because the API has no idempotency keys, so a
+replay after an ambiguous 5xx could duplicate billable work.
+
+Error handling bounds what it trusts from the server: at most
+`MAX_ERROR_BODY_BYTES` decoded bytes of an error body are read, and
+``Retry-After`` is clamped to `MAX_RETRY_AFTER` seconds (non-finite values are
+discarded) before being exposed on `RateLimitError.retry_after`.


Suggested change

`ErrorRaisingTransport` converts HTTP error responses and connection failures

into structured `IonQError` exceptions; `build_transport` assembles the default

stack used by `IonQClient`. Idempotent methods are retried on status codes 429,

500, 502, 503, and 520-529 with exponential backoff (factor 0.5, jitter 0.5,

max 60s); POST is never retried because the API has no idempotency keys, so a

replay after an ambiguous 5xx could duplicate billable work.

Error handling bounds what it trusts from the server: at most

`MAX_ERROR_BODY_BYTES` decoded bytes of an error body are read, and

``Retry-After`` is clamped to `MAX_RETRY_AFTER` seconds (non-finite values are

discarded) before being exposed on `RateLimitError.retry_after`.

This module provides the `ErrorRaisingTransport` that wraps httpx transports

to convert HTTP error responses and connection failures into structured

`IonQError` exceptions. The `build_transport` factory creates the default

transport stack: ``RetryTransport`` (from httpx-retries) wrapped by

``ErrorRaisingTransport``.

The default retry configuration retries on status codes 429, 500, 502, 503,

and 520-529 with exponential backoff (factor 0.5, jitter 0.5, max 60s).

POST, PATCH and other HTTP methods not documented as idempotent are never retried.

Error handling limits the length of the body read from the server to `MAX_ERROR_BODY_BYTES` and limits the maximum ``Retry-After`` delay to at move `MAX_RETRY_AFTER` seconds. Non-finite ``Retry-After`` values are ignored

The original text reads much better, so I manually merged the two.

hodgestar-ionq · 2026-08-27T11:29:02Z

+        # POST is deliberately not retryable: without idempotency keys, a replay
+        # after an ambiguous 5xx could duplicate billable jobs.


Suggested change

# POST is deliberately not retryable: without idempotency keys, a replay

# after an ambiguous 5xx could duplicate billable jobs.

This comment feels out of place. The Retry class should document its own behaviour.

splch added 3 commits August 25, 2026 14:52

splch requested a review from a team as a code owner August 25, 2026 20:29

This was referenced Aug 25, 2026

Regenerate the client from the latest OpenAPI specification #95

Open

Simplify the hand-written layer and fix client contract bugs #97

Open

Remove documentation rot and tighten the prose #98

Open

natestemen approved these changes Aug 26, 2026

View reviewed changes

hodgestar-ionq reviewed Aug 27, 2026

View reviewed changes

Navigation Menu

Uh oh!

Fix security scan findings across the client and codegen pipeline - #96

Fix security scan findings across the client and codegen pipeline#96
splch wants to merge 3 commits into
mainfrom
scan-findings/1-security

splch commented Aug 25, 2026 •

edited

Loading

Uh oh!

natestemen left a comment

Uh oh!

natestemen Aug 25, 2026

Uh oh!

natestemen Aug 26, 2026

Uh oh!

hodgestar-ionq Aug 27, 2026

Uh oh!

hodgestar-ionq left a comment

Uh oh!

hodgestar-ionq Aug 26, 2026

Uh oh!

hodgestar-ionq Aug 27, 2026

Uh oh!

hodgestar-ionq Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# POST is deliberately not retryable: without idempotency keys, a replay
		# after an ambiguous 5xx could duplicate billable jobs.

Uh oh!

Conversation

splch commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

natestemen left a comment

Choose a reason for hiding this comment

Uh oh!

natestemen Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

natestemen Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

hodgestar-ionq Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

hodgestar-ionq left a comment

Choose a reason for hiding this comment

Uh oh!

hodgestar-ionq Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

hodgestar-ionq Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

hodgestar-ionq Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

splch commented Aug 25, 2026 •

edited

Loading