[gonzaloebiz]

The library half of the area/action pair. The receiver half is live; the extension call site follows separately.

Why the library cannot work this out for itself

ctx is derived from PHP_SAPI: cli means cron, anything else means web. But Magento's own cron entry point refuses to run under the CLI SAPI — pub/cron.php exits if php_sapi_name() === 'cli' and then boots the framework's Cron application. An installation running cron over HTTP is a web-SAPI process doing a full sync, and it lands in the web lane. Every web-lane number is contaminated by an unknown amount of cron, and no amount of looking at our own traffic can separate them.

The second reason is narrower: exactly two call sites reach the campaigns family — a storefront controller that fires on any page carrying a foreign mc_cid, and an admin controller that fires only on an explicit click in the order view. Both emit an identical campaigns:404 into a single last-write-wins slot, and both are web, so ctx cannot separate them. The storefront one is the common case by construction, since it needs no merchant action at all — which means suppressing it as noise is the same edit as losing the merchant-facing one.

Shape

setSurface($area, $action) — one call, both tokens, because the host knows them at the same moment and a single entry point is a single method_exists() guard on the caller's side.

Instance state, beside the user agent, not on a bucket. A bucket exists only once an API key has been seen: $_current is null for a store view with no key configured and for everything past MAX_BUCKETS. The dispatch happened regardless, and those are precisely the processes whose surface we would most want named. A process serves one surface; a bucket is an account it reached.

First non-empty wins, per token and independently. A valid area survives an action that does not pass, and a later valid action can still complete an earlier rejected one — but nothing can overwrite a value already taken.

Above the lean cut, with the other facts about the reporting process rather than about the account. A lean report is one sent by a process that never saw the account root, and what that process was doing is the one thing such a report can still say. It costs nothing to be wrong about this either: sendMode() never returns lean off the CLI lane, and both reasons above are web.

The action is attacker-controlled, and that shapes the rest

getFullActionName() is the plain concatenation of route, controller and action. Magento's page-cache block renderer — a frontend HttpGetActionInterface controller, no admin session, no form key, gated only by an X-Requested-With header the caller sets — writes all three from a JSON query parameter for the duration of a block render.

So:

• Rejected whole, never repaired. Stripping the bytes that failed out of sales_order\x00view yields sales_orderview — not what arrived, and possibly a route that exists. Absent says what happened; cleaned does not.
• Length on the raw bytes, before the charset test, so the value measured is the value given and there is no intermediate form whose length could differ from what arrived.
• The receiver validates independently, and does not assume this check happened.

Neither end can tell a forged but well-formed action name from a real one. What both bound is the shape, so the worst case is an action attributed to the wrong surface — not an action that carries anything into a consumer.

\z, not $

The receiver's expression is ^[A-Za-z0-9_]+$ in JavaScript, where $ matches only at the end of input. PHP's $ also matches immediately before a final newline. Measured:

PHP  /^[A-Za-z0-9_]+$/   on "checkout_index\n" => 1
PHP  /^[A-Za-z0-9_]+\z/  on "checkout_index\n" => 0
JS   /^[A-Za-z0-9_]+$/   on "checkout_index\n" => false

Ported literally, this library would have accepted exactly one byte the receiver refuses, and that token would have disappeared with nothing recorded at either end.

The charset settles a second problem for free

json_encode() returns false on malformed UTF-8, and flush() drops a body that is not a string. One hostile byte would therefore have cost the entire report — every count, every timing, for every bucket — not merely this field. An ASCII-only pattern makes that unreachable rather than handled.

Demonstrated rather than argued, on a real envelope:

envelope carrying the raw hostile byte: json_encode = FALSE
  -> flush() discards the whole bucket (!is_string($body) => continue)

Verified

22 values through the real envelope(), every rejection leaving the key absent — never a cleaned or clipped remnant — and never costing the rest of the envelope:

	result
frontend + checkout_index_index	both carried
sales_Order_View (stock mixed-case route)	carried — case is not folded
exactly 128 bytes / 129 bytes	carried / absent
trailing newline	absent
interior NUL	absent
malformed UTF-8	absent, and the envelope still encodes
traversal, CRLF, SQL, markup, JNDI, Cyrillic homoglyph, bidi override, trailing space, namespace separator	absent
non-string (array, int), empty string	absent

An area outside the receiver's closed set is passed through rather than rejected here. The set of areas is the receiver's to hold — duplicating it would mean re-releasing this library whenever the platform gains one, and a value that arrives and is dropped there is how we would learn that happened.

First-wins and independence, on the same path:

setSurface('frontend','checkout_index_index') then ('adminhtml','sales_order_view')
  -> area=frontend  action=checkout_index_index      (second call does not overwrite)

setSurface('frontend', <malformed UTF-8>) then ('adminhtml','sales_order_view')
  -> area=frontend  action=sales_order_view          (the later valid action completes it)

End to end on a running 2.4.8, through the real helper, a real API key and a real root call rather than a fixture — and following the send rather than assuming it:

getenv(MC_TELEMETRY) : 'force'      PHP_SAPI : cli
root->info()         : ok
_enabled / _forced   : true / true
bucket c6eadd32  saw_root=true  calls=1  mode=full
  body real : 711 B
  contains "area":"adminhtml"                    : yes
  contains "action":"mailchimp_surfaceprobe_two" : yes
--- flush ---
  _reported    : true
  _sendsFailed : 0        (0 means the endpoint answered 2xx)
  took         : 121 ms

The body is built by invoking envelope() with the same arguments flush() passes it — the real $cli and the mode sendMode() actually returned — so those are the bytes that went out, not a reconstruction. _sendsFailed is incremented by post() on a curl error or any status outside 200-299, so zero is the transport confirming delivery from this side.

Two more cases on the same install, with the send suppressed rather than sampled away:

host that never calls setSurface     area=ABSENT    action=ABSENT               calls=1
setSurface before any key exists     area=frontend  action=checkout_cart_index  calls=1

The first is the no-regression case: an extension that knows nothing about this method produces the envelope it produced before, key for key. The second is §4.3's argument made rather than asserted — the surface is named before an API key exists, so before any bucket does, and it still arrives. On a bucket it would have been dropped.

tools/no-deprecated-calls.php: 64 files, 0 findings.

Not in this PR

The extension call site, which needs two guards of its own: method_exists() on this method, because an app/code installation pairs whichever library is present with whichever module is present, and a guard on getFullActionName(), which is not on RequestInterface and does not exist on the CLI request implementation at all — where most of the API traffic lives.

[dipola]

Approving. The \z call is right — PHP's $ matches before a final newline and \z does not — and reject-whole with the length measured on the raw bytes before the charset test is the correct order.

Two suggestions, neither blocking.

1. An unrouted request composes the bare delimiters

getFullActionName() is route . '_' . controller . '_' . action. When nothing was routed all three parts are null and it returns __.

That value passes surfaceToken(), and it has to: underscore is inside [A-Za-z0-9_] because it is the separator. What comes through is a token that names nothing, is byte-identical on every installation that produces it, and looks exactly like a route name to anything that reads it later.

By the reasoning already in your description — the library bounds the shape — a value with no name in it is a shape question rather than a semantic one, so it belongs here too and not only at the call site. One more line beside the two checks already there:

if (!preg_match('/[A-Za-z0-9]/', $value)) {
    return null;
}

Worth writing the rule as "at least one character that is not a separator" rather than as anything about double underscores: mailchimp__ should be kept. It carries a real route id, so it says the route resolved and the controller and action did not, which is a true and readable thing to report.

2. A test for surfaceToken(), in the harness this repo already has

tools/tests/ plus the existing workflow give you a runner, an idiom and a CI step with no new dependency and no PHPUnit.

The \z is the case that most wants pinning. It is a deliberate departure from the far more familiar $, the difference shows up on exactly one byte, and the instinct of the next person editing that line will be to "normalise" it back — at which point nothing fails and the tokens quietly stop arriving. The table already in your description, asserted, plus one more run: step, closes that off permanently.

[gonzaloebiz]

Both taken. CI green, five steps.

1. __ now rejected here too

You are right that it is a shape question. The rule is written as "carries at least one character that is not a separator", so mailchimp__ is kept — a route that resolved with the controller and action that did not is a true and readable thing to report, and a rule about double underscores would have thrown it away.

The call site keeps its own copy. Not because this one is insufficient, but because the two know different things: the library sees a shape, while the extension knows why the value is meaningless — nothing was routed — and can decline to send the bytes at all. Either alone is correct; both is one fewer assumption about which library a given install is running.

2. tools/tests/test-surface-token.php, 31 checks

Same idiom as the gate test, no PHPUnit, one more run: step.

The \z case earns its place, exactly as you predicted it would:

mutation: \z -> $     -> 1 check fails: "a trailing newline"
mutation: drop the name filter -> 3 checks fail: bare delimiters, single delimiter, nothing but delimiters

Both mutations killed. Before this, the first of those two edits changed nothing that any test could see.

I also asserted the reason rather than only the behaviour:

check('PHP $ accepts a trailing newline', preg_match('/^[A-Za-z0-9_]+$/', "checkout_index\n") === 1);
check('PHP \z does not',                  preg_match('/^[A-Za-z0-9_]+\z/', "checkout_index\n") === 0);

So if someone does normalise the line back, the failure tells them why the odd-looking escape was there, instead of just that something broke.

One thing I did beyond the two suggestions — say if you would rather I did not

The workflow ran one gate and now runs two, so "Deprecated calls" said less than the workflow does. I renamed it to "Source gates" and the job to "Gates over src". The file name is unchanged and nothing references the old name — develop has no branch protection, so there is no required-check string to break. Purely cosmetic, and easy to revert if you would rather the name stayed put.

[dipola]

Both good, and the rename is an improvement — keep it. A workflow named after one of the two gates it runs is a name that will mislead someone at exactly the wrong moment.

Two things you did that I did not ask for and that are better than what I did:

Asserting the reason, not just the behaviour. check('PHP $ accepts a trailing newline', …) fails with an explanation attached. A test that pins a surprising line should say why the line is surprising, or the next person deletes the line and reads the failure as noise.

Keeping the filter at the call site as well. Your reasoning is the right one and sharper than mine: the library sees a shape, the call site knows why the value is meaningless — nothing was routed — and can decline to send the bytes at all. Two layers that know different things is not duplication.

The mutation result is the part worth keeping in the commit record: before this, changing \z to $ was an edit no test could see.