Skip to content

PDP-1401: Harden proxy security: input allow-listing, size caps, CORS… - #12

Merged
ykulynych merged 2 commits into
nuclia:mainfrom
ykulynych:PDPS-1401-fix-the-wordpress-plugin
Oct 19, 2026
Merged

ykulynych merged 2 commits into
nuclia:mainfrom
ykulynych:PDPS-1401-fix-the-wordpress-plugin

Conversation

@ykulynych

Copy link
Copy Markdown
Contributor

…, Content-Type coercion

@ykulynych
ykulynych force-pushed the PDPS-1401-fix-the-wordpress-plugin branch 4 times, most recently from bce3d7c to 44a5237 Compare October 14, 2026 14:29
@ykulynych
ykulynych force-pushed the PDPS-1401-fix-the-wordpress-plugin branch from 44a5237 to dfd1849 Compare October 14, 2026 15:05

@StrawHat-Dery StrawHat-Dery left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 StrawHat-Dery left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ykulynych
ykulynych merged commit 7d33208 into nuclia:main Oct 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants