[svedge]

task https://app.doboard.com/1/task/58708

[Copilot]

🟡 Changes recommended

Replay protection is not atomic, and IPv6 normalization can reject valid tokens.

2 open issues

• Atomically claim replay tokens before issuing secrets · New
• Canonicalize IPv6 addresses before comparing token IPs · New

What changed in this PR

This PR adds a signed-token entry point for emergency firewall bypass, reusing the existing browser-secret mechanism and removing the legacy access-key bypass.

Changes:

• Add JWT validation, replay tracking, and bypass activation.
• Register authenticated and unauthenticated handlers and let bypass requests reach them.
• Add tests for token validation, single-use behavior, and request recognition.

File	Description
tests/​lib/​CleantalkSP/​SpbctWP/​Firewall/​FirewallBypassJwtTest.php	Tests JWT bypass activation and rejection cases.
security-malware-firewall.php	Registers the JWT bypass handlers.
lib/​CleantalkSP/​SpbctWP/​Firewall/​FirewallBypass.php	Implements JWT validation and browser bypass issuance.
inc/​spbc-firewall.php	Replaces legacy bypass checks with JWT request routing.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Site isolation and valid-token handling have unresolved security and correctness issues.

1 open issue

• Bind multisite JWT audience and issuer to the site path · New

2 previously missed issues

In code that hasn't changed since last review

Verify renewed bypass cookies against the persisted secret

lib/​CleantalkSP/​SpbctWP/​Firewall/​FirewallBypass.php:311

When the browser already sends a bypass cookie, Cookie::getVariable() prefers the original filter_input(INPUT_COOKIE, ...) value over the new secret written to $_COOKIE. This check compares the old secret with the newly stored hash, so a valid renewal redirects home and skips activation auditing even though a new cookie was issued. Verify the persisted hash against $user_secret directly instead of rereading the incoming cookie.

Preserve mapped IPv4 addresses during request-IP validation

lib/​CleantalkSP/​SpbctWP/​Firewall/​FirewallBypass.php:391

When REMOTE_ADDR is ::ffff:203.0.113.10, IP::get() removes the mapped prefix through the shared normalizeIPv6() helper, so this comparison rejects a valid token claiming either the plain or mapped IPv4 address. Preserve the mapped prefix through request-IP selection before canonicalizing it. Add a mapped-request/plain-token regression case; the existing test covers only the opposite direction.

2 resolved since last review

• Atomically claim replay tokens before issuing secrets
• Canonicalize IPv6 addresses before comparing token IPs

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Valid token redemptions can fail IP validation or incorrectly redirect home and omit the activation audit log.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Verify persisted bypass hash instead of stale incoming cookie

lib/​CleantalkSP/​SpbctWP/​Firewall/​FirewallBypass.php:311

When the request includes an old spbc_fw_bypass cookie, Cookie::getVariable() still reads that incoming value through filter_input() after setUserToken() updates $_COOKIE. A successful JWT redemption therefore redirects home and skips the activation audit log despite issuing a new bypass cookie. Verify the persisted hash against $user_secret directly instead of rereading the incoming cookie.

Preserve IPv4-mapped addresses during request IP normalization

lib/​CleantalkSP/​SpbctWP/​Firewall/​FirewallBypass.php:390

For a request address of ::ffff:203.0.113.10, IP::get() calls the shared normalizeIPv6(), which removes the mapped-address marker and returns 0:0:0:0:0:0:cb00:710a. The new comparison therefore rejects valid tokens containing either 203.0.113.10 or ::ffff:203.0.113.10. Preserve the mapped address through IP detection before canonicalizing it, and add a regression case with a mapped request address, not just a mapped token.

1 resolved since last review

• Bind multisite JWT audience and issuer to the site path

_{Review effort: Balanced}