Skip to content

💥 feat(routing): what declares nothing is for administrators (3.0) - #125

Merged
gfazioli merged 5 commits into
v3from
feat/v3-secure-defaults
Oct 6, 2026
Merged

gfazioli merged 5 commits into
v3from
feat/v3-secure-defaults

Conversation

@gfazioli

@gfazioli gfazioli commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

For 3.0, on v3: what declares nothing is for administrators. This is the breaking half of audit S2, S3 and S9. 2.1.2 (#123) shipped the half that changes nothing.

💥 The defaults

What declares nothing Up to 2.x 3.0
A page of config/routes.php or pages/ (no capability key, no capability() method) opened for every logged-in user (read since 2.1.2) needs manage_options
A menu of config/menus.php (no capability); its items inherit it read manage_options
A REST route with no permission_callback public; with a notice since 2.1.2 refuses every request with WordPress's rest_forbidden (rest_authorization_required_code(): 401 for a visitor, 403 when logged in), and the notice says how to make it public

To keep something open, declare it:

  • 'capability' => 'read' for a route page or a menu;
  • a capability() returning 'read' for a pages/ class;
  • 'permission_callback' => '__return_true' for a REST route.

The 14 boilerplates already declare all of these (checked by running the new report on them).

✨ php bones migrate:to-v3 lists them

The command now also lists every page, menu and REST route that declares nothing, one line each, with the file and, for a route, the line. It reads them from the tokens, because including the config files would need WordPress, and it rewrites nothing: who may open a page is the author's call.

Checked on scratch copies of four boilerplates:

  • as shipped: Nothing to change;
  • with every capability and __return_true stripped: every one is listed, and the route that has a callback of its own (/protected) is not.

Tests

  • composer test: 307 tests and 824 assertions, against 287 and 765 on v3 (rebased on 2.1.3).
    • The route, page and REST tests now assert the 3.0 defaults.
    • New: AdminMenuProviderTest (it fails on v3's provider) and MigrateToV3AccessTest.
  • Live, in WPKirk-Developing on wpbones.test, with v3-access-live-smoke.sh:
    • Setup: a subscriber, an administrator and a visitor, against a route, a pages/ class and a menu that declare nothing, and a REST route without a callback, each next to one that declares read or __return_true.
    • master (2.1.2): 7 ✗ of 18. A subscriber opens all three pages, and a visitor gets a 200 from the REST route, with the 2.x wording in the notice.
    • This branch: 18 ✓.

Review

  • Copilot: the account's quota is exhausted.

  • Codex (gpt-6-sol, high), round 1 (0cf2d5e), three P2s, all in the report:

    • it scanned api/ instead of the folder config/api.php names;
    • a config that returns a variable counted as empty;
    • the string permission_callback anywhere in a call counted as declared.
  • Codex, round 2 (973b96b), two P2s: only custom.path names the folder, and only when its value is one whole literal.

  • An independent review (a subagent with a clean context) found more, fixed in a779c5a. The report read some files wrong:

    • a constant or interpolated key;
    • an ABSPATH guard's return [];
    • 'capability' => null and 'permission_callback' => null;
    • an aliased, lower-case or nested Route;
    • capability() in a comment, protected, or with arguments.

    It also found two weak tests. The menu test's spy could not see add_menu_page()'s capability, and the REST status stub returned the value the code would hard-code.

  • Rebased on v3 after 2.1.3: composer test gives 307 tests and 824 assertions, and v3-access-live-smoke.sh 18 ✓.

gfazioli added a commit that referenced this pull request Oct 6, 2026
- the REST folder is the one config/api.php names (api.custom.path), not
  always api/; a path that is not a literal is reported;
- a config/routes.php or config/menus.php that does not return a literal
  array is a review item, never "nothing to change";
- only a key of the options argument counts as a permission_callback, not
  the string anywhere in the call; options that are not a literal array are
  reported.
gfazioli added a commit that referenced this pull request Oct 6, 2026
…tom.path

The REST folder is read from the custom block's own path key, and only
when its value is one whole string literal: another path key earlier in
config/api.php, or a concatenation, made the report scan the wrong folder.
gfazioli added a commit that referenced this pull request Oct 6, 2026
The report said "Nothing to change" where something did:
- a config entry whose key is a constant or an interpolation made the next
  entries' keys count for the previous one: such a file is now unreadable,
  as one that returns a variable is;
- the first return in the file was taken, an ABSPATH guard's included: only
  the file's own top-level return counts, and only one;
- 'capability' => null or '' counted as declared (the providers fall back to
  manage_options), and so did 'permission_callback' => null;
- a Route imported under another name, written in lower case, or nested in
  another call's arguments was not read; an attribute in the arguments made
  a declared route look undeclared;
- a pages/ capability() was found by a regex on the raw text: in a comment
  it counted, a protected one or one with required arguments counted, an
  implicitly public one did not. Read from the tokens now.

Tests: the menu spy kept one capability per slug, so the first item hid the
menu's own (a hard-coded 'read' in add_menu_page() passed); it keeps both,
and the post-type branch has a test. The REST status stub returns 418, so a
hard-coded 401 would fail.
Breaking, for 3.0 (audit S2, S3, S9):
- a page of config/routes.php or of pages/ without a capability asks for
  manage_options; it asked for read (2.1.2), and before that for nothing;
- a menu of config/menus.php without a capability asks for manage_options,
  and its items with it; it asked for read;
- a REST route without a permission_callback refuses every request with
  WordPress's rest_forbidden (401 or 403), and the notice says how to make it
  public; up to 2.x it was public.

A page, menu or route meant for every logged-in user, or for everyone, says so:
'capability' => 'read', capability() returning 'read', or
'permission_callback' => '__return_true'.
The pages of config/routes.php and pages/, the menus of config/menus.php
and the REST routes of api/ that declare no capability or permission_callback,
one line each with the file (and the line, for a route), read from the
tokens. Nothing is rewritten: who may open a page is the author's call.
- the REST folder is the one config/api.php names (api.custom.path), not
  always api/; a path that is not a literal is reported;
- a config/routes.php or config/menus.php that does not return a literal
  array is a review item, never "nothing to change";
- only a key of the options argument counts as a permission_callback, not
  the string anywhere in the call; options that are not a literal array are
  reported.
…tom.path

The REST folder is read from the custom block's own path key, and only
when its value is one whole string literal: another path key earlier in
config/api.php, or a concatenation, made the report scan the wrong folder.
The report said "Nothing to change" where something did:
- a config entry whose key is a constant or an interpolation made the next
  entries' keys count for the previous one: such a file is now unreadable,
  as one that returns a variable is;
- the first return in the file was taken, an ABSPATH guard's included: only
  the file's own top-level return counts, and only one;
- 'capability' => null or '' counted as declared (the providers fall back to
  manage_options), and so did 'permission_callback' => null;
- a Route imported under another name, written in lower case, or nested in
  another call's arguments was not read; an attribute in the arguments made
  a declared route look undeclared;
- a pages/ capability() was found by a regex on the raw text: in a comment
  it counted, a protected one or one with required arguments counted, an
  implicitly public one did not. Read from the tokens now.

Tests: the menu spy kept one capability per slug, so the first item hid the
menu's own (a hard-coded 'read' in add_menu_page() passed); it keeps both,
and the post-type branch has a test. The REST status stub returns 418, so a
hard-coded 401 would fail.
@gfazioli
gfazioli force-pushed the feat/v3-secure-defaults branch from 4f086eb to a779c5a Compare October 6, 2026 14:03
@gfazioli
gfazioli merged commit a779c5a into v3 Oct 6, 2026
4 checks passed
@gfazioli
gfazioli deleted the feat/v3-secure-defaults branch October 6, 2026 14:04
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.

1 participant