Repository navigation
Add capability grants that reach roles created later - #36
Merged
Merged
Conversation
A plugin had no way to grant its capabilities to roles that exist only on some sites, or that appear after the plugin does, such as WooCommerce's shop_manager. WP_Roles::add_cap() does nothing for a missing role and core fires no action when a role is added, so a grant made once at install either fails or never reaches the role. CapabilityGrantsTest covers the first part of CapabilityGrants: - an invalid option name or grants map throws on construction, which reads, writes and hooks nothing; - register_hooks() runs the grants on wp_loaded at 10; - the first run grants each declared pair to its role, in the stored roles row, to the role's users and in the ledger; - a run with nothing changed makes no query; - a declared role that does not exist is skipped, and its pairs are granted once a later request adds it. The branch starts without an "Update packages" commit: composer outdated --direct and npm outdated were empty, composer packages-update and npm update left both lockfiles unchanged, and the workflows already pin configs trunk 6302f1f. Assisted-by: Claude Code:claude-opus-5-5
A plugin could not grant capabilities to a role that is missing at install and created later, because WP_Roles::add_cap() does nothing for a missing role and core fires no action when a role appears. CapabilityGrants takes an autoloaded ledger option, the grants as lists of capabilities keyed by role, and a logger, and validates the first two on construction without reading or writing anything. register_hooks() adds run() on wp_loaded, which fires after init, where plugins add their roles. run() returns at once when a hash of the declared grants and of the declared roles that exist matches the stored one, so a site with nothing new makes no query. Otherwise it grants each declared capability that its role lacks and the ledger does not list, through WP_Role::add_cap(), and stores the ledger with the new hash. Assisted-by: Claude Code:claude-opus-5-5
CapabilityGrants granted what a plugin declared but never took back a pair the plugin stopped declaring, and kept ledger entries for deleted roles, so a role deleted and created again never received its capabilities. WooCommerce's Reset capabilities tool deletes and recreates shop_manager in one request, which leaves the role names unchanged. CapabilityGrantsTest adds rows for: - a pair the role held before the plugin declared it, which is never ledgered and stays when the declaration drops it; - a ledgered pair the declaration drops, which is revoked, unless it was removed or denied by hand, and leaves the ledger; - a ledgered pair whose role is gone, which leaves the ledger; - a declared pair removed or denied by hand, which stays so when a later run grants another; - a declared role deleted and created again, in a later request or within one request as WooCommerce's tool does, which is granted again; - a deleted undeclared role or another option's write, which leaves the ledger row; - a changed declaration, which grants the added pairs and revokes the dropped ones in one run. The hook row now also expects updated_option at 10. The hand-edit row and the guard row pass already, because a run skips ledgered pairs and nothing yet listens to option writes. Assisted-by: Claude Code:claude-opus-5-5
CapabilityGrants never took back a pair the plugin stopped declaring, and it kept the ledger entries of a deleted role, so the role, once created again, never received its capabilities. A run now first walks the ledger: an entry whose role is gone is forgotten, and a pair no longer declared is revoked when its role still holds it as granted, then forgotten either way. A pair removed or denied by hand stays so, since the run grants only pairs the ledger does not list. register_hooks() also adds forget_deleted_roles() on updated_option at 10. When a write of the site's roles row deletes a declared role, it drops that role's ledger entries and clears the stored hash, so the next run grants the role again. A run alone cannot see this when the role is deleted and recreated within one request, as WooCommerce's Reset capabilities tool does, because the role names stay the same. Assisted-by: Claude Code:claude-opus-5-5
CapabilityGrants ledgered every grant it attempted. A plugin that locks roles, a role write by another request between this request's read and its own, or roles defined in code leave the stored roles row unchanged while the request's copy changes, so the ledger listed grants no site had, and a later run read the missing grant as a hand removal and never made it. A request killed between the grants and the ledger write would leave grants the ledger never revokes, and a log call that throws on wp_loaded would break every request. CapabilityGrantsTest adds rows for: - a grant the roles row does not keep, which is not ledgered, is logged as a warning naming the pair, and is made again on each later request until it persists; - a revocation the row does not keep, which stays ledgered and is made again; - a warning whose logger throws, which reaches the PHP error log without breaking the request; - the ledger row listing a run's grants as pending while the run writes the roles, and none after it; - a run interrupted after that write, which grants its pending pairs, ledgers those its role holds and leaves a denied one; - a missing or malformed ledger row, which reads as an empty ledger; - on a network, a run and a role deletion on one site, which leave the other site's roles and ledger alone. The multisite row and the rows for a row that is not an array or holds malformed grants pass already, because options are per site and the ledger read drops malformed entries. Assisted-by: Claude Code:claude-opus-5-5
CapabilityGrants ledgered every grant it attempted. WP_Roles writes the whole roles row from the request's copy, so a role-locking plugin, another request's role write or roles defined in code leave the row unchanged while the copy changes; the ledger then listed a grant no site had, and a later run read it as a hand removal and never made it. A request killed between the grants and the ledger write left grants no ledger would revoke, and a version-less or future row was read as a ledger. A run now: - writes the grants it is about to make as pending before making them, so the next run ledgers a pending grant its role holds instead of taking it for a hand grant; - re-reads the roles row past the options cache through OptionRows, ledgers only the grants the row holds, and keeps ledgered a revocation the row does not reflect; - for a change the row did not keep, logs a warning naming it and leaves the stored hash empty, so the next request makes it again, writing to the PHP error log when the logger throws; - reads a row that is not a version 1 row as empty. Assisted-by: Claude Code:claude-opus-5-5
The file map did not list Permissions/, convention 7 still asked migration steps to throw on a missing role, which CapabilityGrants now handles, and core's description did not mention capability grants. The file map lists Permissions/ with CapabilityGrants. Convention 7 drops the missing-role clause and says that role capabilities are granted by CapabilityGrants, never by a step. Limitation 12 records when grants act, what a hand change keeps, the deletion while inactive, the lost concurrent role write, and the endless retry where the roles row never keeps a change. Core's manifest, its README and the root README name role capability grants. Assisted-by: Claude Code:claude-opus-5-5
forget_deleted_roles() skipped the write when no ledger row existed yet, a guard no behavior depends on: the row it would write holds an empty ledger and an empty hash, which the next run reads as it reads a missing row. run()'s summary said it did nothing "unless nothing either depends on changed", which reads backwards. The listener now writes whenever a declared role is deleted, and run()'s summary says it acts once the declaration or the declared roles have changed since the last complete run. Assisted-by: Claude Code:claude-opus-5-5
Two one-line mutations of CapabilityGrants passed every row. The guard row added the unrelated option instead of updating it, so updated_option never fired for it and a listener that ignored the option name went unnoticed. The interrupted-run row did not check the log, so treating a pending pair held as denied as a grant to verify, which logs a failed grant, went unnoticed. The guard row now adds the unrelated option and then updates it, and the interrupted-run row asserts that nothing was logged. Assisted-by: Claude Code:claude-opus-5-5
Review found three ways CapabilityGrants loses track of what it granted or overrides a denial: - a pending pair whose role holds it, left by a killed run, was ledgered only while still declared, so a declaration dropped before the next run left the capability granted for good; - a grant that persisted but whose check could not read the roles row was dropped from pending, so the next run took it for a hand grant and never revoked it; - a run read the role objects, which WP_Roles::add_cap() leaves unchanged, so a denial made through WP_Roles earlier in the request read as absent and was overwritten with a grant. CapabilityGrantsTest adds a row for each. Assisted-by: Claude Code:claude-opus-5-5
A run read the role objects, which WP_Roles::add_cap() and remove_cap() leave unchanged, so a denial made through WP_Roles earlier in the request read as absent and was overwritten. A pending pair its role held was ledgered only while still declared, so a declaration dropped before the next run left that grant in place for good. A grant whose check could not read the roles row left pending, so the next run took it for a hand grant. A run now reads each role's capabilities from WP_Roles' roles array, which those calls change and which WordPress stores. Pending pairs that their role holds join the ledgered pairs before revocation, so a dropped one is revoked like any other, and the separate pending branch of the grant loop goes. A grant the roles row does not show stays pending. The class docblock keeps one reason, and the rule for hand changes moves to run(), the member it governs. The tests' ledger reader sorts each role's list, since its order is not behavior. Assisted-by: Claude Code:claude-opus-5-5
ahegyes
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A plugin could not grant its capabilities to roles that appear after it, such as WooCommerce's
shop_manager:WP_Roles::add_cap()does nothing for a missing role and core fires no action when a role is added, so a grant made once at install never reaches them, and nothing took back a grant the plugin stopped declaring.Permissions\CapabilityGrantstakes an autoloaded ledger option, the grants as lists of capabilities keyed by role, and a logger. Onwp_loadedit returns at once, with no query, while a hash of the declaration and of the declared roles that exist is unchanged.updated_optionlistener catches a role deleted and recreated within one request, as WooCommerce's Reset capabilities tool does.AGENTS.md records the file map, that migration steps no longer grant role capabilities, and limitation 12; core's description names role capability grants.
Affected packages
wp-framework-bootstrapwp-framework-sharedwp-framework-corewp-framework-settingswp-framework-woocommerceChecklist
tests/Integration/Core/Permissions/CapabilityGrantsTest.php(35 rows, plus 1 in the multisite run) against real roles: grants, the fast path, hand changes, recreated roles including WooCommerce's reset, writes the roles row does not keep, pending grants after a killed run, malformed ledger rows and per-site isolation.composer quality-checkpasses locally (Unit 204, Integration 1005), pluscomposer test:integration:multisite(30) and PHP 7.4php -loverpackages/bootstrapand the woocommerce uninstall filesREADME.md(the package READMEs stay pointer-form; AGENTS.md carries the file map)