SECURITY-10974: raise AuthError on a missing HMAC header, and stop shipping auth tokens in samples

[tomas-amaro]

Verification follow-up to SECURITY-10974 / HackerOne report 3688453, which reported an HMAC disclosure and webhook authentication bypass in the PHP SDK.

This SDK does not have the reported defect

Checked directly, and worth stating plainly so the negative result is on record:

• MessageDigest.isEqual is constant-time — no timing side channel
• AuthError carries a fixed message and discloses neither HMAC
• both notification samples already caught exceptions and returned generic errors

No equivalent of the reported bypass exists here. Verifying that surfaced two adjacent problems.

1. A missing HMAC header raised NullPointerException

NotificationHandler.toObject() called hash.getBytes() before null-checking hash. A request carrying no X-Riskified-Hmac-Sha256 header therefore produced an NPE rather than AuthError — so a caller catching the documented exception mishandled precisely the unauthenticated case.

It failed closed either way; the exception type was the bug. Now rejected as AuthError, matching the .NET reference which guards the missing header explicitly.

Both operands also now pin US_ASCII rather than inheriting the platform default charset. Both values are hex, so behaviour is unchanged today — it removes a latent dependency on deployment-host configuration.

Adds NotificationHandlerTest, which did not exist. It covers the valid path, wrong and foreign signatures, the missing-header case, and asserts AuthError discloses neither HMAC nor the request body.

2. The notification samples carried auth tokens as source literals

SampleServer.java and HTTPPOSTServer.java each supplied the Riskified auth token inline. These samples ship in a public repository, so those were disclosed credentials — and the auth token is exactly what an attacker needs to forge a notification that passes HMAC verification, which is the impact the report describes.

Both now resolve it through SampleAuthToken.fromEnvironment() (RISKIFIED_AUTH_TOKEN), failing with an actionable message when unset rather than starting a server that would reject every notification. The socket sample reads it once at startup and injects it, mirroring how NotificationServlet already received its key.

Important

This does not remove the literals from git history. If either value was ever live it needs rotating, which is a separate action for whoever owns those credentials and is being tracked as its own ticket. Reviewers: please confirm their status rather than assuming they were dummies.

For the record, two similar-looking literals in riskified-sample were checked and deliberately not touched — orderClient/Client.java:218 is a cart token and AccountActionClient.java uses a SessionDetails token, both synthetic fixture data alongside bob.norman@hostmail.com, not credentials.

Verification

Demonstrated RED → GREEN on the NPE: missingHmacThrowsAuthErrorNotNullPointerException fails against unpatched master with "Missing HMAC header must raise AuthError, not NullPointerException", and passes with the fix.

• mvn test — 63 pass (was 56; 7 new), 0 failures. Two errors in RiskifiedClientTest are pre-existing and environmental: it makes live API calls and needs a real sandbox token. Identical with and without this branch.
• mvn -DskipTests package — both modules build, including the sample whose constructor signature changed.

🤖 Generated with Claude Code