From dfd184967b71c14fec7dcbee2f90eafbef7d544d Mon Sep 17 00:00:00 2001 From: ykulynych Date: Fri, 18 Sep 2026 09:35:26 +0200 Subject: [PATCH 1/2] PDP-1401: Harden proxy security: input allow-listing, size caps, CORS, Content-Type coercion --- .github/workflows/tests.yml | 18 +++ progress-agentic-rag.php | 5 +- readme.txt | 37 ++++- src/Proxy/ProxyController.php | 204 +++++++++++++++++++++++----- tests/phpunit/FrontendProxyTest.php | 110 +++++++++++++++ 5 files changed, 332 insertions(+), 42 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 492b8a6..8ff7167 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -97,3 +97,21 @@ jobs: - name: Release artifact check run: RELEASE_DIR=build/progress-agentic-rag-connector docker compose -f docker/docker-compose.yml run --rm release-check + + - name: Plugin Check + run: | + docker run --rm \ + --network progress-agentic-rag-ci \ + --user 0 \ + -e WORDPRESS_DB_HOST=progress-agentic-rag-ci-db \ + -e WORDPRESS_DB_USER=wordpress \ + -e WORDPRESS_DB_PASSWORD=wordpress \ + -e WORDPRESS_DB_NAME=wordpress \ + -e WP_PATH=/var/www/html \ + -e WP_URL=http://progress-agentic-rag-ci-wordpress \ + -e PLUGIN_CHECK_TARGET=progress-agentic-rag-connector/progress-agentic-rag.php \ + -v progress-agentic-rag-ci-wp:/var/www/html \ + -v "$PWD/build/progress-agentic-rag-connector":/var/www/html/wp-content/plugins/progress-agentic-rag-connector:ro \ + -v "$PWD/docker/scripts/run-plugin-check.sh":/run-plugin-check.sh:ro \ + wordpress:cli \ + sh -c 'wp plugin activate progress-agentic-rag-connector/progress-agentic-rag.php --allow-root --path=/var/www/html --url=http://progress-agentic-rag-ci-wordpress && sh /run-plugin-check.sh' diff --git a/progress-agentic-rag.php b/progress-agentic-rag.php index 4a48b77..91b10c6 100644 --- a/progress-agentic-rag.php +++ b/progress-agentic-rag.php @@ -8,7 +8,7 @@ * Plugin Name: Progress Agentic RAG connector * Plugin URI: https://github.com/nuclia/wordpress-plugin * Description: Index WordPress content into Progress Agentic RAG and power knowledge-base search. - * Version: 1.0.2 + * Version: 1.0.3 * Requires at least: 6.8 * Requires PHP: 8.1 * Author: Progress Software @@ -16,7 +16,6 @@ * License: GPL v3 or later * License URI: https://www.gnu.org/licenses/gpl-3.0.html * Text Domain: progress-agentic-rag-connector - * Domain Path: /languages */ use ProgressAgenticRag\Autoloader; @@ -30,7 +29,7 @@ } // phpcs:disable WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedConstantFound -- Public plugin constants already use the Progress Agentic RAG prefix. -define( 'PROGRESS_AGENTIC_RAG_VERSION', '1.0.2' ); +define( 'PROGRESS_AGENTIC_RAG_VERSION', '1.0.3' ); define( 'PROGRESS_AGENTIC_RAG_MIN_PHP_VERSION', '8.1' ); define( 'PROGRESS_AGENTIC_RAG_MIN_WP_VERSION', '6.8' ); define( 'PROGRESS_AGENTIC_RAG_FILE', __FILE__ ); diff --git a/readme.txt b/readme.txt index f8015fd..d6c023b 100644 --- a/readme.txt +++ b/readme.txt @@ -2,8 +2,8 @@ Contributors: progresssoftware Tags: search, ai, knowledge-base, rag, indexing Requires at least: 6.8 -Tested up to: 7.0 -Stable tag: 1.0.2 +Tested up to: 7.1 +Stable tag: 1.0.3 Requires PHP: 8.1 License: GPLv3 or later License URI: https://www.gnu.org/licenses/gpl-3.0.html @@ -41,8 +41,41 @@ Service details: No. Service credentials stay server-side, and public widget requests use the server-side proxy. +== Developer Hooks == + +The plugin's front-end proxy exposes two filters for site owners/developers who need +non-default behavior. Both can be added in a small must-use plugin or a theme's +`functions.php`. + +`progress_agentic_rag_proxy_max_body_bytes` (int) +Maximum size, in bytes, of a request body the proxy will forward to Progress Agentic +RAG. Defaults to 5242880 (5 MB), which covers normal chat/search requests and a +single moderate image upload. Raise it if you enable larger image uploads in the +widget. + + add_filter( 'progress_agentic_rag_proxy_max_body_bytes', function () { + return 10 * MB_IN_BYTES; + } ); + +`progress_agentic_rag_proxy_allowed_origins` (string[]) +Origins allowed to receive a reflected `Access-Control-Allow-Origin` response from +the proxy. Defaults to the site's own origin only. The requesting `Origin` header +value is passed as the second argument. + + add_filter( 'progress_agentic_rag_proxy_allowed_origins', function ( $origins, $requesting_origin ) { + $origins[] = 'https://staging.example.com'; + return $origins; + }, 10, 2 ); + == Changelog == += 1.0.3 = + +* Security: hardened the front-end proxy — allow-listed and size-capped request paths, + query strings, and bodies; restricted CORS to the site's own origin; forced unsafe + upstream Content-Types (HTML/SVG/XML/JS) to download instead of render inline. +* Dev: added CI Plugin Check enforcement; fixed readme/header metadata flagged by it. + = 1.0.2 = * Fix: Admin pages no longer perform blocking upstream sync/labelset calls diff --git a/src/Proxy/ProxyController.php b/src/Proxy/ProxyController.php index 0cf1618..8c64f23 100644 --- a/src/Proxy/ProxyController.php +++ b/src/Proxy/ProxyController.php @@ -12,10 +12,13 @@ defined( 'ABSPATH' ) || exit; final class ProxyController { - private const QUERY_VAR_ENABLED = 'progress_agentic_rag_proxy'; - private const QUERY_VAR_ZONE = 'progress_agentic_rag_proxy_zone'; - private const QUERY_VAR_PATH = 'progress_agentic_rag_proxy_path'; - private const OPTION_REWRITE_VERSION = 'progress_agentic_rag_proxy_rewrite_version'; + private const QUERY_VAR_ENABLED = 'progress_agentic_rag_proxy'; + private const QUERY_VAR_ZONE = 'progress_agentic_rag_proxy_zone'; + private const QUERY_VAR_PATH = 'progress_agentic_rag_proxy_path'; + private const OPTION_REWRITE_VERSION = 'progress_agentic_rag_proxy_rewrite_version'; + private const MAX_PATH_LENGTH = 2048; + private const MAX_QUERY_STRING_LENGTH = 2048; + private const MAX_BODY_BYTES = 5242880; // 5 MB default; filterable via progress_agentic_rag_proxy_max_body_bytes. public function __construct( private readonly SettingsRepository $settings ) { } @@ -82,29 +85,61 @@ public function handle_path_request(): void { $this->send_cors_headers(); + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Must stay raw: sanitize_text_field() strips the %XX octets this check looks for. Only inspected, never output or forwarded. + $raw_path = (string) wp_parse_url( isset( $_SERVER['REQUEST_URI'] ) ? (string) wp_unslash( $_SERVER['REQUEST_URI'] ) : '', PHP_URL_PATH ); + if ( self::has_unsafe_path_bytes( $raw_path ) || self::has_unsafe_path_bytes( rawurldecode( $raw_path ) ) ) { + $this->respond( $this->error( 400, 'progress_agentic_rag_proxy_invalid_path', __( 'Invalid Progress Agentic RAG proxy path.', 'progress-agentic-rag-connector' ) ) ); + } + $method = isset( $_SERVER['REQUEST_METHOD'] ) ? strtoupper( sanitize_text_field( (string) wp_unslash( $_SERVER['REQUEST_METHOD'] ) ) ) : 'GET'; - $body = ( 'GET' !== $method && 'HEAD' !== $method ) ? (string) file_get_contents( 'php://input' ) : ''; - - $result = $this->execute( - $request['zone'], - $request['path'], - $method, - $this->client_query_string(), - isset( $_SERVER['CONTENT_TYPE'] ) ? sanitize_text_field( (string) wp_unslash( $_SERVER['CONTENT_TYPE'] ) ) : '', - isset( $_SERVER['HTTP_ACCEPT'] ) ? sanitize_text_field( (string) wp_unslash( $_SERVER['HTTP_ACCEPT'] ) ) : '', - $body, - $this->passthrough_headers() + $body = ( 'GET' !== $method && 'HEAD' !== $method ) ? $this->read_request_body() : ''; + + if ( null === $body ) { + $this->respond( $this->error( 413, 'progress_agentic_rag_proxy_payload_too_large', __( 'Request body exceeds the maximum allowed size.', 'progress-agentic-rag-connector' ) ) ); + } + + $this->respond( + $this->execute( + $request['zone'], + $request['path'], + $method, + $this->client_query_string(), + isset( $_SERVER['CONTENT_TYPE'] ) ? sanitize_text_field( (string) wp_unslash( $_SERVER['CONTENT_TYPE'] ) ) : '', + isset( $_SERVER['HTTP_ACCEPT'] ) ? sanitize_text_field( (string) wp_unslash( $_SERVER['HTTP_ACCEPT'] ) ) : '', + $body, + $this->passthrough_headers() + ) ); + } + private function respond( array $result ): void { status_header( $result['status'] ); foreach ( $result['headers'] as $name => $value ) { header( (string) $name . ': ' . (string) $value, false ); } - echo $result['body']; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped exit; } + private static function has_unsafe_path_bytes( string $value ): bool { + return 1 === preg_match( '/[\x00-\x1F\x7F]/', $value ) + || str_contains( $value, '..' ) + || str_contains( $value, '\\' ); + } + + + /** @return string|null Null if body exceeds max_body_bytes() (reads +1 byte to detect overflow). */ + private function read_request_body(): ?string { + $max_bytes = $this->max_body_bytes(); + $body = (string) file_get_contents( 'php://input', false, null, 0, $max_bytes + 1 ); + + return strlen( $body ) > $max_bytes ? null : $body; + } + + private function max_body_bytes(): int { + return (int) apply_filters( 'progress_agentic_rag_proxy_max_body_bytes', self::MAX_BODY_BYTES ); + } + /** * @param array $passthrough_headers Allow-listed client request headers. * @return array{status: int, headers: array, body: string} @@ -196,8 +231,7 @@ private function path_request(): array { } } - // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Raw URI is parsed, validated, and normalized before use. - $request_uri = isset( $_SERVER['REQUEST_URI'] ) ? (string) wp_unslash( $_SERVER['REQUEST_URI'] ) : ''; + $request_uri = isset( $_SERVER['REQUEST_URI'] ) ? sanitize_text_field( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : ''; $request_path = (string) wp_parse_url( $request_uri, PHP_URL_PATH ); $home_path = trim( (string) wp_parse_url( home_url( '/' ), PHP_URL_PATH ), '/' ); if ( '' !== $home_path && str_starts_with( $request_path, '/' . $home_path . '/' ) ) { @@ -222,22 +256,21 @@ private function path_request(): array { private function normalize_path( string $path ): ?string { $path = ltrim( $path, '/' ); - $decoded_path = rawurldecode( $path ); if ( '' === $path - || str_contains( $path, '..' ) - || str_contains( $decoded_path, '..' ) - || str_contains( $path, '\\' ) - || str_contains( $decoded_path, '\\' ) - || str_contains( $path, "\r" ) - || str_contains( $decoded_path, "\r" ) - || str_contains( $path, "\n" ) - || str_contains( $decoded_path, "\n" ) + || strlen( $path ) > self::MAX_PATH_LENGTH + // Narrower than RFC 3986: unreserved chars + "/" (segments) + "%" (percent-encoding) + ":" (valid in Nuclia slugs/field ids) only. + // Other sub-delims aren't needed for simple REST path segments/ids, so they're excluded here. + || 1 !== preg_match( '/^[A-Za-z0-9\-._~%:\/]+$/', $path ) ) { return null; } + if ( self::has_unsafe_path_bytes( $path ) || self::has_unsafe_path_bytes( rawurldecode( $path ) ) ) { + return null; + } + if ( str_starts_with( $path, 'api/' ) ) { return $path; } @@ -250,12 +283,11 @@ private function normalize_path( string $path ): ?string { } private function client_query_string(): string { - // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Raw query string is allow-list cleaned before proxying. - $query_string = isset( $_SERVER['QUERY_STRING'] ) ? (string) wp_unslash( $_SERVER['QUERY_STRING'] ) : ''; + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- allow-list validated in clean_query_string() before proxying. + $query_string = isset( $_SERVER['QUERY_STRING'] ) ? (string) wp_unslash( $_SERVER['QUERY_STRING'] ) : ''; - if ( '' === $query_string && isset( $_SERVER['REQUEST_URI'] ) ) { - // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Raw URI is parsed and the query is allow-list cleaned before proxying. - $parsed = wp_parse_url( (string) wp_unslash( $_SERVER['REQUEST_URI'] ), PHP_URL_QUERY ); + if ( '' === $query_string && isset( $_SERVER['REQUEST_URI'] ) ) { + $parsed = wp_parse_url( sanitize_text_field( wp_unslash( $_SERVER['REQUEST_URI'] ) ), PHP_URL_QUERY ); $query_string = is_string( $parsed ) ? $parsed : ''; } @@ -263,7 +295,7 @@ private function client_query_string(): string { } private function clean_query_string( string $query_string ): string { - if ( '' === $query_string ) { + if ( '' === $query_string || strlen( $query_string ) > self::MAX_QUERY_STRING_LENGTH ) { return ''; } @@ -276,8 +308,13 @@ private function clean_query_string( string $query_string ): string { $parts = []; foreach ( explode( '&', $query_string ) as $part ) { + // Allow-list is RFC 3986's "query" grammar (unreserved + sub-delims + : @ / ?), minus "&" since that's already the split delimiter above. + if ( '' === $part || 1 !== preg_match( '/^[A-Za-z0-9\-._~%!$\'()*+,;:@=\/?]*$/', $part ) ) { + continue; + } + $key = rawurldecode( explode( '=', $part, 2 )[0] ?? '' ); - if ( '' !== $part && ! in_array( $key, $strip_keys, true ) ) { + if ( ! in_array( $key, $strip_keys, true ) ) { $parts[] = $part; } } @@ -317,17 +354,71 @@ private function send_cors_headers(): void { } $origin = isset( $_SERVER['HTTP_ORIGIN'] ) ? sanitize_text_field( (string) wp_unslash( $_SERVER['HTTP_ORIGIN'] ) ) : ''; - header( 'Access-Control-Allow-Origin: ' . ( '' !== $origin ? $origin : '*' ) ); + + if ( '' !== $origin && $this->is_allowed_origin( $origin ) ) { + header( 'Access-Control-Allow-Origin: ' . $origin ); + } + + header( 'Vary: Origin', false ); + header( 'Access-Control-Allow-Methods: GET, POST, OPTIONS' ); header( 'Access-Control-Allow-Headers: Content-Type, Authorization, X-Requested-With, x-synchronous, X-Synchronous, x-ndb-client, X-NDB-Client, Range, If-Range' ); header( 'Access-Control-Expose-Headers: X-Nuclia-Upstream-Status, X-Nuclia-Upstream-Content-Type, X-Nuclia-Upstream-URL, Nuclia-Learning-Id, X-NUCLIA-TRACE-ID' ); - header( 'Vary: Origin', false ); + } + + private function is_allowed_origin( string $origin ): bool { + $site_origin = $this->site_origin(); + + /** + * Filters the list of origins allowed to receive CORS access to the proxy. + * + * @param string[] $allowed_origins Allowed origins. Defaults to the site's own origin. + * @param string $origin The requesting Origin header value. + */ + $allowed_origins = (array) apply_filters( 'progress_agentic_rag_proxy_allowed_origins', [ $site_origin ], $origin ); + + foreach ( $allowed_origins as $allowed_origin ) { + if ( is_string( $allowed_origin ) && 0 === strcasecmp( $allowed_origin, $origin ) ) { + return true; + } + } + + return false; + } + + /** Uses home_url(), not $_SERVER, so a spoofed Host header can't widen the allowed origin. */ + private function site_origin(): string { + $home = home_url( '/' ); + $scheme = (string) wp_parse_url( $home, PHP_URL_SCHEME ); + $host = (string) wp_parse_url( $home, PHP_URL_HOST ); + $port = wp_parse_url( $home, PHP_URL_PORT ); + + if ( '' === $scheme || '' === $host ) { + return ''; + } + + return $scheme . '://' . $host . ( null !== $port ? ':' . $port : '' ); } private function is_ephemeral_request( string $query_string ): bool { return (bool) preg_match( '/(^|&)eph-token=/i', $query_string ); } + // MIME types a browser will execute/render as an active document (HTML, JS, or + // XML/SVG which can embed scripts) instead of treating as inert data. + private const UNSAFE_CONTENT_TYPES = [ + 'text/html', + 'application/xhtml+xml', + 'image/svg+xml', + 'text/xml', + 'application/xml', + 'text/javascript', + 'application/javascript', + 'application/x-javascript', + 'application/ecmascript', + 'text/ecmascript', + ]; + /** * @param array $response Upstream response. * @return array @@ -351,15 +442,54 @@ private function response_headers( array $response, string $remote_url, int $sta } $headers['X-Nuclia-Upstream-Status'] = (string) $status; - $content_type = $headers['Content-Type'] ?? $headers['content-type'] ?? ''; + + // Normalize Content-Type casing to one canonical key to avoid duplicate headers. + $content_type_key = null; + foreach ( array_keys( $headers ) as $name ) { + if ( 0 === strcasecmp( $name, 'Content-Type' ) ) { + $content_type_key = $name; + break; + } + } + + $content_type = null !== $content_type_key ? $headers[ $content_type_key ] : ''; + if ( null !== $content_type_key && 'Content-Type' !== $content_type_key ) { + unset( $headers[ $content_type_key ] ); + } + if ( '' !== $content_type ) { $headers['X-Nuclia-Upstream-Content-Type'] = $content_type; + $headers['Content-Type'] = ( ! $this->is_attachment_disposition( $headers ) && $this->is_unsafe_content_type( $content_type ) ) + ? 'application/octet-stream' + : $content_type; } + + // Belt-and-suspenders: block MIME-sniffing regardless of Content-Type above. + $headers['X-Content-Type-Options'] = 'nosniff'; + $headers['X-Nuclia-Upstream-URL'] = substr( str_replace( [ "\r", "\n" ], '', (string) preg_replace( '/(?<=[?&])eph-token=[^&]*/i', 'eph-token=REDACTED', $remote_url ) ), 0, 2048 ); return $headers; } + /** @param string $content_type May include a ";charset=..." suffix, stripped before comparison. */ + private function is_unsafe_content_type( string $content_type ): bool { + $base_type = strtolower( trim( strtok( $content_type, ';' ) ?: $content_type ) ); + + return in_array( $base_type, self::UNSAFE_CONTENT_TYPES, true ); + } + + /** Attachment downloads are saved to disk, so their Content-Type can't execute regardless of value. */ + private function is_attachment_disposition( array $headers ): bool { + foreach ( $headers as $name => $value ) { + if ( 0 === strcasecmp( $name, 'Content-Disposition' ) ) { + return 0 === stripos( trim( $value ), 'attachment' ); + } + } + + return false; + } + /** * @return array{status: int, headers: array, body: string} */ diff --git a/tests/phpunit/FrontendProxyTest.php b/tests/phpunit/FrontendProxyTest.php index e2c71bc..a7ada0f 100644 --- a/tests/phpunit/FrontendProxyTest.php +++ b/tests/phpunit/FrontendProxyTest.php @@ -168,11 +168,78 @@ public function test_proxy_execute_redacts_ephemeral_token_and_filters_headers() self::assertSame( '{"query":"example"}', $request['args']['body'] ); self::assertSame( '207', $result['headers']['X-Nuclia-Upstream-Status'] ); self::assertSame( 'application/json', $result['headers']['X-Nuclia-Upstream-Content-Type'] ); + self::assertSame( 'application/json', $result['headers']['Content-Type'] ); + self::assertSame( 'nosniff', $result['headers']['X-Content-Type-Options'] ); self::assertSame( 'https://europe-1.rag.progress.cloud/api/v1/search?eph-token=REDACTED&foo=bar', $result['headers']['X-Nuclia-Upstream-URL'] ); self::assertArrayNotHasKey( 'content-encoding', $result['headers'] ); self::assertSame( 'trace-1, trace-2', $result['headers']['X-NUCLIA-TRACE-ID'] ); } + public function test_proxy_coerces_unsafe_upstream_content_types(): void { + $GLOBALS['progress_agentic_rag_test_http_responses'][] = [ + 'response' => [ + 'code' => 200, + ], + 'headers' => [ + 'Content-Type' => 'text/html; charset=utf-8', + ], + 'body' => '', + ]; + + $result = $this->execute_proxy( 'europe-1', 'v1/search' ); + + self::assertSame( 'text/html; charset=utf-8', $result['headers']['X-Nuclia-Upstream-Content-Type'] ); + self::assertSame( 'application/octet-stream', $result['headers']['Content-Type'] ); + self::assertSame( 'nosniff', $result['headers']['X-Content-Type-Options'] ); + + $GLOBALS['progress_agentic_rag_test_http_responses'][] = [ + 'response' => [ + 'code' => 200, + ], + 'headers' => [ + 'Content-Type' => 'image/svg+xml', + ], + 'body' => '', + ]; + + $svg_result = $this->execute_proxy( 'europe-1', 'v1/search' ); + + self::assertSame( 'application/octet-stream', $svg_result['headers']['Content-Type'] ); + } + + public function test_proxy_does_not_coerce_content_type_for_attachment_downloads(): void { + $GLOBALS['progress_agentic_rag_test_http_responses'][] = [ + 'response' => [ + 'code' => 200, + ], + 'headers' => [ + 'Content-Type' => 'image/svg+xml', + 'Content-Disposition' => 'attachment; filename="logo.svg"', + ], + 'body' => '', + ]; + + $result = $this->execute_proxy( 'europe-1', 'v1/search' ); + + self::assertSame( 'image/svg+xml', $result['headers']['Content-Type'] ); + self::assertSame( 'nosniff', $result['headers']['X-Content-Type-Options'] ); + + $GLOBALS['progress_agentic_rag_test_http_responses'][] = [ + 'response' => [ + 'code' => 200, + ], + 'headers' => [ + 'Content-Type' => 'image/svg+xml', + 'Content-Disposition' => 'inline; filename="logo.svg"', + ], + 'body' => '', + ]; + + $inline_result = $this->execute_proxy( 'europe-1', 'v1/search' ); + + self::assertSame( 'application/octet-stream', $inline_result['headers']['Content-Type'] ); + } + public function test_proxy_execute_validates_zone_method_token_and_path(): void { self::assertSame( 400, $this->execute_proxy( 'bad zone', 'v1/search' )['status'] ); self::assertSame( 405, $this->execute_proxy( 'europe-1', 'v1/search', 'DELETE' )['status'] ); @@ -184,6 +251,34 @@ public function test_proxy_execute_validates_zone_method_token_and_path(): void self::assertSame( 500, $this->execute_proxy( 'europe-1', 'v1/search' )['status'] ); } + public function test_proxy_normalize_path_rejects_disallowed_characters_and_overlong_paths(): void { + self::assertSame( 400, $this->execute_proxy( 'europe-1', "v1/search\x00" )['status'] ); + self::assertSame( 400, $this->execute_proxy( 'europe-1', 'v1/se arch' )['status'] ); + self::assertSame( 400, $this->execute_proxy( 'europe-1', 'v1/