fix(security): stop register_meta leaking secret post meta over REST#583
Merged
Conversation
register_meta() registered every BeyondWords post-meta key with
show_in_rest => true. WordPress returns show_in_rest meta in the public
`view` context with no capability check, so anonymous
GET /wp/v2/posts/:id leaked the legacy per-post `speechkit_access_key`,
BeyondWords API error messages, the audio preview token and legacy
player URLs/text on every compatible post.
Register only the keys the block editor reads/writes over REST (the
current keys plus the six deprecated keys it still reads for
SpeechKit-upgrade back-compat) with show_in_rest => true; register every
other deprecated key — including speechkit_access_key — with
show_in_rest => false so it never reaches the REST API. All keys stay
registered, so write sanitisation and Custom-Fields hiding are
unchanged.
The sensitive keys the editor genuinely needs in the authenticated
`edit` context (error messages, preview token, legacy player link) are
additionally stripped from non-`edit` responses by a new
rest_prepare_{post_type} filter, so they stay available to editors but
not to anonymous visitors.
Verified with `npm run phpcs` (WordPress-VIP-Go, no phpcs:ignore) and
the SyncTest PHPUnit suite (register_meta, init, and a new
register_meta_does_not_expose_private_meta_to_the_public test).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
galbus
marked this pull request as ready for review
July 19, 2026 07:15
gouravkhunger
approved these changes
Jul 19, 2026
Contributor
|
@galbus there's a merge conflict on this one. |
…0e9f Resolves a conflict in src/post/class-sync.php: main's fix(sync) (2be1ca0) added DELETE_AUDIO_CRON_HOOK immediately after GENERATE_AUDIO_CRON_HOOK, the same spot where this branch added REST_LEGACY_META_KEYS and REST_PRIVATE_META_KEYS. Kept both sides, with the new cron-hook constant next to its GENERATE_AUDIO_CRON_HOOK sibling and the REST constants after it. init() merged cleanly and registers both the delete-audio cron action and the rest_api_init meta-visibility action. Also reset the current user before deleting it in register_meta_does_not_expose_private_meta_to_the_public, so the test no longer leaves the global pointing at a deleted user row. Verified after merge: phpcs clean on class-sync.php; SyncTest passes (80 tests, 185 assertions) across repeated runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@gouravkhunger Conflict resolved — merged latest 🤖 Addressed by Claude Code |
galbus
added a commit
that referenced
this pull request
Jul 19, 2026
…a911 Conflict in src/editor/classic/class-metabox.php between main's stored-XSS fix for the classic player (#583) and this branch's poll-before-embed. Resolved by keeping both, split per integration path: - REST API (has a Content ID): keeps this branch's loading state. The player is embedded by classic-metabox.js only once GET /content reports `processed`, so no inline `onload` is emitted at all. The Content ID and preview token ride in esc_attr()'d data-* attributes and are read back with getAttribute(), so neither is interpolated into JavaScript source — this removes the XSS vector rather than escaping around it. - Client-side (Magic Embed, no Content ID): nothing to poll, so it still embeds inline and keeps main's JSON-encoded (HEX-flagged) + esc_attr()'d onload verbatim. test-metabox.php: player_embed_neutralises_content_id_xss() now asserts the crafted Content ID stays a data attribute with no onload to break out of, and a new player_embed_json_encodes_the_client_side_config() keeps main's JSON-encoding mechanism under test on the path that still uses it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gouravkhunger
self-requested a review
July 19, 2026 07:51
gouravkhunger
approved these changes
Jul 19, 2026
…0e9f Resolves conflicts with main's fix (54a3364) that validates the Content ID charset, which landed in the same two places as this branch's REST exposure fix: - src/post/class-sync.php: main added a per-key sanitize_callback override for beyondwords_content_id inside register_meta(), built off the old per-post_type $options array that this branch replaced with an inline per-key array (to compute show_in_rest per key). Combined both: the options array is still built per key with the show_in_rest split, and sanitize_callback now resolves to Meta::sanitize_content_id() for beyondwords_content_id and sanitize_text_field otherwise. - tests/phpunit/post/test-sync.php: both sides appended a new test right after register_meta(). Kept both (register_meta_does_not_expose_private_meta_to_the_public and register_meta_sanitizes_content_id_on_write). Verified after merge: phpcs clean on class-sync.php; SyncTest passes (81 tests, 187 assertions), including both new register_meta tests. Co-Authored-By: Claude Opus 4.8 <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
2 participants
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.
Problem
Sync::register_meta()registered every BeyondWords post-meta key — viaUtils::get_post_meta_keys('all')(current and deprecated) — with'show_in_rest' => true.WordPress returns
show_in_restmeta in theviewcontext with no capability check (WP_REST_Meta_Fields::get_value()performs no auth;auth_callbackonly gates writes). So an unauthenticatedGET /wp/v2/posts/<id>(or the posts collection) returned, in themetaobject of every compatible published post:speechkit_access_key— a legacy per-post API credential from the v2.x SpeechKit era (on sites upgraded from SpeechKit)beyondwords_error_message/speechkit_error_message— internal BeyondWords API error stringsbeyondwords_preview_token— the audio preview token_speechkit_link/_speechkit_text— legacy player URLs and article textspeechkit_statusand other internal rowsThis is a public information-disclosure bug present on every install.
Fix
Two layers, both in
src/post/class-sync.php:1. Register
show_in_restper key, not blanket-true.Only the keys the block editor actually reads/writes over REST get
show_in_rest => true: the 16currentkeys, plus six deprecated keys the editor still reads for back-compat on SpeechKit-upgraded posts (REST_LEGACY_META_KEYS— error/pending notices, play/preview, the Generate-audio toggle and the legacy player link all depend on them). Every other deprecated key — includingspeechkit_access_key,_speechkit_text, cached API responses — is nowshow_in_rest => falseand never reaches REST at all. All keys stay registered, so write-sanitisation and Custom-Fields-panel hiding (is_protected_meta()) are unchanged.2. Strip the still-needed sensitive keys from public responses.
The keys the editor genuinely needs in the authenticated
editcontext but which must not leak publicly (REST_PRIVATE_META_KEYS:beyondwords_error_message,beyondwords_preview_token,speechkit_error_message,_speechkit_link) are removed from every non-editresponse by a newrest_prepare_{post_type}filter (hide_private_meta_from_rest()). Theeditcontext is itself permission-gated by the posts controller, so editors keep full access; anonymous visitors get nothing.Why not the simpler "current → true, deprecated → false" split?
Two reasons it wouldn't work:
beyondwords_error_messageandbeyondwords_preview_tokenare current keys but still sensitive — a pure current/deprecated split leaves them exposed.editcontext (e.g.error-notice/check.js,open-sidebar/index.js,play-audio,pending-notice), so blanket-hiding all deprecated keys would regress editing of legacy posts.The strip layer resolves both: sensitive keys stay available to authenticated editors, hidden from the public.
Verification
npm run phpcs(WordPress-VIP-Go): PASS, nophpcs:ignoreadded.SyncTest: PASS —register_meta,init, and a newregister_meta_does_not_expose_private_meta_to_the_public(3 tests, 54 assertions) run in the real WP test harness.view, non-sensitive keys stay publicly readable, and an editor still sees the private keys viacontext=edit.Reviewer notes
beyondwords_content_id,beyondwords_project_id, and the four non-secret legacy IDs/flags) remain publicly readable — unchanged in sensitivity from before and useful for headless integrations. Only the flagged secret/internal keys are removed or stripped.speechkit_access_keyis now completely absent from REST (bothviewandedit) — editors never needed it there; the admin Inspect panel reads it server-side viahas_meta().