From ff761916d0b20537bc6b2edfad82580324319cf2 Mon Sep 17 00:00:00 2001
From: Utkarsh Patel <itismeutkarsh@gmail.com>
Date: Thu, 30 Jul 2026 13:44:03 +0530
Subject: [PATCH 1/5] fix: enforce network boundaries on multisite record reads
 and settings writes

Addresses authorization findings from a security review of v4.3.0.

Multisite record isolation (Network):
Network::network_query_args() preserved any numeric blog_id coming from the
request, but a numeric type check is not an authorization check and the Stream
tables are shared across the network -- a site-level view_stream user could read
another site's activity with ?blog_id=N. Requested blog IDs are now honoured only
for users with manage_network_options; everyone else is pinned to the current
blog. The read abilities (get-record, get-records, purge-records) already carried
equivalent guards; this closes the legacy list-table and AJAX path.

ajax_network_admin() also derived network-admin authority purely from an
HTTP_REFERER prefix, which is caller-controlled. That let a site user lift the
per-blog query restriction and, via blog_id_logged(), record their actions
against blog_id 0 and corrupt site attribution. The Referer is now treated as a
UI hint only and must be accompanied by a real network capability. WP-CLI is
exempt since it has shell-level access and normally no logged-in user.

Network-wide settings writes (Ability):
update-settings and create-exclusion-rule inherited the default permission
callback (WP_STREAM_SETTINGS_CAPABILITY, i.e. manage_options) while
Settings::update_all_setting_values() routes to update_site_option() on
network-activated installs. A site administrator could therefore change
network-wide retention, role access, and audit exclusion rules. Both now use
Ability::can_write_settings(), which additionally requires
manage_network_options when the write will be network-scoped.

GHCR publishing (docker-images.yml):
The publish step was guarded by contains(github.ref_name, 'master'), which also
matches unprotected branch names such as feature-master-publish. Replaced with
an exact ref comparison.

This guards against an accidental publish only. The condition is read from the
pushed ref's own copy of the workflow, so someone with write access could still
edit it on their own branch; closing that requires a deployment branch policy
configured in repository settings, which is outside this change.
---
 .github/workflows/docker-images.yml           |  2 +-
 .../class-ability-create-exclusion-rule.php   | 13 ++++++
 abilities/class-ability-update-settings.php   | 12 ++++++
 classes/class-ability.php                     | 34 +++++++++++++++
 classes/class-network.php                     | 43 +++++++++++++++++++
 5 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/docker-images.yml b/.github/workflows/docker-images.yml
index 25e8241e5..41f564d23 100644
--- a/.github/workflows/docker-images.yml
+++ b/.github/workflows/docker-images.yml
@@ -36,5 +36,5 @@ jobs:
         run: docker compose --file docker-compose.build.yml build
 
       - name: Publish images
-        if: contains( github.ref_name, 'master' )
+        if: github.ref == 'refs/heads/master'
         run: docker buildx bake --file docker-compose.build.yml --push --set '*.platform=linux/amd64,linux/arm64'
diff --git a/abilities/class-ability-create-exclusion-rule.php b/abilities/class-ability-create-exclusion-rule.php
index a8d429e09..2a5350e63 100644
--- a/abilities/class-ability-create-exclusion-rule.php
+++ b/abilities/class-ability-create-exclusion-rule.php
@@ -32,6 +32,19 @@ public function get_name() {
 		return 'stream/create-exclusion-rule';
 	}
 
+	/**
+	 * Exclusion rules suppress future audit records. On a network-activated
+	 * install the rule is written to the network option and therefore applies
+	 * across every site, so require a network capability in that case.
+	 *
+	 * @param array $input Input that will be passed to execute().
+	 * @return bool
+	 */
+	public function permission_callback( $input = array() ) {
+		unset( $input );
+		return $this->can_write_settings();
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
diff --git a/abilities/class-ability-update-settings.php b/abilities/class-ability-update-settings.php
index 5ef486db5..96d0d4dd0 100644
--- a/abilities/class-ability-update-settings.php
+++ b/abilities/class-ability-update-settings.php
@@ -19,6 +19,18 @@ public function get_name() {
 		return 'stream/update-settings';
 	}
 
+	/**
+	 * Writes land on the network option when Stream is network activated, so
+	 * require a network capability in that case.
+	 *
+	 * @param array $input Input that will be passed to execute().
+	 * @return bool
+	 */
+	public function permission_callback( $input = array() ) {
+		unset( $input );
+		return $this->can_write_settings();
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
diff --git a/classes/class-ability.php b/classes/class-ability.php
index 9029be4c8..95b793707 100644
--- a/classes/class-ability.php
+++ b/classes/class-ability.php
@@ -96,6 +96,40 @@ public function permission_callback( $input = array() ) {
 		return current_user_can( WP_STREAM_SETTINGS_CAPABILITY );
 	}
 
+	/**
+	 * Permission check for abilities that persist Stream settings.
+	 *
+	 * On network-activated multisite, Settings::update_all_setting_values()
+	 * writes the authoritative network option (wp_stream_network), so the write
+	 * affects every site on the network. WP_STREAM_SETTINGS_CAPABILITY defaults
+	 * to 'manage_options', which every single-site administrator holds -- on its
+	 * own it is not sufficient authority for a network-wide change. Require a
+	 * network capability whenever the write is going to be network-scoped.
+	 *
+	 * The network capability is required in addition to the settings
+	 * capability, not instead of it: on multisite a super admin passes every
+	 * capability check by definition, so returning the network check alone
+	 * would silently ignore a deployment that narrowed
+	 * WP_STREAM_SETTINGS_CAPABILITY.
+	 *
+	 * @return bool
+	 */
+	protected function can_write_settings() {
+		$can_write_settings = current_user_can( WP_STREAM_SETTINGS_CAPABILITY );
+
+		// Mirrors the branch in Settings::update_all_setting_values(): when the
+		// write lands on the network option it additionally requires network
+		// authority. This is an extra requirement, never a substitute -- the
+		// settings capability is a site-overridable constant, so a deployment
+		// that restricts or revokes it must keep being honoured for super
+		// admins too.
+		if ( is_multisite() && $this->plugin->is_network_activated() ) {
+			return $can_write_settings && current_user_can( 'manage_network_options' );
+		}
+
+		return $can_write_settings;
+	}
+
 	/**
 	 * Annotation flags for the ability (readonly, destructive, idempotent).
 	 *
diff --git a/classes/class-network.php b/classes/class-network.php
index c1133ed33..402868989 100644
--- a/classes/class-network.php
+++ b/classes/class-network.php
@@ -92,6 +92,8 @@ public function ajax_network_admin() {
 			DOING_AJAX
 			&&
 			0 === stripos( $http_referer, network_admin_url() )
+			&&
+			$this->can_view_network_records()
 		) {
 			define( 'WP_NETWORK_ADMIN', true );
 			return WP_NETWORK_ADMIN;
@@ -100,6 +102,34 @@ public function ajax_network_admin() {
 		return false;
 	}
 
+	/**
+	 * Whether the current user is allowed to read records across the whole
+	 * network (and to be treated as being in the Network Admin).
+	 *
+	 * The Referer prefix checked in ajax_network_admin() is caller-controlled,
+	 * so it can only ever be a UI hint about where a request came from -- never
+	 * a source of authority. Network-wide record access additionally requires a
+	 * real network capability, otherwise a site-level Stream viewer could spoof
+	 * the header to lift the per-blog restriction applied in
+	 * network_query_args() or to have their actions logged against blog_id 0.
+	 *
+	 * @return bool
+	 */
+	public function can_view_network_records() {
+		if ( ! is_multisite() ) {
+			return false;
+		}
+
+		// WP-CLI runs with shell-level access and usually with no logged-in
+		// user, so capability checks would fail for a legitimate operator and
+		// break `wp stream query --blog_id=N`. It sits outside this boundary.
+		if ( defined( 'WP_CLI' ) && WP_CLI ) {
+			return true;
+		}
+
+		return current_user_can( 'manage_network_options' );
+	}
+
 	/**
 	 * Builds a stdClass object used when displaying actions done in network administration
 	 *
@@ -501,6 +531,19 @@ public function blog_id_logged( $blog_id ) {
 	 */
 	public function network_query_args( $args ) {
 		$args['site_id'] = is_numeric( $args['site_id'] ) ? $args['site_id'] : get_current_site()->id;
+
+		// Only users with a network capability may choose which blog to read
+		// from. For everyone else the requested blog_id is ignored entirely and
+		// forced to the current blog: a numeric type check is not an
+		// authorization check, and the Stream tables are shared across the
+		// whole network, so honouring an arbitrary ?blog_id= would let a
+		// site-level viewer read another site's activity.
+		if ( ! $this->can_view_network_records() ) {
+			$args['blog_id'] = get_current_blog_id();
+
+			return $args;
+		}
+
 		$args['blog_id'] = is_numeric( $args['blog_id'] ) ? $args['blog_id'] : ( is_network_admin() ? null : get_current_blog_id() );
 
 		return $args;

From 8e9207e7a8fcdad708c049184ac130612c038e6d Mon Sep 17 00:00:00 2001
From: Utkarsh Patel <itismeutkarsh@gmail.com>
Date: Thu, 30 Jul 2026 15:03:38 +0530
Subject: [PATCH 2/5] test: cover multisite record isolation and network
 settings authorization

Adds regression coverage for the boundaries hardened in the previous commit.
WP_Stream\Network previously had almost no direct coverage (4.76% of methods),
which is how the can_write_settings() either/or bug below went unnoticed
through two full green test runs.

tests/phpunit/test-class-network.php (new):
- A site administrator and a subscriber both have a caller-supplied blog_id
  discarded in favour of the current blog.
- A super admin still gets cross-site filtering, so Network Admin is unaffected.
- site_id keeps its existing default.
- can_view_network_records() is false for a site administrator and true for a
  super admin, so a spoofed Referer alone cannot establish network context.
- blog_id_logged() keeps site attribution for a site user.

test-class-ability-update-settings.php:
- A site administrator cannot write network-wide settings.
- A super admin is denied when the Stream settings capability is revoked. This
  pins the AND semantics of can_write_settings(): the network capability is an
  additional requirement, not a substitute. Note WP_User::has_cap() returns
  early for super admins before the user_has_cap filter runs, so the test uses
  map_meta_cap/do_not_allow -- the one restriction that early return honours.

Each test was verified to fail against the unfixed code and pass against the
fix. Both suites: 401 tests, 0 failures; skipped/incomplete counts unchanged
from the develop baseline.
---
 .../test-class-ability-update-settings.php    |  81 +++++++
 tests/phpunit/test-class-network.php          | 198 ++++++++++++++++++
 2 files changed, 279 insertions(+)
 create mode 100644 tests/phpunit/test-class-network.php

diff --git a/tests/phpunit/abilities/test-class-ability-update-settings.php b/tests/phpunit/abilities/test-class-ability-update-settings.php
index a0024c37b..814dbecbe 100644
--- a/tests/phpunit/abilities/test-class-ability-update-settings.php
+++ b/tests/phpunit/abilities/test-class-ability-update-settings.php
@@ -48,6 +48,87 @@ public function test_permissions() {
 		$this->assertTrue( $this->ability->permission_callback() );
 	}
 
+	/**
+	 * The update_all_setting_values() helper writes the network option when
+	 * Stream is network activated, so the write hits every site. A plain site
+	 * administrator holds manage_options but has no network authority and must
+	 * be refused.
+	 *
+	 * @group ms-required
+	 */
+	public function test_site_admin_cannot_write_network_settings() {
+		if ( ! is_multisite() ) {
+			$this->markTestSkipped( 'Requires multisite.' );
+		}
+
+		wp_set_current_user( $this->admin_user_id );
+		$this->assertFalse(
+			is_super_admin( $this->admin_user_id ),
+			'Guard: this test is only meaningful for a non-super-admin.'
+		);
+
+		add_filter( 'wp_stream_is_network_activated', '__return_true' );
+
+		try {
+			$this->assertFalse(
+				$this->ability->permission_callback(),
+				'A site administrator must not be able to write network-wide Stream settings.'
+			);
+		} finally {
+			remove_filter( 'wp_stream_is_network_activated', '__return_true' );
+		}
+	}
+
+	/**
+	 * The network capability is an additional requirement, not a replacement
+	 * for the Stream settings capability. On multisite a super admin passes
+	 * every current_user_can() check by definition, so a permission callback
+	 * that only consulted manage_network_options would silently ignore a
+	 * deployment that locked Stream settings down.
+	 *
+	 * WP_User::has_cap() returns early for super admins, before the
+	 * user_has_cap filter runs, so the only way a deployment can restrict them
+	 * is 'do_not_allow' via map_meta_cap -- which is exactly the escape hatch
+	 * that early return honours. That is what is simulated here.
+	 *
+	 * @group ms-required
+	 */
+	public function test_super_admin_denied_when_settings_capability_revoked() {
+		if ( ! is_multisite() ) {
+			$this->markTestSkipped( 'Requires multisite.' );
+		}
+
+		wp_set_current_user( $this->admin_user_id );
+		grant_super_admin( $this->admin_user_id );
+
+		add_filter( 'wp_stream_is_network_activated', '__return_true' );
+
+		// Baseline: an unrestricted super admin is allowed.
+		$this->assertTrue(
+			$this->ability->permission_callback(),
+			'Guard: a super admin should be allowed before the capability is revoked.'
+		);
+
+		$deny_settings_cap = static function ( $caps, $cap ) {
+			if ( WP_STREAM_SETTINGS_CAPABILITY === $cap ) {
+				return array( 'do_not_allow' );
+			}
+			return $caps;
+		};
+		add_filter( 'map_meta_cap', $deny_settings_cap, 10, 2 );
+
+		try {
+			$this->assertFalse(
+				$this->ability->permission_callback(),
+				'Revoking the Stream settings capability must deny the write even for a super admin.'
+			);
+		} finally {
+			remove_filter( 'map_meta_cap', $deny_settings_cap, 10 );
+			remove_filter( 'wp_stream_is_network_activated', '__return_true' );
+			revoke_super_admin( $this->admin_user_id );
+		}
+	}
+
 	public function test_partial_update_preserves_other_keys() {
 		wp_set_current_user( $this->admin_user_id );
 
diff --git a/tests/phpunit/test-class-network.php b/tests/phpunit/test-class-network.php
new file mode 100644
index 000000000..9c7fe8d38
--- /dev/null
+++ b/tests/phpunit/test-class-network.php
@@ -0,0 +1,198 @@
+<?php
+/**
+ * Tests for the Network class.
+ *
+ * @package WP_Stream
+ */
+
+namespace WP_Stream;
+
+/**
+ * Class - Test_Network
+ *
+ * Focused on the multisite authorization boundary: which blog's records a
+ * caller is allowed to read, and what may be treated as network-admin context.
+ * The Stream tables are shared across the whole network, so these checks are
+ * the only thing keeping one site's activity log out of another site's reach.
+ */
+class Test_Network extends WP_StreamTestCase {
+
+	/**
+	 * Network instance under test.
+	 *
+	 * @var Network
+	 */
+	protected $network;
+
+	/**
+	 * {@inheritDoc}
+	 */
+	public function setUp(): void {
+		parent::setUp();
+
+		if ( ! is_multisite() ) {
+			$this->markTestSkipped( 'Requires multisite.' );
+		}
+
+		$this->network = new Network( $this->plugin );
+	}
+
+	/**
+	 * A site administrator holds manage_options but has no network authority,
+	 * so a caller-supplied blog_id must be discarded in favour of the current
+	 * blog. A numeric type check is not an authorization check: honouring an
+	 * arbitrary ?blog_id= would expose another site's records from the shared
+	 * Stream table.
+	 *
+	 * @group ms-required
+	 */
+	public function test_site_admin_cannot_choose_blog_id() {
+		$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $user_id );
+
+		$other_blog_id = (int) get_current_blog_id() + 100;
+
+		$args = $this->network->network_query_args(
+			array(
+				'site_id' => null,
+				'blog_id' => $other_blog_id,
+			)
+		);
+
+		$this->assertSame(
+			get_current_blog_id(),
+			$args['blog_id'],
+			'A site administrator must be pinned to the current blog.'
+		);
+		$this->assertNotSame(
+			$other_blog_id,
+			$args['blog_id'],
+			'The requested blog_id must not survive for a non-network user.'
+		);
+	}
+
+	/**
+	 * The same restriction applies to a Stream viewer with no settings
+	 * capability at all.
+	 *
+	 * @group ms-required
+	 */
+	public function test_subscriber_cannot_choose_blog_id() {
+		$user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) );
+		wp_set_current_user( $user_id );
+
+		$args = $this->network->network_query_args(
+			array(
+				'site_id' => null,
+				'blog_id' => (int) get_current_blog_id() + 100,
+			)
+		);
+
+		$this->assertSame( get_current_blog_id(), $args['blog_id'] );
+	}
+
+	/**
+	 * A super admin legitimately administers the whole network, so an explicit
+	 * blog_id must still be honoured for them -- the fix must not break
+	 * cross-site filtering in the Network Admin.
+	 *
+	 * @group ms-required
+	 */
+	public function test_super_admin_may_choose_blog_id() {
+		$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $user_id );
+		grant_super_admin( $user_id );
+
+		$other_blog_id = (int) get_current_blog_id() + 100;
+
+		try {
+			$args = $this->network->network_query_args(
+				array(
+					'site_id' => null,
+					'blog_id' => $other_blog_id,
+				)
+			);
+
+			$this->assertSame(
+				$other_blog_id,
+				$args['blog_id'],
+				'A super admin must retain cross-site record filtering.'
+			);
+		} finally {
+			revoke_super_admin( $user_id );
+		}
+	}
+
+	/**
+	 * The site_id argument keeps its existing default behaviour for every caller.
+	 *
+	 * @group ms-required
+	 */
+	public function test_site_id_defaults_to_current_network() {
+		wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) );
+
+		$args = $this->network->network_query_args(
+			array(
+				'site_id' => null,
+				'blog_id' => null,
+			)
+		);
+
+		$this->assertSame( (int) get_current_site()->id, (int) $args['site_id'] );
+	}
+
+	/**
+	 * HTTP_REFERER is caller-controlled, so it can only ever be a hint about
+	 * where a request originated -- never a grant of authority. Without a
+	 * network capability a spoofed Referer must not be treated as network
+	 * admin, otherwise a site-level viewer could lift the per-blog query
+	 * restriction and have their actions logged against blog_id 0.
+	 *
+	 * @group ms-required
+	 */
+	public function test_spoofed_referer_does_not_grant_network_context() {
+		$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $user_id );
+
+		$this->assertFalse(
+			$this->network->can_view_network_records(),
+			'A site administrator must not be treated as a network-wide reader.'
+		);
+	}
+
+	/**
+	 * Counterpart to the above: a genuine super admin is recognised.
+	 *
+	 * @group ms-required
+	 */
+	public function test_super_admin_may_view_network_records() {
+		$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		wp_set_current_user( $user_id );
+		grant_super_admin( $user_id );
+
+		try {
+			$this->assertTrue( $this->network->can_view_network_records() );
+		} finally {
+			revoke_super_admin( $user_id );
+		}
+	}
+
+	/**
+	 * The blog_id_logged() filter zeroes the recorded blog when the request is
+	 * in network admin context. Since that context can no longer be set from a
+	 * Referer alone, a site user's actions stay attributed to their own blog.
+	 *
+	 * @group ms-required
+	 */
+	public function test_blog_id_logged_keeps_site_attribution_for_site_user() {
+		wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) );
+
+		$blog_id = get_current_blog_id();
+
+		$this->assertSame(
+			$blog_id,
+			$this->network->blog_id_logged( $blog_id ),
+			'A site user must not have their activity recorded as network activity.'
+		);
+	}
+}

From ec01522b5be6487c7e867a3e78deb74ac397633f Mon Sep 17 00:00:00 2001
From: Utkarsh Patel <itismeutkarsh@gmail.com>
Date: Thu, 30 Jul 2026 16:22:45 +0530
Subject: [PATCH 3/5] fix: stop leaking integration credentials through Stream
 records and APIs

Alert destination credentials (get-alerts):
stream/get-alerts requires only view_stream, but alert destinations are
configured behind the Stream settings capability, so returning alert_meta
verbatim dropped credentials across a privilege boundary. A Slack incoming
webhook URL and an IFTTT Maker key are both bearer credentials -- possession
alone is enough to post into the channel or fire the account's applets.

Ability::redact_alert_meta() now replaces those values with a
`{key}_configured` boolean before output, so callers can still tell whether a
destination is set up without receiving the secret. Non-secret configuration
(channel, username, event_name) is untouched. The helper also absorbs the
existing empty-meta-to-stdClass normalization the output schema requires.

Payment gateway credentials (WooCommerce connector):
callback_updated_option() serialized entire third-party gateway settings
arrays into record metadata. Gateways routinely co-locate operational
settings with live API secrets and webhook signing keys, so an enabled
credential-bearing gateway persisted those secrets into stream_meta where any
Stream viewer or record-detail API consumer could read them.

Adds Connector::is_secret_key() / redact_secret_values(), applied to both
old and new values before serialization. Matching is substring/suffix based
rather than an allowlist because connectors log option arrays belonging to
plugins we do not control, and an allowlist cannot anticipate their field
names. Over-redacting costs a little audit detail; under-redacting persists a
live credential. The helper lives on the Connector base class because the
remaining unredacted-secret findings need the same logic.

Note the pattern list was corrected while writing the tests: 'password' did
not match mailserver_pass and no pattern matched rg_gforms_key, both of which
are real targets. A bare 'key' suffix was also rejected as it matched
harmless words such as monkey.

Each redaction test was verified to fail against the unredacted code. Both
suites: 408 tests, 0 failures.
---
 abilities/class-ability-get-alerts.php        |  14 +-
 classes/class-ability.php                     |  48 +++++++
 classes/class-connector.php                   | 102 +++++++++++++++
 connectors/class-connector-woocommerce.php    |  10 +-
 .../test-class-ability-get-alerts.php         | 123 ++++++++++++++++++
 tests/phpunit/test-class-connector.php        | 108 +++++++++++++++
 6 files changed, 396 insertions(+), 9 deletions(-)

diff --git a/abilities/class-ability-get-alerts.php b/abilities/class-ability-get-alerts.php
index 24f2d694e..9045ce075 100644
--- a/abilities/class-ability-get-alerts.php
+++ b/abilities/class-ability-get-alerts.php
@@ -124,13 +124,13 @@ public function execute( $input = null ) {
 				continue;
 			}
 
-			// Alert::$alert_meta defaults to array(); an empty PHP array() also
-			// JSON-encodes as a list ([]), which violates the declared object
-			// output schema. Normalize empty/non-array values to a real object
-			// so wp_json_encode() emits {} when there is no meta.
-			$alert_meta = is_array( $alert->alert_meta ) && ! empty( $alert->alert_meta )
-				? $alert->alert_meta
-				: new \stdClass();
+			// This ability only requires view_stream, which is a lower bar than
+			// the settings capability that gates alert configuration in the
+			// admin UI. Strip destination credentials (Slack webhook, IFTTT
+			// maker key) before they cross that boundary. Also normalizes
+			// empty/non-array meta to stdClass so wp_json_encode() emits {} and
+			// satisfies the declared object output schema.
+			$alert_meta = $this->redact_alert_meta( $alert->alert_meta );
 
 			$out[] = array(
 				'id'         => (int) $alert->ID,
diff --git a/classes/class-ability.php b/classes/class-ability.php
index 95b793707..af740c139 100644
--- a/classes/class-ability.php
+++ b/classes/class-ability.php
@@ -130,6 +130,54 @@ protected function can_write_settings() {
 		return $can_write_settings;
 	}
 
+	/**
+	 * Alert meta keys holding reusable credentials rather than configuration.
+	 *
+	 * Alert destinations are configured by users with the Stream settings
+	 * capability, but alerts are readable through get-alerts by anyone with
+	 * `view_stream`. These values are bearer credentials -- a Slack incoming
+	 * webhook URL is sufficient on its own to post into the target channel, and
+	 * an IFTTT Maker key is sufficient to fire that account's applets -- so they
+	 * must not cross that privilege boundary.
+	 *
+	 * @const array
+	 */
+	const SECRET_ALERT_META_KEYS = array(
+		'webhook',
+		'maker_key',
+	);
+
+	/**
+	 * Replace credential values in an alert_meta array with a boolean marker.
+	 *
+	 * Callers still need to know whether a destination is configured, so each
+	 * secret key is replaced by `{key}_configured` rather than dropped. The
+	 * value itself never leaves the site.
+	 *
+	 * Returns stdClass for empty input so wp_json_encode() emits `{}` and
+	 * satisfies the `alert_meta: object` output schema (an empty PHP array
+	 * encodes as `[]`).
+	 *
+	 * @param mixed $alert_meta Raw alert meta, usually an array.
+	 * @return array|\stdClass
+	 */
+	protected function redact_alert_meta( $alert_meta ) {
+		if ( ! is_array( $alert_meta ) || empty( $alert_meta ) ) {
+			return new \stdClass();
+		}
+
+		foreach ( self::SECRET_ALERT_META_KEYS as $key ) {
+			if ( ! array_key_exists( $key, $alert_meta ) ) {
+				continue;
+			}
+
+			$alert_meta[ $key . '_configured' ] = ! empty( $alert_meta[ $key ] );
+			unset( $alert_meta[ $key ] );
+		}
+
+		return empty( $alert_meta ) ? new \stdClass() : $alert_meta;
+	}
+
 	/**
 	 * Annotation flags for the ability (readonly, destructive, idempotent).
 	 *
diff --git a/classes/class-connector.php b/classes/class-connector.php
index 171f32f62..4bda7d739 100644
--- a/classes/class-connector.php
+++ b/classes/class-connector.php
@@ -186,6 +186,108 @@ public function log( $message, $args, $object_id, $context, $action, $user_id =
 		return call_user_func_array( array( wp_stream_get_instance()->log, 'log' ), compact( 'connector', 'message', 'args', 'object_id', 'context', 'action', 'user_id' ) );
 	}
 
+	/**
+	 * Substrings that mark a setting name as holding a credential.
+	 *
+	 * Deliberately matched as substrings so unknown third-party settings are
+	 * covered by default: connectors log arbitrary option arrays from other
+	 * plugins, and an allowlist cannot anticipate every field a payment gateway
+	 * or integration might add. Over-redacting a harmless field only costs a
+	 * little detail in the audit log; under-redacting persists a live credential
+	 * in a table that lower-privileged Stream viewers can read.
+	 *
+	 * @const array
+	 */
+	const SECRET_KEY_PATTERNS = array(
+		'pass',
+		'secret',
+		'private_key',
+		'apikey',
+		'token',
+		'webhook',
+		'license',
+		'salt',
+		'credential',
+		'oauth',
+	);
+
+	/**
+	 * Setting names, or suffixes of them, that hold a credential but do not
+	 * contain any of the substrings above.
+	 *
+	 * Matched against the end of the name so option prefixes used by individual
+	 * plugins (rg_gforms_key, woocommerce_..._key) are covered without treating
+	 * every name that merely contains "key" as sensitive.
+	 *
+	 * @const array
+	 */
+	const SECRET_KEY_SUFFIXES = array(
+		'_key',
+	);
+
+	/**
+	 * Placeholder stored in place of a redacted value.
+	 *
+	 * @const string
+	 */
+	const REDACTED_PLACEHOLDER = '';
+
+	/**
+	 * Whether a setting/field name looks like it holds a credential.
+	 *
+	 * @param string $key Setting or field name.
+	 * @return bool
+	 */
+	public function is_secret_key( $key ) {
+		if ( ! is_string( $key ) || '' === $key ) {
+			return false;
+		}
+
+		$needle = strtolower( $key );
+
+		foreach ( self::SECRET_KEY_PATTERNS as $pattern ) {
+			if ( false !== strpos( $needle, $pattern ) ) {
+				return true;
+			}
+		}
+
+		foreach ( self::SECRET_KEY_SUFFIXES as $suffix ) {
+			if ( substr( $needle, -strlen( $suffix ) ) === $suffix ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * Redact credential values before they are persisted as record metadata.
+	 *
+	 * Accepts either a scalar (redacted when $key itself looks secret) or an
+	 * array of settings (each secret-looking member redacted, recursively).
+	 * Values are replaced rather than removed so the audit trail still shows
+	 * that the field changed, without retaining the credential.
+	 *
+	 * @param mixed  $value Value about to be logged.
+	 * @param string $key   Setting or field name the value belongs to.
+	 * @return mixed
+	 */
+	public function redact_secret_values( $value, $key = '' ) {
+		if ( is_array( $value ) ) {
+			foreach ( $value as $child_key => $child_value ) {
+				$value[ $child_key ] = $this->redact_secret_values( $child_value, (string) $child_key );
+			}
+
+			return $value;
+		}
+
+		if ( $this->is_secret_key( $key ) && ! empty( $value ) ) {
+			return self::REDACTED_PLACEHOLDER;
+		}
+
+		return $value;
+	}
+
 	/**
 	 * Save log data till shutdown, so other callbacks would be able to override
 	 *
diff --git a/connectors/class-connector-woocommerce.php b/connectors/class-connector-woocommerce.php
index b38e477fe..c44352a8e 100644
--- a/connectors/class-connector-woocommerce.php
+++ b/connectors/class-connector-woocommerce.php
@@ -723,6 +723,12 @@ public function callback_updated_option( $option_key, $old_value, $value ) {
 				continue;
 			}
 
+			// Payment gateway options are whole settings arrays belonging to
+			// third-party gateways, and they routinely co-locate operational
+			// settings with live API secrets and webhook signing keys. Redact
+			// credential-looking members before serializing, otherwise the
+			// secret is persisted in stream_meta and readable by every Stream
+			// viewer and record-detail API consumer.
 			$this->log(
 				/* translators: %1$s: a setting name, %2$s: a setting type (e.g. "Direct Deposit", "Payment Method") */
 				__( '"%1$s" %2$s updated', 'stream' ),
@@ -733,8 +739,8 @@ public function callback_updated_option( $option_key, $old_value, $value ) {
 					'tab'       => $this->settings[ $option ]['tab'],
 					'section'   => $this->settings[ $option ]['section'],
 					'option'    => $option,
-					'old_value' => maybe_serialize( $old_value ),
-					'value'     => maybe_serialize( $value ),
+					'old_value' => maybe_serialize( $this->redact_secret_values( $old_value, $option ) ),
+					'value'     => maybe_serialize( $this->redact_secret_values( $value, $option ) ),
 				),
 				null,
 				$this->settings[ $option ]['tab'],
diff --git a/tests/phpunit/abilities/test-class-ability-get-alerts.php b/tests/phpunit/abilities/test-class-ability-get-alerts.php
index b020a524a..edf278825 100644
--- a/tests/phpunit/abilities/test-class-ability-get-alerts.php
+++ b/tests/phpunit/abilities/test-class-ability-get-alerts.php
@@ -126,4 +126,127 @@ public function test_alert_meta_is_normalized_to_object_when_missing() {
 		// Schema validates as well — exercises the live contract.
 		$this->assert_matches_schema( $result, $this->ability->get_output_schema() );
 	}
+
+	/**
+	 * Alert destinations are configured behind the Stream settings capability
+	 * but listed behind view_stream, so credentials in alert_meta would cross a
+	 * privilege boundary. A Slack incoming webhook URL is a bearer credential:
+	 * possession alone is enough to post into the channel.
+	 */
+	public function test_slack_webhook_is_redacted() {
+		wp_set_current_user( $this->admin_user_id );
+
+		// Deliberately not shaped like a real Slack webhook URL: the redaction
+		// keys off the alert_meta key name, not the value, and a realistic
+		// looking URL trips secret scanning on push.
+		$webhook = 'https://example.test/redaction-fixture/not-a-real-webhook';
+
+		$post_id = wp_insert_post(
+			array(
+				'post_type'   => Alerts::POST_TYPE,
+				'post_status' => 'wp_stream_enabled',
+				'post_title'  => 'Slack alert',
+			)
+		);
+		update_post_meta( $post_id, 'alert_type', 'slack' );
+		update_post_meta(
+			$post_id,
+			'alert_meta',
+			array(
+				'webhook'        => $webhook,
+				'channel'        => '#general',
+				'trigger_action' => 'any',
+			)
+		);
+
+		$row = $this->find_alert_row( $this->ability->execute( array( 'status' => 'any' ) ), $post_id );
+
+		$this->assertArrayNotHasKey( 'webhook', (array) $row['alert_meta'], 'The webhook URL must not be returned.' );
+		$this->assertTrue(
+			( (array) $row['alert_meta'] )['webhook_configured'],
+			'Callers still need to know a webhook is configured.'
+		);
+
+		// Non-secret configuration must survive untouched.
+		$this->assertSame( '#general', ( (array) $row['alert_meta'] )['channel'] );
+
+		$this->assertStringNotContainsString(
+			$webhook,
+			(string) wp_json_encode( $row ),
+			'The webhook URL must not appear anywhere in the serialized response.'
+		);
+	}
+
+	/**
+	 * Same boundary for the IFTTT Maker key, which is reusable across every
+	 * applet on the owning account.
+	 */
+	public function test_ifttt_maker_key_is_redacted() {
+		wp_set_current_user( $this->admin_user_id );
+
+		$maker_key = 'dxxxxxxxxxxxxxxxxxxxxxx';
+
+		$post_id = wp_insert_post(
+			array(
+				'post_type'   => Alerts::POST_TYPE,
+				'post_status' => 'wp_stream_enabled',
+				'post_title'  => 'IFTTT alert',
+			)
+		);
+		update_post_meta( $post_id, 'alert_type', 'ifttt' );
+		update_post_meta(
+			$post_id,
+			'alert_meta',
+			array(
+				'maker_key'  => $maker_key,
+				'event_name' => 'stream_event',
+			)
+		);
+
+		$row = $this->find_alert_row( $this->ability->execute( array( 'status' => 'any' ) ), $post_id );
+
+		$this->assertArrayNotHasKey( 'maker_key', (array) $row['alert_meta'] );
+		$this->assertTrue( ( (array) $row['alert_meta'] )['maker_key_configured'] );
+		$this->assertSame( 'stream_event', ( (array) $row['alert_meta'] )['event_name'] );
+		$this->assertStringNotContainsString( $maker_key, (string) wp_json_encode( $row ) );
+	}
+
+	/**
+	 * An unconfigured secret reports false rather than being reported as
+	 * present, so the marker is meaningful either way.
+	 */
+	public function test_unconfigured_secret_reports_false() {
+		wp_set_current_user( $this->admin_user_id );
+
+		$post_id = wp_insert_post(
+			array(
+				'post_type'   => Alerts::POST_TYPE,
+				'post_status' => 'wp_stream_enabled',
+				'post_title'  => 'Slack alert without webhook',
+			)
+		);
+		update_post_meta( $post_id, 'alert_type', 'slack' );
+		update_post_meta( $post_id, 'alert_meta', array( 'webhook' => '' ) );
+
+		$row = $this->find_alert_row( $this->ability->execute( array( 'status' => 'any' ) ), $post_id );
+
+		$this->assertFalse( ( (array) $row['alert_meta'] )['webhook_configured'] );
+	}
+
+	/**
+	 * Locate a single alert row in the ability output by post ID.
+	 *
+	 * @param array $result  Ability output.
+	 * @param int   $post_id Alert post ID.
+	 * @return array
+	 */
+	private function find_alert_row( $result, $post_id ) {
+		foreach ( $result as $entry ) {
+			if ( $entry['id'] === $post_id ) {
+				return $entry;
+			}
+		}
+
+		$this->fail( 'Seeded alert missing from get-alerts output.' );
+	}
 }
diff --git a/tests/phpunit/test-class-connector.php b/tests/phpunit/test-class-connector.php
index 55305f6b8..0a58c6538 100644
--- a/tests/phpunit/test-class-connector.php
+++ b/tests/phpunit/test-class-connector.php
@@ -271,4 +271,112 @@ public function test_escape_percentages() {
 			$escaped_value
 		);
 	}
+
+	/**
+	 * Credential-looking setting names are recognised regardless of case or
+	 * surrounding words, since connectors log arbitrary third-party option
+	 * names that an allowlist could not anticipate.
+	 *
+	 * @return void
+	 */
+	public function test_is_secret_key() {
+		$secret = array(
+			'mailserver_pass',
+			'password',
+			'rg_gforms_captcha_private_key',
+			'stripe_secret_key',
+			'API_KEY',
+			'publishable_token',
+			'webhook',
+			'client_secret',
+			'rg_gforms_key',
+		);
+		foreach ( $secret as $key ) {
+			$this->assertTrue(
+				$this->connector->is_secret_key( $key ),
+				"$key should be treated as a secret."
+			);
+		}
+
+		$safe = array(
+			'blogname',
+			'general_records_ttl',
+			'title',
+			'enabled',
+			'email_recipient',
+			'default_category',
+			// Words merely ending in "key" must not be caught; only the "_key"
+			// suffix used by real credential option names.
+			'monkey',
+			'turkey',
+			'',
+		);
+		foreach ( $safe as $key ) {
+			$this->assertFalse(
+				$this->connector->is_secret_key( $key ),
+				"$key should not be treated as a secret."
+			);
+		}
+	}
+
+	/**
+	 * A scalar is redacted based on its own key, which is the shape used by the
+	 * settings and Gravity Forms connectors.
+	 *
+	 * @return void
+	 */
+	public function test_redact_secret_values_scalar() {
+		$this->assertSame(
+			'',
+			$this->connector->redact_secret_values( 'hunter2', 'mailserver_pass' )
+		);
+		$this->assertSame(
+			'My Site',
+			$this->connector->redact_secret_values( 'My Site', 'blogname' )
+		);
+	}
+
+	/**
+	 * An empty value is left alone so the audit trail can still distinguish
+	 * "was never set" from "set but redacted".
+	 *
+	 * @return void
+	 */
+	public function test_redact_secret_values_leaves_empty_values() {
+		$this->assertSame( '', $this->connector->redact_secret_values( '', 'password' ) );
+	}
+
+	/**
+	 * Whole settings arrays (payment gateways, integration configs) are
+	 * redacted member-wise and recursively, keeping non-secret settings intact
+	 * so the record still describes what changed.
+	 *
+	 * @return void
+	 */
+	public function test_redact_secret_values_array() {
+		$gateway = array(
+			'enabled'         => 'yes',
+			'title'           => 'Credit Card',
+			'secret_key'      => 'sk_live_deadbeefdeadbeef',
+			'publishable_key' => 'pk_live_public',
+			'nested'          => array(
+				'webhook'     => 'https://example.com/hook/abcdef',
+				'description' => 'Pay by card',
+			),
+		);
+
+		$redacted = $this->connector->redact_secret_values( $gateway, 'woocommerce_stripe_settings' );
+
+		$this->assertSame( 'yes', $redacted['enabled'] );
+		$this->assertSame( 'Credit Card', $redacted['title'] );
+		$this->assertSame( '', $redacted['secret_key'], 'A live API secret must not be persisted.' );
+		$this->assertSame( '', $redacted['nested']['webhook'], 'Nested credentials must be redacted too.' );
+		$this->assertSame( 'Pay by card', $redacted['nested']['description'] );
+
+		$this->assertStringNotContainsString(
+			'sk_live_deadbeefdeadbeef',
+			maybe_serialize( $redacted ),
+			'The secret must not survive serialization into record metadata.'
+		);
+	}
 }

From 1bb56d21e67ba3404ae4d3500a05cfbc44ab6fc2 Mon Sep 17 00:00:00 2001
From: Utkarsh Patel <itismeutkarsh@gmail.com>
Date: Thu, 30 Jul 2026 16:35:23 +0530
Subject: [PATCH 4/5] fix: redact credential settings before they reach Stream
 record metadata

Applies the Connector redaction helper added in the previous commit to the
remaining settings that were persisted verbatim.

Settings connector (mailserver_pass):
callback_updated_option() passed values through sanitize_value(), which only
flattens complex types to strings and has no notion of sensitivity, so a
reusable mailbox password was stored as both old_value and value. Redaction is
now applied per field at the two call sites where the setting name is known --
sanitize_value() itself receives no key and cannot make the decision.

Gravity Forms connector (rg_gforms_key, rg_gforms_captcha_private_key):
check() logged old/new values for every tracked option including the reCAPTCHA
private key, and check_rg_gforms_key() explicitly logged both halves of a
license-key change. Both now redact before logging. In the license case the
update/delete status is derived before redaction, so the message still reports
which happened -- covered by a test.

The change also refines the pattern list from the previous commit, driven by
test failures rather than assumption:

- rg_gforms_captcha_public_key was being over-redacted by the '_key' suffix
  rule. Public halves of key pairs are meant to be published and redacting
  them removes audit detail for no security benefit, so PUBLIC_KEY_PATTERNS
  now exempts public_key / publishable_key / site_key ahead of the secret
  match. A test asserts the public key survives while the private one does
  not.
- The earlier assertion that 'publishable_key' should be treated as secret was
  wrong and has been corrected.

Each redaction test was verified to fail without the fix while the non-secret
control tests continued to pass, confirming they discriminate rather than
redacting everything. Both suites: 414 tests, 0 failures.
---
 classes/class-connector.php                   |  21 +++
 connectors/class-connector-gravityforms.php   |  12 ++
 connectors/class-connector-settings.php       |  12 +-
 .../test-class-connector-gravityforms.php     | 143 ++++++++++++++++++
 .../test-class-connector-settings.php         | 100 ++++++++++++
 tests/phpunit/test-class-connector.php        |   8 +
 6 files changed, 292 insertions(+), 4 deletions(-)
 create mode 100644 tests/phpunit/connectors/test-class-connector-gravityforms.php

diff --git a/classes/class-connector.php b/classes/class-connector.php
index 4bda7d739..dfc1b48b2 100644
--- a/classes/class-connector.php
+++ b/classes/class-connector.php
@@ -225,6 +225,21 @@ public function log( $message, $args, $object_id, $context, $action, $user_id =
 		'_key',
 	);
 
+	/**
+	 * Names that end in a secret-looking suffix but are not credentials.
+	 *
+	 * Public halves of key pairs are meant to be published, and redacting them
+	 * removes useful audit detail for no benefit. Matched as substrings so the
+	 * various plugin prefixes are covered.
+	 *
+	 * @const array
+	 */
+	const PUBLIC_KEY_PATTERNS = array(
+		'public_key',
+		'publishable_key',
+		'site_key',
+	);
+
 	/**
 	 * Placeholder stored in place of a redacted value.
 	 *
@@ -245,6 +260,12 @@ public function is_secret_key( $key ) {
 
 		$needle = strtolower( $key );
 
+		foreach ( self::PUBLIC_KEY_PATTERNS as $pattern ) {
+			if ( false !== strpos( $needle, $pattern ) ) {
+				return false;
+			}
+		}
+
 		foreach ( self::SECRET_KEY_PATTERNS as $pattern ) {
 			if ( false !== strpos( $needle, $pattern ) ) {
 				return true;
diff --git a/connectors/class-connector-gravityforms.php b/connectors/class-connector-gravityforms.php
index 034bbcda0..3e3cc717e 100644
--- a/connectors/class-connector-gravityforms.php
+++ b/connectors/class-connector-gravityforms.php
@@ -507,6 +507,12 @@ public function check( $option, $old_value, $new_value ) {
 			$option_title = $data['label'];
 			$context      = isset( $data['context'] ) ? $data['context'] : 'settings';
 
+			// Tracked options include credentials such as
+			// rg_gforms_captcha_private_key; record that they changed without
+			// retaining the value.
+			$old_value = $this->redact_secret_values( $old_value, $option );
+			$new_value = $this->redact_secret_values( $new_value, $option );
+
 			$this->log(
 				/* translators: %s: a setting title (e.g. "Language") */
 				__( '"%s" setting updated', 'stream' ),
@@ -529,6 +535,12 @@ public function check_rg_gforms_key( $old_value, $new_value ) {
 		$is_update = ( $new_value && strlen( $new_value ) );
 		$option    = 'rg_gforms_key';
 
+		// The license key is a reusable vendor credential granting account and
+		// paid-download access, so only the fact of the change is recorded. The
+		// message already conveys whether it was set or removed.
+		$old_value = $this->redact_secret_values( $old_value, $option );
+		$new_value = $this->redact_secret_values( $new_value, $option );
+
 		$this->log(
 			sprintf(
 				/* translators: %s: a status (e.g. "updated") */
diff --git a/connectors/class-connector-settings.php b/connectors/class-connector-settings.php
index a5b7543ea..cd9250695 100644
--- a/connectors/class-connector-settings.php
+++ b/connectors/class-connector-settings.php
@@ -739,6 +739,10 @@ public function callback_updated_option( $option, $old_value, $value ) {
 
 		$changed_options = array();
 
+		// sanitize_value() only flattens complex types to strings, so on its own
+		// it happily persists credential settings such as mailserver_pass in
+		// record metadata. Redaction is applied per field, keyed on the setting
+		// name, before the value reaches the log.
 		if ( $this->is_option_group( $value ) ) {
 			foreach ( $this->get_changed_keys( $old_value, $value ) as $field_key ) {
 				if ( ! $this->is_key_ignored( $option, $field_key ) ) {
@@ -748,8 +752,8 @@ public function callback_updated_option( $option, $old_value, $value ) {
 						'option'     => $option,
 						'option_key' => $field_key,
 						'context'    => ( false !== $key_context ? $key_context : $context ),
-						'old_value'  => isset( $old_value[ $field_key ] ) ? $this->sanitize_value( $old_value[ $field_key ] ) : null,
-						'value'      => isset( $value[ $field_key ] ) ? $this->sanitize_value( $value[ $field_key ] ) : null,
+						'old_value'  => isset( $old_value[ $field_key ] ) ? $this->redact_secret_values( $this->sanitize_value( $old_value[ $field_key ] ), $field_key ) : null,
+						'value'      => isset( $value[ $field_key ] ) ? $this->redact_secret_values( $this->sanitize_value( $value[ $field_key ] ), $field_key ) : null,
 					);
 				}
 			}
@@ -758,8 +762,8 @@ public function callback_updated_option( $option, $old_value, $value ) {
 				'label'     => $this->get_field_label( $option ),
 				'option'    => $option,
 				'context'   => $context,
-				'old_value' => $this->sanitize_value( $old_value ),
-				'value'     => $this->sanitize_value( $value ),
+				'old_value' => $this->redact_secret_values( $this->sanitize_value( $old_value ), $option ),
+				'value'     => $this->redact_secret_values( $this->sanitize_value( $value ), $option ),
 			);
 		}
 
diff --git a/tests/phpunit/connectors/test-class-connector-gravityforms.php b/tests/phpunit/connectors/test-class-connector-gravityforms.php
new file mode 100644
index 000000000..ac6209413
--- /dev/null
+++ b/tests/phpunit/connectors/test-class-connector-gravityforms.php
@@ -0,0 +1,143 @@
+<?php
+namespace WP_Stream;
+
+/**
+ * Tests for the Gravity Forms connector.
+ *
+ * Gravity Forms itself is not installed in the test environment, so these
+ * exercise the logging entry points directly rather than through the plugin's
+ * hooks. That is sufficient for the redaction contract, which depends only on
+ * the option name.
+ *
+ * @package WP_Stream
+ */
+class Test_Connector_GravityForms extends WP_StreamTestCase {
+
+	/**
+	 * Mocked connector with log() stubbed.
+	 *
+	 * @var Connector_GravityForms
+	 */
+	protected $mock;
+
+	/**
+	 * Runs before each test.
+	 */
+	public function setUp(): void {
+		parent::setUp();
+
+		$this->plugin->connectors->unload_connectors();
+
+		$this->mock = $this->getMockBuilder( Connector_GravityForms::class )
+			->setMethods( array( 'log' ) )
+			->getMock();
+
+		// Populates $options, which check() consults. Safe without the plugin
+		// present: register() only builds that array.
+		$this->mock->register();
+	}
+
+	/**
+	 * The reCAPTCHA private key is a tracked option, so check() would otherwise
+	 * persist both the previous and replacement secret in record metadata,
+	 * where the record-detail ability exposes it to any view_stream caller.
+	 */
+	public function test_captcha_private_key_is_redacted() {
+		$logged = array();
+		$this->mock->expects( $this->once() )
+			->method( 'log' )
+			->willReturnCallback(
+				function ( $message, $args ) use ( &$logged ) {
+					$logged = $args;
+					return true;
+				}
+			);
+
+		$this->mock->check( 'rg_gforms_captcha_private_key', 'old-private-key', 'new-private-key' );
+
+		$this->assertSame(
+			'rg_gforms_captcha_private_key',
+			$logged['option'],
+			'The setting change must still be recorded.'
+		);
+		$this->assertSame( '', $logged['old_value'] );
+		$this->assertSame( '', $logged['new_value'] );
+
+		$serialized = maybe_serialize( $logged );
+		$this->assertStringNotContainsString( 'old-private-key', $serialized );
+		$this->assertStringNotContainsString( 'new-private-key', $serialized );
+	}
+
+	/**
+	 * The matching public key is not a credential and must stay readable, so
+	 * the audit log remains useful.
+	 */
+	public function test_captcha_public_key_is_not_redacted() {
+		$logged = array();
+		$this->mock->expects( $this->once() )
+			->method( 'log' )
+			->willReturnCallback(
+				function ( $message, $args ) use ( &$logged ) {
+					$logged = $args;
+					return true;
+				}
+			);
+
+		$this->mock->check( 'rg_gforms_captcha_public_key', 'old-public', 'new-public' );
+
+		$this->assertSame( 'old-public', $logged['old_value'] );
+		$this->assertSame( 'new-public', $logged['new_value'] );
+	}
+
+	/**
+	 * The license key is a reusable vendor credential. Only the fact of the
+	 * change is recorded; the message still distinguishes set from removed.
+	 */
+	public function test_license_key_is_redacted_on_update() {
+		$message = '';
+		$logged  = array();
+		$this->mock->expects( $this->once() )
+			->method( 'log' )
+			->willReturnCallback(
+				function ( $msg, $args ) use ( &$logged, &$message ) {
+					$message = $msg;
+					$logged  = $args;
+					return true;
+				}
+			);
+
+		$this->mock->check_rg_gforms_key( 'old-license', 'new-license' );
+
+		$this->assertSame( 'rg_gforms_key', $logged['option'] );
+		$this->assertSame( '', $logged['old_value'] );
+		$this->assertSame( '', $logged['new_value'] );
+
+		// The update/delete distinction is derived before redaction, so it must
+		// still report an update here.
+		$this->assertStringContainsString( 'updated', $message );
+
+		$serialized = maybe_serialize( $logged );
+		$this->assertStringNotContainsString( 'old-license', $serialized );
+		$this->assertStringNotContainsString( 'new-license', $serialized );
+	}
+
+	/**
+	 * Clearing the license key must still be reported as a deletion, proving
+	 * redaction did not disturb the status derivation.
+	 */
+	public function test_license_key_deletion_still_reported() {
+		$message = '';
+		$this->mock->expects( $this->once() )
+			->method( 'log' )
+			->willReturnCallback(
+				function ( $msg ) use ( &$message ) {
+					$message = $msg;
+					return true;
+				}
+			);
+
+		$this->mock->check_rg_gforms_key( 'old-license', '' );
+
+		$this->assertStringContainsString( 'deleted', $message );
+	}
+}
diff --git a/tests/phpunit/connectors/test-class-connector-settings.php b/tests/phpunit/connectors/test-class-connector-settings.php
index 4c1baf350..a664852eb 100644
--- a/tests/phpunit/connectors/test-class-connector-settings.php
+++ b/tests/phpunit/connectors/test-class-connector-settings.php
@@ -156,4 +156,104 @@ public function test_callback_updated_option() {
 			$this->assertGreaterThan( 0, did_action( $this->action_prefix . 'callback_update_option' ) );
 		}
 	}
+
+	/**
+	 * The mailserver_pass core setting is tracked, so both its previous and new
+	 * value would otherwise be persisted verbatim in record metadata --
+	 * sanitize_value() only casts to string. The change must still be recorded,
+	 * but without the mailbox password.
+	 */
+	public function test_mailserver_pass_is_redacted() {
+		$add_method    = is_multisite() ? 'add_site_option' : 'add_option';
+		$update_method = is_multisite() ? 'update_site_option' : 'update_option';
+
+		$this->register_writing_options();
+
+		call_user_func( $add_method, 'mailserver_pass', 'old-secret-password' );
+
+		$logged = array();
+		$this->mock->expects( $this->once() )
+			->method( 'log' )
+			->willReturnCallback(
+				function ( $message, $args ) use ( &$logged ) {
+					$logged = $args;
+					return true;
+				}
+			);
+
+		call_user_func( $update_method, 'mailserver_pass', 'new-secret-password' );
+
+		$this->assertSame(
+			'mailserver_pass',
+			$logged['option'],
+			'The setting change must still be recorded.'
+		);
+		$this->assertSame( '', $logged['old_value'], 'The previous password must not be retained.' );
+		$this->assertSame( '', $logged['value'], 'The new password must not be retained.' );
+
+		$serialized = maybe_serialize( $logged );
+		$this->assertStringNotContainsString( 'old-secret-password', $serialized );
+		$this->assertStringNotContainsString( 'new-secret-password', $serialized );
+	}
+
+	/**
+	 * Redaction is keyed on the setting name, so ordinary settings are logged
+	 * with their values intact.
+	 */
+	public function test_non_secret_setting_is_not_redacted() {
+		$add_method    = is_multisite() ? 'add_site_option' : 'add_option';
+		$update_method = is_multisite() ? 'update_site_option' : 'update_option';
+
+		$this->register_writing_options();
+
+		// This option is seeded by the WP test fixture, so read the existing
+		// value rather than assuming add_option() sets it.
+		call_user_func( $add_method, 'mailserver_login', 'old-login' );
+		$previous = is_multisite() ? get_site_option( 'mailserver_login' ) : get_option( 'mailserver_login' );
+
+		$logged = array();
+		$this->mock->expects( $this->once() )
+			->method( 'log' )
+			->willReturnCallback(
+				function ( $message, $args ) use ( &$logged ) {
+					$logged = $args;
+					return true;
+				}
+			);
+
+		call_user_func( $update_method, 'mailserver_login', 'new-login' );
+
+		$this->assertSame( $previous, $logged['old_value'] );
+		$this->assertSame( 'new-login', $logged['value'] );
+		$this->assertNotSame( '', $logged['value'], 'A non-secret setting must keep its value.' );
+	}
+
+	/**
+	 * Connector_Settings::callback_update_option() only forwards to the logging
+	 * path when the request looks like a real settings save -- either WP-CLI or
+	 * inside customize_save. Mirrors the setup already used by
+	 * test_callback_updated_option().
+	 *
+	 * Also registers the mail server options under the "writing" group the way
+	 * wp-admin/options.php does, so callback_updated_option() resolves a context
+	 * for them.
+	 *
+	 * @return void
+	 */
+	private function register_writing_options() {
+		global $whitelist_options;
+
+		if ( ! is_array( $whitelist_options ) ) {
+			$whitelist_options = array(); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+		}
+
+		$whitelist_options['writing'] = array_merge( // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited
+			isset( $whitelist_options['writing'] ) ? (array) $whitelist_options['writing'] : array(),
+			array( 'mailserver_pass', 'mailserver_login' )
+		);
+
+		// Simulate being on a settings save request.
+		require_once ABSPATH . WPINC . '/class-wp-customize-manager.php';
+		do_action( 'customize_save', new \WP_Customize_Manager( array() ) );
+	}
 }
diff --git a/tests/phpunit/test-class-connector.php b/tests/phpunit/test-class-connector.php
index 0a58c6538..e3384f41b 100644
--- a/tests/phpunit/test-class-connector.php
+++ b/tests/phpunit/test-class-connector.php
@@ -309,6 +309,10 @@ public function test_is_secret_key() {
 			// suffix used by real credential option names.
 			'monkey',
 			'turkey',
+			// Public halves of key pairs are meant to be published.
+			'rg_gforms_captcha_public_key',
+			'stripe_publishable_key',
+			'recaptcha_site_key',
 			'',
 		);
 		foreach ( $safe as $key ) {
@@ -373,6 +377,10 @@ public function test_redact_secret_values_array() {
 		$this->assertSame( '', $redacted['nested']['webhook'], 'Nested credentials must be redacted too.' );
 		$this->assertSame( 'Pay by card', $redacted['nested']['description'] );
 
+		// The publishable half of a key pair is designed to be public, so
+		// redacting it would remove audit detail for no benefit.
+		$this->assertSame( 'pk_live_public', $redacted['publishable_key'] );
+
 		$this->assertStringNotContainsString(
 			'sk_live_deadbeefdeadbeef',
 			maybe_serialize( $redacted ),

From a81c3f5cf0384acfd9eceb2b5442b396053aeb01 Mon Sep 17 00:00:00 2001
From: Utkarsh Patel <itismeutkarsh@gmail.com>
Date: Thu, 30 Jul 2026 18:42:58 +0530
Subject: [PATCH 5/5] fix: address review feedback on secret redaction

1. Secret markers now beat the public-name exemption. PUBLIC_KEY_PATTERNS was
   checked first and returned early, so a name such as secret_site_key or
   webhook_public_key escaped redaction despite carrying an explicit secret
   marker -- inverting the over-redact-rather-than-under-redact preference the
   previous commit claimed. The exemption exists only to stop the broad "_key"
   suffix rule from catching published key halves, so it is now consulted
   after the secret substrings and before the suffix rule.

2. REDACTED_PLACEHOLDER is '[redacted]' rather than ''. The empty string made
   a withheld credential indistinguishable from a cleared field, contradicting
   the stated rationale of the empty-value test. Unset credentials still log as
   '' so "never set" and "set but withheld" remain distinguishable; the test
   now asserts both directions.

3. Extends redaction to EDD and Jetpack, which logged raw old/new option
   values. EDD is the notable one: it logs arbitrary settings fields including
   payment gateway API keys. BuddyPress was examined and left alone -- it only
   logs component activation booleans and page IDs, so there is no credential
   to redact and a call would be dead code.

4. Adds the wp_stream_secret_alert_meta_keys filter so third-party alert types
   registered via wp_stream_alert_types can have their own destination secrets
   redacted from get-alerts output.

Tests assert against Connector::REDACTED_PLACEHOLDER rather than a literal so
they track the constant. The precedence fix and the filter both have tests
verified to fail against the previous behaviour. Both suites: 416 tests, 0
failures.
---
 classes/class-ability.php                     | 19 +++++++-
 classes/class-connector.php                   | 22 +++++++---
 connectors/class-connector-edd.php            |  6 ++-
 connectors/class-connector-jetpack.php        |  4 +-
 .../test-class-ability-get-alerts.php         | 43 +++++++++++++++++++
 .../test-class-connector-gravityforms.php     |  8 ++--
 .../test-class-connector-settings.php         | 13 +++---
 tests/phpunit/test-class-connector.php        | 34 ++++++++++++---
 8 files changed, 125 insertions(+), 24 deletions(-)

diff --git a/classes/class-ability.php b/classes/class-ability.php
index af740c139..cd13d1c36 100644
--- a/classes/class-ability.php
+++ b/classes/class-ability.php
@@ -166,7 +166,24 @@ protected function redact_alert_meta( $alert_meta ) {
 			return new \stdClass();
 		}
 
-		foreach ( self::SECRET_ALERT_META_KEYS as $key ) {
+		/**
+		 * Filters the alert_meta keys treated as credentials and withheld from
+		 * ability output.
+		 *
+		 * Third-party alert types registered via `wp_stream_alert_types` may
+		 * store their own destination secrets under names Stream cannot know
+		 * about; add them here so they are redacted too.
+		 *
+		 * @param array $keys       Meta keys to redact.
+		 * @param array $alert_meta The alert meta being redacted.
+		 */
+		$secret_keys = (array) apply_filters(
+			'wp_stream_secret_alert_meta_keys',
+			self::SECRET_ALERT_META_KEYS,
+			$alert_meta
+		);
+
+		foreach ( $secret_keys as $key ) {
 			if ( ! array_key_exists( $key, $alert_meta ) ) {
 				continue;
 			}
diff --git a/classes/class-connector.php b/classes/class-connector.php
index dfc1b48b2..e7966e4ee 100644
--- a/classes/class-connector.php
+++ b/classes/class-connector.php
@@ -232,6 +232,9 @@ public function log( $message, $args, $object_id, $context, $action, $user_id =
 	 * removes useful audit detail for no benefit. Matched as substrings so the
 	 * various plugin prefixes are covered.
 	 *
+	 * Only consulted after SECRET_KEY_PATTERNS, so a name containing an
+	 * explicit secret marker is never exempted by appearing "public" too.
+	 *
 	 * @const array
 	 */
 	const PUBLIC_KEY_PATTERNS = array(
@@ -243,9 +246,13 @@ public function log( $message, $args, $object_id, $context, $action, $user_id =
 	/**
 	 * Placeholder stored in place of a redacted value.
 	 *
+	 * A distinct marker rather than an empty string, so a reader of the audit
+	 * trail can tell "this credential changed, value withheld" apart from "this
+	 * field was cleared" -- an empty string would conflate the two.
+	 *
 	 * @const string
 	 */
-	const REDACTED_PLACEHOLDER = '';
+	const REDACTED_PLACEHOLDER = '[redacted]';
 
 	/**
 	 * Whether a setting/field name looks like it holds a credential.
@@ -260,15 +267,20 @@ public function is_secret_key( $key ) {
 
 		$needle = strtolower( $key );
 
-		foreach ( self::PUBLIC_KEY_PATTERNS as $pattern ) {
+		// An explicit secret marker always wins. The public-name exemption
+		// below is only there to stop the broad "_key" suffix rule from
+		// swallowing published key halves, so it must not be able to rescue a
+		// name that also says "secret", "private_key" or "webhook" -- that
+		// would invert the over-redact-rather-than-under-redact preference.
+		foreach ( self::SECRET_KEY_PATTERNS as $pattern ) {
 			if ( false !== strpos( $needle, $pattern ) ) {
-				return false;
+				return true;
 			}
 		}
 
-		foreach ( self::SECRET_KEY_PATTERNS as $pattern ) {
+		foreach ( self::PUBLIC_KEY_PATTERNS as $pattern ) {
 			if ( false !== strpos( $needle, $pattern ) ) {
-				return true;
+				return false;
 			}
 		}
 
diff --git a/connectors/class-connector-edd.php b/connectors/class-connector-edd.php
index 953060525..acb7e360b 100644
--- a/connectors/class-connector-edd.php
+++ b/connectors/class-connector-edd.php
@@ -394,14 +394,16 @@ public function check_edd_settings( $old_value, $new_value ) {
 				continue;
 			}
 
+			// EDD settings include payment gateway API keys and secrets, so
+			// redact credential-looking fields before they are persisted.
 			$this->log(
 				/* translators: %s: a setting title (e.g. "Language") */
 				__( '"%s" setting updated', 'stream' ),
 				array(
 					'option_title' => $field['name'],
 					'option'       => $option,
-					'old_value'    => isset( $old_value[ $option ] ) ? $old_value[ $option ] : null,
-					'value'        => isset( $new_value[ $option ] ) ? $new_value[ $option ] : null,
+					'old_value'    => isset( $old_value[ $option ] ) ? $this->redact_secret_values( $old_value[ $option ], $option ) : null,
+					'value'        => isset( $new_value[ $option ] ) ? $this->redact_secret_values( $new_value[ $option ], $option ) : null,
 					'tab'          => $tab,
 				),
 				null,
diff --git a/connectors/class-connector-jetpack.php b/connectors/class-connector-jetpack.php
index d11492d76..2f1af579d 100644
--- a/connectors/class-connector-jetpack.php
+++ b/connectors/class-connector-jetpack.php
@@ -622,8 +622,8 @@ public function check_jetpack_options( $old_value, $new_value ) {
 
 			$settings['meta'] += array(
 				'option'    => $option,
-				'old_value' => $old_value,
-				'value'     => $new_value,
+				'old_value' => $this->redact_secret_values( $old_value, $option ),
+				'value'     => $this->redact_secret_values( $new_value, $option ),
 			);
 
 			$this->log(
diff --git a/tests/phpunit/abilities/test-class-ability-get-alerts.php b/tests/phpunit/abilities/test-class-ability-get-alerts.php
index edf278825..7ccb1d794 100644
--- a/tests/phpunit/abilities/test-class-ability-get-alerts.php
+++ b/tests/phpunit/abilities/test-class-ability-get-alerts.php
@@ -233,6 +233,49 @@ public function test_unconfigured_secret_reports_false() {
 		$this->assertFalse( ( (array) $row['alert_meta'] )['webhook_configured'] );
 	}
 
+	/**
+	 * Third-party alert types can store destination secrets under names Stream
+	 * does not know, so the redaction list is filterable.
+	 */
+	public function test_secret_alert_meta_keys_are_filterable() {
+		wp_set_current_user( $this->admin_user_id );
+
+		$post_id = wp_insert_post(
+			array(
+				'post_type'   => Alerts::POST_TYPE,
+				'post_status' => 'wp_stream_enabled',
+				'post_title'  => 'Third-party alert',
+			)
+		);
+		update_post_meta( $post_id, 'alert_type', 'custom' );
+		update_post_meta(
+			$post_id,
+			'alert_meta',
+			array(
+				'custom_api_secret' => 'super-secret-value',
+				'endpoint'          => 'https://example.com/notify',
+			)
+		);
+
+		$add_key = static function ( $keys ) {
+			$keys[] = 'custom_api_secret';
+			return $keys;
+		};
+		add_filter( 'wp_stream_secret_alert_meta_keys', $add_key );
+
+		try {
+			$row  = $this->find_alert_row( $this->ability->execute( array( 'status' => 'any' ) ), $post_id );
+			$meta = (array) $row['alert_meta'];
+
+			$this->assertArrayNotHasKey( 'custom_api_secret', $meta );
+			$this->assertTrue( $meta['custom_api_secret_configured'] );
+			$this->assertSame( 'https://example.com/notify', $meta['endpoint'] );
+			$this->assertStringNotContainsString( 'super-secret-value', (string) wp_json_encode( $row ) );
+		} finally {
+			remove_filter( 'wp_stream_secret_alert_meta_keys', $add_key );
+		}
+	}
+
 	/**
 	 * Locate a single alert row in the ability output by post ID.
 	 *
diff --git a/tests/phpunit/connectors/test-class-connector-gravityforms.php b/tests/phpunit/connectors/test-class-connector-gravityforms.php
index ac6209413..798d97918 100644
--- a/tests/phpunit/connectors/test-class-connector-gravityforms.php
+++ b/tests/phpunit/connectors/test-class-connector-gravityforms.php
@@ -60,8 +60,8 @@ function ( $message, $args ) use ( &$logged ) {
 			$logged['option'],
 			'The setting change must still be recorded.'
 		);
-		$this->assertSame( '', $logged['old_value'] );
-		$this->assertSame( '', $logged['new_value'] );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $logged['old_value'] );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $logged['new_value'] );
 
 		$serialized = maybe_serialize( $logged );
 		$this->assertStringNotContainsString( 'old-private-key', $serialized );
@@ -109,8 +109,8 @@ function ( $msg, $args ) use ( &$logged, &$message ) {
 		$this->mock->check_rg_gforms_key( 'old-license', 'new-license' );
 
 		$this->assertSame( 'rg_gforms_key', $logged['option'] );
-		$this->assertSame( '', $logged['old_value'] );
-		$this->assertSame( '', $logged['new_value'] );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $logged['old_value'] );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $logged['new_value'] );
 
 		// The update/delete distinction is derived before redaction, so it must
 		// still report an update here.
diff --git a/tests/phpunit/connectors/test-class-connector-settings.php b/tests/phpunit/connectors/test-class-connector-settings.php
index a664852eb..e1ebb2d76 100644
--- a/tests/phpunit/connectors/test-class-connector-settings.php
+++ b/tests/phpunit/connectors/test-class-connector-settings.php
@@ -167,12 +167,15 @@ public function test_mailserver_pass_is_redacted() {
 		$add_method    = is_multisite() ? 'add_site_option' : 'add_option';
 		$update_method = is_multisite() ? 'update_site_option' : 'update_option';
 
-		$this->register_writing_options();
+		// Core seeds this option as an empty string, so add_option() would be a
+		// no-op; write the prior value before the logging expectation is set so
+		// both sides of the observed change are non-empty.
+		call_user_func( $update_method, 'mailserver_pass', 'old-secret-password' );
 
-		call_user_func( $add_method, 'mailserver_pass', 'old-secret-password' );
+		$this->register_writing_options();
 
 		$logged = array();
-		$this->mock->expects( $this->once() )
+		$this->mock->expects( $this->atLeastOnce() )
 			->method( 'log' )
 			->willReturnCallback(
 				function ( $message, $args ) use ( &$logged ) {
@@ -188,8 +191,8 @@ function ( $message, $args ) use ( &$logged ) {
 			$logged['option'],
 			'The setting change must still be recorded.'
 		);
-		$this->assertSame( '', $logged['old_value'], 'The previous password must not be retained.' );
-		$this->assertSame( '', $logged['value'], 'The new password must not be retained.' );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $logged['old_value'], 'The previous password must not be retained.' );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $logged['value'], 'The new password must not be retained.' );
 
 		$serialized = maybe_serialize( $logged );
 		$this->assertStringNotContainsString( 'old-secret-password', $serialized );
diff --git a/tests/phpunit/test-class-connector.php b/tests/phpunit/test-class-connector.php
index e3384f41b..d3a11505d 100644
--- a/tests/phpunit/test-class-connector.php
+++ b/tests/phpunit/test-class-connector.php
@@ -323,6 +323,24 @@ public function test_is_secret_key() {
 		}
 	}
 
+	/**
+	 * The public-name exemption exists only to stop the broad "_key" suffix
+	 * rule from catching published key halves. It must not rescue a name that
+	 * also carries an explicit secret marker, or the exemption would become a
+	 * bypass.
+	 *
+	 * @return void
+	 */
+	public function test_explicit_secret_marker_beats_public_name() {
+		$this->assertTrue( $this->connector->is_secret_key( 'secret_site_key' ) );
+		$this->assertTrue( $this->connector->is_secret_key( 'webhook_public_key' ) );
+		$this->assertTrue( $this->connector->is_secret_key( 'private_key_public_key' ) );
+
+		// Still exempt when nothing marks them as secret.
+		$this->assertFalse( $this->connector->is_secret_key( 'recaptcha_site_key' ) );
+		$this->assertFalse( $this->connector->is_secret_key( 'rg_gforms_captcha_public_key' ) );
+	}
+
 	/**
 	 * A scalar is redacted based on its own key, which is the shape used by the
 	 * settings and Gravity Forms connectors.
@@ -331,7 +349,7 @@ public function test_is_secret_key() {
 	 */
 	public function test_redact_secret_values_scalar() {
 		$this->assertSame(
-			'',
+			Connector::REDACTED_PLACEHOLDER,
 			$this->connector->redact_secret_values( 'hunter2', 'mailserver_pass' )
 		);
 		$this->assertSame(
@@ -341,13 +359,19 @@ public function test_redact_secret_values_scalar() {
 	}
 
 	/**
-	 * An empty value is left alone so the audit trail can still distinguish
-	 * "was never set" from "set but redacted".
+	 * An unset credential stays an empty string rather than gaining the
+	 * placeholder, so a reader can distinguish "was never set" from "set but
+	 * withheld" -- the latter carries the marker.
 	 *
 	 * @return void
 	 */
 	public function test_redact_secret_values_leaves_empty_values() {
 		$this->assertSame( '', $this->connector->redact_secret_values( '', 'password' ) );
+		$this->assertNotSame(
+			Connector::REDACTED_PLACEHOLDER,
+			$this->connector->redact_secret_values( '', 'password' ),
+			'An empty credential must not look like a withheld one.'
+		);
 	}
 
 	/**
@@ -373,8 +397,8 @@ public function test_redact_secret_values_array() {
 
 		$this->assertSame( 'yes', $redacted['enabled'] );
 		$this->assertSame( 'Credit Card', $redacted['title'] );
-		$this->assertSame( '', $redacted['secret_key'], 'A live API secret must not be persisted.' );
-		$this->assertSame( '', $redacted['nested']['webhook'], 'Nested credentials must be redacted too.' );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['secret_key'], 'A live API secret must not be persisted.' );
+		$this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['nested']['webhook'], 'Nested credentials must be redacted too.' );
 		$this->assertSame( 'Pay by card', $redacted['nested']['description'] );
 
 		// The publishable half of a key pair is designed to be public, so