Harden multisite authorization and stop logging integration credentials

.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'

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}
 	 */

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,

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}
 	 */

classes/class-ability.php

@@ -96,6 +96,105 @@ 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;
+	}
+
+	/**
+	 * 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();
+		}
+
+		/**
+		 * 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;
+			}
+
+			$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).
 	 *

classes/class-connector.php

@@ -186,6 +186,141 @@ 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',
+	);
+
+	/**
+	 * 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.
+	 *
+	 * 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(
+		'public_key',
+		'publishable_key',
+		'site_key',
+	);
+
+	/**
+	 * 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 = '[redacted]';
+
+	/**
+	 * 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 );
+
+		// 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 true;
+			}
+		}
+
+		foreach ( self::PUBLIC_KEY_PATTERNS as $pattern ) {
+			if ( false !== strpos( $needle, $pattern ) ) {
+				return false;
+			}
+		}
+
+		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
 	 *

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;

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,