Skip to content

Leave the Composer routes open while SIMPLE_UPDATE_KEY is unset #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
justinmaurerdotdev merged 1 commit into from
Oct 9, 2026
Merged

Leave the Composer routes open while SIMPLE_UPDATE_KEY is unset #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/runbook.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
state until the key is enforced.
* Composer routes (vnext sites): `/packages.json`, `/p2/reason-dev/<slug>.json`,
`/dist/reason-dev/<slug>/<slug>-<version>.zip`. Key via `Authorization: Bearer`
only. Answer 503 if `SIMPLE_UPDATE_KEY` is unset.
only. Same rule as above: ungated while `SIMPLE_UPDATE_KEY` is unset, so a
Composer site can build against production before the legacy fleet sends the
key. Setting the key gates both routes in the same deploy.

This server only reads the bucket. Everything under `packages/`, `dist/`,
`meta/` and `p2/` is written by `reason-wp-repo-actions`.
Expand Down
13 changes: 5 additions & 8 deletions includes/Wpup/ComposerRepository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
* /p2/reason-dev/<slug>.json p2/reason-dev/<slug>.json
* /dist/reason-dev/<slug>/<slug>-<version>.zip 302 to a presigned URL
*
* Every route requires SIMPLE_UPDATE_KEY as "Authorization: Bearer <key>" and
* fails closed (503) when the key isn't configured: premium plugin code sits
* behind these routes. The key is never accepted from the query string here,
* because Composer records download URLs in composer.lock.
* The key rule is the update-checker route's: while SIMPLE_UPDATE_KEY is unset
* the routes are open, and once it is set every route requires it as
* "Authorization: Bearer <key>". The key is never accepted from the query
* string here, because Composer records download URLs in composer.lock.
*/
class Wpup_ComposerRepository {
const VENDOR = 'reason-dev';
Expand Down Expand Up @@ -53,10 +53,7 @@ public function handle($path, Wpup_Headers $headers) {
}
$path = ltrim((string)$path, '/');

if ( $this->key === '' ) {
return Wpup_ComposerResponse::error(503, 'The Composer repository is disabled: SIMPLE_UPDATE_KEY is not set.');
}
if ( !$this->isAuthorized($headers) ) {
if ( $this->key !== '' && !$this->isAuthorized($headers) ) {

@claude claude Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

馃敶 Anyone can now download premium plugin zips from /dist/reason-dev/.../*.zip whenever SIMPLE_UPDATE_KEY is unset. The old code fail-closed (503) specifically to keep "premium plugin code" (base branch includes/Wpup/ComposerRepository.php:13-14) behind this route when no key was set; that line is deleted and the gate at line 56 now only checks isAuthorized when $this->key !== ''. Composer-only/vnext premium plugins that never shipped via the legacy route were never exposed unauthenticated before this change. Fix: keep dist (or all Composer routes) fail-closed, or require a Composer-specific key, until the specific vnext premium packages involved are confirmed safe to leave open; don't rely solely on URL-guessing difficulty as the safeguard.

Why this was flagged

Trigger: SIMPLE_UPDATE_KEY unset in an environment, which per the PR author is production's state immediately after merge and deploy. Entry point: GET /dist/reason-dev//-.zip, routed via wpup_route_composer()->handle() in includes/Wpup/ComposerRepository.php:50-60. At line 56 the 401 check now only runs when $this->key !== '', so with no key the request falls through to serveDist() (line 111), which issues a 302 to a presigned S3 URL with no authorization at all. On the base branch this path answered 503 specifically because, per the deleted docblock comment (base branch lines 13-14), "premium plugin code sits behind these routes." The PR calls this the same exposure as the legacy packages/ zips, but that only holds for plugins also served by the legacy route; plugins distributed only via the new Composer/vnext path were never reachable unauthenticated before this diff. No other safeguard limits this, since valid slugs are guessable or discoverable via /packages.json and /p2/..., which are also open.

Verification: Base includes/Wpup/ComposerRepository.php returned 503 when $this->key === '', per the docblock keeping "premium plugin code" behind these routes. The diff deletes that check; the merged gate at line 56 skips authorization when the key is unset, letting GET /dist/reason-dev//-.zip fall through to serveDist() (line 111), which returns a 302 to a presigned S3 URL with no authentication.

$response = Wpup_ComposerResponse::error(401, 'Invalid or missing update key.');
$response->headers['WWW-Authenticate'] = 'Bearer realm="packages.reason.com"';
return $response;
Expand Down
3 changes: 1 addition & 2 deletions serverless.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ functions:
S3_PREFIX: ${env:S3_PREFIX, 'packages/'}
# Shared secret that gates both the update-checker route (`get_metadata`/`download`)
# and the Composer routes (`/packages.json`, `/p2/...`, `/dist/...`). When empty or
# unset, the update-checker route is ungated, but the Composer routes return 503
# (they fail closed). Read at deploy time from SSM Parameter Store at
# unset, both are ungated. Read at deploy time from SSM Parameter Store at
# /${self:service}/<stage>/SIMPLE_UPDATE_KEY. Set/rotate it with:
# aws ssm put-parameter --region us-east-1 --name /wp-update-server/<stage>/SIMPLE_UPDATE_KEY --type SecureString --value '<key>'
SIMPLE_UPDATE_KEY: ${ssm:/${self:service}/${self:provider.stage}/SIMPLE_UPDATE_KEY, ''}
Expand Down
10 changes: 7 additions & 3 deletions tests/composer-repository-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,14 @@ function s3_error($status, $code) {
check('not composer: slug that merely starts with p2', $repo->handle('/p2x/', bearer(KEY)) === null);
check('isComposerPath: dist', Wpup_ComposerRepository::isComposerPath('/dist/reason-dev/a/a-1.0.zip'));

// --- fail closed when the key is not configured ---
// --- no key configured: open, like the update-checker route ---
$mock = new MockHandler();
$res = make_repo('', $mock)->handle('/packages.json', bearer('anything'));
check('empty key: 503', $res->status === 503);
$mock->append(new Result(array('Body' => Utils::streamFor('{"packages":{}}'))));
$open = make_repo('', $mock);
check('empty key, no header: 200', $open->handle('/packages.json', new Wpup_Headers(array()))->status === 200);
check('empty key, stray bearer: 200', $open->handle('/packages.json', bearer('anything'))->status === 200);
check('empty key: p2 served', $open->handle('/p2/reason-dev/acme-widget.json', new Wpup_Headers(array()))->status === 200);
check('empty key: unknown path still 404', $open->handle('/p2/', new Wpup_Headers(array()))->status === 404);

// --- authorization ---
$mock = new MockHandler();
Expand Down
Loading