security: harden CI workflows, session cache, debug dump and cookie opt-out (PER-8912, PER-8915, PER-8916, PER-8920, PER-8922)

.github/workflows/changelog.yml

@@ -2,10 +2,15 @@ name: Changelog
 on:
   push:
     branches: [master]
+permissions:
+  contents: read
 jobs:
   update_draft:
+    permissions:
+      contents: write  # create/update the draft release
+      pull-requests: read
     runs-on: ubuntu-latest
     steps:
-      - uses: release-drafter/release-drafter@v5
+      - uses: release-drafter/release-drafter@09c613e259eb8d4e7c81c2cb00618eb5fc4575a7 # v5.25.0
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/lint.yml

@@ -1,15 +1,17 @@
 name: Lint
 on: push
+permissions:
+  contents: read
 jobs:
   lint:
     name: Lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-python@v4
+      - uses: actions/checkout@a37ce9120846195fa4ece8f58b268e6043cb2f26 # v3.7.0
+      - uses: actions/setup-python@7f4fc3e22c37d6ff65e88745f38bd3157c663f7c # v4.9.1
         with:
           python-version: 3.11
-      - uses: actions/cache@v3
+      - uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c # v3.5.0
         with:
           path: ~/.cache/pip
           key: v1/${{ runner.os }}/pip/${{ hashFiles('{requirements,development}.txt') }}

.github/workflows/release.yml

@@ -2,15 +2,17 @@ name: Release
 on:
   release:
     types: [published]
+permissions:
+  contents: read
 jobs:
   publish:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-python@v4
+      - uses: actions/checkout@a37ce9120846195fa4ece8f58b268e6043cb2f26 # v3.7.0
+      - uses: actions/setup-python@7f4fc3e22c37d6ff65e88745f38bd3157c663f7c # v4.9.1
         with:
           python-version: 3.x
-      - uses: actions/cache@v3
+      - uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c # v3.5.0
         with:
           path: ~/.cache/pip
           key: v1/${{ runner.os }}/pip/${{ hashFiles('{requirements,development}.txt') }}

.github/workflows/stale.yml

@@ -3,11 +3,17 @@ on:
   schedule:
     - cron: '0 19 * * 2'
 
+permissions:
+  contents: read
+
 jobs:
   stale:
+    permissions:
+      issues: write
+      pull-requests: write
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/stale@v8
+      - uses: actions/stale@1160a2240286f5da8ec72b1c0816ce2481aabf84 # v8.0.0
         with:
           stale-issue-message: >-
             This issue is stale because it has been open for more than 14 days with no activity.

.github/workflows/test.yml

@@ -8,6 +8,9 @@ on:
         type: string
         default: master
 
+permissions:
+  contents: read
+
 jobs:
   test:
     name: Test
@@ -17,7 +20,7 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions-ecosystem/action-regex-match@v2
+      - uses: actions-ecosystem/action-regex-match@9e6c4fb3d5e898f505be7a1fb6e7b0a278f6665b # v2.0.2
         id: regex-match
         if: ${{ github.event_name == 'workflow_dispatch' }}
         with:
@@ -26,22 +29,22 @@ jobs:
       - name: Break on invalid branch name
         run: exit 1
         if: ${{ github.event_name == 'workflow_dispatch' && steps.regex-match.outputs && steps.regex-match.outputs.match == '' }}
-      - uses: actions/checkout@v3
-      - uses: actions/setup-python@v4
+      - uses: actions/checkout@a37ce9120846195fa4ece8f58b268e6043cb2f26 # v3.7.0
+      - uses: actions/setup-python@7f4fc3e22c37d6ff65e88745f38bd3157c663f7c # v4.9.1
         with:
           python-version: ${{ matrix.python }}
-      - uses: actions/cache@v3
+      - uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c # v3.5.0
         with:
           path: ~/.cache/pip
           key: v1/${{ runner.os }}/pypi-${{matrix.python}}/${{ hashFiles('{requirements,development}.txt') }}
           restore-keys: v1/${{ runner.os }}/pypi-${matrix.python}/
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1
         with:
           node-version: 16
       - name: Get yarn cache directory path
         id: yarn-cache-dir-path
         run: echo "::set-output name=dir::$(yarn cache dir)"
-      - uses: actions/cache@v3
+      - uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c # v3.5.0
         with:
           path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
           key: v1/${{ runner.os }}/node-${{ matrix.node }}/${{ hashFiles('**/yarn.lock') }}
@@ -65,7 +68,7 @@ jobs:
           # no node project to link into); PATH is what makes percy resolvable.
           echo "$(yarn global bin)" >> "$GITHUB_PATH"
           export PATH="$(yarn global bin):$PATH"
-          cd ${{ github.workspace }}
+          cd "$GITHUB_WORKSPACE"
           yarn remove @percy/cli >/dev/null 2>&1 || true
           yarn link `echo $PERCY_PACKAGES` >/dev/null 2>&1 || true
           percy --version

README.md

@@ -60,6 +60,13 @@ $ percy exec -- [python test command]
 - `name` (**required**) - The snapshot name; must be unique to each snapshot
 - `**kwargs` - [See per-snapshot configuration options](https://www.browserstack.com/docs/percy/take-percy-snapshots/overview#per-snapshot-configuration)
 
+### Cookies
+
+By default the SDK sends the browser's cookies with each snapshot so Percy can load pages and
+assets that need a logged-in session during asset discovery. To stop sending cookies, set
+`PERCY_DO_NOT_USE_CAPTURED_COOKIES=true`. Assets behind authentication may then fail to load
+and snapshots can render incorrectly; the SDK logs a warning once when this is enabled.
+
 ### Migrating Config
 
 If you have a previous Percy configuration file, migrate it to the newest version with the

percy/cache.py

@@ -1,9 +1,12 @@
+import threading
 import time
 
 class Cache:
     CACHE  = {}
     CACHE_TIMEOUT = 5 * 60 # 300 seconds
     TIMEOUT_KEY = 'last_access_time'
+    # Guards CACHE so parallel runners sharing a process can't mutate it mid-iteration
+    LOCK = threading.RLock()
 
     # Caching Keys
     capabilities = 'capabilities'
@@ -19,24 +22,27 @@ def check_types(cls, session_id, property):
     @classmethod
     def set_cache(cls, session_id, property, value):
         cls.check_types(session_id, property)
-        session = cls.CACHE.get(session_id, {})
-        session[cls.TIMEOUT_KEY] = time.time()
-        session[property] = value
-        cls.CACHE[session_id] = session
+        with cls.LOCK:
+            session = cls.CACHE.get(session_id, {})
+            session[cls.TIMEOUT_KEY] = time.time()
+            session[property] = value
+            cls.CACHE[session_id] = session
 
     @classmethod
     def get_cache(cls, session_id, property):
         cls.cleanup_cache()
         cls.check_types(session_id, property)
-        session = cls.CACHE.get(session_id, {})
-        return session.get(property, None)
+        with cls.LOCK:
+            session = cls.CACHE.get(session_id, {})
+            return session.get(property, None)
 
     @classmethod
     def cleanup_cache(cls):
         now = time.time()
-        session_ids = []
-        for session_id, session in cls.CACHE.items():
-            timestamp = session[cls.TIMEOUT_KEY]
-            if now - timestamp >= cls.CACHE_TIMEOUT:
-                session_ids.append(session_id)
-        list(map(cls.CACHE.pop, session_ids))
+        with cls.LOCK:
+            session_ids = [
+                session_id for session_id, session in cls.CACHE.items()
+                if now - session[cls.TIMEOUT_KEY] >= cls.CACHE_TIMEOUT
+            ]
+            for session_id in session_ids:
+                cls.CACHE.pop(session_id, None)

percy/snapshot.py

@@ -2,6 +2,7 @@
 import os
 import platform
 import json
+import tempfile
 from contextlib import contextmanager
 from functools import lru_cache
 from time import sleep
@@ -35,6 +36,9 @@ def _get_bool_env(key):
     RESIZE_SETTLE_SECONDS = float(os.environ.get('PERCY_RESIZE_SETTLE_SECONDS') or 0.5)
 except (TypeError, ValueError):
     RESIZE_SETTLE_SECONDS = 0.5
+# Set to true to stop sending browser cookies to the CLI (read per snapshot)
+DO_NOT_USE_CAPTURED_COOKIES_ENV = 'PERCY_DO_NOT_USE_CAPTURED_COOKIES'
+_cookie_opt_out_warned = False
 # for logging
 LABEL = '[\u001b[35m' + ('percy:python' if PERCY_DEBUG else 'percy') + '\u001b[39m]'
 CDP_SUPPORT_SELENIUM = (str(SELENIUM_VERSION)[0].isdigit() and int(
@@ -52,6 +56,25 @@ def log(message, lvl = 'info'):
         if lvl != 'debug' or PERCY_DEBUG:
             print(message)
 
+def get_cookies_for_snapshot(driver):
+    """Collect browser cookies so the CLI can load authenticated assets during
+    discovery. Honours the PERCY_DO_NOT_USE_CAPTURED_COOKIES opt-out and never
+    raises: a cookie read failure only means no cookies are forwarded."""
+    global _cookie_opt_out_warned # pylint: disable=global-statement
+    if _get_bool_env(DO_NOT_USE_CAPTURED_COOKIES_ENV):
+        if not _cookie_opt_out_warned:
+            _cookie_opt_out_warned = True
+            log('Not sending browser cookies to Percy because '
+                f'{DO_NOT_USE_CAPTURED_COOKIES_ENV} is set. Pages or assets that need a '
+                'logged-in session may fail to load during asset discovery, so snapshots '
+                'can render incorrectly.', 'warn')
+        return []
+    try:
+        return driver.get_cookies()
+    except Exception as e:  # pylint: disable=broad-except
+        log(f'Could not read browser cookies, continuing without them: {e}', 'debug')
+        return []
+
 # Check if Percy is enabled, caching the result so it is only checked once
 @lru_cache(maxsize=None)
 def is_percy_enabled():
@@ -991,18 +1014,29 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa
         dom_snapshots.append(dom_snapshot)
     # Optional debug dump — gated to avoid polluting the user's CWD on every CI run.
     if PERCY_DEBUG:
-        try:
-            with open("output_file.json", "w", encoding="utf-8") as file_handle:
-                json.dump(dom_snapshots, file_handle, indent=4)
-        except Exception as e:  # pylint: disable=broad-except
-            log(f"Could not write debug snapshot dump: {type(e).__name__}: {e}", "debug")
+        _write_responsive_debug_dump(dom_snapshots)
     # Restore the original window size after capture. No snapshot follows, so
     # skip the resize-event poll (wait_for_resize=False) — it serves no purpose
     # here and is the call that hangs geckodriver in CI.
     change_window_dimension_and_wait(
         driver, current_width, current_height, resize_count + 1, wait_for_resize=False)
     return dom_snapshots
 
+def _write_responsive_debug_dump(dom_snapshots):
+    # Owner-only temp file with an unpredictable name; cookies are left out
+    # because they're not needed to debug the captured DOM.
+    try:
+        redacted = [
+            {k: v for k, v in snap.items() if k != 'cookies'} if isinstance(snap, dict) else snap
+            for snap in dom_snapshots
+        ]
+        fd, path = tempfile.mkstemp(prefix='percy-responsive-dom-', suffix='.json')
+        with os.fdopen(fd, "w", encoding="utf-8") as file_handle:
+            json.dump(redacted, file_handle, indent=4)
+        log(f"Wrote responsive snapshot debug dump to {path}", "debug")
+    except Exception as e:  # pylint: disable=broad-except
+        log(f"Could not write debug snapshot dump: {type(e).__name__}: {e}", "debug")
+
 def is_responsive_snapshot_capture(config, **kwargs):
     # Don't run resposive snapshot capture when defer uploads is enabled
     if 'percy' in config and config['percy'].get('deferUploads', False): return False
@@ -1028,7 +1062,7 @@ def percy_snapshot(driver, name, **kwargs):
         # Expose closed shadow roots via CDP before serialization so PercyDOM
         # can find them through the WeakMap (Chromium-only; non-Chromium no-ops).
         expose_closed_shadow_roots(driver)
-        cookies = driver.get_cookies()
+        cookies = get_cookies_for_snapshot(driver)
 
         # Serialize and capture the DOM
         if is_responsive_snapshot_capture(data['config'], **kwargs):

tests/test_cache.py

@@ -1,4 +1,5 @@
 # . # pylint: disable=[arguments-differ, protected-access]
+import threading
 import time
 import unittest
 from unittest.mock import patch
@@ -57,3 +58,31 @@ def test_cleanup_cache(self):
         self.assertIn(self.session_id, self.cache.CACHE)
         self.cache.cleanup_cache()
         self.assertNotIn(self.session_id, self.cache.CACHE)
+
+    def test_cleanup_cache_blocks_concurrent_writers_while_iterating(self):
+        cache = self.cache
+        writer_blocked = []
+        writers = []
+
+        class InterleavingDict(dict):
+            # Simulates another thread writing to the cache mid-iteration
+            def items(self):
+                writer = threading.Thread(
+                    target=cache.set_cache,
+                    args=('other_session', Cache.capabilities, {}))
+                writer.start()
+                writers.append(writer)
+                writer.join(timeout=0.2)
+                writer_blocked.append(writer.is_alive())
+                for item in list(super().items()):
+                    yield item
+
+        original = cache.CACHE
+        cache.CACHE = InterleavingDict(original)
+        try:
+            cache.cleanup_cache()  # must not raise "dictionary changed size during iteration"
+            writers[0].join(timeout=5)
+            self.assertEqual(writer_blocked, [True])
+            self.assertIn('other_session', cache.CACHE)
+        finally:
+            cache.CACHE = original