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-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/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/changelog.md b/changelog.md index cfcdc555b..684528062 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,34 @@ # Stream Changelog +## 4.4.0 - August 31, 2026 + +### Security + +- Keep multisite record reads inside the current site. `Network::network_query_args()` kept any numeric `blog_id` from the request. Because the Stream tables are shared across the network, a site-level `view_stream` user could read the activity of another site with `?blog_id=N`. Stream now accepts a requested blog ID only from users with `manage_network_options`. All other users get the current blog. WP-CLI keeps its exemption ([#1958](https://github.com/xwp/stream/pull/1958)). +- Stop the use of `HTTP_REFERER` as proof of network-admin authority in `ajax_network_admin()`. The Referer comes from the caller. A site user could use it to remove the per-blog query restriction, and, through `blog_id_logged()`, record their actions against `blog_id` 0. This made the site attribution incorrect. Stream now uses the Referer only as a hint about the user interface. A true network capability is also necessary ([#1958](https://github.com/xwp/stream/pull/1958)). +- Make a network capability necessary for the `update-settings` and `create-exclusion-rule` abilities when the write goes to `update_site_option()` on a network-activated multisite. Both abilities used the default `manage_options` permission callback before ([#1958](https://github.com/xwp/stream/pull/1958)). +- Remove secret values from option-change records. The WooCommerce, EDD, Jetpack, Gravity Forms, and Settings connectors wrote integration credentials into `stream_meta` as cleartext. This included payment gateway API keys, webhook signing keys, the `mailserver_pass` mailbox password, and the Gravity Forms license and reCAPTCHA private keys. Any user who can read the activity log or export it could read these values. Stream now replaces a secret value with `[redacted]`. A credential that was never set continues to log as an empty value, so you can tell the difference between "not set" and "set but withheld" ([#1958](https://github.com/xwp/stream/pull/1958)). +- Withhold alert destination credentials from the `stream/get-alerts` ability. The ability needs only `view_stream`, but you configure alert destinations behind the Stream settings capability. The ability returned `alert_meta` without change, which moved Slack webhook URLs and IFTTT Maker keys across a privilege boundary. Stream now replaces each secret with a `{key}_configured` boolean, so a caller can still see if a destination is configured. Non-secret configuration, such as the channel, the user name, and the event name, does not change ([#1958](https://github.com/xwp/stream/pull/1958)). +- Log only the changed field for Jetpack options, and withhold Publicize connection payloads. Jetpack attached the complete `jetpack_options` value to a record about one field, so a single miss could expose all tokens ([#1958](https://github.com/xwp/stream/pull/1958)). +- Correct the GHCR publish condition in `docker-images.yml`. The `contains( github.ref_name, 'master' )` guard also matched an unprotected branch name such as `feature-master-publish`. The workflow now compares the ref exactly ([#1958](https://github.com/xwp/stream/pull/1958)). + +### Bug Fixes + +- Correct a false "SITE IS DISCONNECTED" error from `wp stream query`. The CLI treated every empty result as a disconnected site, because `db->query()` returns an empty array both for "no matching records" and for a failure. The CLI now reads `$wpdb->last_error`, which WordPress sets only after a true database error. A site with no records no longer reports the error. This affected new installations and multisite subsites with no activity ([#1829](https://github.com/xwp/stream/issues/1829), [#1966](https://github.com/xwp/stream/pull/1966)). +- Correct a fatal error in `role_can_view()` when the capability check runs before `init` and the `general_role_access` setting is not yet available ([#1963](https://github.com/xwp/stream/pull/1963)). +- Correct a PHP warning for a password reset request that gives an unknown user. The `retrieve_password` handler now stops when `get_user_by()` returns `false`. Before, it read properties from `false` ([#1838](https://github.com/xwp/stream/issues/1838), [#1931](https://github.com/xwp/stream/pull/1931)). + +### Enhancements + +- Add the `wp_stream_secret_alert_meta_keys` filter. Use it to declare the destination secrets of a custom alert type that you register with `wp_stream_alert_types`, so Stream removes them from ability output ([#1958](https://github.com/xwp/stream/pull/1958)). +- Add `Connector::is_secret_key()` and `Connector::redact_secret_values()`, with the `SECRET_KEY_PATTERNS`, `SECRET_KEY_SUFFIXES`, `PUBLIC_KEY_PATTERNS`, and `REDACTED_PLACEHOLDER` constants. Custom connectors can use these helpers for the same redaction. Matching uses substrings and suffixes, because connectors log option values that belong to plugins Stream does not control. `PUBLIC_KEY_PATTERNS` keeps the published half of a key pair, such as `public_key`, `publishable_key`, and `site_key`, unless the name also holds a secret marker ([#1958](https://github.com/xwp/stream/pull/1958)). +- Add `Network::can_view_network_records()` as the single authority check for access to network-scoped records ([#1958](https://github.com/xwp/stream/pull/1958)). + +### Development + +- Add PHPUnit coverage for the hardened paths: `Network`, the `Connector` redaction helpers, `CLI::connection()`, `Admin`, the Gravity Forms, Jetpack, Settings, and Users connectors, and the `get-alerts` and `update-settings` abilities. `Network` had almost no direct coverage before ([#1958](https://github.com/xwp/stream/pull/1958), [#1966](https://github.com/xwp/stream/pull/1966)). +- Update Node.js to `^24.20.0` ([#1951](https://github.com/xwp/stream/pull/1951), [#1954](https://github.com/xwp/stream/pull/1954), [#1970](https://github.com/xwp/stream/pull/1970)), `globals` to `^17.11.0` ([#1947](https://github.com/xwp/stream/pull/1947), [#1953](https://github.com/xwp/stream/pull/1953), [#1964](https://github.com/xwp/stream/pull/1964), [#1965](https://github.com/xwp/stream/pull/1965)), `@playwright/test` to `^1.62.1` ([#1943](https://github.com/xwp/stream/pull/1943), [#1952](https://github.com/xwp/stream/pull/1952)), `npm-run-all2` to `^9.0.3` ([#1949](https://github.com/xwp/stream/pull/1949)), and `squizlabs/php_codesniffer` to 3.13.6 ([#1960](https://github.com/xwp/stream/pull/1960)). + ## 4.3.0 - July 18, 2026 ### Enhancements diff --git a/classes/class-ability.php b/classes/class-ability.php index 9029be4c8..cd13d1c36 100644 --- a/classes/class-ability.php +++ b/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). * diff --git a/classes/class-admin.php b/classes/class-admin.php index 51848a38f..ea2b0f2b1 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -1748,16 +1748,20 @@ public function register_list_table() { /** * Check if a particular role has access * + * The user_has_cap/role_has_cap filters that call this are registered in the + * constructor, but the Settings object is not constructed until init priority 9. + * A capability check fired before then (e.g. by a security plugin evaluating + * firewall rules on plugins_loaded) must be denied rather than fatal on the + * null options chain. + * * @param string $role User role. * * @return bool */ private function role_can_view( $role ) { - if ( in_array( $role, $this->plugin->settings->options['general_role_access'], true ) ) { - return true; - } + $allowed_roles = $this->plugin->settings->options['general_role_access'] ?? array(); - return false; + return in_array( $role, (array) $allowed_roles, true ); } /** diff --git a/classes/class-cli.php b/classes/class-cli.php index 5d7b9cde4..7258d2ed9 100644 --- a/classes/class-cli.php +++ b/classes/class-cli.php @@ -223,14 +223,19 @@ private function csv_format( $records ) { * @return void */ private function connection() { - $query = wp_stream_get_instance()->db->query( + global $wpdb; + + wp_stream_get_instance()->db->query( array( 'records_per_page' => 1, 'fields' => 'created', ) ); - if ( ! $query ) { + // An empty result set is valid (e.g. a fresh site with no logged + // activity yet); only a genuine database error means the site is + // disconnected. + if ( ! empty( $wpdb->last_error ) ) { \WP_CLI::error( esc_html__( 'SITE IS DISCONNECTED', 'stream' ) ); } } diff --git a/classes/class-connector.php b/classes/class-connector.php index 171f32f62..f45119f70 100644 --- a/classes/class-connector.php +++ b/classes/class-connector.php @@ -186,6 +186,150 @@ 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', + // PayPal NVP `api_signature`; over-matches e.g. `email_signature`. + 'signature', + ); + + /** + * 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 ) ) { + // A secret parent may key credentials by opaque IDs (e.g. Jetpack + // `user_tokens` by user ID), so pass it down over child names. + $inherited_key = $this->is_secret_key( $key ) ? $key : null; + + foreach ( $value as $child_key => $child_value ) { + $value[ $child_key ] = $this->redact_secret_values( + $child_value, + null !== $inherited_key ? $inherited_key : (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/classes/class-network.php b/classes/class-network.php index c1133ed33..15209145c 100644 --- a/classes/class-network.php +++ b/classes/class-network.php @@ -86,12 +86,17 @@ public function __construct( $plugin ) { public function ajax_network_admin() { $http_referer = isset( $_SERVER['HTTP_REFERER'] ) ? esc_url_raw( wp_unslash( $_SERVER['HTTP_REFERER'] ) ) : ''; + // Prefer filterable `wp_doing_ajax()` (WP 4.7+); the plugin supports 4.6. + $doing_ajax = function_exists( 'wp_doing_ajax' ) + ? wp_doing_ajax() + : ( defined( 'DOING_AJAX' ) && DOING_AJAX ); + if ( - defined( 'DOING_AJAX' ) - && - DOING_AJAX + $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 +105,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 +534,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; diff --git a/classes/class-plugin.php b/classes/class-plugin.php index 204ec575f..157a5336c 100755 --- a/classes/class-plugin.php +++ b/classes/class-plugin.php @@ -20,7 +20,7 @@ class Plugin { * * @const string */ - const VERSION = '4.3.0'; + const VERSION = '4.4.0'; /** * WP-CLI command diff --git a/composer.lock b/composer.lock index cc70ef19a..5ce9ff977 100644 --- a/composer.lock +++ b/composer.lock @@ -4911,16 +4911,16 @@ }, { "name": "squizlabs/php_codesniffer", - "version": "3.10.2", + "version": "3.13.6", "source": { "type": "git", "url": "https://github.com/PHPCSStandards/PHP_CodeSniffer.git", - "reference": "86e5f5dd9a840c46810ebe5ff1885581c42a3017" + "reference": "4c378e1a528ea066890fc2397cbdd2f94eb2fc91" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/86e5f5dd9a840c46810ebe5ff1885581c42a3017", - "reference": "86e5f5dd9a840c46810ebe5ff1885581c42a3017", + "url": "https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/4c378e1a528ea066890fc2397cbdd2f94eb2fc91", + "reference": "4c378e1a528ea066890fc2397cbdd2f94eb2fc91", "shasum": "" }, "require": { @@ -4937,11 +4937,6 @@ "bin/phpcs" ], "type": "library", - "extra": { - "branch-alias": { - "dev-master": "3.x-dev" - } - }, "notification-url": "https://packagist.org/downloads/", "license": [ "BSD-3-Clause" @@ -4985,9 +4980,13 @@ { "url": "https://opencollective.com/php_codesniffer", "type": "open_collective" + }, + { + "url": "https://thanks.dev/u/gh/phpcsstandards", + "type": "thanks_dev" } ], - "time": "2024-07-21T23:26:44+00:00" + "time": "2026-08-06T00:17:32+00:00" }, { "name": "symfony/config", 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-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-jetpack.php b/connectors/class-connector-jetpack.php index d11492d76..9d8fe7f84 100644 --- a/connectors/class-connector-jetpack.php +++ b/connectors/class-connector-jetpack.php @@ -620,10 +620,12 @@ public function check_jetpack_options( $old_value, $new_value ) { continue; } + // Log only the changed field: attaching the full `jetpack_options` + // blob would expose every credential on a single redaction miss. $settings['meta'] += array( 'option' => $option, - 'old_value' => $old_value, - 'value' => $new_value, + 'old_value' => $this->get_redacted_option_field( $old_value, $option ), + 'value' => $this->get_redacted_option_field( $new_value, $option ), ); $this->log( @@ -636,6 +638,42 @@ public function check_jetpack_options( $old_value, $new_value ) { } } + /** + * Get one changed field from a Jetpack option value, and redact it. + * + * `get_changed_keys()` gives nested fields as `parent::child`. Thus this + * method goes through the path one segment at a time. If the path does not + * exist, the result is null. The field was added or removed. + * + * @param mixed $option_value Whole option value, before or after the change. + * @param string $field_path Changed field, e.g. `publicize_connections::facebook`. + * @return mixed + */ + private function get_redacted_option_field( $option_value, $field_path ) { + if ( ! is_array( $option_value ) ) { + return null; + } + + $segments = explode( '::', $field_path ); + $field = $option_value; + + foreach ( $segments as $segment ) { + if ( ! is_array( $field ) || ! array_key_exists( $segment, $field ) ) { + return null; + } + + $field = $field[ $segment ]; + } + + // Publicize connections hold only credentials under opaque IDs, and + // the `connection` meta already names the service, so withhold them. + if ( 'publicize_connections' === $segments[0] && ! empty( $field ) ) { + return self::REDACTED_PLACEHOLDER; + } + + return $this->redact_secret_values( $field, $segments[ count( $segments ) - 1 ] ); + } + /** * Logs Google+ profile display status * 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/connectors/class-connector-users.php b/connectors/class-connector-users.php index 91b6287e7..a0edd9ee9 100644 --- a/connectors/class-connector-users.php +++ b/connectors/class-connector-users.php @@ -260,10 +260,14 @@ public function callback_password_reset( $user ) { * @param string $user_login User login. */ public function callback_retrieve_password( $user_login ) { - if ( wp_stream_filter_var( $user_login, FILTER_VALIDATE_EMAIL ) ) { + // Core passes user_login; try login first so email-shaped logins resolve. + $user = get_user_by( 'login', $user_login ); + if ( ! $user && is_email( $user_login ) ) { $user = get_user_by( 'email', $user_login ); - } else { - $user = get_user_by( 'login', $user_login ); + } + + if ( ! is_a( $user, 'WP_User' ) ) { + return; } $this->log( 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/package-lock.json b/package-lock.json index 14238a416..417e454d3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,19 +11,19 @@ "timeago": "^1.6.7" }, "devDependencies": { - "@playwright/test": "^1.61.1", + "@playwright/test": "^1.62.1", "@types/node": "^24.13.3", "@wordpress/e2e-test-utils-playwright": "^1.46.0", "@wordpress/eslint-plugin": "^25.2.0", "@wordpress/scripts": "^32.2.0", "copy-webpack-plugin": "^14.0.0", "eslint-plugin-react-hooks": "^7.1.1", - "globals": "^17.7.0", + "globals": "^17.11.0", "jquery": "4.0.0", - "npm-run-all2": "^9.0.2" + "npm-run-all2": "^9.0.3" }, "engines": { - "node": "^24.18.0" + "node": "^24.20.0" } }, "node_modules/@ampproject/remapping": { @@ -4466,19 +4466,19 @@ } }, "node_modules/@playwright/test": { - "version": "1.61.1", - "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.61.1.tgz", - "integrity": "sha512-8nKv6+0RJSL9FE4jYOEGXnPeM/Hg12qZpmqzZjRh3qM0Y7c3z1mrOTfFLids72RDQYVh9WpLEfR5WdpNX4fkig==", + "version": "1.62.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.62.1.tgz", + "integrity": "sha512-DTcUc8qii+cpHvtOwggMtBRMjKZHXYWdw8syRYu2vtzuq4Wxphqq4NfCs5Zt44L6mA8rfDfj+PHnxFc/FeK6mQ==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright": "1.61.1" + "playwright": "1.62.1" }, "bin": { "playwright": "cli.js" }, "engines": { - "node": ">=18" + "node": ">=20" } }, "node_modules/@pmmmwh/react-refresh-webpack-plugin": { @@ -12131,9 +12131,9 @@ } }, "node_modules/globals": { - "version": "17.7.0", - "resolved": "https://registry.npmjs.org/globals/-/globals-17.7.0.tgz", - "integrity": "sha512-Czmyns5dUsq4seFBR/Kdydhmo8y9kC79hiSkPn0YcGtNnYWnrgt0vjrSjx9tspoDGWm2CMarffRuLjM4xUz8xg==", + "version": "17.11.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-17.11.0.tgz", + "integrity": "sha512-Z2I8hM+PbJDXQDq3Icgpzv+mPdwr68iZUU9d5WW4FuXfDUQfkZaZuvjMv42/5crNyw154+9+VWXbYrUgDXbxNw==", "dev": true, "license": "MIT", "engines": { @@ -16087,13 +16087,13 @@ } }, "node_modules/npm-run-all2": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/npm-run-all2/-/npm-run-all2-9.0.2.tgz", - "integrity": "sha512-+dd4SO2jAlLE06OzmJKzIe6QvvjXezcbmobnh8usR0a8BzQCABTdqTXqVPji0ICOhSQpIIrkGd7IzNl5iDaRSA==", + "version": "9.0.3", + "resolved": "https://registry.npmjs.org/npm-run-all2/-/npm-run-all2-9.0.3.tgz", + "integrity": "sha512-BQAEdU1PtYc48qYRdghW2BVTQT3VqWCoFQmO87NlM1h1PYwMCKQpUWaNyB20V26caNzFsXQDfyOdznLlHCih6g==", "dev": true, "license": "MIT", "dependencies": { - "ansi-styles": "^6.2.1", + "ansi-styles": "^7.0.0", "cross-spawn": "^7.0.6", "memorystream": "^0.3.1", "picomatch": "^4.0.2", @@ -16114,13 +16114,13 @@ } }, "node_modules/npm-run-all2/node_modules/ansi-styles": { - "version": "6.2.3", - "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.3.tgz", - "integrity": "sha512-4Dj6M28JB+oAH8kFkTLUo+a2jwOFkuqb3yucU0CANcRRUbxS0cP0nZYCGjcc3BNXwRIsUVmDGgzawme7zvJHvg==", + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-7.0.0.tgz", + "integrity": "sha512-kKvt3m4uwzqL0wlkPd09CmljPJGOZZ4D0fP65sqFSvPkMRKhNi+74MgIJ5QxE6SxqB4t4KyUFGg8+n5zjo6hew==", "dev": true, "license": "MIT", "engines": { - "node": ">=12" + "node": ">=22" }, "funding": { "url": "https://github.com/chalk/ansi-styles?sponsor=1" @@ -16137,9 +16137,9 @@ } }, "node_modules/npm-run-all2/node_modules/picomatch": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", - "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", + "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", "dev": true, "license": "MIT", "engines": { @@ -16948,35 +16948,35 @@ } }, "node_modules/playwright": { - "version": "1.61.1", - "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.61.1.tgz", - "integrity": "sha512-DWnY5o3YbLWK4GovuAVwpqL+1VwGNdUGrRr++8j8PtQQzvAVZUIMjKQ90fY689sEJZJBbZVw1rXaOKSTitkzPQ==", + "version": "1.62.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.62.1.tgz", + "integrity": "sha512-0M+L3LAD8/nm554LOla9Ayx0j0tmFZ0FBcoQ7F1VuVHpM/XpiC8RcDzBQB8W5+hA8L22THxELzeF+2WcUzvcLg==", "dev": true, "license": "Apache-2.0", "dependencies": { - "playwright-core": "1.61.1" + "playwright-core": "1.62.1" }, "bin": { "playwright": "cli.js" }, "engines": { - "node": ">=18" + "node": ">=20" }, "optionalDependencies": { "fsevents": "2.3.2" } }, "node_modules/playwright-core": { - "version": "1.61.1", - "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.61.1.tgz", - "integrity": "sha512-h7Qlt6m4REp25qvIdvbDtVmD4LqVXfpRxhORv9L0jzETM05p4fuPJ3dKyuSXQxDSbXnmS79HAgi9589lGSpLkg==", + "version": "1.62.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.62.1.tgz", + "integrity": "sha512-wPYSwEBJY9GHraISXqyqtx0na0LpO3XEX7jNDhntbex7tzUS7kLnZsOlFruFJB4Hi/rhDMjXGqHewDZ68nYZVw==", "dev": true, "license": "Apache-2.0", "bin": { "playwright-core": "cli.js" }, "engines": { - "node": ">=18" + "node": ">=20" } }, "node_modules/playwright/node_modules/fsevents": { diff --git a/package.json b/package.json index 5f2571291..294eee69e 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "author": "XWP", "license": "GPLv2+", "engines": { - "node": "^24.18.0" + "node": "^24.20.0" }, "repository": { "type": "git", @@ -24,16 +24,16 @@ "track" ], "devDependencies": { - "@playwright/test": "^1.61.1", + "@playwright/test": "^1.62.1", "@types/node": "^24.13.3", "@wordpress/e2e-test-utils-playwright": "^1.46.0", "@wordpress/eslint-plugin": "^25.2.0", "@wordpress/scripts": "^32.2.0", "copy-webpack-plugin": "^14.0.0", "eslint-plugin-react-hooks": "^7.1.1", - "globals": "^17.7.0", + "globals": "^17.11.0", "jquery": "4.0.0", - "npm-run-all2": "^9.0.2" + "npm-run-all2": "^9.0.3" }, "scripts": { "build": "wp-scripts build", diff --git a/readme.txt b/readme.txt index 1dd9594be..f9393f17d 100644 --- a/readme.txt +++ b/readme.txt @@ -3,7 +3,7 @@ Contributors: xwp Tags: activity log, audit log, event log, user tracking, security Requires at least: 4.6 Tested up to: 7.0 -Stable tag: 4.3.0 +Stable tag: 4.4.0 License: GPLv2 or later License URI: https://www.gnu.org/licenses/gpl-2.0.html @@ -132,6 +132,10 @@ Thank you for wanting to make Stream better for everyone! == Upgrade Notice == += 4.4.0 = + +On multisite, access to the records of another site and changes to network-wide settings now need the `manage_network_options` capability. Stream no longer writes integration credentials, such as API keys and tokens, into option-change records. See the changelog for details. + = 4.0.0 = Use only `$_SERVER['REMOTE_ADDR']` as the client IP address for event logs without additional support for `X-Forwarded-For` HTTP request header value which could be spoofed. See the changelog for additional details. @@ -139,6 +143,22 @@ Use only `$_SERVER['REMOTE_ADDR']` as the client IP address for event logs witho == Changelog == += 4.4.0 - August 31, 2026 = + +Security: + +* Keep multisite record reads inside the current site, and make a network capability necessary for changes to network-wide settings. +* Remove integration credentials from option-change records. Secret values now show as `[redacted]`. +* Withhold alert destination credentials, such as Slack webhook URLs, from the `stream/get-alerts` ability. + +Bug Fixes: + +* Correct a false "SITE IS DISCONNECTED" error from `wp stream query` on a site with no records. +* Correct a fatal error when a Stream capability check runs before `init`. +* Correct a PHP warning for a password reset request that gives an unknown user. + +[View the full release notes on GitHub.](https://github.com/xwp/stream/blob/master/changelog.md#440---august-31-2026) + = 4.3.0 - July 18, 2026 = Enhancements: @@ -175,29 +195,6 @@ Bug Fixes: = 4.2.0 - May 28, 2026 = -New Features: - -* Expose Stream abilities via the WordPress MCP Adapter when present, enabling AI tools to query Stream records through the Abilities API. - -Bug Fixes: - -* Fix unbounded growth of `stream` / `stream_meta` tables: the TTL-based auto-purge now runs via Action Scheduler with batched deletion, resolving database bloat on large sites. -* Fix orphan `stream_meta` rows accumulating across repeated purge cycles with a terminal orphan reaper at the end of every auto-purge chain. -* Skip Action Scheduler queries on front-end pageloads, eliminating unnecessary queries per pageview. - -Enhancements: - -* Add a Clean Orphaned Meta link under Settings → Advanced for one-shot cleanup on already-bloated installs. -* Replace the legacy `wp_stream_auto_purge` WP-Cron event with a recurring Action Scheduler action, with run history visible under Tools → Scheduled Actions. - -[View the full release notes on GitHub.](https://github.com/xwp/stream/blob/master/changelog.md#420---may-28-2026) - -= 4.1.2 - February 19, 2026 = - -[View the release notes.](https://github.com/xwp/stream/blob/master/changelog.md#412---february-19-2026) - -= 4.1.1 - February 3, 2025 = - -[View the release notes.](https://github.com/xwp/stream/blob/master/changelog.md#411---february-3-2025) +[View the release notes.](https://github.com/xwp/stream/blob/master/changelog.md#420---may-28-2026) [See the full changelog for all releases.](https://github.com/xwp/stream/blob/master/changelog.md) diff --git a/stream.php b/stream.php index 3b644a88a..cd3d28420 100644 --- a/stream.php +++ b/stream.php @@ -3,7 +3,7 @@ * Plugin Name: Stream - Activity Log & Audit Trail * Plugin URI: https://xwp.co/work/stream/ * Description: Stream is an activity log and audit trail for WordPress — track every change made on your site in beautifully organized detail. All activity is organized by context, action and IP address for easy filtering. Developers can extend Stream with custom connectors to log any kind of action. - * Version: 4.3.0 + * Version: 4.4.0 * Author: XWP * Author URI: https://xwp.co * License: GPLv2+ diff --git a/tests/phpunit/abilities/test-class-ability-get-alerts.php b/tests/phpunit/abilities/test-class-ability-get-alerts.php index b020a524a..7ccb1d794 100644 --- a/tests/phpunit/abilities/test-class-ability-get-alerts.php +++ b/tests/phpunit/abilities/test-class-ability-get-alerts.php @@ -126,4 +126,170 @@ 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'] ); + } + + /** + * 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. + * + * @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/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/connectors/test-class-connector-gravityforms.php b/tests/phpunit/connectors/test-class-connector-gravityforms.php new file mode 100644 index 000000000..798d97918 --- /dev/null +++ b/tests/phpunit/connectors/test-class-connector-gravityforms.php @@ -0,0 +1,143 @@ +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( Connector::REDACTED_PLACEHOLDER, $logged['old_value'] ); + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $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( 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. + $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-jetpack.php b/tests/phpunit/connectors/test-class-connector-jetpack.php index 7696e6583..de35757a0 100644 --- a/tests/phpunit/connectors/test-class-connector-jetpack.php +++ b/tests/phpunit/connectors/test-class-connector-jetpack.php @@ -309,4 +309,135 @@ public function test_track_post_by_email() { $this->assertFalse( 0 === did_action( $this->action_prefix . 'callback_wp_ajax_jetpack_post_by_email_regenerate' ) ); $this->assertFalse( 0 === did_action( $this->action_prefix . 'callback_wp_ajax_jetpack_post_by_email_disable' ) ); } + + /** + * `jetpack_options` holds all Jetpack credentials in one blob. A record + * about one changed setting must carry only that field, and no token from + * the other fields. + */ + public function test_check_jetpack_options_logs_only_the_changed_field() { + $logged = array(); + + $connector = $this->getMockBuilder( Connector_Jetpack::class ) + ->setMethods( array( 'log' ) ) + ->getMock(); + + $connector->method( 'log' )->willReturnCallback( + function ( $message, $meta ) use ( &$logged ) { + $logged[] = $meta; + } + ); + + $old_value = array( + 'videopress' => array( + 'access' => 'edit_posts', + ), + 'user_tokens' => array( + '1' => 'user.token.one', + ), + 'publicize_connections' => array( + 'facebook' => array( + 'id_1' => array( + 'access_token' => 'facebook.access.token', + ), + ), + ), + ); + + $new_value = $old_value; + $new_value['videopress']['access'] = 'read'; + + $connector->check_jetpack_options( $old_value, $new_value ); + + $this->assertNotEmpty( $logged, 'A VideoPress setting change must be logged.' ); + + $serialized = maybe_serialize( $logged ); + + $this->assertStringNotContainsString( 'user.token.one', $serialized, 'A user token must not reach record metadata.' ); + $this->assertStringNotContainsString( 'facebook.access.token', $serialized, 'A Publicize token must not reach record metadata.' ); + + $this->assertSame( 'edit_posts', $logged[0]['old_value'] ); + $this->assertSame( 'read', $logged[0]['value'] ); + } + + /** + * A Publicize connection holds only credentials, thus the value payload is + * withheld. The service label still says what changed. + */ + public function test_check_jetpack_options_withholds_publicize_connection_payload() { + $logged = array(); + + $connector = $this->getMockBuilder( Connector_Jetpack::class ) + ->setMethods( array( 'log' ) ) + ->getMock(); + + $connector->method( 'log' )->willReturnCallback( + function ( $message, $meta ) use ( &$logged ) { + $logged[] = $meta; + } + ); + + // The connector reads the service label from Publicize's UI global, + // which only exists when the module is active. + global $publicize_ui; + + $original_publicize_ui = $publicize_ui; + $publicize_ui = new class() { // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited + /** + * Publicize stub. + * + * @var object + */ + public $publicize; + + /** + * Give the label lookup the connector needs. + */ + public function __construct() { + $this->publicize = new class() { + /** + * Service label for a connection name. + * + * @param string $name Service name. + * @return string + */ + public function get_service_label( $name ) { + return ucfirst( $name ); + } + }; + } + }; + + $old_value = array( + 'publicize_connections' => array(), + ); + + $new_value = array( + 'publicize_connections' => array( + 'facebook' => array( + 'id_1' => array( + 'access_token' => 'facebook.access.token', + 'access_token_secret' => 'facebook.token.secret', + ), + ), + ), + ); + + try { + $connector->check_jetpack_options( $old_value, $new_value ); + } finally { + $publicize_ui = $original_publicize_ui; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited + } + + $serialized = maybe_serialize( $logged ); + + $this->assertStringNotContainsString( 'facebook.access.token', $serialized ); + $this->assertStringNotContainsString( 'facebook.token.secret', $serialized ); + + $this->assertNotEmpty( $logged, 'A new Publicize connection must be logged.' ); + + foreach ( $logged as $meta ) { + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $meta['value'] ); + } + } } diff --git a/tests/phpunit/connectors/test-class-connector-settings.php b/tests/phpunit/connectors/test-class-connector-settings.php index 4c1baf350..e1ebb2d76 100644 --- a/tests/phpunit/connectors/test-class-connector-settings.php +++ b/tests/phpunit/connectors/test-class-connector-settings.php @@ -156,4 +156,107 @@ 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'; + + // 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' ); + + $this->register_writing_options(); + + $logged = array(); + $this->mock->expects( $this->atLeastOnce() ) + ->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( 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 ); + $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/connectors/test-class-connector-users.php b/tests/phpunit/connectors/test-class-connector-users.php index 6f5f0e371..2b3f4f9a0 100644 --- a/tests/phpunit/connectors/test-class-connector-users.php +++ b/tests/phpunit/connectors/test-class-connector-users.php @@ -133,6 +133,49 @@ public function test_callback_retrieve_password_and_profile_update() { $this->assertFalse( 0 === did_action( $this->action_prefix . 'callback_retrieve_password' ) ); } + /** + * Email-shaped login that is not the user_email must still log. + * + * Core passes user_login to retrieve_password. Looking up by email first + * returns false when login looks like an email but differs from user_email. + */ + public function test_callback_retrieve_password_with_email_shaped_login() { + $user_id = self::factory()->user->create( + array( + 'user_login' => 'john@example.com', + 'user_email' => 'different@example.com', + 'display_name' => 'EmailLoginGuy', + ) + ); + $user = get_user_by( 'id', $user_id ); + + $this->mock->expects( $this->once() ) + ->method( 'log' ) + ->with( + $this->equalTo( __( '%s\'s password was requested to be reset', 'stream' ) ), + $this->equalTo( array( 'display_name' => 'EmailLoginGuy' ) ), + $this->equalTo( $user_id ), + $this->equalTo( 'sessions' ), + $this->equalTo( 'forgot-password' ), + $this->equalTo( $user_id ) + ); + + do_action( 'retrieve_password', $user->user_login ); + + $this->assertFalse( 0 === did_action( $this->action_prefix . 'callback_retrieve_password' ) ); + } + + /** + * Unknown login must not call log or trigger property-on-bool warnings. + */ + public function test_callback_retrieve_password_with_unknown_user() { + $this->mock->expects( $this->never() )->method( 'log' ); + + do_action( 'retrieve_password', 'nonexistent-user-xyz' ); + + $this->assertFalse( 0 === did_action( $this->action_prefix . 'callback_retrieve_password' ) ); + } + public function test_callback_set_logged_in_cookie() { // Create user. $user_id = self::factory()->user->create( array( 'display_name' => 'TestGuy' ) ); diff --git a/tests/phpunit/test-class-admin.php b/tests/phpunit/test-class-admin.php index ba29f6869..b16fee4b4 100644 --- a/tests/phpunit/test-class-admin.php +++ b/tests/phpunit/test-class-admin.php @@ -73,6 +73,45 @@ public function test_init() { $this->assertInstanceOf( '\WP_Stream\Export', $this->admin->export ); } + /** + * The user_has_cap filter is registered in the Admin constructor, but the + * Settings object is only built on init priority 9. A capability check for + * the view cap fired before then (e.g. a firewall plugin on plugins_loaded) + * must be denied, not fatal on the null options chain. + */ + public function test_filter_user_caps_before_settings_initialized() { + $settings = $this->plugin->settings; + $this->plugin->settings = null; + + $user = get_user_by( 'id', $this->admin_user_id ); + $allcaps = $this->admin->filter_user_caps( + array(), + array( $this->admin->view_cap ), + array( $this->admin->view_cap, $this->admin_user_id ), + $user + ); + + $this->plugin->settings = $settings; + + $this->assertArrayNotHasKey( $this->admin->view_cap, $allcaps ); + } + + /** + * Once Settings exists, the view cap is granted to allowed roles as before. + */ + public function test_filter_user_caps_grants_view_cap_to_allowed_role() { + $user = get_user_by( 'id', $this->admin_user_id ); + $allcaps = $this->admin->filter_user_caps( + array(), + array( $this->admin->view_cap ), + array( $this->admin->view_cap, $this->admin_user_id ), + $user + ); + + $this->assertArrayHasKey( $this->admin->view_cap, $allcaps ); + $this->assertTrue( $allcaps[ $this->admin->view_cap ] ); + } + public function test_prepare_admin_notices() { // Test no notices $this->admin->notices = array(); diff --git a/tests/phpunit/test-class-cli.php b/tests/phpunit/test-class-cli.php new file mode 100644 index 000000000..939621ec0 --- /dev/null +++ b/tests/phpunit/test-class-cli.php @@ -0,0 +1,74 @@ +setAccessible( true ); + $capture_exit->setValue( null, true ); + + $connection = new ReflectionMethod( CLI::class, 'connection' ); + $connection->setAccessible( true ); + + try { + $connection->invoke( new CLI() ); + } finally { + $capture_exit->setValue( null, false ); + } + } + + /** + * A query that legitimately matches zero records is not a disconnection. + */ + public function test_connection_does_not_error_on_empty_result() { + global $wpdb; + + // Force the connection check's query to match nothing, without + // touching any actual data other tests rely on. + $force_no_matches = function ( $where ) { + return $where . ' AND 1=0'; + }; + add_filter( 'wp_stream_db_query_where', $force_no_matches ); + + try { + $this->invoke_connection(); + } finally { + remove_filter( 'wp_stream_db_query_where', $force_no_matches ); + } + + // Reaching this line means WP_CLI::error() was never triggered. + $this->assertEmpty( $wpdb->last_error ); + } + + /** + * A genuine database error should still be reported as a disconnected site. + */ + public function test_connection_errors_on_database_failure() { + global $wpdb; + + $original_table = $wpdb->stream; + $wpdb->stream = $wpdb->prefix . 'stream_table_that_does_not_exist'; + $wpdb->suppress_errors( true ); + + try { + $this->expectException( ExitException::class ); + $this->invoke_connection(); + } finally { + $wpdb->stream = $original_table; + $wpdb->suppress_errors( false ); + $wpdb->last_error = ''; + } + } +} diff --git a/tests/phpunit/test-class-connector.php b/tests/phpunit/test-class-connector.php index 55305f6b8..73cdb7cf0 100644 --- a/tests/phpunit/test-class-connector.php +++ b/tests/phpunit/test-class-connector.php @@ -271,4 +271,213 @@ 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', + // PayPal NVP gateways match none of the other patterns. + 'api_signature', + 'paypal_pro_signature', + ); + 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', + // 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 ) { + $this->assertFalse( + $this->connector->is_secret_key( $key ), + "$key should not be treated as a secret." + ); + } + } + + /** + * 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. + * + * @return void + */ + public function test_redact_secret_values_scalar() { + $this->assertSame( + Connector::REDACTED_PLACEHOLDER, + $this->connector->redact_secret_values( 'hunter2', 'mailserver_pass' ) + ); + $this->assertSame( + 'My Site', + $this->connector->redact_secret_values( 'My Site', 'blogname' ) + ); + } + + /** + * 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.' + ); + } + + /** + * 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( 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 + // 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 ), + 'The secret must not survive serialization into record metadata.' + ); + } + + /** + * A secret parent can hold credentials under opaque keys. Jetpack keys + * `user_tokens` by user ID, so no child name shows that the value is a + * token. The parent name must make all children secret. + * + * @return void + */ + public function test_redact_secret_values_secret_parent_map() { + $input = array( + '1' => 'user.token.one', + '42' => 'user.token.forty.two', + ); + + $redacted = $this->connector->redact_secret_values( $input, 'user_tokens' ); + + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['1'] ); + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['42'] ); + $this->assertStringNotContainsString( + 'user.token.one', + maybe_serialize( $redacted ), + 'A token under an opaque key must not reach record metadata.' + ); + } + + /** + * The parent name goes down through every level, so a deep tree under a + * secret parent cannot keep a credential. + * + * @return void + */ + public function test_redact_secret_values_secret_parent_nested_map() { + $input = array( + 'facebook_1' => array( + 'id' => '1234', + 'access_token' => 'live.access.token', + 'profile' => array( + 'name' => 'Example Page', + ), + ), + ); + + $redacted = $this->connector->redact_secret_values( $input, 'publicize_credentials' ); + + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['facebook_1']['id'] ); + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['facebook_1']['access_token'] ); + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['facebook_1']['profile']['name'] ); + } + + /** + * A parent that is not secret keeps the current behaviour: each child is + * judged by its own name, so non-secret settings stay readable. + * + * @return void + */ + public function test_redact_secret_values_keeps_children_of_safe_parent() { + $input = array( + 'blogname' => 'My Site', + 'api_key' => 'live-key', + ); + + $redacted = $this->connector->redact_secret_values( $input, 'jetpack_options' ); + + $this->assertSame( 'My Site', $redacted['blogname'] ); + $this->assertSame( Connector::REDACTED_PLACEHOLDER, $redacted['api_key'] ); + } } diff --git a/tests/phpunit/test-class-network.php b/tests/phpunit/test-class-network.php new file mode 100644 index 000000000..8c1bf1103 --- /dev/null +++ b/tests/phpunit/test-class-network.php @@ -0,0 +1,271 @@ +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.' + ); + } + + /** + * End-to-end form of the same rule: an ajax request that carries a + * network-admin Referer, from a user with no network capability, must not + * become network-admin context. + * + * WP_NETWORK_ADMIN is a constant, thus a true result would stay for all + * later tests in the process. The assertion is that the method refuses, + * which keeps the constant undefined. + * + * @group ms-required + */ + public function test_spoofed_referer_does_not_set_network_admin_over_ajax() { + $user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); + wp_set_current_user( $user_id ); + + $original_referer = isset( $_SERVER['HTTP_REFERER'] ) ? $_SERVER['HTTP_REFERER'] : null; + $_SERVER['HTTP_REFERER'] = network_admin_url(); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated + add_filter( 'wp_doing_ajax', '__return_true' ); + + try { + $this->assertFalse( + $this->network->ajax_network_admin(), + 'A spoofed Referer must not grant network-admin context.' + ); + $this->assertFalse( + defined( 'WP_NETWORK_ADMIN' ), + 'WP_NETWORK_ADMIN must stay undefined for a site-level user.' + ); + } finally { + remove_filter( 'wp_doing_ajax', '__return_true' ); + + if ( null === $original_referer ) { + unset( $_SERVER['HTTP_REFERER'] ); + } else { + $_SERVER['HTTP_REFERER'] = $original_referer; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated + } + } + } + + /** + * Counterpart: a super admin on the same ajax request does get network + * context. The check runs through can_view_network_records() to avoid the + * WP_NETWORK_ADMIN constant, which would stay for all later tests. + * + * @group ms-required + */ + public function test_super_admin_gets_network_context_over_ajax() { + $user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); + wp_set_current_user( $user_id ); + grant_super_admin( $user_id ); + + $original_referer = isset( $_SERVER['HTTP_REFERER'] ) ? $_SERVER['HTTP_REFERER'] : null; + $_SERVER['HTTP_REFERER'] = network_admin_url(); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated + add_filter( 'wp_doing_ajax', '__return_true' ); + + try { + $this->assertTrue( + 0 === stripos( $_SERVER['HTTP_REFERER'], network_admin_url() ) // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated + && $this->network->can_view_network_records(), + 'A super admin must keep network-admin context over ajax.' + ); + } finally { + remove_filter( 'wp_doing_ajax', '__return_true' ); + revoke_super_admin( $user_id ); + + if ( null === $original_referer ) { + unset( $_SERVER['HTTP_REFERER'] ); + } else { + $_SERVER['HTTP_REFERER'] = $original_referer; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated + } + } + } + + /** + * 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.' + ); + } +}