From f7823b48436be080bca3174af499b3669ca3c787 Mon Sep 17 00:00:00 2001 From: jar-stripe Date: Fri, 28 Aug 2026 09:43:48 -0700 Subject: [PATCH 1/6] Dispatch discriminated union fields to their variant class (#1860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add discriminated union serialization tests Tests discriminated union type shapes for both request-side (TypedDict params with Literal discriminator) and response-side (StripeObject deserialization), covering standalone and inline variants. Co-Authored-By: Claude Opus 4.6 Committed-By-Agent: claude * Clarify test docstring scope and dict() comment The module docstring now explicitly states these tests exercise runtime semantics (dict construction, field access, round-trip), not static type narrowing. The dict() comment explains what it's actually testing. Co-Authored-By: Claude Opus 4.6 Committed-By-Agent: claude * Rewrite DU tests: correct inline pattern + route through _api_encode Inline union tests now use the flattened TypedDict pattern (discriminator and per-variant payload fields on the parent) rather than the incorrect per-variant TypedDicts-with-type-field pattern that was there before. Request-side tests now exercise `_api_encode` so they verify real SDK encoding behavior (bracket notation, nested dicts) rather than just dict construction and key lookup. Co-Authored-By: Claude Sonnet 4.6 Committed-By-Agent: claude * Dispatch discriminated union fields to their variant class A discriminated union field arrived as a dict with no class attached, so it became a bare StripeObject. That object carries no `_field_encodings`, so an int64 or decimal field inside a variant stayed a string — `luminance` came back as "1500" rather than 1500. Codegen already emits `_inner_class_union_variant_types` on the parent (`{"color": ("model", {"rgb": RgbColor, ...})}`); nothing read it. Consume it in `_update_attributes` so the discriminator inside the value selects the variant class, which then applies its own encodings. Mirrors stripe-ruby#1923. Falls back to a plain StripeObject when the discriminator is absent, is not a string, or names a variant this release does not know, so a variant the API adds later still deserializes. Rewrites tests/test_discriminated_unions.py, which could not detect any of this: every response-side test ran `StripeObject.construct_from` on the base class with no variant map, so all seven passed identically against `{"foo": 1}`. The fixtures now mirror the generated shape — two color variants with *different* encodings — so identical wire bytes hydrate differently based only on the discriminator. Seven of the 24 tests fail with the dispatch line reverted. The request side moves from `_api_encode` to `_coerce_v2_params`. `_api_encode` is v1 form encoding, which treats any dict identically and so asserted nothing about unions; v2 requests coerce through the method-level schema. One test pins the generator's deliberate flattening of variants into one field-name-keyed map. Co-Authored-By: Claude Opus 5 Committed-By-Agent: claude --------- Co-authored-by: Claude Opus 4.6 --- stripe/_stripe_object.py | 34 +++- tests/test_discriminated_unions.py | 309 +++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+), 1 deletion(-) create mode 100644 tests/test_discriminated_unions.py diff --git a/stripe/_stripe_object.py b/stripe/_stripe_object.py index f29bbce42..b37075ab1 100644 --- a/stripe/_stripe_object.py +++ b/stripe/_stripe_object.py @@ -413,7 +413,9 @@ def _refresh_from( for k, v in values.items(): # Apply field encoding coercion (e.g. int64_string: str → int) v = self._coerce_field_value(k, v) - inner_class = self._get_inner_class_type(k) + inner_class = self._get_union_variant_class( + k, v + ) or self._get_inner_class_type(k) is_dict = self._get_inner_class_is_beneath_dict(k) if is_dict: obj = { @@ -682,11 +684,41 @@ def __deepcopy__(self, memo: Dict[int, Any]) -> "StripeObject": _inner_class_dicts: ClassVar[List[str]] = [] _field_encodings: ClassVar[Dict[str, str]] = {} + # Maps a discriminated-union field to (discriminator, {value: class}). Generated + # subclasses override this; every other object keeps the empty default so the + # lookup in _update_attributes stays cheap. + _inner_class_union_variant_types: ClassVar[ + Dict[str, Tuple[str, Dict[str, Type["StripeObject"]]]] + ] = {} + def _get_inner_class_type( self, field_name: str ) -> Optional[Type["StripeObject"]]: return self._inner_class_types.get(field_name) + def _get_union_variant_class( + self, field_name: str, value: Any + ) -> Optional[Type["StripeObject"]]: + """ + Returns the variant class that a discriminated union field's value should + become, based on the discriminator carried in the value itself. + + Returns None rather than raising when the discriminator is absent, is not a + string, or names a variant this version of the SDK does not know about. The + caller then converts without a class, so a variant the API adds after this + release still deserializes instead of blowing up. + """ + union = self._inner_class_union_variant_types.get(field_name) + if union is None or not isinstance(value, dict): + return None + + discriminator, variants = union + discriminator_value = cast(Dict[str, Any], value).get(discriminator) + if not isinstance(discriminator_value, str): + return None + + return variants.get(discriminator_value) + def _get_inner_class_is_beneath_dict(self, field_name: str): return field_name in self._inner_class_dicts diff --git a/tests/test_discriminated_unions.py b/tests/test_discriminated_unions.py new file mode 100644 index 000000000..ff20cc1a6 --- /dev/null +++ b/tests/test_discriminated_unions.py @@ -0,0 +1,309 @@ +""" +Tests for discriminated union runtime behavior. + +A discriminated union field arrives as a plain JSON object, and the SDK has to +pick the variant class out of the discriminator carried inside that object. The +fixtures below mirror what codegen emits for the fake spec's `test.llama` +resource, including the part that makes dispatch *observable*: the two color +variants declare different `_field_encodings`, so identical wire bytes hydrate +differently based only on the discriminator. Without dispatch the value becomes +a bare StripeObject carrying no encodings, and every coercion assertion here +fails. + +Static type narrowing (Literal discriminators, Union resolution) is checked by +pyright, not here. +""" + +from decimal import Decimal +from typing import Any, Dict, Optional, Union + +from typing_extensions import Literal + +from stripe._encode import _coerce_v2_params +from stripe._stripe_object import StripeObject + + +# --------------------------------------------------------------------------- +# Fixtures — shaped the way codegen emits them +# --------------------------------------------------------------------------- + + +class RgbColor(StripeObject): + luminance: Optional[int] + model: Literal["rgb"] + _field_encodings = {"luminance": "int64_string"} + + +class HsvColor(StripeObject): + model: Literal["hsv"] + saturation_precision: Optional[Decimal] + _field_encodings = {"saturation_precision": "decimal_string"} + + +class HslColor(StripeObject): + model: Literal["hsl"] + + +class MagicLlama(StripeObject): + mana_cost: Optional[int] + _field_encodings = {"mana_cost": "int64_string"} + + +class Llama(StripeObject): + """ + Carries both union shapes the generator produces: a standalone `color` + union whose variants are separate classes, and an inline `magic_llama` + union whose discriminator lives on the parent and whose payload is a + plain inner class. + """ + + color: Union[RgbColor, HsvColor, HslColor] + magic_llama: Optional[MagicLlama] + name: str + type: Literal["earth_llama", "magic_llama"] + _inner_class_types = {"magic_llama": MagicLlama} + _inner_class_union_variant_types = { + "color": ( + "model", + {"rgb": RgbColor, "hsv": HsvColor, "hsl": HslColor}, + ), + } + + +def _llama(**values: Any) -> Llama: + return Llama.construct_from( + {"name": "kuzco", **values}, key="sk_test", api_mode="V2" + ) + + +# Copied from the generated `LlamaService.create` call site. The generator +# flattens every variant's fields into one map keyed by field name, so this +# single schema covers both `luminance` (rgb) and `saturation_precision` (hsv). +_COLOR_REQUEST_SCHEMA: Dict[str, Any] = { + "color": { + "luminance": "int64_string", + "saturation_precision": "decimal_string", + }, +} + + +# --------------------------------------------------------------------------- +# Response side — variant dispatch +# --------------------------------------------------------------------------- + + +class TestVariantDispatch: + """The discriminator selects the variant class, not the base.""" + + def test_dispatches_to_the_rgb_variant(self): + llama = _llama(color={"model": "rgb", "luminance": "1500"}) + assert isinstance(llama.color, RgbColor) + + def test_dispatches_to_the_hsv_variant(self): + llama = _llama(color={"model": "hsv", "saturation_precision": "0.125"}) + assert isinstance(llama.color, HsvColor) + + def test_dispatches_to_a_variant_with_no_payload_fields(self): + llama = _llama(color={"model": "hsl"}) + assert isinstance(llama.color, HslColor) + assert llama.color.model == "hsl" + + def test_the_variants_int64_encoding_applies(self): + llama = _llama(color={"model": "rgb", "luminance": "1500"}) + assert llama.color.luminance == 1500 + assert isinstance(llama.color.luminance, int) + + def test_the_variants_decimal_encoding_applies(self): + llama = _llama(color={"model": "hsv", "saturation_precision": "0.125"}) + assert llama.color.saturation_precision == Decimal("0.125") + assert isinstance(llama.color.saturation_precision, Decimal) + + def test_only_the_discriminator_decides_which_field_coerces(self): + """ + The sharpest statement of what dispatch buys: two payloads differing + in nothing but the discriminator coerce different fields, because each + variant class knows only its own encodings. + """ + payload = {"luminance": "1500", "saturation_precision": "0.125"} + + as_rgb = _llama(color={"model": "rgb", **payload}).color + assert as_rgb.luminance == 1500 + assert as_rgb.saturation_precision == "0.125" + + as_hsv = _llama(color={"model": "hsv", **payload}).color + assert as_hsv.luminance == "1500" + assert as_hsv.saturation_precision == Decimal("0.125") + + def test_the_discriminator_itself_is_readable_on_the_variant(self): + llama = _llama(color={"model": "rgb", "luminance": "1"}) + assert llama.color.model == "rgb" + assert llama.color["model"] == "rgb" + + +# --------------------------------------------------------------------------- +# Response side — fallback +# --------------------------------------------------------------------------- + + +class TestUnknownVariantFallback: + """ + A variant the API adds after this release must still deserialize. The + fallback is a plain StripeObject: readable, but with no encodings, since + the SDK has no idea what the new variant's fields mean. + """ + + def test_an_unknown_discriminator_falls_back(self): + llama = _llama(color={"model": "cmyk", "cyan": "1"}) + assert type(llama.color) is StripeObject + assert llama.color.model == "cmyk" + assert llama.color.cyan == "1" + + def test_an_absent_discriminator_falls_back(self): + llama = _llama(color={"luminance": "1500"}) + assert type(llama.color) is StripeObject + assert llama.color.luminance == "1500" + + def test_a_non_string_discriminator_falls_back(self): + llama = _llama(color={"model": 7}) + assert type(llama.color) is StripeObject + + def test_a_null_union_value_stays_none(self): + assert _llama(color=None).color is None + + def test_a_non_object_union_value_passes_through(self): + """ + Not a shape the API produces, but the lookup must not raise on it — + the union field is read before anything has validated its type. + """ + assert _llama(color="rgb").color == "rgb" + + +# --------------------------------------------------------------------------- +# Response side — inline unions are unaffected +# --------------------------------------------------------------------------- + + +class TestInlineUnionsUseInnerClassTypes: + """ + Inline union variants are namespaced by field name, so they need no + discriminator lookup and keep going through `_inner_class_types`. These + pin that the union lookup did not displace it. + """ + + def test_the_inline_variant_gets_its_inner_class(self): + llama = _llama(type="magic_llama", magic_llama={"mana_cost": "42"}) + assert isinstance(llama.magic_llama, MagicLlama) + + def test_the_inline_variants_encoding_applies(self): + llama = _llama(type="magic_llama", magic_llama={"mana_cost": "42"}) + assert llama.magic_llama.mana_cost == 42 + assert isinstance(llama.magic_llama.mana_cost, int) + + def test_the_non_selected_variant_is_not_fabricated(self): + llama = _llama(type="earth_llama") + assert llama.type == "earth_llama" + # `__getattr__` raises for a key absent from `_data`, so this is a + # real statement that nothing was materialized for the other variant. + assert not hasattr(llama, "magic_llama") + + +# --------------------------------------------------------------------------- +# Response side — serialization back out +# --------------------------------------------------------------------------- + + +class TestUnionValueSerialization: + def test_to_dict_recurses_into_the_variant(self): + llama = _llama(color={"model": "rgb", "luminance": "1500"}) + assert llama.to_dict()["color"] == { + "model": "rgb", + "luminance": 1500, + } + + def test_to_dict_for_json_restringifies_the_decimal(self): + """ + The variant hydrates `saturation_precision` to a Decimal, which is not + JSON-serializable, so `for_json` has to put the string back. + """ + llama = _llama(color={"model": "hsv", "saturation_precision": "0.125"}) + + plain = llama.to_dict()["color"]["saturation_precision"] + assert isinstance(plain, Decimal) + + for_json = llama.to_dict(for_json=True)["color"] + assert for_json["saturation_precision"] == "0.125" + assert isinstance(for_json["saturation_precision"], str) + + def test_to_dict_preserves_an_unknown_variant_verbatim(self): + llama = _llama(color={"model": "cmyk", "cyan": "1"}) + assert llama.to_dict()["color"] == {"model": "cmyk", "cyan": "1"} + + +# --------------------------------------------------------------------------- +# Request side +# --------------------------------------------------------------------------- + + +class TestUnionRequestCoercion: + """ + Outbound coercion runs off the method-level schema, which is keyed by + field name only — there is no discriminator in it. + """ + + def test_the_rgb_variants_int64_field_is_stringified(self): + result = _coerce_v2_params( + {"color": {"model": "rgb", "luminance": 1500}}, + _COLOR_REQUEST_SCHEMA, + ) + assert result == {"color": {"model": "rgb", "luminance": "1500"}} + + def test_the_hsv_variants_decimal_field_is_stringified(self): + result = _coerce_v2_params( + { + "color": { + "model": "hsv", + "saturation_precision": Decimal("0.125"), + } + }, + _COLOR_REQUEST_SCHEMA, + ) + assert result == { + "color": {"model": "hsv", "saturation_precision": "0.125"} + } + + def test_a_payload_free_variant_passes_through_untouched(self): + result = _coerce_v2_params( + {"color": {"model": "hsl"}}, _COLOR_REQUEST_SCHEMA + ) + assert result == {"color": {"model": "hsl"}} + + def test_coercion_is_by_field_name_not_by_variant(self): + """ + Pins the generator's flattening decision: every variant's fields land + in one map, so a field is coerced whenever it appears, whatever the + discriminator says. Safe while variants do not share a field name with + conflicting encodings. + """ + result = _coerce_v2_params( + { + "color": { + "model": "rgb", + "saturation_precision": Decimal("0.5"), + } + }, + _COLOR_REQUEST_SCHEMA, + ) + assert result == { + "color": {"model": "rgb", "saturation_precision": "0.5"} + } + + def test_unknown_variant_fields_pass_through(self): + result = _coerce_v2_params( + {"color": {"model": "cmyk", "cyan": 1}}, + _COLOR_REQUEST_SCHEMA, + ) + assert result == {"color": {"model": "cmyk", "cyan": 1}} + + def test_a_null_union_is_not_coerced(self): + result = _coerce_v2_params({"color": None}, _COLOR_REQUEST_SCHEMA) + assert result == {"color": None} From 9f0feff4fcccfe8b3c91495a234e22029451cbc5 Mon Sep 17 00:00:00 2001 From: jar-stripe Date: Fri, 28 Aug 2026 10:48:52 -0700 Subject: [PATCH 2/6] fix: show ruff parse errors on format failure (#1837) Replace --quiet with stdout redirection. Ruff's --quiet flag suppresses all output including parse errors (e.g. merge markers), making format failures in CI impossible to diagnose. Redirecting stdout to /dev/null suppresses the file list but lets errors (which go to stderr) through. Committed-By-Agent: claude Co-authored-by: Claude Sonnet 4.6 --- justfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/justfile b/justfile index a56d62314..c9a7a6800 100644 --- a/justfile +++ b/justfile @@ -35,11 +35,11 @@ typecheck: install-test-deps install-dev-deps # ⭐ format all code format: install-dev-deps - ruff format . --quiet + ruff format . > /dev/null # verify formatting, but don't modify files format-check: install-dev-deps - ruff format . --check --quiet + ruff format . --check > /dev/null # remove venv & build artifacts clean: From c0282c71110a39a3ad598fa774839913243db43e Mon Sep 17 00:00:00 2001 From: jar-stripe Date: Fri, 28 Aug 2026 16:01:08 -0700 Subject: [PATCH 3/6] Carry private-preview's CI workflow branch list on master (#1897) private-preview's ci.yml differs from master's by one additive hunk: `private-preview` in `on.push.branches`. Because that hunk lives only on private-preview, every merge of master into private-preview yields a workflow blob matching neither parent. GitHub refuses a push from a GitHub App lacking `workflows` permission when it introduces a workflow blob that does not already exist in the repository, so the codegen repo's Codegen job's push to latest-codegen-private-preview is rejected and a human has to perform the merge by hand. Holding the hunk on master too means both sides of the merge carry the same change, the merge result is byte-identical to master's blob, and the App only ever carries an already-committed file forward. The hunk is a no-op on master. For a push event the workflow file comes from the pushed ref, so master's copy listing private-preview is never consulted for a push to private-preview, and it cannot affect pushes to master or beta. `on.pull_request.branches` already lists private-preview. Committed-By-Agent: claude Co-authored-by: Claude Opus 5 --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2407e5b55..f59e56c82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,7 @@ on: branches: - master - beta + - private-preview - sdk-release/** - feature/** tags: From 94f8c70678d10530576aa43c035f86c7c8025efe Mon Sep 17 00:00:00 2001 From: zacchua-stripe Date: Mon, 31 Aug 2026 11:40:24 -0700 Subject: [PATCH 4/6] Add open vs closed enum section to README (#1877) * Add open vs closed enum section to README * Put type sig in code block --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index f6929e52c..8b40fd997 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,17 @@ sends by default. If you are overriding `stripe.api_version` / `stripe_version` [webhook endpoint](https://stripe.com/docs/webhooks#api-versions) tied to an older version, be aware that the data you see at runtime may not match the types. +### Open and Closed Enums + +Many of Stripe API enums are open, meaning Stripe may add new values even on older API versions. +To reflect this, open enum fields are typed as `Union[Literal[...], str]` rather than a plain `Literal[...]`. +This ensures the field has the correct type for both values known at SDK release time and other values that may be added later. + +A small number of enums are closed, meaning Stripe guarantees no new values will be added without an API version change. + +Refer to the [API Reference](https://docs.stripe.com) for the latest set of allowed values. + + ### Public Preview SDKs Stripe has features in the [public preview phase](https://docs.stripe.com/release-phases) that can be accessed via versions of this package that have the `bX` suffix like `12.2.0b2`. From 5231afbb0349542b384becceb5dfa6744917d809 Mon Sep 17 00:00:00 2001 From: David Brownman <1231935+xavdid@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:53:20 -0700 Subject: [PATCH 5/6] Use cryptographically secure boundaries for multipart file uploads (#1898) * swap to a secure multipart boundary * use monkeypatch insteado of bare assignment * remove unneded comments --- stripe/_multipart_data_generator.py | 30 +++++++++++---- tests/api_resources/test_file.py | 12 ++++-- tests/api_resources/test_file_upload.py | 12 ++++-- tests/services/test_file_upload.py | 9 ++++- tests/test_multipart_data_generator.py | 49 +++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 15 deletions(-) diff --git a/stripe/_multipart_data_generator.py b/stripe/_multipart_data_generator.py index 3151df83e..ec91d22b4 100644 --- a/stripe/_multipart_data_generator.py +++ b/stripe/_multipart_data_generator.py @@ -1,13 +1,29 @@ -import random import io +import secrets from stripe._encode import _api_encode +# Number of random bytes used to build the boundary, matching stripe-ruby's +# `SecureRandom.hex(30)` and Go's mime/multipart. This must come from a CSPRNG: +# multipart/form-data is only safe if the delimiter cannot be guessed by anyone +# able to influence the content, since a value containing the delimiter gets +# parsed as additional parts. +BOUNDARY_BYTES = 30 + + +def _escape_header_value(value: str) -> str: + """Make a value safe to interpolate into a part header. + + An unescaped quote would end the quoted-string early, and CR/LF would + introduce additional header lines or parts. + """ + return value.replace('"', "%22").replace("\r", " ").replace("\n", " ") + class MultipartDataGenerator(object): data: io.BytesIO line_break: str - boundary: int + boundary: str chunk_size: int def __init__(self, chunk_size: int = 1028): @@ -36,9 +52,9 @@ def add_params(self, params): filename = str(value.name) self._write('Content-Disposition: form-data; name="') - self._write(key) + self._write(_escape_header_value(key)) self._write('"; filename="') - self._write(filename) + self._write(_escape_header_value(filename)) self._write('"') self._write(self.line_break) self._write("Content-Type: application/octet-stream") @@ -48,7 +64,7 @@ def add_params(self, params): self._write_file(value) else: self._write('Content-Disposition: form-data; name="') - self._write(key) + self._write(_escape_header_value(key)) self._write('"') self._write(self.line_break) self._write(self.line_break) @@ -83,5 +99,5 @@ def _write_file(self, f): break self._write(file_contents) - def _initialize_boundary(self): - return random.randint(0, 2**63) + def _initialize_boundary(self) -> str: + return secrets.token_hex(BOUNDARY_BYTES) diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index 4494f543e..fe7037961 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -32,8 +32,14 @@ def test_is_retrievable(self, http_client_mock): ) assert isinstance(resource, stripe.File) - def test_is_creatable(self, setup_upload_api_base, http_client_mock): - MultipartDataGenerator._initialize_boundary = lambda self: 1234567890 + def test_is_creatable( + self, setup_upload_api_base, http_client_mock, monkeypatch + ): + monkeypatch.setattr( + MultipartDataGenerator, + "_initialize_boundary", + lambda self: "abc123", + ) test_file = tempfile.TemporaryFile() resource = stripe.File.create( purpose="dispute_evidence", @@ -44,7 +50,7 @@ def test_is_creatable(self, setup_upload_api_base, http_client_mock): "post", path="/v1/files", api_base=stripe.upload_api_base, - content_type="multipart/form-data; boundary=1234567890", + content_type="multipart/form-data; boundary=abc123", ) assert isinstance(resource, stripe.File) diff --git a/tests/api_resources/test_file_upload.py b/tests/api_resources/test_file_upload.py index 8e52cf103..a2896a378 100644 --- a/tests/api_resources/test_file_upload.py +++ b/tests/api_resources/test_file_upload.py @@ -33,8 +33,14 @@ def test_is_retrievable(self, http_client_mock): ) assert isinstance(resource, File) - def test_is_creatable(self, setup_upload_api_base, http_client_mock): - MultipartDataGenerator._initialize_boundary = lambda self: 1234567890 + def test_is_creatable( + self, setup_upload_api_base, http_client_mock, monkeypatch + ): + monkeypatch.setattr( + MultipartDataGenerator, + "_initialize_boundary", + lambda self: "abc123", + ) test_file = tempfile.TemporaryFile() resource = File.create( purpose="dispute_evidence", @@ -45,7 +51,7 @@ def test_is_creatable(self, setup_upload_api_base, http_client_mock): "post", api_base=stripe.upload_api_base, path="/v1/files", - content_type="multipart/form-data; boundary=1234567890", + content_type="multipart/form-data; boundary=abc123", ) assert isinstance(resource, File) diff --git a/tests/services/test_file_upload.py b/tests/services/test_file_upload.py index cae1c79b7..fa671c1ff 100644 --- a/tests/services/test_file_upload.py +++ b/tests/services/test_file_upload.py @@ -28,8 +28,13 @@ def test_is_creatable( self, file_stripe_mock_stripe_client, http_client_mock, + monkeypatch, ): - MultipartDataGenerator._initialize_boundary = lambda self: 1234567890 + monkeypatch.setattr( + MultipartDataGenerator, + "_initialize_boundary", + lambda self: "abc123", + ) test_file = tempfile.TemporaryFile() # We create a new client here instead of re-using the stripe_mock_stripe_client fixture @@ -46,6 +51,6 @@ def test_is_creatable( "post", api_base=stripe.upload_api_base, path="/v1/files", - content_type="multipart/form-data; boundary=1234567890", + content_type="multipart/form-data; boundary=abc123", ) assert isinstance(resource, File) diff --git a/tests/test_multipart_data_generator.py b/tests/test_multipart_data_generator.py index b8f4fa738..0b9d5f93e 100644 --- a/tests/test_multipart_data_generator.py +++ b/tests/test_multipart_data_generator.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- +import random import re import io @@ -87,3 +88,51 @@ def test_multipart_data_unicode_file_name(self): string = io.StringIO("foo") string.name = "паспорт.png" self.run_test_multipart_data_with_file(string) + + def test_boundary_is_not_derived_from_the_random_module(self): + # Seeding the `random` module must not determine the boundary. A + # boundary an attacker can predict lets a caller-influenced value + # (including file bytes) inject additional parts. + random.seed(0) + first = MultipartDataGenerator().boundary + random.seed(0) + second = MultipartDataGenerator().boundary + + assert first != second + assert re.fullmatch(r"[0-9a-f]{60}", first) + assert re.fullmatch(r"[0-9a-f]{60}", second) + + @staticmethod + def lines_starting_with(http_body, prefix): + return [ + line for line in http_body.split("\r\n") if line.startswith(prefix) + ] + + def test_escapes_quotes_and_crlf_in_param_names(self): + injected = 'a\r\nContent-Disposition: form-data; name="purpose' + generator = MultipartDataGenerator() + generator.add_params({injected: "value"}) + http_body = generator.get_post_data().decode("utf-8") + + # The injected CRLF must not begin a second header line, and the + # injected quote must not end the quoted-string early. + assert self.lines_starting_with(http_body, "Content-Disposition:") == [ + 'Content-Disposition: form-data; name="a Content-Disposition: ' + 'form-data; name=%22purpose"' + ] + # One opening delimiter and one closing delimiter: a single part. + assert http_body.count("--%s" % generator.boundary) == 2 + + def test_escapes_quotes_and_crlf_in_file_names(self): + test_file = io.StringIO("foo") + test_file.name = 'a\r\nX-Injected: yes"b.png' + generator = MultipartDataGenerator() + generator.add_params({"file": test_file}) + http_body = generator.get_post_data().decode("utf-8") + + assert self.lines_starting_with(http_body, "Content-Disposition:") == [ + 'Content-Disposition: form-data; name="file"; ' + 'filename="a X-Injected: yes%22b.png"' + ] + assert self.lines_starting_with(http_body, "X-Injected:") == [] + assert http_body.count("--%s" % generator.boundary) == 2 From a9e3979905054fa462c03a0584d3140ba93d8a2d Mon Sep 17 00:00:00 2001 From: David Brownman <1231935+xavdid@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:31:56 -0700 Subject: [PATCH 6/6] Harden API requestor code against malicious URLs (#1896) * validate that incoming urls don't redirect requests * shorten comments --- stripe/_api_requestor.py | 2 + stripe/_util.py | 38 +++++++++- stripe/v2/core/_event.py | 6 +- tests/test_api_requestor.py | 146 ++++++++++++++++++++++++++++++++++-- 4 files changed, 178 insertions(+), 14 deletions(-) diff --git a/stripe/_api_requestor.py b/stripe/_api_requestor.py index 96f5e3c9b..e0f22186d 100644 --- a/stripe/_api_requestor.py +++ b/stripe/_api_requestor.py @@ -29,6 +29,7 @@ log_debug, log_info, dashboard_link, + validate_path, _convert_to_stripe_object, get_api_mode, ) @@ -618,6 +619,7 @@ def _args_for_request_with_retries( "questions." ) + validate_path(url) abs_url = "%s%s" % ( self._options.base_addresses.get(base_address), url, diff --git a/stripe/_util.py b/stripe/_util.py index 386894080..8b378c63c 100644 --- a/stripe/_util.py +++ b/stripe/_util.py @@ -7,7 +7,7 @@ from stripe._api_mode import ApiMode -from urllib.parse import quote_plus +from urllib.parse import quote_plus, urlsplit from typing_extensions import Type, TYPE_CHECKING from typing import ( @@ -260,8 +260,12 @@ def _convert_to_stripe_object( klass = get_object_class(api_mode, klass_name) # TODO: this is a horrible hack. The API needs # to return something for `object` here. - - elif "data" in resp and "next_page_url" in resp: + # + # Gated on V2: this runs recursively over every nested value, so without + # the mode check any nested map in a v1 payload carrying `data` and + # `next_page_url` becomes an auto-paginating v2 collection. A malicious webhook + # could potentially choose the host of a subsequent authenticated request. + elif api_mode == "V2" and "data" in resp and "next_page_url" in resp: klass = stripe.v2.ListObject elif klass_ is not None: klass = klass_ @@ -346,6 +350,34 @@ def sanitize_id(id): return quotedId +def validate_path(path: str) -> None: + """ + Assert that a request path is origin-relative: that it begins with a single + "/" and carries no scheme, authority or userinfo. + + The absolute URL is built by concatenating a base address onto this path, and + no base address ends in a slash. A path like "@evil.example/v1/x" or + ".evil.example/v1/x" would modify the resulting host and direct the request + (including the API key) to a non-Stripe host. + + Because some relative urls arrive from potentially untrusted sources (like + webhook bodies), we have to be a little defensive. + + So, we require that a path starts with a leading slash and that urlsplit + finds no scheme or authority in it. + """ + if not path.startswith("/") or path.startswith("//"): + raise ValueError( + f'Request path must be a string beginning with a single "/", got: {path!r}' + ) + + parts = urlsplit(path) + if parts.scheme or parts.netloc: + raise ValueError( + f"Request path may not contain a scheme or authority, got: {path!r}" + ) + + def get_api_mode(url: str) -> ApiMode: if url.startswith("/v2"): return "V2" diff --git a/stripe/v2/core/_event.py b/stripe/v2/core/_event.py index 9f054f2fc..fe014db67 100644 --- a/stripe/v2/core/_event.py +++ b/stripe/v2/core/_event.py @@ -8,7 +8,7 @@ from typing_extensions import Literal, TYPE_CHECKING from stripe._stripe_object import StripeObject, UntypedStripeObject -from stripe._util import get_api_mode +from stripe._util import get_api_mode, sanitize_id from stripe._stripe_context import StripeContext from stripe._webhook import WebhookPayload @@ -219,7 +219,7 @@ def __repr__(self) -> str: def fetch_event(self) -> Event: response = self._client.raw_request( "get", - f"/v2/core/events/{self.id}", + f"/v2/core/events/{sanitize_id(self.id)}", stripe_context=self.context, headers={"Stripe-Request-Trigger": f"event={self.id}"}, usage=["pushed_event_pull"], @@ -229,7 +229,7 @@ def fetch_event(self) -> Event: async def fetch_event_async(self) -> Event: response = await self._client.raw_request_async( "get", - f"/v2/core/events/{self.id}", + f"/v2/core/events/{sanitize_id(self.id)}", stripe_context=self.context, headers={"Stripe-Request-Trigger": f"event={self.id}"}, usage=["pushed_event_pull", "pushed_event_pull_async"], diff --git a/tests/test_api_requestor.py b/tests/test_api_requestor.py index 84deff6b9..c49803c46 100644 --- a/tests/test_api_requestor.py +++ b/tests/test_api_requestor.py @@ -23,8 +23,13 @@ StripeStreamResponse, StripeStreamResponseAsync, ) +from stripe._util import ( + validate_path, + _convert_to_stripe_object, +) from stripe.v2._deleted_object import DeletedObject from tests.http_client_mock import HTTPClientMock +from tests.test_webhook import generate_header VALID_API_METHODS = ("get", "post", "delete") @@ -175,10 +180,16 @@ def test_param_encoding(self, requestor, http_client_mock): urlencode(expectation).replace("%5B", "[").replace("%5D", "]") ) http_client_mock.stub_request( - "get", query_string=query_string, rbody="{}", rcode=200 + "get", + path=self.v1_path, + query_string=query_string, + rbody="{}", + rcode=200, ) - requestor.request("get", "", self.ENCODE_INPUTS, base_address="api") + requestor.request( + "get", self.v1_path, self.ENCODE_INPUTS, base_address="api" + ) http_client_mock.assert_requested("get", query_string=query_string) @@ -247,10 +258,11 @@ def test_ordereddict_encoding(self): assert encoded[4][0] == "ordered[nested][b]" def test_url_construction(self, requestor, http_client_mock): + # Paths must be origin-relative -- see validate_path. CASES = ( - (f"{stripe.api_base}?foo=bar", "", {"foo": "bar"}), - (f"{stripe.api_base}?foo=bar", "?", {"foo": "bar"}), - (stripe.api_base, "", {}), + (f"{stripe.api_base}/v1/foo?foo=bar", "/v1/foo", {"foo": "bar"}), + (f"{stripe.api_base}/v1/foo?foo=bar", "/v1/foo?", {"foo": "bar"}), + (f"{stripe.api_base}/v1/foo", "/v1/foo", {}), ( f"{stripe.api_base}/%20spaced?baz=5&foo=bar%24", "/%20spaced?foo=bar%24", @@ -258,8 +270,8 @@ def test_url_construction(self, requestor, http_client_mock): ), # duplicate query params keys should be deduped ( - f"{stripe.api_base}?foo=bar", - "?foo=bar", + f"{stripe.api_base}/v1/foo?foo=bar", + "/v1/foo?foo=bar", {"foo": "bar"}, ), ) @@ -982,7 +994,7 @@ def test_invalid_json(self, requestor, http_client_mock): def test_invalid_method(self, requestor): with pytest.raises(stripe.APIConnectionError): - requestor.request("foo", "bar", base_address="api") + requestor.request("foo", self.v1_path, base_address="api") def test_oauth_invalid_requestor_error(self, requestor, http_client_mock): http_client_mock.stub_request( @@ -1135,6 +1147,124 @@ def test_raw_request_with_file_param(self, requestor, http_client_mock): ) assert supplied_headers["Content-Type"] == "multipart/form-data" + ORIGIN_RELATIVE_PATHS = [ + "/v1/customers/cus_123", + "/v1/customers", + "/v2/core/accounts?page=page_123&limit=2", + # "@" is legal inside a path or query string -- it only opens an + # authority when it precedes the first "/". + "/v1/customers?email=user%40example.com", + "/v1/invoices/in_123@456", + # A backslash does not open an authority: the "/" already closed it. + "/v1/\\evil.example", + ] + + HOSTILE_PATHS = [ + # Concatenated onto a base address with no trailing slash, each of these + # moves the request's authority off api.stripe.com. + "@evil.example/v1/leak", + ":pw@evil.example/v1/leak", + ":80@evil.example/v1/leak", + # Extends the host into an attacker-owned subdomain + # (api.stripe.com.evil.example), which has a valid certificate. + ".evil.example/v1/leak", + "-evil.example/v1/leak", + "https://evil.example/v1/leak", + "//evil.example/v1/leak", + "", + "v1/customers", + ] + + @pytest.mark.parametrize("path", ORIGIN_RELATIVE_PATHS) + def test_accepts_origin_relative_path(self, path): + validate_path(path) + + @pytest.mark.parametrize("path", HOSTILE_PATHS) + def test_rejects_hostile_path(self, path): + with pytest.raises(ValueError): + validate_path(path) + + @pytest.mark.parametrize("path", HOSTILE_PATHS) + def test_request_rejects_hostile_path_without_issuing_request( + self, path, requestor, http_client_mock + ): + with pytest.raises(ValueError): + requestor.request("get", path, base_address="api") + + http_client_mock.assert_no_request() + + def test_raw_request_rejects_hostile_path_without_issuing_request( + self, http_client_mock + ): + client = stripe.StripeClient( + "sk_test_123", http_client=http_client_mock.get_mock_http_client() + ) + + with pytest.raises(ValueError): + client.raw_request("get", "@evil.example/v1/leak") + + http_client_mock.assert_no_request() + + def test_fetch_related_object_rejects_hostile_url_without_issuing_request( + self, http_client_mock + ): + client = stripe.StripeClient( + "sk_test_123", http_client=http_client_mock.get_mock_http_client() + ) + payload = json.dumps( + { + "id": "evt_123", + "object": "v2.core.event", + "type": "v2.core.account.created", + "created": "2026-01-01T00:00:00Z", + "related_object": { + "id": "acct_123", + "type": "account", + "url": "@evil.example/v1/leak", + }, + } + ) + secret = "whsec_test_secret" + header = generate_header(payload=payload, secret=secret) + + notification = client.parse_event_notification(payload, header, secret) + + with pytest.raises(ValueError): + notification.fetch_related_object() + + http_client_mock.assert_no_request() + + def test_v1_payload_does_not_produce_v2_list_object(self, requestor): + # A signature-verified v1 webhook body is attacker-shaped. Without the + # api_mode gate, `lines` here became an auto-paginating v2 collection + # whose next_page_url chose the host of the next authenticated request. + obj = _convert_to_stripe_object( + resp={ + "id": "in_123", + "object": "invoice", + "lines": { + "data": [{"id": "il_123"}], + "next_page_url": "@evil.example/v1/leak", + }, + }, + requestor=requestor, + api_mode="V1", + ) + + assert not isinstance(obj["lines"], stripe.v2.ListObject) + + def test_v2_response_still_produces_v2_list_object(self, requestor): + obj = _convert_to_stripe_object( + resp={ + "data": [{"id": "acct_123"}], + "next_page_url": "/v2/core/accounts?page=page_123", + }, + requestor=requestor, + api_mode="V2", + ) + + assert isinstance(obj, stripe.v2.ListObject) + class TestDefaultClient(object): @pytest.fixture(autouse=True)