You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR needs a human approval. Before you give it, these are the things I'd want resolved.
This PR updates Risk SDK to 3.0.1, raises minSdk to 23, adds validated baseUrlPrefix forwarding as the mssd param, and adds ProGuard rules. The logic is mostly correct, but there is a gap in validation coverage: the baseUrlPrefix passed to RiskSdkUseCase (and thus RiskInstanceProvider) bypasses BaseUrlPrefixValidator when called from CheckoutApiServiceFactory, but the RiskInstanceProvider test feeds already-valid prefixes directly, masking whether invalid values could reach Risk.
Concerns
In CheckoutApiServiceFactory.create, validatedBaseUrlPrefix is correctly passed to provideTokenRepository, but on line ~97 the baseUrlPrefix passed into RiskSdkUseCase (via the second provideRiskSdkUseCase call) uses the raw baseUrlPrefix parameter rather than validatedBaseUrlPrefix — meaning invalid prefixes are NOT sanitized before reaching Risk.
The RiskInstanceProvider and RiskSdkUseCase tests only exercise valid prefixes and null; there is no test verifying that an invalid prefix (e.g. '-bad', 'a.b') fed through CheckoutApiServiceFactory does not reach RiskInstanceProvider.provide with the invalid value.
The EnvironmentExtension.kt change removes the local baseUrlPrefixValidator call and now trusts that callers pass already-validated input — this is fine for toBaseUrl but only holds if all call sites validate first; the Risk path currently does not.
The consumer-rules.pro file still lacks a trailing newline, which is a minor file hygiene issue but not a blocker.
The regex ^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?$ correctly implements the stated 1–63 character, no-leading/trailing-hyphen rule, including accepting single-character labels.
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
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Risk caching ignores prefix changes, new checked exceptions are undeclared for Java callers, and empty-prefix validation breaks existing default-host configurations.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The process-wide Risk cache ignores subsequent prefix changes, causing later clients to publish Risk data to the wrong host, while the null-stubbed test bypasses this behavior.
For rejected prefixes such as -merchant, merchant-, or é, this message omits the rule they violate. Consider including the ASCII-only and start/end restrictions so integrators can correct the configuration, matching the public KDoc.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The process-wide Risk cache ignores subsequent prefix changes, causing later clients to publish Risk data to the wrong endpoint, while the null-returning test misses this cached-instance behavior.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The breaking minSdk 21→23 increase and major Risk SDK upgrade require human release sign-off and minified API 23 validation without optional Google Play services classes, which was unavailable here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
PIMOB-5427
Proposed changes
baseUrlPrefixas themssdparam.Minor supporting changes: fixed a coroutine test race, removed obsolete API checks, and updated README SDK requirements.
Test Steps
Kotlin w/ mssd:
Screen.Recording.2026-11-12.at.10.03.07.mov
Kotlin w/o mssd:
Screen.Recording.2026-11-12.at.10.05.16.mov
Java w/ mssd:
Screen.Recording.2026-11-12.at.09.58.34.mov
Java w/o mssd:
Screen.Recording.2026-11-12.at.10.00.51.mov
Checklist