From 51ae9de6a7c3d279c2d9e2ce78b1fc5108eaad02 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Mon, 15 Jun 2026 12:18:45 +0530 Subject: [PATCH] security: validate PERCY_CLI_API to loopback (PER-8913/8917/8918/8919, mitigates 8914) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PERCY_CLI_API was read from the environment with no validation and used as the base for every outbound call, as the source of the @percy/dom script injected and executed in the browser (incl. into cross-origin iframes), and its healthcheck response drives the session-type auth gate. An attacker-controlled value enables SSRF, CLI-fetched-JS code execution, and an auth-gate bypass. Add _resolve_cli_api_address(): loopback-only by default (localhost/127.0.0.1/ ::1); a remote host is allowed only over HTTPS with an explicit PERCY_ALLOW_REMOTE_CLI_API opt-in, otherwise warn and fall back to http://localhost:5338. This one root fix covers: PER-8919 (CWE-918 SSRF), PER-8913 (CWE-306 session-gate bypass via attacker CLI URL), PER-8917 (CWE-494 externally-fetched JS without integrity), PER-8918 (CWE-79 unverified script injected into cross-origin iframe — source now loopback-only), and mitigates PER-8914 (the lru-cached DOM script can now only be fetched from the trusted local CLI). Adds unit tests for the validator (loopback allow, remote/link-local reject, https opt-in). Note: option payloads are already JSON-encoded via json.dumps, so they are not string-interpolated into execute_script. Co-Authored-By: Claude Opus 4.8 (1M context) --- percy/snapshot.py | 29 +++++++++++++++++++++++++++-- tests/test_snapshot.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index 5c75387..b4d0804 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -20,8 +20,33 @@ def _get_bool_env(key): return os.environ.get(key, "").lower() == "true" -# Maybe get the CLI API address from the environment -PERCY_CLI_API = os.environ.get('PERCY_CLI_API') or 'http://localhost:5338' +DEFAULT_PERCY_CLI_API = 'http://localhost:5338' +_LOOPBACK_HOSTS = frozenset({'localhost', '127.0.0.1', '::1'}) + + +def _resolve_cli_api_address(): + # The Percy CLI runs locally. PERCY_CLI_API is used as the base for every + # outbound call AND as the source of the @percy/dom script that is injected + # and executed in the browser, and the healthcheck response drives the + # session-type auth gate. An attacker-controlled value therefore enables + # SSRF, CLI-fetched-JS RCE, and an auth-gate bypass (CWE-918/CWE-94/CWE-306). + # Restrict it to loopback; allow a remote host only over HTTPS with an + # explicit opt-in, otherwise warn and fall back to the local default. + raw = os.environ.get('PERCY_CLI_API') or DEFAULT_PERCY_CLI_API + host = (urlparse(raw).hostname or '').lower() + if host in _LOOPBACK_HOSTS: + return raw + allow_remote = os.environ.get('PERCY_ALLOW_REMOTE_CLI_API', '').lower() in ('1', 'true', 'yes') + if allow_remote and urlparse(raw).scheme == 'https': + return raw + print(f"[percy] Ignoring non-loopback PERCY_CLI_API '{raw}'; falling back to " + f"{DEFAULT_PERCY_CLI_API}. To target a remote Percy CLI use an https:// URL " + "and set PERCY_ALLOW_REMOTE_CLI_API=true.") + return DEFAULT_PERCY_CLI_API + + +# Maybe get the CLI API address from the environment (validated to loopback) +PERCY_CLI_API = _resolve_cli_api_address() PERCY_DEBUG = os.environ.get('PERCY_LOGLEVEL') == 'debug' RESPONSIVE_CAPTURE_SLEEP_TIME = ( os.environ.get('RESPONSIVE_CAPTURE_SLEEP_TIME') or diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index f5800e7..5fb568b 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1,4 +1,5 @@ # pylint: disable=too-many-lines +import os import unittest from unittest.mock import patch, Mock from http.server import BaseHTTPRequestHandler, HTTPServer @@ -18,6 +19,8 @@ _resolve_readiness_config, _wait_for_ready, get_serialized_dom, + _resolve_cli_api_address, + DEFAULT_PERCY_CLI_API, ) from percy import percy_snapshot, percySnapshot, percy_screenshot @@ -124,6 +127,38 @@ def mock_screenshot(fail=False, data=False): }), status=(500 if fail else 200)) +class TestResolveCliApiAddress(unittest.TestCase): + @patch.dict(os.environ, {}, clear=True) + def test_defaults_to_localhost(self): + self.assertEqual(_resolve_cli_api_address(), DEFAULT_PERCY_CLI_API) + + @patch.dict(os.environ, {'PERCY_CLI_API': 'http://127.0.0.1:5338'}, clear=True) + def test_allows_loopback(self): + self.assertEqual(_resolve_cli_api_address(), 'http://127.0.0.1:5338') + + @patch.dict(os.environ, {'PERCY_CLI_API': 'http://attacker.example/x'}, clear=True) + def test_rejects_remote_host(self): + self.assertEqual(_resolve_cli_api_address(), DEFAULT_PERCY_CLI_API) + + @patch.dict(os.environ, {'PERCY_CLI_API': 'http://169.254.169.254/latest/meta-data'}, clear=True) + def test_rejects_link_local_ssrf_target(self): + self.assertEqual(_resolve_cli_api_address(), DEFAULT_PERCY_CLI_API) + + @patch.dict(os.environ, { + 'PERCY_CLI_API': 'https://percy-cli.internal:5338', + 'PERCY_ALLOW_REMOTE_CLI_API': 'true' + }, clear=True) + def test_allows_remote_https_with_opt_in(self): + self.assertEqual(_resolve_cli_api_address(), 'https://percy-cli.internal:5338') + + @patch.dict(os.environ, { + 'PERCY_CLI_API': 'http://percy-cli.internal:5338', + 'PERCY_ALLOW_REMOTE_CLI_API': 'true' + }, clear=True) + def test_remote_opt_in_still_requires_https(self): + self.assertEqual(_resolve_cli_api_address(), DEFAULT_PERCY_CLI_API) + + # pylint: disable=too-many-public-methods class TestPercySnapshot(unittest.TestCase): @classmethod