Skip to content

Plugin Directory: Block a release when a security scan reports a high risk score - #777

Open
obenland wants to merge 4 commits into
WordPress:trunkfrom
obenland:feature/gandalf-risk-score-suspension
Open

Plugin Directory: Block a release when a security scan reports a high risk score#777
obenland wants to merge 4 commits into
WordPress:trunkfrom
obenland:feature/gandalf-risk-score-suspension

Conversation

@obenland

@obenland obenland commented Aug 10, 2026

Copy link
Copy Markdown
Member

Security scan callbacks now include a max_risk_score (0–10) and a bounded findings array. This builds on the advisory integration and acts on that evidence.

Builds on #779. Supersedes the scan-driven half of #720/#729.

What it does

  • A completed scan with max_risk_score >= 8.0 (Plugin_Scan_Gandalf::BLOCK_RISK_SCORE, filterable) blocks the scanned release via Plugin Directory: Add an API to block a release from being served #785's API_Update_Updater::block_release(), regardless of the plugin's install count: the version is held out of the update API — the previously served version keeps being served — until a reviewer force-releases it. The plugin itself stays published; nothing closes.
  • A verdict that can't un-ship anything — the version is already being served, or a newer release superseded it mid-scan — blocks nothing and stays advisory; the alert still carries the risk score for manual action.
  • The review team receives the findings as context: an internal note on the plugin (bold risk score and title per finding, file location on the next line), a Slack alert, and an evidence snapshot in post meta bounded to the ten highest-risk findings (all that ever surface). Blocks always alert; below-threshold results stay advisory and deduplicate as before.

Callback handling

  • Contract validation lives in Plugin Directory: Validate the security scan callback contract at the route #787; the reported max_risk_score is trusted as-is — the scanner is authoritative for it.
  • Consumed callbacks are recorded per scan_id under a canonical (key-order-insensitive) digest: an identical retry is acknowledged without repeating effects, a different body for a consumed scan is rejected as a conflict, and a completed verdict supersedes an earlier failure report for the same scan — the pending entry survives a failure for exactly that reason. A short per-plugin lock (released even if processing throws) prevents concurrent callbacks from double-processing or clobbering each other's records.
  • Finding strings are treated as untrusted and escaped per output context: esc_html() for the wp-admin note, htmlspecialchars( …, ENT_NOQUOTES ) for Slack (the idiom of the existing Slack integrations — Slack only decodes &/</> entities).

Testing

tests/Security_Scan_Block_Test.php (20 tests) covers the threshold boundary, replay and conflict handling (including a re-marshalled retry with different key order), a completed verdict superseding a failure report, blocking refused for served and superseded versions, blocking independent of install count, a reported score blocking even without findings detail, the snapshot bound, escaping of hostile finding strings, the row holding the served version through a scan-driven block, and force-release closing the loop.

🤖 Generated with Claude Code

@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch 11 times, most recently from 720e3d1 to eae73fc Compare August 10, 2026 20:34
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch 4 times, most recently from 9f85496 to e9fa87e Compare August 10, 2026 21:38
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch 3 times, most recently from 05834de to c284616 Compare August 11, 2026 14:38
@obenland obenland changed the title Plugin Directory: Suspend a plugin when a security scan reports a high risk score Plugin Directory: Block a release when a security scan reports a high risk score Aug 11, 2026
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch 2 times, most recently from b1e5832 to db30b40 Compare August 11, 2026 20:40
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch 4 times, most recently from dd5c180 to 2429313 Compare August 12, 2026 13:07
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch 2 times, most recently from 8cd0daa to 1ebec61 Compare August 12, 2026 18:47
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch from 1ebec61 to 374f9c4 Compare August 13, 2026 18:49
@obenland
obenland marked this pull request as ready for review August 13, 2026 20:37
Copilot AI lite review requested due to automatic review settings August 13, 2026 20:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props obenland.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

… risk score.

Security scan callbacks now carry a max_risk_score and a bounded findings
array. A completed scan at or above the threshold blocks the scanned
release pending review — the previously served version keeps being
served — and the plugin review team receives the findings as context via
an internal note, a Slack alert, and a stored evidence snapshot. A verdict
for a version already served, or superseded by a newer release, can't
un-ship anything and stays advisory.

Scanner retries are acknowledged idempotently under a canonical digest:
an identical retry repeats no effects, and a completed verdict supersedes
an earlier failure report for the same scan. Finding fields beyond the
risk score are optional per the callback contract and read defensively.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@obenland
obenland force-pushed the feature/gandalf-risk-score-suspension branch from d31aad9 to 70622ec Compare August 14, 2026 19:11
obenland and others added 3 commits August 14, 2026 14:47
…route.

The callback contract has two halves that `required` cannot express, being
unconditional: a completed scan reports a verdict, a failed one reports an
error. Neither was checked, so a payload missing its half reached code that
indexes it directly. A completed callback without `findings` fatalled before
the delivery was acknowledged, leaving the scanner to retry it forever and
the release unblocked; one without `max_risk_score` recorded a fabricated
score of zero. The status validator now rejects both with a 400 the scanner
can act on.

A verdict at or above the threshold also alerts when the release can't be
held — already served, or superseded — instead of passing silently whenever
it carried no findings of its own. The stored evidence snapshot is slashed
so backslashes in finding text survive `update_post_meta()`, and the review
note reports the risk score to the same precision as the Slack alert.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing reads `_gandalf_scan_last_result`. The findings reach reviewers
through the internal note and the Slack alert, and the release hold carries
the scan ID and risk score it needs on the release row itself, so the meta
was written on every completed callback and never surfaced. Its findings
were bounded and stripped of snippets solely to keep that row small, which
the display paths already do for themselves at five and ten.

The alert gate goes back to keying on the findings count: a clean scan
reports a maximum risk score of zero, so a verdict cannot reach the block
threshold without findings behind it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A completed verdict was matched against the plugin's Version header before
its release was held, which is the lookup WordPress#807 replaced: the header is
author-controlled, so committing a rename into the scanned tag made the
scan's version no longer the plugin's and the block was refused. Blocking
now resolves the current release the way the updater does, from the stable
tag, and applies only when that release is the one the verdict examined.

The test environment gains WP_GANDALF_SCAN_SHARED_SECRET so the callback
route can be driven by hand; the PHPUnit suite defines it itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants