security: validate env-controlled inputs + harden release pipeline (PER-8514–8518)

.github/workflows/release.yml

@@ -2,15 +2,19 @@ name: Release
 on:
   release:
     types: [published]
+permissions:
+  contents: read
 jobs:
   publish:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-python@v4
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0
         with:
           python-version: 3.x
-      - uses: actions/cache@v3
+      - uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0
         with:
           path: ~/.cache/pip
           key: v1/${{ runner.os }}/pip/${{ hashFiles('{requirements,development}.txt') }}

development.txt

@@ -1,4 +1,4 @@
 httpretty==1.1.*
 pylint==2.*
-twine
+twine==5.1.1
 coverage==7.*

percy/lib/cli_wrapper.py

@@ -1,13 +1,46 @@
 from functools import lru_cache
+from urllib.parse import urlparse
 import os
 import requests
 
 from percy.errors import CLIException
 from percy.common import log
 from percy.environment import Environment
 
-# 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'
+
+# Hostnames that resolve to the local machine. The Percy CLI runs locally, so
+# the SDK only ever needs to talk to a loopback address.
+_LOOPBACK_HOSTS = frozenset({'localhost', '127.0.0.1', '::1'})
+
+
+def _resolve_cli_api_address():
+    # The CLI API address can be overridden via the environment. Because the SDK
+    # POSTs request bodies that may carry BrowserStack credentials (capabilities)
+    # to this address, an attacker-controlled value would enable SSRF and
+    # credential exfiltration to an arbitrary host (CWE-306 / CWE-918). Restrict
+    # it to loopback by default; allow a remote host only over HTTPS and with an
+    # explicit opt-in.
+    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
+
+    log(
+        f"Ignoring non-loopback PERCY_CLI_API '{raw}'; falling back to {DEFAULT_PERCY_CLI_API}. "
+        "To target a remote Percy CLI, use an https:// URL and set PERCY_ALLOW_REMOTE_CLI_API=true.",
+        error=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()
 
 class CLIWrapper:
     def __init__(self) -> None:

percy/providers/generic_provider.py

@@ -180,11 +180,33 @@ def get_device_name(self):
         return ''
 
     def _get_dir(self):
+        return self._resolve_tmp_dir()
+
+    @staticmethod
+    def _resolve_tmp_dir():
         dir_path = os.environ.get('PERCY_TMP_DIR') or None
-        if dir_path:
-            Path(dir_path).mkdir(parents=True, exist_ok=True)
-            return dir_path
-        return tempfile.mkdtemp()
+        if not dir_path:
+            return tempfile.mkdtemp()
+
+        # PERCY_TMP_DIR is attacker-influenceable. Resolve symlinks and relative
+        # segments, then confirm the result stays within an approved temp root so
+        # it cannot be used to write screenshots to arbitrary locations such as
+        # web-served directories or dotfiles (CWE-22).
+        resolved = os.path.realpath(dir_path)
+        allowed_bases = []
+        for base in (tempfile.gettempdir(), '/tmp', '/var/tmp'):
+            try:
+                allowed_bases.append(os.path.realpath(base))
+            except OSError:
+                continue
+
+        if not any(resolved == base or resolved.startswith(base + os.sep) for base in allowed_bases):
+            log(f"Ignoring unsafe PERCY_TMP_DIR '{dir_path}' (outside the system temp directory); "
+                "using a secure temporary directory instead.", error=True)
+            return tempfile.mkdtemp()
+
+        Path(resolved).mkdir(parents=True, exist_ok=True)
+        return resolved
 
     def _get_path(self, directory):
         suffix = '.png'

tests/test_cli_wrapper.py

@@ -1,4 +1,5 @@
 # pylint: disable=[protected-access]
+import os
 import unittest
 from unittest.mock import Mock, patch
 from percy.errors import CLIException
@@ -7,6 +8,41 @@
 from percy.lib.tile import Tile
 
 
+class ResolveCliApiAddressTestCase(unittest.TestCase):
+    @patch.dict(os.environ, {}, clear=True)
+    def test_defaults_to_localhost(self):
+        self.assertEqual(cli_wrapper._resolve_cli_api_address(), cli_wrapper.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(cli_wrapper._resolve_cli_api_address(), "http://127.0.0.1:5338")
+
+    @patch.dict(os.environ, {"PERCY_CLI_API": "http://attacker.example.com/x"}, clear=True)
+    def test_rejects_remote_host(self):
+        # Falls back to the safe default instead of POSTing credentials off-box.
+        self.assertEqual(cli_wrapper._resolve_cli_api_address(), cli_wrapper.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(cli_wrapper._resolve_cli_api_address(), cli_wrapper.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(cli_wrapper._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(cli_wrapper._resolve_cli_api_address(), cli_wrapper.DEFAULT_PERCY_CLI_API)
+
+
 class CLIWrapperTestCase(unittest.TestCase):
     def setUp(self):
         self.cli_wrapper = cli_wrapper.CLIWrapper()

tests/test_generic_provider.py

@@ -2,6 +2,7 @@
 # pylint: disable=R0904
 import shutil
 import os
+import tempfile
 import unittest
 from unittest.mock import MagicMock, patch, PropertyMock
 from appium.webdriver.webdriver import WebDriver
@@ -52,6 +53,42 @@ def test_get_dir_with_env_var(self):
         self.assertEqual(os.listdir(dir_created), [])
         os.removedirs(dir_created)
 
+    def test_get_dir_rejects_path_outside_temp(self):
+        # A PERCY_TMP_DIR that resolves outside the system temp root must be
+        # rejected to prevent arbitrary file writes via path traversal (CWE-22).
+        escape_dir = os.path.join(os.getcwd(), "percy-escape-test")
+        with patch.dict(os.environ, {"PERCY_TMP_DIR": escape_dir}):
+            dir_created = self.generic_provider._get_dir()
+        self.assertTrue(os.path.realpath(dir_created).startswith(os.path.realpath(tempfile.gettempdir())))
+        self.assertFalse(os.path.exists(escape_dir))
+        os.removedirs(dir_created)
+
+    def test_get_dir_rejects_dotdot_traversal(self):
+        traversal = os.path.join(tempfile.gettempdir(), "..", "..", "etc", "percy-evil")
+        with patch.dict(os.environ, {"PERCY_TMP_DIR": traversal}):
+            dir_created = self.generic_provider._get_dir()
+        self.assertTrue(os.path.realpath(dir_created).startswith(os.path.realpath(tempfile.gettempdir())))
+        self.assertFalse(os.path.exists(os.path.realpath(traversal)))
+        os.removedirs(dir_created)
+
+    def test_get_dir_skips_unresolvable_allowed_base(self):
+        # If resolving an allowed temp base raises OSError it must be skipped
+        # rather than crash; with no resolvable base the env dir is rejected and
+        # a secure temporary directory is used instead.
+        real_realpath = os.path.realpath
+        fake_dir = os.path.join(tempfile.gettempdir(), "percy-base-error")
+
+        def flaky_realpath(path):
+            if path in (tempfile.gettempdir(), "/tmp", "/var/tmp"):
+                raise OSError("cannot resolve base")
+            return real_realpath(path)
+
+        with patch.dict(os.environ, {"PERCY_TMP_DIR": fake_dir}):
+            with patch("percy.providers.generic_provider.os.path.realpath", side_effect=flaky_realpath):
+                dir_created = self.generic_provider._get_dir()
+        self.assertTrue(os.path.realpath(dir_created).startswith(os.path.realpath(tempfile.gettempdir())))
+        os.removedirs(dir_created)
+
     def test_get_path(self):
         count = 1000  # Generate count unique paths and check their uniqueness
         png_paths = [