fix(security): CodeQL #196 XSS + post-PR #865 dashboard - #866
POWERFULMOVES wants to merge 2 commits into
Conversation
Strengthen URL sanitization in chrome-extension options.js from regex to URL constructor with strict protocol allowlist (http:/https: only). Update production audit dashboard with Supabase unification milestone and CodeQL alert resolution status. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughReplaced regex URL scheme check in the Chrome extension options with URL constructor validation and try/catch; updated production audit docs to reflect Supabase unification and CodeQL Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pmoves/chrome-extension/options/options.js (1)
262-268: URL validation logic is correct and addresses CodeQL#196.The
new URL()constructor with protocol allowlist is the proper approach for breaking the taint chain. Usingparsed.hrefensures normalization.However, when validation fails (invalid URL or disallowed protocol), the link element is still rendered with "View SVG" text but no
href. Consider either hiding the link or providing visual feedback when the URL is invalid.🛠️ Optional: Hide link when URL is invalid
const link = document.createElement('a'); const gatewayBase = $('#url-gateway')?.value || 'http://localhost:8085'; const vizUrl = `${gatewayBase}/viz/shape/${encodeURIComponent(s.shape_id)}.svg`; // Validate URL via constructor to prevent XSS (CodeQL `#194`, `#196`) + let validUrl = false; try { const parsed = new URL(vizUrl); if (parsed.protocol === 'http:' || parsed.protocol === 'https:') { link.href = parsed.href; + validUrl = true; } } catch { /* invalid URL — skip href assignment */ } - link.target = '_blank'; - link.style.cssText = 'margin-left:8px;color:`#667eea`;'; - link.textContent = 'View SVG'; - div.appendChild(link); + if (validUrl) { + link.target = '_blank'; + link.style.cssText = 'margin-left:8px;color:`#667eea`;'; + link.textContent = 'View SVG'; + div.appendChild(link); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/chrome-extension/options/options.js` around lines 262 - 268, When URL validation fails in the block that parses vizUrl with new URL() (where parsed is checked for parsed.protocol and link.href is assigned), hide or disable the link element instead of rendering a "View SVG" anchor with no href: update the logic around link (the link element created/used for the SVG preview) so that if the try/catch falls through or the protocol is not http/https you either remove the link from the DOM, set link.hidden = true, or add a disabled/aria-disabled state and alternative text so users get clear visual feedback when vizUrl is invalid; ensure any change is applied wherever link and vizUrl are handled so no empty anchor is shown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/chrome-extension/options/options.js`:
- Around line 262-268: In popup.js, before calling chrome.tabs.create({ url:
r.url }) (the place using r.url returned from shapeSvgUrl() in pmoves-api.js),
validate r.url by constructing new URL(r.url) inside a try/catch and ensure its
protocol is 'http:' or 'https:'; only pass the parsed.href to chrome.tabs.create
when the check succeeds, otherwise skip opening the tab or handle the error.
This mirrors the URL validation used in options.js and ensures untrusted
_services.gateway values (including 'javascript:' or 'data:') are not opened.
---
Nitpick comments:
In `@pmoves/chrome-extension/options/options.js`:
- Around line 262-268: When URL validation fails in the block that parses vizUrl
with new URL() (where parsed is checked for parsed.protocol and link.href is
assigned), hide or disable the link element instead of rendering a "View SVG"
anchor with no href: update the logic around link (the link element created/used
for the SVG preview) so that if the try/catch falls through or the protocol is
not http/https you either remove the link from the DOM, set link.hidden = true,
or add a disabled/aria-disabled state and alternative text so users get clear
visual feedback when vizUrl is invalid; ensure any change is applied wherever
link and vizUrl are handled so no empty anchor is shown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2faa0c0b-de47-4ec0-9016-c7dd3597b449
📒 Files selected for processing (2)
pmoves/chrome-extension/options/options.jspmoves/docs/PRODUCTION_AUDIT_DASHBOARD.md
| // Validate URL via constructor to prevent XSS (CodeQL #194, #196) | ||
| try { | ||
| const parsed = new URL(vizUrl); | ||
| if (parsed.protocol === 'http:' || parsed.protocol === 'https:') { | ||
| link.href = parsed.href; | ||
| } | ||
| } catch { /* invalid URL — skip href assignment */ } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for other unvalidated URL usages in the chrome extension
echo "=== Checking popup.js for URL handling ==="
rg -n -C3 'chrome\.tabs\.create' pmoves/chrome-extension/
echo ""
echo "=== Checking for direct href assignments without URL validation ==="
rg -n '\.href\s*=' pmoves/chrome-extension/ --type js
echo ""
echo "=== Checking pmoves-api.js for URL construction ==="
rg -n -C3 '_services\.gateway' pmoves/chrome-extension/Repository: POWERFULMOVES/PMOVES.AI
Length of output: 1372
🏁 Script executed:
#!/bin/bash
# Examine the msg() function and where r.url originates
echo "=== Examining popup.js msg function and getShapeSvgUrl action ==="
rg -n -B5 -A10 "getShapeSvgUrl" pmoves/chrome-extension/
echo ""
echo "=== Looking for message handler in popup.js or background script ==="
rg -n -B5 -A10 "getShapeSvgUrl" pmoves/chrome-extension/ --type js
echo ""
echo "=== Checking if pmoves-api.js exists ==="
find pmoves -name "*api*" -o -name "*pmoves*api*" 2>/dev/nullRepository: POWERFULMOVES/PMOVES.AI
Length of output: 5368
🏁 Script executed:
#!/bin/bash
# Get more context around the msg() function in popup.js
echo "=== Finding msg function definition ==="
rg -n "msg\s*=" pmoves/chrome-extension/popup/popup.js -A3
echo ""
echo "=== Looking for background script or service worker ==="
find pmoves/chrome-extension -name "*.js" | grep -E "(background|service|worker)" | head -5Repository: POWERFULMOVES/PMOVES.AI
Length of output: 201
🏁 Script executed:
#!/bin/bash
# Find shapeSvgUrl function definition
echo "=== Finding shapeSvgUrl function ==="
rg -n "shapeSvgUrl" pmoves/chrome-extension/background.js -B2 -A5
echo ""
echo "=== Looking at pmoves-api.js for URL construction ==="
head -50 pmoves/chrome-extension/lib/pmoves-api.js
echo ""
echo "=== Searching for _services.gateway in pmoves-api.js ==="
rg -n "_services\\.gateway\|gateway" pmoves/chrome-extension/lib/pmoves-api.js -C2Repository: POWERFULMOVES/PMOVES.AI
Length of output: 2179
🏁 Script executed:
#!/bin/bash
# Find shapeSvgUrl function in pmoves-api.js
echo "=== Finding shapeSvgUrl implementation ==="
rg -n "shapeSvgUrl" pmoves/chrome-extension/lib/pmoves-api.js -B2 -A8
echo ""
echo "=== Full pmoves-api.js to see all exports ==="
wc -l pmoves/chrome-extension/lib/pmoves-api.jsRepository: POWERFULMOVES/PMOVES.AI
Length of output: 488
URL validation missing in popup.js: chrome.tabs.create() passes unvalidated URLs.
At popup.js:188, the code passes r.url directly to chrome.tabs.create({ url: r.url }) without validation. This URL originates from shapeSvgUrl() (pmoves-api.js:319), which constructs it from the configurable _services.gateway without protocol validation:
return `${_services.gateway}/viz/shape/${encodeURIComponent(shapeId)}.svg`;If _services.gateway is set to a javascript: or data: URL, the extension would attempt to open it, bypassing the protocol validation now applied in options.js.
Apply the same new URL() constructor + protocol check before calling chrome.tabs.create() to ensure consistent security posture.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/chrome-extension/options/options.js` around lines 262 - 268, In
popup.js, before calling chrome.tabs.create({ url: r.url }) (the place using
r.url returned from shapeSvgUrl() in pmoves-api.js), validate r.url by
constructing new URL(r.url) inside a try/catch and ensure its protocol is
'http:' or 'https:'; only pass the parsed.href to chrome.tabs.create when the
check succeeds, otherwise skip opening the tab or handle the error. This mirrors
the URL validation used in options.js and ensures untrusted _services.gateway
values (including 'javascript:' or 'data:') are not opened.
- Remap supabase-realtime default host port 4000→4010 to avoid collision with tensorzero-ui - Increase grep_context window from 30→55 lines in realtime healthcheck/JWT tests to capture healthcheck block below long env var sections Local-only (gitignored) fixes applied separately: - Add NATS_URL to env.tier-agent and env.tier-worker - Remove duplicate SUPABASE_JWT_SECRET from env.shared and env.tier-api Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Docker Hardening ValidationHardening Validation ReportValidated: Wed Mar 11 17:51:30 UTC 2026Services CheckedPMOVES.AI Docker Hardening Validation[INFO] Checking: pmoves/docker-compose.hardened.yml [INFO] Validating: hi-rag-gateway-v2 [INFO] Validating: extract-worker [INFO] Validating: langextract [INFO] Validating: presign [INFO] Validating: render-webhook [INFO] Validating: retrieval-eval [INFO] Validating: pdf-ingest [INFO] Validating: jellyfin-bridge [INFO] Validating: invidious-companion-proxy [INFO] Validating: ffmpeg-whisper [INFO] Validating: media-video [INFO] Validating: media-audio [INFO] Validating: hi-rag-gateway-v2-gpu [INFO] Validating: hi-rag-gateway-gpu [INFO] Validating: deepresearch [INFO] Validating: supaserch [INFO] Validating: publisher-discord [INFO] Validating: mesh-agent [INFO] Validating: nats-echo-req [INFO] Validating: nats-echo-res [INFO] Validating: publisher [INFO] Validating: analysis-echo [INFO] Validating: graph-linker [INFO] Validating: comfy-watcher [INFO] Validating: grayjay-plugin-host [INFO] Validating: agent-zero [INFO] Validating: archon [INFO] Validating: channel-monitor [INFO] Validating: pmoves-yt [INFO] Validating: notebook-sync [INFO] Validating: supabase_service_role_key [INFO] Validating: supabase_jwt_secret ====================================== |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pmoves/tests/smoke/test_supabase_realtime_tenant.py (1)
27-30:⚠️ Potential issue | 🟡 MinorUpdate default port to 4010 to match docker-compose change.
The default fallback URL uses port
4000, butpmoves/docker-compose.yml(line 712) now defaults the host port to4010. Tests running against the default compose configuration will fail to connect.🔧 Proposed fix
SUPABASE_REALTIME_URL = os.getenv( "SUPABASE_REALTIME_URL", - "ws://localhost:4000/socket/websocket" + "ws://localhost:4010/socket/websocket" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/tests/smoke/test_supabase_realtime_tenant.py` around lines 27 - 30, The default fallback URL for SUPABASE_REALTIME_URL in the test uses port 4000 which no longer matches the docker-compose change; update the default value in the SUPABASE_REALTIME_URL declaration to use "ws://localhost:4010/socket/websocket" so tests using the fallback will connect to the compose service (modify the constant where SUPABASE_REALTIME_URL is set in pmoves/tests/smoke/test_supabase_realtime_tenant.py).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/docker-compose.yml`:
- Around line 711-712: Update the SUPABASE_REALTIME_PORT default in
pmoves/env.supabase (change SUPABASE_REALTIME_PORT from 4000 to 4010) and update
the hardcoded websocket URLs in pmoves/tests/smoke/test_supabase_selfhosted.py
(line with "ws://localhost:4000/socket/websocket") and
pmoves/tests/smoke/test_supabase_realtime_tenant.py (line with
"ws://localhost:4000/socket/websocket") to use port 4010 or derive the host URL
from SUPABASE_REALTIME_PORT so tests follow the new docker-compose host port;
leave the internal container port (4000) references unchanged.
---
Outside diff comments:
In `@pmoves/tests/smoke/test_supabase_realtime_tenant.py`:
- Around line 27-30: The default fallback URL for SUPABASE_REALTIME_URL in the
test uses port 4000 which no longer matches the docker-compose change; update
the default value in the SUPABASE_REALTIME_URL declaration to use
"ws://localhost:4010/socket/websocket" so tests using the fallback will connect
to the compose service (modify the constant where SUPABASE_REALTIME_URL is set
in pmoves/tests/smoke/test_supabase_realtime_tenant.py).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b53bc4e9-2d45-48bb-90b8-617ac4fd55b0
📒 Files selected for processing (2)
pmoves/docker-compose.ymlpmoves/tests/smoke/test_supabase_realtime_tenant.py
| ports: | ||
| - ${SUPABASE_REALTIME_PORT:-4000}:4000 | ||
| - ${SUPABASE_REALTIME_PORT:-4010}:4000 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify env files and test defaults for port consistency
echo "=== Checking env.supabase for SUPABASE_REALTIME_PORT ==="
rg -n "SUPABASE_REALTIME_PORT" pmoves/env.supabase pmoves/env.tier-supabase 2>/dev/null || echo "Files not found or no matches"
echo ""
echo "=== Checking test files for hardcoded port 4000 references ==="
rg -n "localhost:4000" pmoves/tests/Repository: POWERFULMOVES/PMOVES.AI
Length of output: 460
Update env.supabase and test file defaults to match the new 4010 host port.
The docker-compose change from port 4000→4010 avoids a port collision, but pmoves/env.supabase (line 167) still sets SUPABASE_REALTIME_PORT=4000, which will override this default. Additionally, test files at pmoves/tests/smoke/test_supabase_selfhosted.py:31 and test_supabase_realtime_tenant.py:29 use hardcoded ws://localhost:4000/socket/websocket, causing test failures against the new compose default.
The internal container port (4000) correctly remains unchanged, so container-to-container references continue working.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/docker-compose.yml` around lines 711 - 712, Update the
SUPABASE_REALTIME_PORT default in pmoves/env.supabase (change
SUPABASE_REALTIME_PORT from 4000 to 4010) and update the hardcoded websocket
URLs in pmoves/tests/smoke/test_supabase_selfhosted.py (line with
"ws://localhost:4000/socket/websocket") and
pmoves/tests/smoke/test_supabase_realtime_tenant.py (line with
"ws://localhost:4000/socket/websocket") to use port 4010 or derive the host URL
from SUPABASE_REALTIME_PORT so tests follow the new docker-compose host port;
leave the internal container port (4000) references unchanged.
Summary
chrome-extension/options/options.js:263-268from regex (/^https?:\/\//) tonew URL()constructor with strictparsed.protocolallowlist — gives CodeQL a clear taint-break signalTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests
Chores