Repository navigation
PDP-1401: Harden proxy security: input allow-listing, size caps, CORS… - #12
Conversation
bce3d7c to
44a5237
Compare
…, Content-Type coercion
44a5237 to
dfd1849
Compare
StrawHat-Dery
left a comment
There was a problem hiding this comment.
Overall, the proxy hardening looks good in both automated and runtime testing.
Validated successfully:
PHP lint and PHPUnit pass (132 tests / 722 assertions)
normal same-origin proxy requests continue to work
service-account credentials remain server-side
CORS only reflects the allowed site origin; hostile/suffix origins are not allowed
no wildcard or credentialed CORS
the 5 MiB request-body cap works at the exact boundary and rejects oversized bodies locally with 413
JSON responses remain JSON
HTML/SVG responses are coerced to application/octet-stream with nosniff
One high-value design question remains around oversized query strings.
At exactly 2048 bytes, the query is preserved. At 2049 bytes, the entire query string is dropped and the upstream request still proceeds. In runtime testing, a request containing a dummy eph-token returned 401 at 2048 bytes, while the 2049-byte version lost the query parameters and returned 200, matching the normal service-account path.
Is this behavior intentional as a graceful degradation mechanism?
I can understand the UX motivation for allowing the request to continue rather than surfacing a raw 400/414, but the current fallback also removes auth-critical parameters such as eph-token, which means an oversized request can silently continue under a different authentication context.
If graceful degradation is intentional, a few safer options may be worth considering:
preserve auth-critical parameters such as eph-token even if nonessential query parameters are dropped;
move large request metadata/configuration into the request body where the upstream API allows it;
or, if the query must remain bounded, reject the oversized request and let the frontend present a user-friendly error.
It may also be worth confirming whether 2048 bytes is the right limit for legitimate PAR traffic.
Non-blocking follow-up: on this plain-permalink test site, some raw punctuation/non-ASCII path characters are rejected, while percent-encoded forms can be altered by WordPress sanitization before forwarding. No current widget/customer break was reproduced, so I’d treat that as a compatibility follow-up rather than a blocker.
No blocking vulnerability was found. From my side, the main item I’d like clarified before final merge is whether the oversized-query fallback is intentional and, if so, whether we want to preserve the current auth semantics more explicitly.
StrawHat-Dery
left a comment
There was a problem hiding this comment.
Follow-up validation for 121757a is complete.
The oversized-query behavior is now runtime-confirmed fixed:
- normal proxy requests still work
- a 2049-byte query is preserved and no longer gets silently dropped
- exactly 8192 bytes is accepted by the plugin when the request reaches PHP
- 8193 bytes returns a local 414 with no upstream call
- invalid query characters return 400 and are not partially forwarded
- dummy eph-token requests remain on the ephemeral path rather than falling through to the service-account path
Ready to merge from my side.
…, Content-Type coercion