Plugin dir: draft patch to allow reviewers to preview pending plugins

[tellyworth]

This adds a link with corresponding REST APIs to allow plugin reviewers to easily preview a pending/new plugin that has not yet been published.

Note that in this initial version the REST APIs are not authenticated, since Playground does not pass auth cookies back to the API when fetching blueprints and zip files.

https://meta.trac.wordpress.org/ticket/7380

[tellyworth]

The main questions I have:

• How safe is it to add the unauthenticated zip endpoint? (It might allow an enumeration discovery attack on pending zip uploads, but it's unclear how important that is)
• If authentication is required, what method? (Best idea so far is a nonce-like URL parameter that's unique to each file)
• In class-plugin-zip.php, readfile() works in testing on my sandbox, will it work in production also?
• In short: worth doing?

[dd32]

• How safe is it to add the unauthenticated zip endpoint? (It might allow an enumeration discovery attack on pending zip uploads, but it's unclear how important that is)

Unfortunately not at all, we don't go to extreme efforts to hide ZIPs, but we also don't make it super easy to guess a URL.

• If authentication is required, what method? (Best idea so far is a nonce-like URL parameter that's unique to each file)

We'd have to do something like a signed URL, if Auth cookies are not present to verify it.

Something we could do instead is a Blueprint endpoint that accepts the ZIP as a parameter (not the ID, the actual zip filename/path), which would resolve the enumeration aspect.

• In class-plugin-zip.php, readfile() works in testing on my sandbox, will it work in production also?

Yup, although I'm not sure why it's needed. It's not clear to me why the playground doesn't just download it from the URL that the ZIP currently lives at, or is this a CORS issue again?

If CORS, since we're serving via ms-files, we can likely inject the appropriate CORS headers for ZIPs uploaded to the site.

We can possible also use /wp-content/blogs.dir/$site_id/files/... which bypassses PHP and allows nginx to serve the ZIP directly, which might also have appropriate headers (but I doubt)

• In short: worth doing?

Probably!

[tellyworth]

Thanks @dd32 !

I pushed an update that uses a hash instead of an ID (see notes in commit message).

Re the zip endpoint: yes its entire reason for existing is CORS. I'd be happy to use an alternative method if it's feasible.

[tellyworth]

Note to self: here's the code where Playground fetches the blueprint with credentials: omit
https://github.com/WordPress/wordpress-playground/blob/ca8ca28a8703c137760e176c66e28d850869f496/packages/playground/website/src/lib/resolve-blueprint.ts#L16

[tellyworth]

If CORS, since we're serving via ms-files, we can likely inject the appropriate CORS headers for ZIPs uploaded to the site.

ms-files uses SHORTINIT which means plugins are bypassed. Were you thinking of a dropin or some other angle for hooking into that?

[dd32]

ms-files uses SHORTINIT which means plugins are bypassed. Were you thinking of a dropin or some other angle for hooking into that?

Yeah, I guess I was thinking of a filter in sunrise, where we've got a bunch of other ms-files hacks. Something like this:

if (
	defined( 'SHORTINIT' ) && SHORTINIT &&
	'ms-files.php' === basename( $_SERVER['SCRIPT_FILENAME'] ) &&
	preg_match( '~^/plugins/files/\d+/\d+/[^?/]+.zip$~i', $_SERVER['REQUEST_URI'] )
) {
  // Add headers
}

There's one in there too that allows s.w.org/patterns/files to load too.. so it could even just be that s.w.org/plugins/files gets added there too, and the CORS headers added when via that CDN, if we wanted to be super careful.

I'm not 100% set on that direction though.. just seems odd that we need an entire rest api endpoint to serve the ZIP

[tellyworth]

The zip endpoint was very much a CORS workaround. It does have some potential benefits, like hiding the true location of the uploaded file, or providing url-signed auth. That could be useful if we intend to expose these links to plugin authors during the review process (which is definitely something worth considering). But in general I agree that an endpoint just for serving the zips is an overcomplication if an alternative works well enough.

[tellyworth]

Seems to work well using sunrise.php. I pushed an update with some better code; I think it's probably good enough to open a meta ticket in its current state.