fix: feed shop allowlists into the SDK URL-safety validator (remote profile fetches always rejected) - #150
Conversation
The SDK bundle compiles its UrlSafetyValidator from the static ucp_sdk.allowed_profile_hosts semantic config, which this plugin never populates. The validator therefore always ran with an empty allowlist and rejected every remote platform-profile fetch (and order-webhook target) with 'Profile host ... is not allowed' - no matter what the merchant configured. The per-channel remoteProfileAllowlist only guards the earlier assertSafeProfileUri() check in the request-context factory and never reaches the fetcher, so any real platform calling a correctly configured shop got a 422 on every UCP runtime request. Fix: a compiler pass replaces the SDK's compiled definition with one built at runtime by ConfiguredUrlSafetyValidatorFactory, which unions every sales channel's (and the global scope's) remoteProfileAllowlist and platformAllowlist plus all sales-channel domain hosts. Per-channel policy stays enforced upstream by assertSafeProfileUri(); the validator keeps acting as the SSRF defense-in-depth layer behind it, and the webhook dispatcher picks up the same allowlist.
|
Read this properly while adjudicating the overlap between our UCP PRs. No conflict with #159/#160 — 1. Your design note asks a question we can now answer differentlyYou wrote:
That reasoning was right when you wrote it on 28 July. It has since changed: the SDK has moved three times this week — #111 and #112 are merged, #113 prepares 0.0.4, and the plugin will require The SDK-level shape you describe — I'm not asking you to redo it. I'm asking which we want, because the two are genuinely different bets:
Given it's a live breakage on PaaS, I lean toward merging this now and filing the SDK change, with this PR's compiler pass documented as a stopgap to remove when the SDK lands it. That way nobody waits and the architecture still converges. 2. The union is the reviewable claim, and it deserves a named reviewerYour argument is that the cross-channel union is safe because a per-channel gate always runs first. That holds cleanly for profile fetches — That's probably fine — 3. It does not fix O7, and I want to correct myself publiclyI had this PR filed in my notes as "probably the There are two independent defects in
That one is SDK code, so this PR cannot reach it — and it's the natural companion to the fetch-config change in 1. If we go the SDK route, both land together. Happy to write the SDK PR for the fetch-time configuration + |
Problem
Any UCP platform calling a correctly configured shop gets a 422 on every runtime request:
Setting the sales channel's
remoteProfileAllowlist(admin orucp:config:set) does not help — the rejection persists no matter what is configured.Root cause
There are two profile-host checks with different config sources:
DefaultHttpRequestContextFactory::assertSafeProfileUri()— uses the per-channel runtime configuration (remoteProfileAllowlist). This one works.UrlSafetyValidatorinsideHttpAgentProfileFetcher(also injected intoDefaultOrderWebhookDispatcher) — compiled from the static bundle configucp_sdk.allowed_profile_hosts, which this plugin never populates. It therefore always runs with an empty allowlist and rejects every remote profile fetch, right after check 1 passes.The two checks are easy to conflate because their error messages are similar; the observed message (
Profile host "…" is not allowed., without the "Allowed hosts: …" suffix) is the fetch-time validator's empty-list rejection.Order webhooks are affected by the same empty allowlist through the shared service.
Fix
ConfiguredUrlSafetyValidatorFactorybuilds the SDK validator from what the shop already manages: the union of every sales channel's (and the global scope's)remoteProfileAllowlist+platformAllowlist, plus all sales-channel domain hosts (parity with the runtime fallback so own-host profile URIs stay fetchable).ReplaceSdkUrlSafetyValidatorPassswaps the SDK bundle's compiled definition for the factory-built one (compiler pass so the override wins regardless of bundle load order, same pattern asReplaceSdkSigningKeyCommandsPass).The union is safe: per-channel policy is still enforced by
assertSafeProfileUri()before any fetch; the validator remains the SSRF defense-in-depth layer (blocked hosts/IP checks unchanged).profile_fetching_development_modeis passed through unchanged.Verification
Reproduced and verified against a live shop (Shopware PaaS) with a channel-scoped
remoteProfileAllowlist:POST /ucp/v1/catalog/searchwithUCP-Agent: profile="https://<allowed-host>/.well-known/ucp"→ 422Profile host … is not allowed.searchProductsround-trip returns products.6 new unit tests (factory union/dedup/lowercase, dev-mode passthrough, DB-failure fallback; pass replacement + no-op guards). php-cs-fixer/phpstan clean on touched files.
Design note (why plugin-level, and the SDK alternative)
The root architectural gap arguably sits in the SDK:
DefaultHttpRequestContextFactoryhas already resolved a per-requestRuntimeConfiguration(withallowedProfileHosts) when it callsAgentProfileFetcherInterface::fetch(string $uri)— but the fetch API has no config slot, so the fetcher falls back to a global static list. The deepest fix would be SDK-level: validate the fetch against the per-request configuration. This PR is the correct fix at the plugin layer for SDK ^0.0.2 as-is, since the SDK deliberately exposesallowed_profile_hostsas an integration-owned knob and the plugin is the layer that owns the (dynamic, DB-backed) merchant config.Why the cross-channel union is safe: both consumers of the validator run after a per-channel policy gate — the profile fetch only happens once
assertSafeProfileUri()passed for the resolved channel, and webhook targets come from aRequestContextcreated through that same gate (or fromwebhookUrlOverride, whichUcpConfigvalidates against the same channel's allowlists at write time). The validator's SSRF checks (blocked hosts, private/reserved IP ranges, ports, DNS) are unchanged.Trade-offs to be aware of: the validator is built lazily per container lifecycle (a few small queries on first UCP/webhook use per request cycle; long-running workers pick up allowlist changes on the next worker cycle).