Skip to content

feat: require environment_subdomain or an explicit use_legacy_domain opt-out - #226

Open
armando-rodriguez-cko wants to merge 5 commits into
mainfrom
feat/INT-1688-mandatory-subdomain
Open

feat: require environment_subdomain or an explicit use_legacy_domain opt-out#226
armando-rodriguez-cko wants to merge 5 commits into
mainfrom
feat/INT-1688-mandatory-subdomain

Conversation

@armando-rodriguez-cko

@armando-rodriguez-cko armando-rodriguez-cko commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes the merchant-specific subdomain (MSSD) mandatory. Callers must now call environment_subdomain(...), or explicitly opt out with the new use_legacy_domain(), which raises a DeprecationWarning from its first release. Setting both, or neither, raises CheckoutArgumentException at build time. MSSD is no longer beta and non-MSSD usage will be deprecated, so the previous silent fallback to api.checkout.com had to go.

Changes

  • checkout_sdk/checkout_sdk_builder.py — the subdomain is held as a string and the EnvironmentSubdomain is built when the configuration is assembled, so environment_subdomain() no longer has to be called after environment(); new use_legacy_domain(); new _validate_environment_settings() and _requires_environment_subdomain()
  • checkout_sdk/environment_subdomain.pycreate_url_with_subdomain raises on an invalid subdomain instead of returning the URL unchanged
  • checkout_sdk/default_sdk.py, oauth_sdk.py, previous/previous_sdk.py — validate before building; Previous/ABC exempted; the duplicated with/without-subdomain branches in all three build() methods are gone
  • tests/checkout_default_sdk_test.py — covers all four combinations plus an invalid subdomain
  • tests/checkout_configuration_test.py — the parameterised bad-subdomain case now asserts the raise instead of the silent fallback
  • tests/conftest.py + five other fixtures — every client the suite builds now chooses a domain, through configure_domain

Fixed along the way

environment_subdomain() used to build the URLs from whatever environment was set at call time, so calling it before environment() silently produced the wrong host.

The long import line in tests/payments/request_apm_payments_integration_test.py is wrapped only because pre-commit lints staged files, so touching that file surfaced a pre-existing line-too-long.

Verification

544 tests passing, 0 failing, 217 skipped. flake8 and pylint pre-commit hooks pass.

API Reference

Breaking changes

Yes, two. This needs a major release, classified and versioned when the release is cut.

  1. The merchant-specific subdomain is mandatory for the Default and DefaultOAuth platforms. Code that omitted it and relied on the implicit fallback to api.checkout.com / access.checkout.com now fails at client construction. Migration: set the subdomain, or use the legacy-domain opt-out as a temporary measure. The Previous (ABC) platform is unaffected.
  2. An invalid subdomain now fails instead of being silently ignored. Callers passing a malformed value keep working against the shared host today; after this change they fail fast. This one is easy to miss because it is not what the ticket asked for, so it needs its own line in the release notes.

README

Updated in this PR: a "Subdomain value" section above the Default example, the subdomain added to the configuration samples, and a "Legacy domain (emergency use only)" section at the bottom.

Notes

The suite routes every client it builds through a single helper that uses the shared hosts. Applying the merchant-specific subdomain there looked better, since it is the path merchants are being moved to, but the sandbox OAuth clients are not provisioned for it: .NET CI failed 224 integration tests with invalid_client when the token request went to {subdomain}.access.sandbox.checkout.com. Binding those OAuth clients to the subdomain is a platform task and should land before merchants are told the subdomain is mandatory.

Reference implementation: checkout-sdk-net#590. Tracked as INT-1688.

No version bump here: that happens on master when the release is cut, per the release workflow.

…opt-out

The merchant-specific subdomain is how merchants should reach the API, but it
was optional and an unset value silently fell back to api.checkout.com, so a
forgotten subdomain looked exactly like a deliberate opt-out and the SDK could
not warn about either. Callers must now choose: set environment_subdomain, or
call use_legacy_domain(), which raises a DeprecationWarning from its first
release. Both, or neither, raises CheckoutArgumentException.

An invalid subdomain now raises instead of being quietly ignored, which is a
second breaking change: callers passing a malformed value are currently served
by the shared host and never find out.

environment_subdomain no longer needs environment() to be set first, since the
EnvironmentSubdomain is now built when the configuration is assembled. That also
removed the duplicated with/without-subdomain branches in all three build()
methods.

The Previous (ABC) platform predates merchant-specific subdomains and stays
exempt via _requires_environment_subdomain().

Fixtures route clients through conftest.configure_domain, which uses the shared
hosts: the sandbox OAuth clients are not provisioned for the subdomain, so
applying it makes every client_credentials request return invalid_client.

The long import line in the APM test is wrapped only because pre-commit lints
staged files, so touching that file surfaced a pre-existing violation.

Mirrors checkout-sdk-net#590. Refs INT-1688.
@agent-wall-e

agent-wall-e Bot commented Aug 10, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:checkout_sdk/oauth_sdk.py
  • security_sensitive_path:tests/oauth_integration_test.py

Operational gates

  • ✅ jira_ticket (INT-1688)
  • ✅ independent_review

Files analysed: 15


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Aug 10, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_pathcheckout_sdk/oauth_sdk.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathtests/oauth_integration_test.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Aug 10, 2026

Copy link
Copy Markdown

🔵 Advisory review: Sound, but needs your judgement

This PR needs a human approval. The code itself reads as correct; whether it should land depends on context I don't have.

This PR enforces that Default and OAuth SDK callers must now explicitly set either environment_subdomain or use_legacy_domain at build time, removing the silent fallback to shared hosts. The implementation is technically correct and internally consistent, but the breaking-change severity and the acknowledged gap (sandbox OAuth clients not yet provisioned for MSSD, forcing every integration test fixture to opt into the deprecated path) are business/deployment decisions a human must weigh.

For you to decide

  • The PR description explicitly acknowledges that sandbox OAuth clients are not provisioned for MSSD, meaning every integration test fixture must use use_legacy_domain() — this creates a situation where the 'deprecated emergency fallback' is the only working path for tests, which undermines the signal the deprecation warning is meant to send.
  • The _environment_subdomain property is evaluated lazily at build time (once in oauth_sdk.py via environment_subdomain = self._environment_subdomain), so _validate_environment_settings is called and then _environment_subdomain is called a second time independently — this is correct but slightly fragile since the property constructs a new EnvironmentSubdomain object each call; no bug today, but worth noting.
  • In oauth_sdk.py, _validate_environment_settings() is called before environment_subdomain is assigned to the local variable; if _subdomain is non-None and valid, EnvironmentSubdomain.__init__ will be called once during validation (via the property) and once when assigned — EnvironmentSubdomain.__init__ itself calls create_url_with_subdomain which can raise, but validation already guards this, so no double-raise risk; still, the property is called twice per build() invocation.
  • The PreviousSdk._validate_environment_settings() is now called in build() with _requires_environment_subdomain() returning False, so for Previous: neither subdomain nor legacy domain is required, but if a caller sets both, the 'cannot both be set' exception still fires — this is the correct and documented behaviour, just worth a reviewer confirming it is intentional.
  • The README 'Legacy domain' section says 'Exactly one of environment_subdomain(...) or use_legacy_domain() must be set' but does not caveat the Previous platform exception in that same sentence, which could mislead Previous SDK users reading that section.
  • The decision of whether to ship a mandatory-MSSD breaking change before sandbox OAuth infrastructure is ready (forcing all test fixtures onto the deprecated path) is a product/platform timing call that cannot be resolved from the diff alone.

This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02

Flagged in review: the test called use_legacy_domain() directly, so it emitted
the deprecation warning on every run, inconsistent with every other fixture.
It now goes through conftest.configure_domain like the rest.
@agent-wall-e

agent-wall-e Bot commented Aug 11, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:checkout_sdk/oauth_sdk.py
  • security_sensitive_path:tests/oauth_integration_test.py

Operational gates

  • ✅ jira_ticket (INT-1688)
  • ✅ independent_review

Files analysed: 15


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Aug 11, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_pathcheckout_sdk/oauth_sdk.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathtests/oauth_integration_test.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

The suite could only run against the shared hosts, so the subdomain path this PR
makes mandatory had no integration coverage. Reviewers flagged that on every SDK,
and it is the right thing to flag.

The domain helper now has two modes. Default is unchanged, the shared hosts,
because the sandbox OAuth clients are not provisioned for the subdomain and the
token request returns invalid_client. Set CHECKOUT_TEST_USE_SUBDOMAIN=true and the
suite runs against CHECKOUT_MERCHANT_SUBDOMAIN instead, so once sandbox is
provisioned like production it is a one-line change in the workflows, already
wired and documented, rather than a rewrite of every fixture.

The switch is deliberately separate from CHECKOUT_MERCHANT_SUBDOMAIN, which CI
already exports: provisioning should drive the behaviour, not the presence of a
secret.
@agent-wall-e

agent-wall-e Bot commented Aug 11, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:.github/workflows/build-main.yml
  • security_sensitive_path:.github/workflows/build-pull-request.yml
  • security_sensitive_path:.github/workflows/build-release.yml
  • security_sensitive_path:checkout_sdk/oauth_sdk.py
  • security_sensitive_path:tests/oauth_integration_test.py

Operational gates

  • ✅ jira_ticket (INT-1688)
  • ✅ independent_review

Files analysed: 18


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Aug 11, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_path.github/workflows/build-main.yml classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path.github/workflows/build-pull-request.yml classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path.github/workflows/build-release.yml classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathcheckout_sdk/oauth_sdk.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathtests/oauth_integration_test.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

Versions are bumped on master during the release, not in a feature branch, per
the release workflow. This branch should carry only the change itself; the major
bump is classified and applied when the release is cut.
@agent-wall-e

agent-wall-e Bot commented Aug 12, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:.github/workflows/build-main.yml
  • security_sensitive_path:.github/workflows/build-pull-request.yml
  • security_sensitive_path:.github/workflows/build-release.yml
  • security_sensitive_path:checkout_sdk/oauth_sdk.py
  • security_sensitive_path:tests/oauth_integration_test.py

Operational gates

  • ✅ jira_ticket (INT-1688)
  • ✅ independent_review

Files analysed: 17


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Aug 12, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_path.github/workflows/build-main.yml classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path.github/workflows/build-pull-request.yml classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_path.github/workflows/build-release.yml classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathcheckout_sdk/oauth_sdk.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathtests/oauth_integration_test.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

Two problems with the previous approach. It needed a new variable in 21 workflow
files, which is not viable without access to create secrets. And it wrapped the
builder chain in a configureDomain helper that is not part of the public API, so
the tests stopped looking like the code a merchant would actually write.

Every fixture now calls the real opt-out inline, in the chain, with a comment
saying why: the sandbox OAuth clients are not provisioned for the merchant-specific
subdomain, so the token request comes back invalid_client. When sandbox is
provisioned, those calls become the subdomain setter.

The unit tests covering all four combinations are untouched: they already used the
public API directly.
@agent-wall-e

agent-wall-e Bot commented Aug 12, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • security_sensitive_path:checkout_sdk/oauth_sdk.py
  • security_sensitive_path:tests/oauth_integration_test.py

Operational gates

  • ✅ jira_ticket (INT-1688)
  • ✅ independent_review

Files analysed: 14


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Aug 12, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
security_sensitive_pathcheckout_sdk/oauth_sdk.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathtests/oauth_integration_test.py classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant