diff --git a/.cspell/custom-words.txt b/.cspell/custom-words.txt index ce73c361..759960cb 100644 --- a/.cspell/custom-words.txt +++ b/.cspell/custom-words.txt @@ -118,6 +118,7 @@ okhttp opentelemetry otelgrpc otelhttp +otherpisp Otherville OURCYGPATTERN Palo @@ -127,6 +128,11 @@ Payoneer paypal Payplug pids +PISP +Pisp +Pisps +pisp +pisps pmezard proguard Proguard @@ -149,6 +155,7 @@ ropeproject RPCURL Rulebook screenreaders +SECP setlocal sharedpref Shopcider @@ -158,6 +165,7 @@ Signifyd skus solana Splitit +spoofable Spyder spyderproject spyproject diff --git a/.github/workflows/linter.yaml b/.github/workflows/linter.yaml index fea1b9c1..663a5b19 100644 --- a/.github/workflows/linter.yaml +++ b/.github/workflows/linter.yaml @@ -4,6 +4,11 @@ on: pull_request: branches: [main] +permissions: + contents: read + statuses: write + pull-requests: write + jobs: build: name: Lint Code Base @@ -11,12 +16,13 @@ jobs: steps: - name: Checkout Code - uses: actions/checkout@v5 + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 with: fetch-depth: 0 + persist-credentials: false - name: Lint Code Base - uses: super-linter/super-linter/slim@v8 + uses: super-linter/super-linter/slim@4ce20838b8ab83717e78138c5b3a1407148e0918 # v8.7.0 env: DEFAULT_BRANCH: main GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -25,6 +31,7 @@ jobs: VALIDATE_ALL_CODEBASE: false FILTER_REGEX_EXCLUDE: "^(\\.github/|\\.vscode/|code/samples/).*|CODE_OF_CONDUCT.md|CHANGELOG.md" VALIDATE_BIOME_FORMAT: false + VALIDATE_BIOME_LINT: false VALIDATE_PYTHON_BLACK: false VALIDATE_PYTHON_FLAKE8: false VALIDATE_PYTHON_ISORT: false diff --git a/code/sdk/python/ap2/sdk/constraints.py b/code/sdk/python/ap2/sdk/constraints.py index 48aed798..4b4bbcb5 100644 --- a/code/sdk/python/ap2/sdk/constraints.py +++ b/code/sdk/python/ap2/sdk/constraints.py @@ -43,27 +43,18 @@ class MandateContext(BaseModel): def merchant_matches(candidate: Merchant, target: Merchant) -> bool: - """Match merchants by ``id`` (preferred) or by ``name`` + ``website``.""" + """Match merchants by a stable, non-empty ``id``. + + Merchant/payee identity must be established by ``id``. The display + fields (``name`` and ``website``) are attacker-controllable and are + never sufficient to prove that two merchant/payee objects are the same + actor, so an empty or missing ``id`` on either side never matches (see + issue #315). + """ candidate_id = candidate.id + target_id = target.id if isinstance(target, Merchant) else target.get('id') - if isinstance(target, Merchant): - target_id = target.id - target_name = target.name - target_website = target.website - else: - target_id = target.get('id') - target_name = target.get('name') - target_website = target.get('website') - - if candidate_id and target_id: - return candidate_id == target_id - - return ( - candidate.name == target_name - and bool(candidate.name) - and candidate.website == target_website - and bool(candidate.website) - ) + return bool(candidate_id) and candidate_id == target_id class PaymentConstraintEvaluator(ABC): diff --git a/code/sdk/python/ap2/tests/constraints_tests.py b/code/sdk/python/ap2/tests/constraints_tests.py index 910c97fc..1430ff76 100644 --- a/code/sdk/python/ap2/tests/constraints_tests.py +++ b/code/sdk/python/ap2/tests/constraints_tests.py @@ -96,29 +96,17 @@ def _checkout(merchant=None, line_items=None, **kw): pytest.param( Merchant(id='m-1', name='A', website='https://a.com'), Merchant(id='m-1', name='B', website='https://b.com'), - id='by_id', - ), - pytest.param( - Merchant(id='', name='Shop', website='https://shop.com'), - Merchant(id='', name='Shop', website='https://shop.com'), - id='by_name_and_website', + id='by_id_ignores_display_fields', ), pytest.param( Merchant(id='m-1', name='A'), Merchant(id='m-1', name='B').model_dump(mode='json'), id='dict_target_by_id', ), - pytest.param( - Merchant(id='', name='Shop', website='https://shop.com'), - Merchant(id='', name='Shop', website='https://shop.com').model_dump( - mode='json' - ), - id='dict_target_by_name_and_website', - ), ], ) def test_merchant_matches(candidate, target): - """Merchants that should match do match.""" + """Merchants with equal, non-empty ids match regardless of display.""" assert merchant_matches(candidate, target) @@ -130,16 +118,37 @@ def test_merchant_matches(candidate, target): Merchant(id='m-2', name='A'), id='different_id', ), - pytest.param( - Merchant(id='', name='Shop'), - Merchant(id='', name='Shop'), - id='name_only_without_website', - ), pytest.param( Merchant(id='m-1', name='A'), Merchant(id='m-2', name='A').model_dump(mode='json'), id='dict_target_different_id', ), + # Regression for issue #315: display fields (name + website) are + # spoofable and must never establish identity when an id is empty. + pytest.param( + Merchant(id='', name='Shop', website='https://shop.com'), + Merchant(id='', name='Shop', website='https://shop.com'), + id='spoofed_name_website_empty_id', + ), + pytest.param( + Merchant(id='', name='Shop', website='https://shop.com'), + Merchant(id='', name='Shop', website='https://shop.com').model_dump( + mode='json' + ), + id='spoofed_name_website_empty_id_dict', + ), + # An authorized merchant with a real id must not be matched by an + # attacker who supplies an empty id but copies name + website. + pytest.param( + Merchant(id='m-1', name='Shop', website='https://shop.com'), + Merchant(id='', name='Shop', website='https://shop.com'), + id='real_id_vs_empty_id_same_display', + ), + pytest.param( + Merchant(id='', name='Shop'), + Merchant(id='', name='Shop'), + id='name_only_without_website', + ), pytest.param( Merchant(id='', name='Shop', website=''), Merchant(id='', name='Shop', website=''),