feat: SurfaceGuard — env.php kill switches for the unauthenticated upload entry points

[ssx]

Implements the SurfaceGuard build spec. An env.php-only hardening module that lets an operator switch off the public, unauthenticated file-upload entry points a store does not use.

Why

The StyleSmuggler campaign (CVE-2026-75650, 835 deduplicated attempts across the fleet, first probe ~12 days before public disclosure) staged a GIF+PHP polyglot through the guest custom-option upload path and then named that file as the source for an arbitrary-instantiation gadget. The upload half of that chain does not depend on the CVE — it is ordinary Magento behaviour on an endpoint most of our stores never use.

The root cause of the upload is a blacklist: when a custom option declares no file_extension, module-catalog/etc/config.xml falls back to forbidding php,exe only, so .phtml, .phar, .pht and .shtml pass. This module does not try to validate its way out of that — a tightened whitelist is still a guess. Each entry point gets an explicit on/off instead, and off means refused.

Configuration contract

One top-level harden key in app/etc/env.php, read through DeploymentConfig only. No admin UI, no database, so nothing a compromised admin session can reach will flip a switch.

'harden' => [
    'graphql' => ['enabled' => true],
    'uploads' => [
        'cart_add_file'             => true,
        'guest_cart_items_file'     => true,
        'customer_address_file'     => true,
        'customer_custom_attr_file' => true,
    ],
],

Fail-safe: only a strict boolean false denies. An absent key, null, true, 0, '0', 'false', '' and an unreadable env.php all resolve to core behaviour. The module installs inert and can be deployed fleet-wide before any per-site decision is made.

Enforcement

Layer	Plugin target	Denied response
Frontend	Checkout\Controller\Cart\Add::execute	Raw 403, only when the request actually carries files
REST	Quote\Api\CartItemRepositoryInterface::save	Webapi\Exception 403, only when the item carries a file option
Frontend	Customer\Controller\Address\File\Upload::execute	Raw 403
Backstop	Customer\Model\FileUploaderFactory::create	LocalizedException, entity type picks the switch
GraphQL	GraphQl\Controller\GraphQl::dispatch	Raw 403, empty body
Backstop	Catalog ValidatorFile::validate + ValidatorInfo::validate	LocalizedException before Uploader::save

The two catalog validators are the guarantee behind "off means neutered". Every custom-option file upload crosses one of them before the file is moved into pub/media, so a future resolver or an entry point nobody has enumerated still dies there.

Denials write one line to a dedicated var/log/surfaceguard.log — endpoint, switch, client IP. No filename, no request body, no headers: a log carrying attacker-supplied content is its own liability.

Deviations from the spec

1. quickorder_file dropped from v1. Its controller exists only on B2B installs, and a plugin naming a missing class fails setup:di:compile on every non-B2B site. Shipping the key without enforcement would be dead config. If wanted, it belongs in a companion module that hard-depends on Magento_QuickOrder.
2. Customer uploads guarded at FileUploaderFactory, not FileProcessor. The spec assumed FileProcessor exposes its entity type; it does not — $entityTypeCode is private with no getter. FileUploaderFactory::create() receives it as a plugin argument, and all four upload controllers construct through it, including the Commerce AbstractUploadFile. So one open-source seam covers the Commerce routes without this module ever naming a Commerce-only class. FileUploader is built only to upload, never to read, so this does not disturb rendering of files already on disk.
3. Extra controller guard for customer/address_file/upload. The factory guard throws a LocalizedException, which those controllers catch and turn into a JSON error envelope at HTTP 200. A controller guard was added so the endpoint named in the surface audit returns a true flat 403. The Commerce custom-attribute routes still answer with their JSON error envelope rather than a bare 403 — noted here and in the README.
4. REST guard scoped to webapi_rest. So a storefront add-to-cart is answered by its own guard rather than this one.
5. REST file-option detection is marker-based (quote_path / order_path / secret_key, the keys ValidatorInfo itself looks for) rather than loading product option metadata on every cart write. The ValidatorInfo backstop is what guarantees the denial if a payload shape ever slips past the markers.

Documented consequence

The catalog backstop is layer-agnostic, so with either custom-option switch off the admin-side custom-option file flow is denied too. Intentional — a store that has switched the feature off is not selling file-option products — but stated in the README so nobody debugs it as a bug.

graphql.enabled = false is blunt and kills every headless/PWA call. Core GraphQL cart mutations take string-only option inputs and carry no file-write sink, so there is no upload variant to deny selectively.

Verification

• Every plugin target class, method and signature confirmed present in the real EE 2.4.8-p5 tree.
• 28 unit tests, 50 assertions, all green — run against that tree with the generated-factory autoloader, so the mocks bind to real classes.
• All five XML files validate against the framework XSDs (module.xsd, ObjectManager/config.xsd, resolved through an XML catalog).
• php -l clean on every file; no double hyphens in XML comments.
• phpcs --standard=Magento2 was not run — not installed in the available vendor tree.
• setup:di:compile has not been run against a live install; worth doing on staging before merge, particularly for the plugin on the generated FileUploaderFactory.

Not in scope

Not a whitelist, not incident response, not a substitute for the VULN-39341 hotfix, and no detection — that lives in the mjolnir IOC sweep (DeployEcommerce/mjolnir#125).

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[ssx]

Thanks — all three reproduce on 2.4.8-p5, so all three are fixed in e7c73b5. Since the findings were validated against a 2.4.6 tree, I re-derived each one from the 2.4.8-p5 source before touching anything; the evidence is below in case it is useful for the next review.

1. Optional file options blocking purchases — confirmed, fixed

Reproduced exactly as described, and the mechanism is worth spelling out because it is subtler than "wrong exception type".

ValidatorFile::validate() opens with:

$runValidation = $option->getIsRequire() || $upload->isUploaded($file);
if (!$runValidation) {
    throw new \Magento\Framework\Validator\Exception(...);
}

So core itself throws for an optional option with nothing uploaded. The caller in Model/Product/Option/Type/File.php has this catch chain:

} catch (ProductException $e) {   ...
} catch (Exception $e) {                 // Magento\Framework\Validator\Exception, imported line 27
    $this->setUserValue(null);           // swallowed, purchase continues
} catch (LocalizedException $e) {
    $this->setIsValid(false);
    throw new LocalizedException(...);   // rethrown, add-to-cart fails
}

Validator\Exception extends InputException extends LocalizedException, and its catch is listed first, which is what makes core's own throw benign. Our LocalizedException fell through to the third block and was rethrown. With either custom-option switch off, every add-to-cart for a product that merely offers an optional file option would have failed, uploaded file or not.

Fixed by checking whether a file is genuinely present before denying, using core's own file key ($processingParams->getFilesPrefix() . 'options_' . $option->getId() . '_file') through the same FileTransferFactory core uses. When no file is present we call $proceed and let core's normal no-upload path run.

ValidatorInfo is deliberately left unconditional: File.php only reaches it inside if ($fileInfo !== null), so there is no empty case there to protect.

Undeterminable cases (unexpected argument shape, or the upload adapter throwing) resolve to "no upload" and defer to core. Blocking a legitimate purchase is the worse failure, and a real upload is still refused by the controller guards on the way in and by ValidatorInfo on the re-materialize path.

2. Cached GraphQL bypassing the kill switch — confirmed, fixed

Confirmed, and the sortOrder detail is the opposite of what the plugin list's own sort function suggests, so here is how I pinned it down.

Magento_GraphQlCache declares its plugins on Magento\Framework\App\FrontControllerInterface — not on the concrete controller — with no sortOrder attribute:

<type name="Magento\Framework\App\FrontControllerInterface">
    <plugin name="graphql-dispatch-plugin" type="Magento\GraphQlCache\Controller\Plugin\GraphQl"/>
    <plugin name="front-controller-builtin-cache" type="Magento\PageCache\Model\App\FrontController\BuiltinPlugin"/>
    <plugin name="front-controller-varnish-cache" type="Magento\PageCache\Model\App\FrontController\VarnishPlugin"/>
</type>

Magento_GraphQl sets <preference for="Magento\Framework\App\FrontControllerInterface" type="Magento\GraphQl\Controller\GraphQl"/>, so those plugins land on the same dispatch chain as ours, and BuiltinPlugin::aroundDispatch short-circuits on a hit:

$result = $this->kernel->load();
if ($result === false) {
    $result = $proceed($request);
    ...
} else {
    $this->addDebugHeader($result, 'X-Magento-Cache-Debug', 'HIT', true);
}
return $result;

On the ordering: PluginList::_sort() reads ($itemA['sortOrder'] ?? PHP_INT_MIN), which would imply a missing attribute sorts first and is unbeatable. That fallback never fires for plugins read from di.xml — ObjectManager/Config/Mapper/Dom.php:112 maps the attribute as $pluginSortOrderNode ? (int)$pluginSortOrderNode->nodeValue : 0, so the cache plugins are sortOrder 0 and our sortOrder="10" ran strictly inside them. A negative sortOrder does win, and xs:int permits it.

The guard now sits on FrontControllerInterface in the graphql area at sortOrder="-100" — same type as the cache plugins, so the comparison is direct rather than relying on interface-vs-class merge semantics. The plugin's subject type hint widened to FrontControllerInterface accordingly, which also drops the module's only hard reference to a Magento_GraphQl class.

One limit I could not engineer away, now documented in the README: a response already cached at Varnish or Fastly is served before PHP is reached, so no PHP-layer guard can refuse it. Switching GraphQL off on a CDN-fronted site needs a CDN purge too.

3. REST detection rejecting ordinary text options — confirmed, fixed

Confirmed by inspection — this one was my bug, plainly. str_contains($value, $marker) against a text option's value means "Please print secret_key on the label" was a 403.

Detection now requires the markers to be keys of a map: an array, or the JSON encoding of one. A text option's value decodes to null (or to a scalar or a list) and is allowed; a genuine file option's keyed metadata is still caught. Only JSON is decoded — attacker-controlled input is never unserialized, so no unserialize() was added.

I stayed with structured metadata rather than looking up the actual option type, to avoid a product-option load on every cart write; ValidatorInfo remains the guarantee if a payload shape ever slips past the keys. Happy to switch to a type lookup if you would rather have the certainty than the saved query.

Verification

• 36 tests, 59 assertions, all green (was 28/50), run against the real 2.4.8-p5 tree with Magento's generated-factory autoloader.
• New coverage: optional option with no upload proceeds; an actual upload is still denied; the re-materialize path is still denied unconditionally; six innocent text values including ["quote_path","order_path"] and a bare quote_path are no longer mistaken for uploads.
• All five XML files still validate against the framework XSDs; php -l clean.
• Still not run, unchanged from the original PR note: phpcs --standard=Magento2 (not installed in the available vendor tree) and setup:di:compile against a live install. The compile is worth doing on staging before merge — it is the real check on the new negative-sortOrder declaration and on the plugin against the generated FileUploaderFactory.

[ssx]

Heads-up: this PR merged (db15182, 11:01:45Z) carrying up to e7c73b5, while a fourth finding was being fixed. That fix was pushed to feature/surfaceguard-v1 after the merge, so it is not on main — it has moved to #2.

What main is currently missing, and why it matters:

[P1] With cart_add_file switched off, an ordinary purchase returns 403 whenever the product has an optional file option the customer left empty. The controller guard read a non-empty file collection as proof of an upload, but PHP puts every file input on a submitted multipart form into $_FILES including empty ones (UPLOAD_ERR_NO_FILE), Laminas keeps them all when mapping the superglobal, and the product view form is multipart whenever the product has any option. The 403 landed at the controller, before the backstop's no-upload handling from finding 1 could run — same bug class, one layer up.

This does not affect any site today, because every switch defaults to allow and the module installs inert. It would bite the first site that switches cart_add_file off, which is exactly the rollout step this module exists for, so it is worth landing #2 before anyone flips a switch in production.

#2 is a single cherry-picked commit on hotfix/empty-file-input-denial, branched from main, with the other three guards audited for the same assumption (only this one had it) and the suite at 43 tests / 73 assertions.

My apologies for the mis-sequencing here — I read the frozen head SHA on this PR as GitHub lag and spent a couple of minutes polling it before checking the PR state and finding it had been merged.