Plugin Directory: Block a release when a security scan reports a high risk score#729
Draft
obenland wants to merge 3 commits into
Draft
Plugin Directory: Block a release when a security scan reports a high risk score#729obenland wants to merge 3 commits into
obenland wants to merge 3 commits into
Conversation
The test environment was missing three things the development environment already had, which between them made whole classes of test impossible: - `update_source` was never created, so anything touching the update API's table failed. - `wp-content/env-bin` was not mapped, so `database-tables.sql` was not reachable from the test container to import. - `PLUGINS_TABLE_PREFIX` was undefined, so `API\Base::load_routes()` fatalled and no `plugins/v1` route was registered. Shared verbatim with WordPress#717; whichever of the two PRs lands second should drop this commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UNnLdnE7etihGNvsWY1qGE
Reviewers could already skip the release cooldown, but had no way to hold a release back short of closing the whole plugin. Add a Block control to the Controls metabox, alongside the force-release one and sharing a single reason field, so a version still inside its cooldown can be held out of `update_source` until it's force-released. - API_Update_Updater::block_release() is the counterpart to force_release(): it records a `release_block` on the release row, and a single gate in update_single_plugin() honours it, so both the deferred cooldown cron and the backup reconciliation cron keep the version out of `update_source`. - A version that's already live can't be un-shipped, so a block on one is refused rather than recorded as a hold that never takes effect. - force_release() clears the block and notes the override in the audit log. Holding a version back means update_single_plugin() can't rewrite the row at all, since every version-specific column describes the held version. That was already true of the cooldown, and it means a status change never reached sites while one was in flight: closing a plugin whose next version was deferred left `update_source` serving the current version with `available = 1`. A block makes that unbounded rather than a few hours, so sync_availability() now writes the availability and closure fields on their own and leaves the served version alone. Covered by Reviewer_Release_Block_Test, which exercises the block through the same path the metabox uses, including closing and re-opening a plugin while a version is held. Its fixture lives in Release_Block_Test_Case, resolved by a small autoloader in the test bootstrap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… risk score. Builds on the reviewer release block: when a Gandalf scan reports a high `risk_score` for a plugin version still inside its release cooldown, that version is held out of the update API indefinitely — the previously served version keeps being served — until a reviewer force-releases it. - Threshold: Plugin_Scan_Gandalf::RISK_SCORE_BLOCK_THRESHOLD (default 8). A completed scan at or above it blocks; below, or absent, it does not. - Precedence: block (risk >= threshold) -> findings alert (existing) -> clean. - Only within cooldown: a verdict for a version that is already live, was superseded by a newer commit mid-scan, or had no delay can't un-ship anything. It's escalated to Slack for manual action instead of held. - force_release() and the Controls metabox name the risk score when a scan set the block, rather than describing it as a reviewer's. - The findings and block Slack alerts now share one sender, and their user-facing wording says "security scan" rather than naming the scanner. Auto-blocking on a risk score is left disabled, so this ships independently of any upstream Gandalf work: a high score alerts Slack, but a human decides. The tests covering that path are skipped rather than removed, and the call is one line to restore. Gandalf does not send a `risk_score` yet either, so the field is read defensively and the feature stays inert until it does. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Adds the scan-driven half of the release block: when a Gandalf scan reports a high
risk_scorefor a plugin version still inside its release cooldown, that version is held out of the update
API indefinitely — the previously served version keeps being served — until a reviewer
force-releases it.
This is the second half of #720, which is now split in two. It builds on #728, which adds the
blocking mechanism itself and the reviewer control.
Important
Stacked on #728. GitHub can't base a fork PR on another fork branch, so this PR's diff
includes #728's commits. Review only the last commit —
Block a release when a security scan reports a high risk score. This should merge after #728.How it works
Plugin_Scan_Gandalf::RISK_SCORE_BLOCK_THRESHOLD(default8). A completedscan at or above it blocks; below, or absent, it does not.
newer commit mid-scan, or had no delay can't un-ship anything; it's escalated to Slack for
manual action instead of held.
force_release()and the Controls metabox name the risk score when a scanset the block, rather than describing it as a reviewer's.
user-facing wording says "security scan" rather than naming the scanner.
What ships inert
Auto-blocking on a risk score is disabled. A high score alerts Slack; a human decides. The
call is one commented-out line in
handle_callback()to restore.Gandalf doesn't send
risk_scoreyet either, so the field is read defensively and the path staysinert until both change.
Tests
Gandalf_Risk_Score_Block_Testdriveshandle_callback()directly for the handler's branches.Gandalf_Risk_Score_Block_Callback_Testcovers the full chain over the REST route — auth,routing, the write to
update_source, force-release closing the loop, the block outlastingthe cooldown.
Release_Block_Test_CaseviaGandalf_Scan_Test_Case→Gandalf_Callback_Test_Case.190 tests pass locally, 8 skipped.
Note
Because auto-blocking is off, the 8 tests that assert a hold are skipped, and several of the
remaining
..._does_not_blocktests pass vacuously — nothing blocks, so "it didn't block"is not evidence the threshold logic works. Real coverage of this path only exists once the
commented-out call is restored. Flagging it so the green checkmark isn't read as more than
it is.
Notes for reviewers
risk_scoreshould join it as an optional{ "type": "number", "minimum": 0 }. It's read defensively here because trunk's routevalidates none of the callback body.
🤖 Generated with Claude Code