From d5abd926c45e9938ef60a685ddc0c67a603bc0ef Mon Sep 17 00:00:00 2001 From: ABB65 Date: Sun, 18 Oct 2026 23:47:02 +0300 Subject: [PATCH 01/10] fix(key): a request with the connection key runs as no WordPress user; exports carry an explicit owner Key::authorize() no longer calls wp_set_current_user(). The key is valid while its creator still has export + manage_options, checked with user_can() on the stored user id, which never changes the request's user. A malformed key is refused before it is hashed and counts as a wrong key. Key::$active is cleared in rest_request_after_callbacks so nothing leaks into the next request of the same PHP process. Jobs::create() takes a principal (Jobs::principal(): user + via = admin | app_password | key, plus the key's hash) and records owner, via and key in the state; Jobs::owned() lets a key read only the exports it started (via key, its hash, listed in the plugin's option) and a signed-in user only their own non-key exports. Remote indexes a key's exports in the plugin's option, never in user meta, and locks starts per key, so a key can no longer reuse, bind or replace an export the administrator started with an application password. Revoking or replacing a key removes its exports; DELETE /key works with the key alone, as before. Every route, status, error code and body shape Migrate relies on is unchanged. REST routes now declare their arguments with validate and sanitize callbacks. tests/key.php records the user id, is_user_logged_in() and manage_options before and after every keyed callback (0 / false / false), checks user meta, the state's owner/via, reuse isolation, parity with an application-password export of the same scope, the malformed key, and that /wp/v2 routes stay closed. --- contentrain-bridge.php | 1 + includes/class-contentrain-bridge-jobs.php | 49 ++++++-- includes/class-contentrain-bridge-key.php | 118 +++++++++++++++---- includes/class-contentrain-bridge-remote.php | 107 +++++++++++++---- tests/key.php | 96 ++++++++++++++- 5 files changed, 310 insertions(+), 61 deletions(-) diff --git a/contentrain-bridge.php b/contentrain-bridge.php index d3732b0..250ec73 100644 --- a/contentrain-bridge.php +++ b/contentrain-bridge.php @@ -25,6 +25,7 @@ unset( $contentrain_bridge_class ); \Contentrain\Bridge\Admin::register(); +\Contentrain\Bridge\Key::register(); \Contentrain\Bridge\Robots::register(); foreach ( array( 'save_post', 'deleted_post', 'added_post_meta', 'updated_post_meta', 'deleted_post_meta', 'created_term', 'edited_term', 'delete_term', 'added_term_meta', 'updated_term_meta', 'deleted_term_meta', 'comment_post', 'edit_comment', 'deleted_comment', 'transition_comment_status', 'profile_update', 'switch_theme' ) as $contentrain_bridge_hook ) { add_action( $contentrain_bridge_hook, array( '\Contentrain\Bridge\Source', 'changed' ), 10, 3 ); diff --git a/includes/class-contentrain-bridge-jobs.php b/includes/class-contentrain-bridge-jobs.php index 775b47a..19c93e5 100644 --- a/includes/class-contentrain-bridge-jobs.php +++ b/includes/class-contentrain-bridge-jobs.php @@ -5,13 +5,27 @@ defined( 'ABSPATH' ) || exit; final class Jobs { + /** + * Who an export belongs to, said explicitly: `user` (the administrator's id) and `via` — `admin` (the Bridge + * screen), `app_password` (REST, signed in) or `key` (REST with the connection key, which signs nobody in: + * the user is the key's creator, read from the key's record, never from `get_current_user_id()`). + */ + public static function principal( $remote = false ) { + if ( Key::active() ) { + return array( 'user' => Key::owner(), 'via' => 'key', 'key' => Key::hash() ); + } + return array( 'user' => get_current_user_id(), 'via' => $remote ? 'app_password' : 'admin' ); + } + /** * `$remote`: started over REST (`Remote`), listed there rather than as the * admin screen's one export, and never waiting on a person: no source scan, - * so no text candidates to review. + * so no text candidates to review. `$principal`: whose export it is + * (`principal()`); the state records it as `owner` and `via`. */ - public static function create( $input, $remote = false ) { + public static function create( $input, $remote = false, $principal = null ) { Files::cleanup(); + $principal = is_array( $principal ) ? $principal : self::principal( $remote ); if ( $remote ) { $input = array_merge( $input, array( 'scan_plugins' => false, 'scan_sources' => false, 'scan_render' => false, 'selected_meta' => array(), 'labels' => array() ) ); } @@ -20,7 +34,7 @@ public static function create( $input, $remote = false ) { if ( ! $types ) { throw new \RuntimeException( 'Select at least one content type.' ); } - $old = $remote ? null : get_user_meta( get_current_user_id(), 'contentrain_bridge_job_' . get_current_blog_id(), true ); + $old = 'admin' === $principal['via'] ? get_user_meta( get_current_user_id(), 'contentrain_bridge_job_' . get_current_blog_id(), true ) : null; if ( $old && is_dir( Files::dir( $old ) ) ) { throw new \RuntimeException( 'Resume or delete your previous export before starting another.' ); } @@ -34,7 +48,7 @@ public static function create( $input, $remote = false ) { // taken here because it selects the paths every later step writes to. $languages = array_values( array_unique( array_map( array( Source::class, 'locale' ), (array) $inventory['languages'] ) ) ); $job = array( - 'id' => $id, 'remote' => (bool) $remote, 'owner' => get_current_user_id(), 'blog' => get_current_blog_id(), 'created_at' => gmdate( 'c' ), + 'id' => $id, 'remote' => (bool) $remote, 'owner' => (int) $principal['user'], 'via' => $principal['via'], 'key' => $principal['key'] ?? null, 'blog' => get_current_blog_id(), 'created_at' => gmdate( 'c' ), 'revision' => Source::revision(), 'phase' => 'media', 'cursor' => 0, 'step' => 0, 'inventory' => $inventory, 'default_locale' => $locale, 'locales' => array( $locale => true ), 'i18n' => count( $languages ) > 1, 'options' => array( 'types' => $types, 'private' => ! empty( $input['private'] ), 'comments' => ! empty( $input['comments'] ), 'media_files' => ! array_key_exists( 'media_files', $input ) || ! empty( $input['media_files'] ), 'scan_plugins' => ! empty( $input['scan_plugins'] ), 'scan_sources' => ! empty( $input['scan_sources'] ), 'scan_render' => ! empty( $input['scan_render'] ), 'selected_meta' => array_values( array_filter( array_map( 'sanitize_text_field', (array) ( $input['selected_meta'] ?? array() ) ) ) ), 'labels' => array_map( 'sanitize_text_field', (array) ( $input['labels'] ?? array() ) ) ), @@ -70,15 +84,31 @@ public static function create( $input, $remote = false ) { } self::warning( $job, array( 'source' => 'rendered-states', 'reason' => 'Source scan does not execute dynamic WordPress/plugin states. Rendered coverage requires the Migrate capture adapter.' ) ); self::save( $job ); - if ( ! $remote ) { + if ( 'admin' === $principal['via'] ) { update_user_meta( get_current_user_id(), 'contentrain_bridge_job_' . get_current_blog_id(), $id ); } return self::summary( $job ); } + /** + * Whether the caller may read this export. A key reads the exports it started (`via` key, listed in `Key::exports()`) + * and nothing else; a signed-in administrator reads their own screen and application-password exports and never a + * key's. Neither side's rule touches the other's identity: with a key there is no current user to compare. + */ + public static function owned( $job ) { + if ( ! is_array( $job ) || ! isset( $job['owner'], $job['blog'] ) || (int) $job['blog'] !== get_current_blog_id() ) { + return false; + } + $via = isset( $job['via'] ) ? (string) $job['via'] : ''; + if ( Key::active() ) { + return 'key' === $via && ( $job['key'] ?? '' ) === Key::hash() && in_array( $job['id'] ?? '', Key::exports(), true ); + } + return 'key' !== $via && get_current_user_id() > 0 && (int) $job['owner'] === get_current_user_id(); + } + public static function read( $id ) { $job = json_decode( Files::read( Files::dir( $id ), 'state.json' ), true ); - if ( ! is_array( $job ) || (int) $job['owner'] !== get_current_user_id() || (int) $job['blog'] !== get_current_blog_id() ) { + if ( ! self::owned( $job ) ) { throw new \RuntimeException( 'Export is not owned by the current user.' ); } if ( strtotime( $job['created_at'] ) < time() - DAY_IN_SECONDS ) { @@ -228,7 +258,7 @@ public static function time_limit() { if ( $max <= 0 ) { return INF; } - $start = isset( $_SERVER['REQUEST_TIME_FLOAT'] ) ? (float) $_SERVER['REQUEST_TIME_FLOAT'] : microtime( true ); + $start = isset( $_SERVER['REQUEST_TIME_FLOAT'] ) ? (float) wp_unslash( $_SERVER['REQUEST_TIME_FLOAT'] ) : microtime( true ); return $start + $max - 5; } @@ -757,7 +787,10 @@ private static function sources( &$job ) { return; } $files = $job['source_files']; - $states = $job['options']['scan_render'] ? ( $job['render_states'] = $job['render_states'] ?? Text::render_states() ) : array(); + if ( $job['options']['scan_render'] && ! isset( $job['render_states'] ) ) { + $job['render_states'] = Text::render_states(); + } + $states = $job['options']['scan_render'] ? $job['render_states'] : array(); $step = $job['cursor']; self::progress( $job, count( $files ) + 1 + count( $states ) ); $job['text'] = $job['text'] ?? array( 'occurrences' => array(), 'errors' => array(), 'counts' => array( 'unlisted_excluded' => 0, 'unlisted_by_reason' => array(), 'sources' => array( 'files' => count( $files ), 'settings' => 1, 'render_states' => count( $states ) ) ) ); diff --git a/includes/class-contentrain-bridge-key.php b/includes/class-contentrain-bridge-key.php index 6924c6b..8a53554 100644 --- a/includes/class-contentrain-bridge-key.php +++ b/includes/class-contentrain-bridge-key.php @@ -14,15 +14,19 @@ * from the query string: an address ends up in access logs and caches. * - Only its SHA-256 is stored; the key is shown once. One key at a time: a new * one retires the previous one. - * - It opens the `contentrain-bridge/v1` routes only, acting as the administrator - * who created it, and only while that user still has `export` + `manage_options`. - * WordPress's own REST API is not opened by it. + * - It opens the `contentrain-bridge/v1` routes only, and runs with no WordPress + * user: the request is never signed in as anyone (`get_current_user_id()` stays + * 0), so no capability check anywhere passes on its account. It is valid only + * while the administrator who created it still has `export` + `manage_options`, + * checked with `user_can()` on their id. WordPress's own REST API is not opened by it. * - Unpaired, it lives an hour: the time to paste it into Migrate. The first * request that carries a `contentrain_pairing` (Migrate's order id) pairs it; * paired, it works for that order until 14 days after it was created, with no * idle limit (reruns can be days apart), until it is revoked or replaced — * by the administrator, or by Migrate itself (`DELETE /key`) when the order closes. - * - It reads only the exports it started, one running at a time. + * - It reads only the exports it started, one running at a time. Those exports are + * indexed in the plugin's own option (`EXPORTS`), never in any user's meta, and + * their state names the key as `via`. * - A wrong key counts against its address: after ten in 15 minutes, wrong keys * from there are answered 429. The right key is always compared first, so a * shared address (a proxy, a CDN) cannot lock Migrate out. @@ -43,9 +47,22 @@ final class Key { const RETIRED = 5; const KEPT_EXPORTS = 20; + /** The format of a key as `create()` returns it: 32 random bytes, base64url, no padding. */ + const FORMAT = '/^[A-Za-z0-9_-]{43}$/D'; + /** Whether this request authenticated with the key: the remote routes then show only the key's own exports. */ private static $active = false; + /** Clear the key after every REST callback: one PHP process can serve several requests, and nothing may leak into the next. */ + public static function register() { + add_filter( 'rest_request_after_callbacks', array( self::class, 'after_callbacks' ), 1000 ); + } + + public static function after_callbacks( $response ) { + self::clear(); + return $response; + } + public static function secure() { return (bool) apply_filters( 'contentrain_bridge_key_secure', is_ssl() || 'local' === wp_get_environment_type() ); } @@ -59,7 +76,7 @@ public static function clear() { self::$active = false; } - /** A new key for the current administrator; the previous one stops working. Returned once, never stored. */ + /** A new key for the current administrator; the previous one stops working and its exports are removed. Returned once, never stored. */ public static function create() { if ( ! Admin::permitted() ) { throw new \RuntimeException( 'Export and administrator permissions are required.' ); @@ -67,7 +84,10 @@ public static function create() { if ( ! self::secure() ) { throw new \RuntimeException( 'Use HTTPS on this site before creating a connection key.' ); } - $key = rtrim( strtr( base64_encode( random_bytes( 32 ) ), '+/', '-_' ), '=' ); + foreach ( self::exports() as $id ) { + self::remove_export( $id ); + } + $key = rtrim( strtr( base64_encode( random_bytes( 32 ) ), '+/', '-_' ), '=' ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode -- Random bytes as base64url text: the key itself, nothing encoded to hide. update_option( self::OPTION, array( 'hash' => hash( 'sha256', $key ), 'user' => get_current_user_id(), @@ -80,16 +100,43 @@ public static function create() { return $key; } - /** Stop the current key — the administrator on the Bridge screen, or Migrate with the key itself. A retired key is answered as revoked, not as unknown. */ + /** + * Stop the current key — the administrator on the Bridge screen, or Migrate with the key itself (the request + * then carries no user, so the key's own authorization stands in for the screen's permission). A retired key is + * answered as revoked, not as unknown. The key's exports go with it: nothing else can read them once it is gone. + */ public static function revoke() { - if ( ! Admin::permitted() ) { + if ( ! self::$active && ! Admin::permitted() ) { throw new \RuntimeException( 'Export and administrator permissions are required.' ); } + foreach ( self::exports() as $id ) { + self::remove_export( $id ); + } update_option( self::OPTION, array( 'retired' => self::retired() ), false ); delete_option( self::SEEN ); delete_option( self::EXPORTS ); } + /** Remove one of the key's export directories unless a request holds it; a held one expires with `Files::cleanup()`. */ + private static function remove_export( $id ) { + try { + $dir = Files::dir( $id ); + } catch ( \RuntimeException $invalid ) { + return; + } + if ( ! is_dir( $dir ) ) { + return; + } + $lock = fopen( $dir . '/lock', 'c' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fopen -- Native advisory lock; filesystem API has no locking primitive. + if ( $lock && flock( $lock, LOCK_EX | LOCK_NB ) ) { + Files::remove( $dir ); + flock( $lock, LOCK_UN ); + } + if ( $lock ) { + fclose( $lock ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- Native advisory lock handle. + } + } + /** What the Bridge screen shows about the current key; never the key or its hash. */ public static function status() { $record = self::record(); @@ -130,12 +177,20 @@ public static function presented( $request ) { return null !== $header ? $header : $field; } - /** Accept the key for this request, as its administrator, or say why not. */ + /** + * Accept the key for this request, or say why not. Nothing here signs anyone in: the request keeps user 0 for its + * whole life, and the export code receives the key's owner explicitly (`Jobs::principal()`). The creator's + * continued `export` + `manage_options` is a validity condition of the key, read with `user_can()` on their id. + */ public static function authorize( $request, $key ) { if ( ! self::secure() ) { return self::error( 'insecure', 'This site is not served over HTTPS, so a connection key cannot be used.', 403 ); } $ip = self::ip(); + // A string that cannot be a key is refused before anything is hashed or compared; it counts as a wrong key. + if ( ! is_string( $key ) || ! preg_match( self::FORMAT, $key ) ) { + return self::wrong( $ip ); + } $hash = hash( 'sha256', $key ); $record = self::record(); // The key is compared before the address is held back: behind a shared address (a proxy, a CDN) @@ -144,30 +199,22 @@ public static function authorize( $request, $key ) { if ( isset( self::retired()[ $hash ] ) ) { return self::error( 'revoked', 'This connection key was revoked or replaced in WordPress. Create a new one on the Contentrain Bridge screen.', 401 ); } - $failures = (int) get_transient( self::failures( $ip ) ); - if ( $failures >= self::FAILURES ) { - return self::error( 'rate_limited', 'Too many wrong connection keys from this address; wait 15 minutes.', 429, array( 'retry_after' => self::WINDOW ) ); - } - set_transient( self::failures( $ip ), $failures + 1, self::WINDOW ); - return self::error( 'invalid', 'The connection key is not valid. Copy it again from the Contentrain Bridge screen.', 401 ); + return self::wrong( $ip ); } if ( time() > self::ends( $record ) ) { return null === $record['pairing'] ? self::error( 'expired', 'The connection key was not used within an hour of being created. Create a new one on the Contentrain Bridge screen.', 401 ) : self::error( 'expired', 'The connection key is 14 days old. Create a new one on the Contentrain Bridge screen.', 401 ); } - wp_set_current_user( (int) $record['user'] ); - if ( ! Admin::permitted() ) { - wp_set_current_user( 0 ); + $user = (int) $record['user']; + if ( ! user_can( $user, 'export' ) || ! user_can( $user, 'manage_options' ) ) { return self::error( 'forbidden', 'The administrator who created the connection key can no longer export this site.', 403 ); } $pairing = $request->get_param( self::PAIRING ); if ( ! is_string( $pairing ) || ! preg_match( '/^[A-Za-z0-9_-]{8,64}$/D', $pairing ) ) { - wp_set_current_user( 0 ); return self::error( 'pairing_required', 'contentrain_pairing (8 to 64 letters, digits, - or _) is required with a connection key.', 400 ); } if ( null !== $record['pairing'] && ! hash_equals( $record['pairing'], $pairing ) ) { - wp_set_current_user( 0 ); return self::error( 'paired_elsewhere', 'This connection key belongs to another Migrate order. Create a new one for this order.', 403 ); } // Paired by the first accepted request of any route — Migrate's access check included, which reads an @@ -180,13 +227,34 @@ public static function authorize( $request, $key ) { // The export in the route itself, never a body or query field of the same name. $id = $request->get_url_params()['id'] ?? null; if ( null !== $id && ! in_array( $id, self::exports(), true ) ) { - wp_set_current_user( 0 ); return new \WP_Error( 'bridge_not_found', 'No such export for this connection key.', array( 'status' => 404 ) ); } self::$active = true; return true; } + /** A wrong key from `$ip`: counted, and 429 once the address has had its ten in the window. */ + private static function wrong( $ip ) { + $failures = (int) get_transient( self::failures( $ip ) ); + if ( $failures >= self::FAILURES ) { + return self::error( 'rate_limited', 'Too many wrong connection keys from this address; wait 15 minutes.', 429, array( 'retry_after' => self::WINDOW ) ); + } + set_transient( self::failures( $ip ), $failures + 1, self::WINDOW ); + return self::error( 'invalid', 'The connection key is not valid. Copy it again from the Contentrain Bridge screen.', 401 ); + } + + /** The administrator who created the active key: the owner every export it starts is recorded under. */ + public static function owner() { + $record = self::record(); + return $record ? (int) $record['user'] : 0; + } + + /** The SHA-256 of the active key, as the option stores it: what a key export's state names. */ + public static function hash() { + $record = self::record(); + return $record ? (string) $record['hash'] : ''; + } + /** DELETE /key: Migrate closes its own key when the order is done. Only the key itself can ask. */ public static function permitted_self( $request ) { self::clear(); @@ -211,11 +279,9 @@ public static function exports() { return is_array( $ids ) ? array_values( array_filter( $ids, static function ( $id ) { return is_string( $id ) && preg_match( '/^[a-f0-9]{32}$/D', $id ); } ) ) : array(); } - /** Record an export as this key's; called under the remote start lock. */ - public static function bind( $id ) { - $ids = array_values( array_diff( self::exports(), array( $id ) ) ); - $ids[] = $id; - update_option( self::EXPORTS, array_slice( $ids, -self::KEPT_EXPORTS ), false ); + /** The key's export list, newest last, at most KEPT_EXPORTS. */ + public static function set( $ids ) { + update_option( self::EXPORTS, array_slice( array_values( $ids ), -self::KEPT_EXPORTS ), false ); } private static function record() { diff --git a/includes/class-contentrain-bridge-remote.php b/includes/class-contentrain-bridge-remote.php index 225d087..cc18176 100644 --- a/includes/class-contentrain-bridge-remote.php +++ b/includes/class-contentrain-bridge-remote.php @@ -9,8 +9,11 @@ * (BR-19). Same permission as the read API (`export` + `manage_options`), same * jobs, same snapshot. The program signs in with an application password, or, * where the host strips the Authorization header, with the connection key an - * administrator created on the Bridge screen (`Key`, BR-27): the key's exports - * only, one running at a time. + * administrator created on the Bridge screen (`Key`, BR-27). A keyed request + * runs with no WordPress user at all: the key is valid only while its creator + * still has `export` + `manage_options`, its exports are listed in the plugin's + * own option rather than in anyone's user meta, and it sees those exports only, + * one running at a time. * * GET /contentrain-bridge/v1/about → { version, auth: [ "app_password", "key" ] }, no sign-in * @@ -54,17 +57,71 @@ final class Remote { const READ_GUARD = 600; public static function routes() { + $permitted = array( self::class, 'permitted' ); // Public by design: what a reader needs to choose how to sign in. No site data. register_rest_route( 'contentrain-bridge/v1', '/about', array( 'methods' => 'GET', 'permission_callback' => '__return_true', 'callback' => array( self::class, 'about' ) ) ); // Published counts for a price before payment (Migrate gate item 10): the same reader permission as an export, never public. - register_rest_route( 'contentrain-bridge/v1', '/counts', array( 'methods' => 'GET', 'permission_callback' => array( self::class, 'permitted' ), 'callback' => array( self::class, 'counts' ) ) ); + register_rest_route( 'contentrain-bridge/v1', '/counts', array( 'methods' => 'GET', 'permission_callback' => $permitted, 'callback' => array( self::class, 'counts' ), 'args' => self::key_args() ) ); register_rest_route( 'contentrain-bridge/v1', '/exports', array( - array( 'methods' => 'POST', 'permission_callback' => array( self::class, 'permitted' ), 'callback' => array( self::class, 'start' ) ), - array( 'methods' => 'GET', 'permission_callback' => array( self::class, 'permitted' ), 'callback' => array( self::class, 'index' ) ), + array( 'methods' => 'POST', 'permission_callback' => $permitted, 'callback' => array( self::class, 'start' ), 'args' => self::start_args() ), + array( 'methods' => 'GET', 'permission_callback' => $permitted, 'callback' => array( self::class, 'index' ), 'args' => self::key_args() ), ) ); // Migrate closes its own key when the order is done (BR-27). - register_rest_route( 'contentrain-bridge/v1', '/key', array( 'methods' => 'DELETE', 'permission_callback' => array( Key::class, 'permitted_self' ), 'callback' => array( Key::class, 'revoke_self' ) ) ); - register_rest_route( 'contentrain-bridge/v1', '/exports/(?P[a-f0-9]{32})/advance', array( 'methods' => 'POST', 'permission_callback' => array( self::class, 'permitted' ), 'callback' => array( self::class, 'advance' ) ) ); + register_rest_route( 'contentrain-bridge/v1', '/key', array( 'methods' => 'DELETE', 'permission_callback' => array( Key::class, 'permitted_self' ), 'callback' => array( Key::class, 'revoke_self' ), 'args' => self::key_args() ) ); + register_rest_route( 'contentrain-bridge/v1', '/exports/(?P[a-f0-9]{32})/advance', array( 'methods' => 'POST', 'permission_callback' => $permitted, 'callback' => array( self::class, 'advance' ), 'args' => self::key_args() ) ); + } + + /** + * The two fields a keyed call carries in its body or query. `Key` reads both inside the permission callback, + * which runs before WordPress validates arguments, so these declare and bound them for the schema and refuse a + * malformed one before the callback; they do not replace the checks in `Key::authorize()`. + */ + public static function key_args() { + return array( + Key::PAIRING => array( 'type' => 'string', 'sanitize_callback' => 'sanitize_text_field', 'validate_callback' => array( self::class, 'validate_pairing' ) ), + Key::FIELD => array( 'type' => 'string', 'sanitize_callback' => 'sanitize_text_field', 'validate_callback' => array( self::class, 'validate_key' ) ), + ); + } + + /** `POST /exports`: the scope and the reuse window. The inline checks in `start()` stay; these bound each value first. */ + public static function start_args() { + $flag = array( 'type' => 'boolean', 'sanitize_callback' => 'rest_sanitize_boolean', 'validate_callback' => 'rest_is_boolean' ); + return array( + 'types' => array( 'type' => 'array', 'items' => array( 'type' => 'string' ), 'sanitize_callback' => array( self::class, 'sanitize_types' ), 'validate_callback' => array( self::class, 'validate_types' ) ), + 'private' => $flag, + 'comments' => $flag, + 'media_files' => $flag, + 'fresh' => $flag, + 'max_age' => array( 'type' => 'integer', 'sanitize_callback' => 'absint', 'validate_callback' => array( self::class, 'validate_max_age' ) ), + ) + self::key_args(); + } + + public static function validate_pairing( $value ) { + return is_string( $value ) && 1 === preg_match( '/^[A-Za-z0-9_-]{8,64}$/D', $value ); + } + + public static function validate_key( $value ) { + return is_string( $value ) && 1 === preg_match( Key::FORMAT, trim( $value ) ); + } + + public static function validate_types( $value ) { + if ( ! is_array( $value ) || count( $value ) > 200 ) { + return false; + } + foreach ( $value as $type ) { + if ( ! is_string( $type ) || 1 !== preg_match( '/^[a-z0-9_-]{1,20}$/D', $type ) ) { + return false; + } + } + return true; + } + + public static function sanitize_types( $value ) { + return array_values( array_map( 'sanitize_key', (array) $value ) ); + } + + public static function validate_max_age( $value ) { + return ( is_int( $value ) && $value >= 0 && $value <= 999999999 ) || ( is_string( $value ) && 1 === preg_match( '/^[0-9]{1,9}$/D', $value ) ); } /** The read API's permission, or the connection key when the request carries one. */ @@ -160,22 +217,16 @@ public static function start( $request ) { } ++$live; if ( $same ) { - if ( Key::active() ) { - Key::bind( $job['id'] ); - } return new \WP_REST_Response( array( 'export' => Jobs::summary( $job ), 'reused' => true ), 200, self::headers() ); } } if ( $live >= self::LIVE_LIMIT ) { return self::error( 'too_many_exports', 'Three exports are already in progress for this user; let one finish or expire.', 429 ); } - $summary = Jobs::create( $scope, true ); + $summary = Jobs::create( $scope, true, Jobs::principal( true ) ); $ids = self::ids(); $ids[] = $summary['id']; - update_user_meta( get_current_user_id(), self::META . get_current_blog_id(), $ids ); - if ( Key::active() ) { - Key::bind( $summary['id'] ); - } + self::remember( $ids ); return new \WP_REST_Response( array( 'export' => $summary, 'reused' => false ), 201, self::headers() ); } ); } catch ( \Throwable $error ) { @@ -184,13 +235,10 @@ public static function start( $request ) { } } - /** The caller's remote exports that still exist, oldest first; with a key, the key's own. */ + /** The caller's remote exports that still exist, oldest first; with a key, the key's own (`ids()`). */ public static function index() { try { $jobs = self::jobs(); - if ( Key::active() ) { - $jobs = array_values( array_filter( $jobs, static function ( $job ) { return in_array( $job['id'], Key::exports(), true ); } ) ); - } return new \WP_REST_Response( array( 'exports' => array_map( array( Jobs::class, 'summary' ), $jobs ) ), 200, self::headers() ); } catch ( \Throwable $error ) { // Input is checked above; what is left is the server's fault, not "not ready". @@ -267,7 +315,7 @@ private static function discard( $id ) { flock( $lock, LOCK_UN ); fclose( $lock ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- Native advisory lock handle. } - update_user_meta( get_current_user_id(), self::META . get_current_blog_id(), array_values( array_diff( self::ids(), array( $id ) ) ) ); + self::remember( array_values( array_diff( self::ids(), array( $id ) ) ) ); return true; } @@ -290,11 +338,24 @@ private static function code( $message ) { return 'export_failed'; } + /** The caller's export ids: the key's own list (the plugin's option) with a key, the signed-in user's meta otherwise. */ private static function ids() { + if ( Key::active() ) { + return Key::exports(); + } $ids = get_user_meta( get_current_user_id(), self::META . get_current_blog_id(), true ); return is_array( $ids ) ? array_values( array_filter( $ids, static function ( $id ) { return is_string( $id ) && preg_match( '/^[a-f0-9]{32}$/D', $id ); } ) ) : array(); } + /** Store the caller's export ids where `ids()` reads them. */ + private static function remember( $ids ) { + if ( Key::active() ) { + Key::set( $ids ); + return; + } + update_user_meta( get_current_user_id(), self::META . get_current_blog_id(), $ids ); + } + /** Readable jobs only; an expired or deleted one leaves the list. */ private static function jobs() { $jobs = array(); @@ -308,14 +369,14 @@ private static function jobs() { } } if ( $kept !== self::ids() ) { - update_user_meta( get_current_user_id(), self::META . get_current_blog_id(), $kept ); + self::remember( $kept ); } return $jobs; } - /** One start at a time per user: two concurrent requests cannot both create. */ + /** One start at a time per caller (the key, or the signed-in user): two concurrent requests cannot both create. */ private static function locked( $callback ) { - $lock = fopen( Files::root() . '/remote-' . get_current_user_id() . '.lock', 'c' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fopen -- Native advisory lock; filesystem API has no locking primitive. + $lock = fopen( Files::root() . '/remote-' . ( Key::active() ? 'key' : get_current_user_id() ) . '.lock', 'c' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fopen -- Native advisory lock; filesystem API has no locking primitive. if ( ! $lock || ! flock( $lock, LOCK_EX ) ) { throw new \RuntimeException( 'Cannot lock export start.' ); } diff --git a/tests/key.php b/tests/key.php index 05e7b43..fea9b02 100644 --- a/tests/key.php +++ b/tests/key.php @@ -11,6 +11,7 @@ } require_once WP_PLUGIN_DIR . '/contentrain-bridge/contentrain-bridge.php'; use Contentrain\Bridge\Files; +use Contentrain\Bridge\Jobs; use Contentrain\Bridge\Key; use Contentrain\Bridge\Remote; @@ -25,9 +26,23 @@ function check( $value, $message ) { * One REST request as an anonymous caller (or `$as`), the key where `$via` says: * `header`, `body` (form fields), `json`, `both` or `none`. */ +/** + * What WordPress saw during every keyed request, recorded before and after the route callback: the user id, + * whether anyone is signed in, and whether `manage_options` holds. With a key all three must stay 0 / false / false. + */ +$observed = array(); +$observe = static function ( $response ) use ( &$observed ) { + $observed[] = array( get_current_user_id(), is_user_logged_in(), current_user_can( 'manage_options' ) ); + return $response; +}; +add_filter( 'rest_request_before_callbacks', $observe, 1 ); +add_filter( 'rest_request_after_callbacks', $observe, 1 ); +$keyed_calls = 0; function call( $method, $route, $params = array(), $key = null, $via = 'header', $as = 0 ) { + global $observed, $keyed_calls; wp_set_current_user( $as ); - $request = new WP_REST_Request( $method, '/contentrain-bridge/v1' . $route ); + $observed = array(); + $request = new WP_REST_Request( $method, 0 === strpos( $route, '/wp/' ) ? $route : '/contentrain-bridge/v1' . $route ); $body = $params; if ( null !== $key && in_array( $via, array( 'header', 'both' ), true ) ) { $request->set_header( 'X-Contentrain-Key', $key ); } if ( null !== $key && in_array( $via, array( 'body', 'json', 'both' ), true ) ) { $body['contentrain_key'] = $key; } @@ -40,6 +55,18 @@ function call( $method, $route, $params = array(), $key = null, $via = 'header', $request->set_body_params( $body ); } $response = rest_do_request( $request ); + if ( null !== $key && 0 === $as ) { + // The explicit-principal rule, checked on every keyed call rather than once: no WordPress user before, during or after. + foreach ( $observed as $seen ) { + if ( array( 0, false, false ) !== $seen ) { + throw new RuntimeException( 'FAIL: a keyed request ran as a WordPress user: ' . wp_json_encode( $seen ) . " ($method $route)" ); + } + } + if ( 0 !== get_current_user_id() || is_user_logged_in() || Key::active() ) { + throw new RuntimeException( "FAIL: a keyed request left a user or the key behind ($method $route)" ); + } + ++$keyed_calls; + } return array( $response->get_status(), $response->get_data() ); } function code( $data ) { @@ -50,7 +77,7 @@ function code( $data ) { $meta = Remote::META . get_current_blog_id(); $_SERVER['REMOTE_ADDR'] = '203.0.113.7'; $reset = static function () use ( $admin, $meta ) { - foreach ( (array) get_user_meta( $admin->ID, $meta, true ) as $id ) { if ( is_string( $id ) && preg_match( '/^[a-f0-9]{32}$/D', $id ) ) { Files::remove( Files::dir( $id ) ); } } + foreach ( array_merge( (array) get_user_meta( $admin->ID, $meta, true ), Key::exports() ) as $id ) { if ( is_string( $id ) && preg_match( '/^[a-f0-9]{32}$/D', $id ) ) { Files::remove( Files::dir( $id ) ); } } delete_user_meta( $admin->ID, $meta ); foreach ( array( Key::OPTION, Key::SEEN, Key::EXPORTS ) as $option ) { delete_option( $option ); } delete_transient( 'contentrain_bridge_key_fail_' . md5( '203.0.113.7' ) ); @@ -83,6 +110,14 @@ function code( $data ) { list( $status, $body ) = call( 'POST', '/exports', array( 'types' => array( 'post' ) ), $key ); check( 400 === $status && 'bridge_key_pairing_required' === code( $body ), 'a key without contentrain_pairing: 400 bridge_key_pairing_required' ); check( null === get_option( Key::OPTION )['pairing'], 'and it does not pair the key' ); +$failures_before = (int) get_transient( 'contentrain_bridge_key_fail_' . md5( '203.0.113.7' ) ); +list( $status, $body ) = call( 'POST', '/exports', array( 'types' => array( 'post' ), 'contentrain_pairing' => $pair ), 'x' ); +check( 401 === $status && 'bridge_key_invalid' === code( $body ) && (int) get_transient( 'contentrain_bridge_key_fail_' . md5( '203.0.113.7' ) ) === $failures_before + 1, 'a malformed key (not 43 base64url characters): 401 bridge_key_invalid, counted as a wrong key' ); +delete_transient( 'contentrain_bridge_key_fail_' . md5( '203.0.113.7' ) ); +list( $status ) = call( 'GET', '/wp/v2/users/me', array( 'contentrain_pairing' => $pair ), $key ); +check( 401 === $status, "the key opens no WordPress route: GET /wp/v2/users/me with the key is $status" ); +list( $status ) = call( 'GET', '/wp/v2/posts', array( 'status' => 'draft', 'contentrain_pairing' => $pair ), $key ); +check( in_array( $status, array( 400, 401 ), true ), "nor GET /wp/v2/posts?status=draft ($status)" ); list( $status, $body ) = call( 'POST', '/exports/' . str_repeat( '0', 32 ) . '/read', array( 'contentrain_pairing' => $pair ), $key, 'json' ); check( 404 === $status && 'bridge_not_found' === code( $body ) && $pair === get_option( Key::OPTION )['pairing'], 'Migrate\'s access check (a read of no export): 404 bridge_not_found, and the key is now paired with the order' ); // ---- Counts before payment (Migrate gate item 10): the paired key reads WordPress's own published counts. ---- @@ -95,6 +130,13 @@ function code( $data ) { check( 201 === $status && false === $started['reused'], 'the header key starts an export: 201' ); $a = $started['export']['id']; check( array( $a ) === Key::exports() && $pair === get_option( Key::OPTION )['pairing'], 'the export is the key\'s, and the key is paired with the order' ); +check( '' === get_user_meta( $admin->ID, $meta, true ), 'and nothing was written to the administrator\'s user meta' ); +$state = json_decode( Files::read( Files::dir( $a ), 'state.json' ), true ); +check( $admin->ID === (int) $state['owner'] && 'key' === $state['via'] && hash( 'sha256', $key ) === $state['key'], 'its state names the owner explicitly: the key\'s administrator, via key' ); +wp_set_current_user( $admin->ID ); +try { Jobs::read( $a ); $read = true; } catch ( RuntimeException $e ) { $read = false; } +wp_set_current_user( 0 ); +check( ! $read, 'signed in as that administrator, Jobs::read() refuses the key\'s export: the key is the only principal that reads it' ); list( $status, $again ) = call( 'POST', '/exports', $scope, $key, 'body' ); check( 200 === $status && $a === $again['export']['id'], 'the key as a body field: the same scope is the running export, reused' ); list( $status, $body ) = call( 'POST', '/exports', array( 'types' => array( 'post' ) ) + $scope, $key ); @@ -112,6 +154,8 @@ function code( $data ) { list( $status, $own ) = call( 'POST', '/exports', array( 'types' => array( 'page' ), 'media_files' => false ), null, 'none', $admin->ID ); check( 201 === $status, 'the administrator starts an export of their own' ); $b = $own['export']['id']; +$state = json_decode( Files::read( Files::dir( $b ), 'state.json' ), true ); +check( $admin->ID === (int) $state['owner'] && 'app_password' === $state['via'] && array( $b ) === (array) get_user_meta( $admin->ID, $meta, true ), 'its state says via app_password, and it is listed in the administrator\'s user meta, not the key\'s option' ); list( $status, $body ) = call( 'POST', "/exports/$b/advance", array( 'contentrain_pairing' => $pair ), $key ); check( 404 === $status && 'bridge_not_found' === code( $body ), 'the key cannot advance it: 404' ); list( $status, $index ) = call( 'GET', '/exports', array( 'contentrain_pairing' => $pair ), $key ); @@ -135,8 +179,45 @@ function code( $data ) { $offset += $chunk['length']; } check( hash( 'sha256', $assembled ) === $info['sha256'], 'rawir.json read in chunks through the body-keyed route assembles to its hash' ); +check( '' === get_user_meta( $admin->ID, $meta, true ) || array( $b ) === (array) get_user_meta( $admin->ID, $meta, true ), 'after start, advance and read with the key, the administrator\'s user meta holds only their own export' ); + +// ---- Parity: the same scope with the application password reads the same content (no capability-dependent read). ---- +list( $status, $parity ) = call( 'POST', '/exports', array( 'types' => array( 'post', 'page' ), 'media_files' => false ), null, 'none', $admin->ID ); +check( 201 === $status, 'the administrator starts the key\'s scope with the application password' ); +$p = $parity['export']['id']; +for ( $i = 0; $i < 60; ++$i ) { + list( $status, $body ) = call( 'POST', "/exports/$p/advance", array(), null, 'none', $admin->ID ); + if ( in_array( $body['export']['phase'], array( 'ready', 'failed' ), true ) ) { break; } +} +check( 'ready' === $body['export']['phase'], 'and advances it to ready' ); +list( , $plist ) = call( 'GET', "/exports/$p", array(), null, 'none', $admin->ID ); +$timed = array( 'bridge/manifest.json', 'bridge/site.json' ); +$differ = array(); +foreach ( $list['files'] + $plist['files'] as $path => $meta_row ) { + if ( ! isset( $list['files'][ $path ], $plist['files'][ $path ] ) || $list['files'][ $path ]['sha256'] !== $plist['files'][ $path ]['sha256'] ) { $differ[] = $path; } +} +check( array() === array_diff( $differ, $timed ), 'the key export and the application-password export have the same files and hashes apart from the timestamped manifest and site.json (differing: ' . implode( ', ', $differ ) . ')' ); + +// ---- Reuse isolation: an export the administrator started is never reused, bound or replaced by the key. ---- +$scope2 = array( 'types' => array( 'page' ), 'media_files' => false, 'comments' => true ); +list( $status, $theirs ) = call( 'POST', '/exports', $scope2, null, 'none', $admin->ID ); +check( 201 === $status, 'the administrator starts an export with the application password' ); +$s = $theirs['export']['id']; +list( $status, $mine ) = call( 'POST', '/exports', $scope2 + array( 'contentrain_pairing' => $pair ), $key ); +check( 201 === $status && false === $mine['reused'] && $s !== $mine['export']['id'], 'the key asking for the same scope gets a new export (201), not the administrator\'s (reused)' ); +check( ! in_array( $s, Key::exports(), true ) && in_array( $mine['export']['id'], Key::exports(), true ), 'the administrator\'s export is not bound to the key' ); +list( $status, $renewed ) = call( 'POST', '/exports', $scope2 + array( 'fresh' => true, 'contentrain_pairing' => $pair ), $key ); +check( 201 === $status && $mine['export']['id'] !== $renewed['export']['id'] && ! is_dir( Files::dir( $mine['export']['id'] ) ) && is_dir( Files::dir( $s ) ), 'fresh with the key replaces the key\'s own export and leaves the administrator\'s intact' ); +list( $status, $index ) = call( 'GET', '/exports', array(), null, 'none', $admin->ID ); +check( in_array( $s, array_column( $index['exports'], 'id' ), true ) && ! in_array( $renewed['export']['id'], array_column( $index['exports'], 'id' ), true ), 'the administrator still lists their export, and not the key\'s' ); +$n = $renewed['export']['id']; +for ( $i = 0; $i < 60; ++$i ) { + list( $status, $body ) = call( 'POST', "/exports/$n/advance", array( 'contentrain_pairing' => $pair ), $key ); + if ( in_array( $body['export']['phase'], array( 'ready', 'failed' ), true ) ) { break; } +} +check( 'ready' === $body['export']['phase'], 'the key\'s replacement export advances to ready' ); list( $status, $rerun ) = call( 'POST', '/exports', array( 'types' => array( 'post' ) ) + $scope, $key ); -check( 201 === $status && $a !== $rerun['export']['id'] && array( $a, $rerun['export']['id'] ) === Key::exports(), 'once it is ready, a rerun with another scope starts: the same key, the same order' ); +check( 201 === $status && $a !== $rerun['export']['id'] && array( $a, $n, $rerun['export']['id'] ) === Key::exports(), 'once it is ready, a rerun with another scope starts: the same key, the same order' ); $seen = get_option( Key::SEEN ); check( '203.0.113.7' === $seen['ip'] && time() - $seen['at'] < 60 && true === Key::status()['used'], 'the last use is recorded: when, and from where' ); @@ -182,8 +263,12 @@ function code( $data ) { check( 401 === $status && 'bridge_key_required' === code( $body ), 'DELETE /key without the key: 401 bridge_key_required' ); list( $status, $body ) = call( 'DELETE', '/key', array( 'contentrain_pairing' => 'order_other_0009' ), $replacement ); check( 403 === $status && 'bridge_key_paired_elsewhere' === code( $body ), 'nor for another order' ); +list( $status, $started ) = call( 'POST', '/exports', array( 'types' => array( 'page' ), 'media_files' => false, 'contentrain_pairing' => 'order_test_0003' ), $replacement ); +$r = $started['export']['id']; +check( 201 === $status && is_dir( Files::dir( $r ) ), 'the new key starts an export of its own' ); list( $status, $body ) = call( 'DELETE', '/key', array( 'contentrain_pairing' => 'order_test_0003' ), $replacement ); check( 200 === $status && true === $body['revoked'], 'DELETE /key with the key and its order: revoked' ); +check( ! is_dir( Files::dir( $r ) ) && array() === Key::exports(), 'and the key\'s export is removed with it' ); list( $status, $body ) = call( 'GET', '/exports', array( 'contentrain_pairing' => 'order_test_0003' ), $replacement ); check( 401 === $status && 'bridge_key_revoked' === code( $body ) && false === Key::status()['active'], 'then the key is answered as revoked, and the screen says no key' ); wp_set_current_user( $admin->ID ); @@ -210,7 +295,10 @@ function code( $data ) { // ---- The application-password path is unchanged. ---- list( $status, $index ) = call( 'GET', '/exports', array(), null, 'none', $admin->ID ); -check( 200 === $status && in_array( $b, array_column( $index['exports'], 'id' ), true ) && in_array( $a, array_column( $index['exports'], 'id' ), true ), 'signed in as the administrator, every remote export is listed as before' ); +check( 200 === $status && in_array( $b, array_column( $index['exports'], 'id' ), true ) && in_array( $p, array_column( $index['exports'], 'id' ), true ) && ! in_array( $a, array_column( $index['exports'], 'id' ), true ), 'signed in as the administrator, their own remote exports are listed and the key\'s are not' ); +list( $status, $body ) = call( 'GET', "/exports/$a", array(), null, 'none', $admin->ID ); +check( 404 === $status && 'bridge_not_found' === code( $body ), 'the administrator cannot read the key\'s export through REST either: 404' ); +check( $keyed_calls > 40, "every one of the $keyed_calls keyed requests ran with user 0, signed out, without manage_options, before and after its callback" ); $reset(); delete_transient( 'contentrain_bridge_key_fail_' . md5( '203.0.113.8' ) ); From eab3862f660d731f8017cbb0fc223b45642db5c2 Mon Sep 17 00:00:00 2001 From: ABB65 Date: Sun, 18 Oct 2026 23:47:30 +0300 Subject: [PATCH 02/10] fix(admin): the screen posts form fields read one by one; REST reads declare their args; the .htaccess note names a server class Admin::ajax() no longer decodes a JSON blob from $_POST['payload']. admin.js sends plain form fields (name, name[], name[key]) and Admin::input() reads each once after check_ajax_referer() with the function for its type: sanitize_key + allowlist for op, a hex check for id, absint for step/offset/cursor, rest_sanitize_boolean for flags, sanitize_key / sanitize_text_field for lists, map_deep + shape checks for labels and decisions, and GitHub's own format checks for token, repository, base branch and conflict choice. The phpcs:ignore that covered the blob is gone. The download handler checks the id format at the read. GET /exports/{id} and POST /exports/{id}/read declare file, offset and length (and the key fields) with validate and sanitize callbacks. Redirects::htaccess() sanitizes SERVER_SOFTWARE and the exported note names a class of server (nginx, iis, other) instead of repeating the header, which did reach bridge/redirects.json; that ignore is gone too. The Connect card says what the key lets Migrate read, that it runs with no WordPress user and cannot change the site, and how to revoke it; the privacy policy text says the same. The DOM test harness decodes the form fields. --- assets/admin.js | 17 ++- includes/class-contentrain-bridge-admin.php | 124 +++++++++++++++--- .../class-contentrain-bridge-redirects.php | 10 +- tests/admin-dom.mjs | 19 ++- 4 files changed, 143 insertions(+), 27 deletions(-) diff --git a/assets/admin.js b/assets/admin.js index feb3001..4b6f6f3 100644 --- a/assets/admin.js +++ b/assets/admin.js @@ -14,8 +14,23 @@ const STEP_TIMEOUT = 120000; const wait = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); function retryable(message) { return Object.assign(new Error(message), { retryable: true }); } + /** + * The request as plain form fields, one per value, the way PHP reads them: a list as `name[]`, an object as + * `name[key]`, a flag as 1 or 0. The server reads each field once with the function for its type; no JSON blob. + */ + function fields(payload, body = new URLSearchParams(), prefix = '') { + for (const [name, value] of Object.entries(payload)) { + const key = prefix ? `${prefix}[${name}]` : name; + if (value === null || value === undefined) continue; + if (Array.isArray(value)) value.forEach((item) => body.append(`${key}[]`, String(item))); + else if (typeof value === 'object') fields(value, body, key); + else if (typeof value === 'boolean') body.append(key, value ? '1' : '0'); + else body.append(key, String(value)); + } + return body; + } async function api(payload, timeout = 0) { - const body = new URLSearchParams({ action: 'contentrain_bridge', nonce: ContentrainBridge.nonce, payload: JSON.stringify(payload) }); + const body = fields({ action: 'contentrain_bridge', nonce: ContentrainBridge.nonce, ...payload }); const controller = new AbortController(); const timer = timeout ? setTimeout(() => controller.abort(), timeout) : 0; let response; diff --git a/includes/class-contentrain-bridge-admin.php b/includes/class-contentrain-bridge-admin.php index 4134cda..d294458 100644 --- a/includes/class-contentrain-bridge-admin.php +++ b/includes/class-contentrain-bridge-admin.php @@ -63,8 +63,10 @@ public static function page() {

-

-

+

+

+

+