fix(ci): bump pillow and pytest to resolve audit CVEs#222
Closed
BANANASJIM wants to merge 3 commits into
Closed
Conversation
Two fixes for multi-GPU replay crashes: 1. _is_arm_studio_dir() falsely detected upstream renderdoc builds as ARM Performance Studio, triggering RTLD_GLOBAL preload of librenderdoc.so which corrupted Vulkan replay. Now requires "arm-performance-studio" in the path. 2. ReplayOptions now pins forceGPUVendor/forceGPUDeviceID to the GPU that was used for capture, by matching the deviceName from structured data against GetAvailableGPUs(). Prevents replay from picking a different GPU than what captured the frame. Also resolves RENDERDOC_PYTHON_PATH relative path handling.
Resolves CVE-2026-40192 (pillow) and CVE-2025-71176 (pytest) that cause the pip-audit CI step to fail on all open PRs.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe pull request enhances the replay system with GPU detection and matching logic. The daemon now infers the GPU used during capture by reading structured device data, derives appropriate GPU force-settings via Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DaemonServer
participant Capture
participant StructuredData as Structured Data
participant GPUMatcher
participant ReplayOptions
participant RenderDoc
Client->>DaemonServer: Load replay (capture file)
DaemonServer->>Capture: Read capture
Capture-->>DaemonServer: Capture object
DaemonServer->>Capture: GetStructuredData()
Capture-->>StructuredData: Device info (physProps.deviceName)
DaemonServer->>Capture: GetAvailableGPUs()
Capture-->>DaemonServer: GPU list
DaemonServer->>GPUMatcher: _match_capture_gpu(capture, structured_data)
GPUMatcher->>GPUMatcher: Match device name against<br/>available GPUs
GPUMatcher-->>DaemonServer: GPU vendor & device ID
DaemonServer->>ReplayOptions: Create with forceGPUVendor,<br/>forceGPUDeviceID
ReplayOptions-->>DaemonServer: Configured options
DaemonServer->>RenderDoc: OpenCapture(capture, options)
RenderDoc-->>Client: Replay ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/rdc/daemon_server.py (1)
179-192: Consider adding a comment noting Vulkan-specific parsing.The structured data traversal specifically looks for
vkEnumeratePhysicalDevices, which only applies to Vulkan captures. For D3D/OpenGL captures, the function falls back to the first available GPU. This is reasonable behavior but a brief comment would clarify the intent.📝 Suggested documentation
+ # Parse Vulkan-specific structured data to find capture GPU name. + # For non-Vulkan captures, falls back to first available GPU. for i in range(len(sd.chunks)): c = sd.chunks[i] if c.name == "vkEnumeratePhysicalDevices":🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rdc/daemon_server.py` around lines 179 - 192, The traversal in the loop examines sd.chunks and looks explicitly for the Vulkan-specific call name "vkEnumeratePhysicalDevices" and then checks children named "physProps" and "deviceName" to match against gpus; add a concise inline comment near this block (referencing sd.chunks, vkEnumeratePhysicalDevices, physProps, deviceName, and gpus) stating that this parsing is Vulkan-specific and that for D3D/OpenGL captures the logic intentionally falls back to the first available GPU, to clarify intent for future readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rdc/daemon_server.py`:
- Around line 169-196: In _match_capture_gpu, avoid the UnboundLocalError by
initializing gpus before the try block (e.g., gpus = [] or gpus = None) so
references in the except/final return are safe; update the function
(_match_capture_gpu) to set a default for gpus, keep the existing try/except
logic, and ensure the final return uses a safe check (return gpus[0] if gpus
else None) without referencing an uninitialized variable.
- Around line 359-370: The code currently uses _match_capture_gpu(tmp_cap) on
the local capture and writes its vendor/device into
remote_opts.forceGPUVendor/forceGPUDeviceID before calling remote.OpenCapture,
which can pick a GPU that doesn't exist on the remote; instead, remove the
assignment of remote_opts.forceGPUVendor and remote_opts.forceGPUDeviceID when
opening a remote server (i.e., when using remote.OpenCapture) so the remote can
pick NoPreference, or if you must force, query the remote's available GPUs first
and validate the local IDs against that list and log a clear error before
calling remote.OpenCapture; update the block that handles
state.local_capture_path and tmp_cap (and references to
remote_opts.forceGPUVendor/forceGPUDeviceID and _match_capture_gpu) to omit
forcing for remote replay or add remote-side validation+error logging
accordingly.
---
Nitpick comments:
In `@src/rdc/daemon_server.py`:
- Around line 179-192: The traversal in the loop examines sd.chunks and looks
explicitly for the Vulkan-specific call name "vkEnumeratePhysicalDevices" and
then checks children named "physProps" and "deviceName" to match against gpus;
add a concise inline comment near this block (referencing sd.chunks,
vkEnumeratePhysicalDevices, physProps, deviceName, and gpus) stating that this
parsing is Vulkan-specific and that for D3D/OpenGL captures the logic
intentionally falls back to the first available GPU, to clarify intent for
future readers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5b14e83-ca4a-4345-ae36-14b33a5e5835
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
src/rdc/daemon_server.pysrc/rdc/discover.pytests/mocks/mock_renderdoc.pytests/unit/test_discover.py
Comment on lines
+169
to
+196
| def _match_capture_gpu(cap: Any, sd: Any = None) -> Any | None: | ||
| """Find the GPU used for capture by matching structured data against available GPUs.""" | ||
| try: | ||
| gpus = cap.GetAvailableGPUs() | ||
| if not gpus: | ||
| return None | ||
| if len(gpus) == 1: | ||
| return gpus[0] | ||
| if sd is None: | ||
| return gpus[0] | ||
| for i in range(len(sd.chunks)): | ||
| c = sd.chunks[i] | ||
| if c.name == "vkEnumeratePhysicalDevices": | ||
| for j in range(c.NumChildren()): | ||
| child = c.GetChild(j) | ||
| if child.name == "physProps": | ||
| for k in range(child.NumChildren()): | ||
| prop = child.GetChild(k) | ||
| if prop.name == "deviceName": | ||
| name = prop.AsString() | ||
| for g in gpus: | ||
| if g.name == name: | ||
| return g | ||
| break | ||
| except Exception: # noqa: BLE001 | ||
| pass | ||
| return gpus[0] if gpus else None | ||
|
|
There was a problem hiding this comment.
Potential UnboundLocalError if GetAvailableGPUs() raises.
If cap.GetAvailableGPUs() at line 172 raises an exception, the except block at line 193 catches it and falls through to line 195, where gpus is referenced but was never assigned.
🐛 Proposed fix
def _match_capture_gpu(cap: Any, sd: Any = None) -> Any | None:
"""Find the GPU used for capture by matching structured data against available GPUs."""
+ gpus: list[Any] = []
try:
gpus = cap.GetAvailableGPUs()
if not gpus:
return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rdc/daemon_server.py` around lines 169 - 196, In _match_capture_gpu,
avoid the UnboundLocalError by initializing gpus before the try block (e.g.,
gpus = [] or gpus = None) so references in the except/final return are safe;
update the function (_match_capture_gpu) to set a default for gpus, keep the
existing try/except logic, and ensure the final return uses a safe check (return
gpus[0] if gpus else None) without referencing an uninitialized variable.
Comment on lines
+359
to
+370
| remote_opts = rd.ReplayOptions() | ||
| if state.local_capture_path: | ||
| tmp_cap = rd.OpenCaptureFile() | ||
| if tmp_cap.OpenFile(state.local_capture_path, "", None) == rd.ResultCode.Succeeded: | ||
| gpu = _match_capture_gpu(tmp_cap) | ||
| if gpu is not None: | ||
| remote_opts.forceGPUVendor = gpu.vendor | ||
| remote_opts.forceGPUDeviceID = gpu.deviceID | ||
| tmp_cap.Shutdown() | ||
|
|
||
| result, controller = remote.OpenCapture( | ||
| rd.RemoteServer.NoPreference, remote_path, rd.ReplayOptions(), None | ||
| rd.RemoteServer.NoPreference, remote_path, remote_opts, None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if RemoteServer has a method to get available GPUs
ast-grep --pattern $'class RemoteServer {
$$$
GetAvailableGPUs($$$) {
$$$
}
$$$
}'
# Search for any remote GPU enumeration patterns
rg -n -C3 'GetAvailableGPUs|AvailableGPUs' --type=pyRepository: BANANASJIM/rdc-cli
Length of output: 2721
🏁 Script executed:
# Search for RemoteServer class definition
rg -n 'class RemoteServer' --type=py
# Look at the remote module/import to understand what it exposes
rg -n 'import.*[Rr]emote|from.*[Rr]emote' src/rdc/daemon_server.py
# Check the rd module type hints and available GPU-related APIs
fd -e pyi -e py | xargs rg -l 'RemoteServer|remote' | head -10Repository: BANANASJIM/rdc-cli
Length of output: 435
🏁 Script executed:
# Look at the context around line 359-370 in daemon_server.py
sed -n '350,380p' src/rdc/daemon_server.py
# Check how remote is created/instantiated
rg -n -B5 'remote\.OpenCapture|RemoteServer' src/rdc/daemon_server.py | head -40
# Look for any comments or documentation about GPU matching on remote
rg -n -C5 '_match_capture_gpu' src/rdc/daemon_server.pyRepository: BANANASJIM/rdc-cli
Length of output: 3475
🏁 Script executed:
# Look at how local replay handles GPU matching for comparison
sed -n '210,230p' src/rdc/daemon_server.py
# Search for any remote GPU enumeration patterns or comments
rg -n 'remote.*GPU|GPU.*remote|forceGPU' src/rdc/
# Check if there's any documentation or workarounds for remote GPU issues
rg -n 'NoPreference|forceGPU' src/rdc/ -C2
# Look for test cases about remote GPU matching
fd -e py tests/ | xargs rg -l 'remote.*GPU|GPU.*remote' 2>/dev/nullRepository: BANANASJIM/rdc-cli
Length of output: 2975
🏁 Script executed:
# Look for comments or documentation about remote GPU matching
rg -n 'remote.*GPU|GPU|NoPreference' src/rdc/daemon_server.py -C3
# Check the _match_capture_gpu function in detail
sed -n '169,195p' src/rdc/daemon_server.py
# Look for any error handling or fallback when GPU forcing fails
rg -n 'forceGPU' src/rdc/ -A3 -B1
# Check if there are any issues or TODOs related to remote GPU
rg -n 'TODO|FIXME|BUG|XXX' src/rdc/daemon_server.py | grep -i gpuRepository: BANANASJIM/rdc-cli
Length of output: 3752
🏁 Script executed:
# Look for the full remote replay function context
sed -n '320,390p' src/rdc/daemon_server.py
# Check if there's any error handling or logging related to remote GPU forcing
rg -n 'remote_opts|RemoteServer' src/rdc/daemon_server.py -C2
# Look for any documentation files that might explain remote GPU handling
fd -e md -e txt | xargs rg -l 'remote.*GPU|GPU' 2>/dev/null | head -5
# Check if structured data is available during remote replay
rg -n 'GetStructuredData' src/rdc/daemon_server.pyRepository: BANANASJIM/rdc-cli
Length of output: 4076
🏁 Script executed:
# Check if there are any tests for remote GPU matching
fd . tests/ -name '*.py' | xargs rg -l 'remote.*GPU|test.*remote.*replay' 2>/dev/null
# Look at the test file for remote replay
find tests -name '*remote*' -type f | head -5
# Check if there's any documentation about the limitation
rg -n 'InitialiseReplay|remote.*GPU' docs -r . 2>/dev/null | head -20
# Check the exact error checking around remote.OpenCapture
sed -n '369,385p' src/rdc/daemon_server.pyRepository: BANANASJIM/rdc-cli
Length of output: 1328
🏁 Script executed:
# Look at the remote replay test to understand expected behavior
cat tests/unit/test_open_remote.py | head -100
# Check if there's any documentation about remote GPU limitations
cat docs-astro/src/pages/docs/remote.astro 2>/dev/null | head -150
# Check the proposal/changelog for any discussion about GPU handling
cat openspec/changes/2026-03-03-bugfix-p2-batch/proposal.md 2>/dev/null | head -200Repository: BANANASJIM/rdc-cli
Length of output: 13644
Remote GPU forcing uses local capture's available GPUs, not remote server's.
The code calls _match_capture_gpu(tmp_cap) on the local copy of the capture, returning the first available GPU from the local machine. This GPU vendor/device ID is then passed to remote.OpenCapture() via remote_opts.forceGPUVendor and remote_opts.forceGPUDeviceID.
If the remote server has different GPUs available than the local machine, the forced GPU may not exist on the remote, causing undefined behavior. The RenderDoc API's rd.RemoteServer.NoPreference fallback suggests graceful degradation is possible, but there is no explicit handling or logging if GPU forcing fails.
Consider either:
- Omitting GPU forcing for remote replay (let
NoPreferencehandle GPU selection on the remote) - Adding explicit error handling or logging if the forced GPU is not available
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rdc/daemon_server.py` around lines 359 - 370, The code currently uses
_match_capture_gpu(tmp_cap) on the local capture and writes its vendor/device
into remote_opts.forceGPUVendor/forceGPUDeviceID before calling
remote.OpenCapture, which can pick a GPU that doesn't exist on the remote;
instead, remove the assignment of remote_opts.forceGPUVendor and
remote_opts.forceGPUDeviceID when opening a remote server (i.e., when using
remote.OpenCapture) so the remote can pick NoPreference, or if you must force,
query the remote's available GPUs first and validate the local IDs against that
list and log a clear error before calling remote.OpenCapture; update the block
that handles state.local_capture_path and tmp_cap (and references to
remote_opts.forceGPUVendor/forceGPUDeviceID and _match_capture_gpu) to omit
forcing for remote replay or add remote-side validation+error logging
accordingly.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
These two CVEs cause the
pip-auditCI step to fail on all open PRs (#219, #220, #221).Test plan
pip-auditpasses locally with 0 vulnerabilitiesSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests