Skip to content

Require a nonce to change Story Budget filters#985

Merged
GaryJones merged 1 commit into
developfrom
vipplug-19-story-budget-csrf
Jun 7, 2026
Merged

Require a nonce to change Story Budget filters#985
GaryJones merged 1 commit into
developfrom
vipplug-19-story-budget-csrf

Conversation

@GaryJones

Copy link
Copy Markdown
Contributor

Summary

The Story Budget's filter controls change state over GET. The Reset link and the filter form both write the current user's saved filter preferences to user meta, and update_user_filters() persisted whatever was in $_GET on every render — none of it nonce-protected. A crafted link could therefore permanently change a victim's saved Story Budget filters: a self-targeted, cosmetic CSRF, but a state change without intent all the same.

The Reset link now carries a nonce that handle_filter_reset() verifies before writing, mirroring the existing date-range form. The filter form carries a nonce too, and update_user_filters() only persists to user meta when the form was actually submitted with a valid nonce. A crafted URL can still filter that single page view, but it can no longer alter what the user has saved.

Removing the persist-on-every-render behaviour surfaced a latent assumption: update_user_filters_from_form_date_range_change() read the stored filters and wrote date keys onto them, but get_user_meta() returns an empty string when nothing is stored, which throws on PHP 8 when indexed. It now treats absent filters as an empty array.

Fixes VIPPLUG-19.

Test plan

  • composer test:integration -- --filter test_filters_persist_only_with_valid_nonce — confirms filters are not persisted without a valid nonce and are persisted with one.
  • Existing Story Budget filter tests still pass.
  • PHPCS and php -l clean.
  • Manual: change filters and Reset on the Story Budget; confirm they still work and persist; confirm a filter/reset URL without a current nonce does not change your saved view.

Reviewer note: the reset and date-range handlers exit/redirect, so they aren't unit-tested directly; the persistence-gating (the substantive fix) is covered by the new test.

@GaryJones GaryJones requested a review from a team as a code owner June 7, 2026 10:00
@GaryJones GaryJones added this to the Next milestone Jun 7, 2026
@GaryJones GaryJones self-assigned this Jun 7, 2026
@GaryJones GaryJones merged commit 00f3341 into develop Jun 7, 2026
10 checks passed
@GaryJones GaryJones deleted the vipplug-19-story-budget-csrf branch June 7, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant