fix(security): P2 triage sweep — query injection, CVE pins, CodeQL fixes - #839
POWERFULMOVES wants to merge 5 commits into
Conversation
…tion Fixes 4 pre-existing build-gate findings from BuildKit --check audit: - pmoves-archon: rename MCP_CREDENTIALS_PATH → MCP_CONFIG_PATH (eliminates SecretsUsedInArgOrEnv false positive) - pmoves-archon-ui: add USER directive (uid 65532, non-root) - pmoves-firefly-iii: add USER www-data defense-in-depth - pmoves-llama-throughput-lab: add USER directive + nginx non-root setup Dashboard: correct runner status from "3/4 online" to actual state (all 4 offline, ai-lab-win now online after manual start). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add _SAFE_VID_RE.match(video_id) check on video IDs extracted from Hi-RAG search results before passing to supa_get(). Prevents query injection via crafted video_id values. Closes P2 #7. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add post-install pip overrides for 4 Trivy-flagged CVEs: - archon: crawl4ai>=0.8.0 (CVE-2026-26216), langchain-core>=1.2.5 (CVE-2025-68664) - deepresearch: ray>=2.52.0 (CVE-2025-62593), vllm>=0.14.1 (CVE-2026-22778) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- options.js: Replace innerHTML template literal with DOM API (textContent) to eliminate XSS vector - mock-server.js: Guard routes[key] lookup with Object.hasOwn() to prevent prototype chain access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR encompasses multiple updates across documentation, Docker configurations, and application code. Key changes include: submodule commit updates, environment variable renaming from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (1)
pmoves/services/archon/Dockerfile (1)
155-157: Use exact override versions here, not floating minimums.
>=turns these security overrides into moving targets, so a rebuild can pull a newercrawl4aiorlangchain-corerelease without any Dockerfile diff. If0.8.0and1.2.5are the vetted fixes, pin them exactly and bump intentionally.Suggested change
- && pip install --no-cache-dir \ - "crawl4ai>=0.8.0" \ - "langchain-core>=1.2.5" \ + && pip install --no-cache-dir \ + "crawl4ai==0.8.0" \ + "langchain-core==1.2.5" \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/services/archon/Dockerfile` around lines 155 - 157, The Dockerfile currently uses floating-minimum pins "crawl4ai>=0.8.0" and "langchain-core>=1.2.5" which makes rebuilds pull newer, unvetted releases; change these to exact pins (e.g. "crawl4ai==0.8.0" and "langchain-core==1.2.5") in the pip install line so the image reproducibly uses the vetted fixes, and update any accompanying comment or release notes to document intentional bumps in future.
🤖 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/docs/PRODUCTION_AUDIT_DASHBOARD.md`:
- Line 30: Normalize all AB-9 status mentions to a single authoritative
snapshot: pick the correct current state for AB-9 (e.g., "All 4 self-hosted
runners offline" or "1/4 online"), then update every occurrence—the
"latest-changes" paragraph, the "executive summary", and the "AB-9 blocker
row"—so they all state the same status and supporting evidence (mention affected
runners `ai-lab-runner`, `ai-lab-win`, `hotfix-runner`, `vps-runner` and the
required restart commands `svc.sh install`/`run.cmd`); ensure GHCR build impact
(`[self-hosted, Linux, X64, vps]`) is consistently described and align the
runbook/smoke evidence text with that chosen snapshot.
- Around line 16-21: The summary line "P2 Tier 1 triage sweep — 3 of 4" is
inconsistent with the four bullets marked FIXED (P2 `#4` Open-Notebook, P2 `#1`
BoTZ, P2 `#8` DoX, P2 `#7` PMOVES.YT) and the later "4 production-blocking items
closed" statement; update the header to match the evidence (change "3 of 4" to
"4 of 4") or, alternatively, if one of the bullets is incorrect, remove or
unmark that specific bullet so the count and the list align; ensure the phrase
"P2 Tier 1 triage sweep" and the later "P2 tracker updated — 4
production-blocking items closed" are consistent.
---
Nitpick comments:
In `@pmoves/services/archon/Dockerfile`:
- Around line 155-157: The Dockerfile currently uses floating-minimum pins
"crawl4ai>=0.8.0" and "langchain-core>=1.2.5" which makes rebuilds pull newer,
unvetted releases; change these to exact pins (e.g. "crawl4ai==0.8.0" and
"langchain-core==1.2.5") in the pip install line so the image reproducibly uses
the vetted fixes, and update any accompanying comment or release notes to
document intentional bumps in future.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19d23de6-2b51-4e24-bffe-6b2a1567c113
📒 Files selected for processing (11)
PMOVES-ArchonPMOVES-llama-throughput-labdocs/phase2-buildkit-secrets-migration-plan.mdpmoves/chrome-extension/options/options.jspmoves/chrome-extension/test/mock-server.jspmoves/docs/PRODUCTION_AUDIT_DASHBOARD.mdpmoves/docs/security/P2_SUBMODULE_TRACKER.mdpmoves/images/firefly-iii/Dockerfilepmoves/services/archon/Dockerfilepmoves/services/deepresearch/Dockerfilepmoves/services/pmoves-yt/yt.py
| - **P2 Tier 1 triage sweep** — 3 of 4 production-blocking P2 items verified FIXED in submodules: | ||
| - P2 #4 Open-Notebook: Auth fail-open → **FIXED** (fail-closed `HTTPException 500` at `auth.py:32-36`) | ||
| - P2 #1 BoTZ: MCP Gateway unauthenticated GET → **FIXED** (`_require_auth()` on `/servers`, `/tools`) | ||
| - P2 #8 DoX: NATS no auth → **FIXED** (auth block added to `nats.conf`; `no_tls: true` is documented dev-only) | ||
| - P2 #7 PMOVES.YT: Query injection → **FIXED** (added `_SAFE_VID_RE` validation on Hi-RAG-sourced `video_id` at `yt.py:3710`) | ||
| - **P2 tracker updated** — 4 production-blocking items closed → 11 open (all Tier 2/3, non-blocking) |
There was a problem hiding this comment.
The blocker closeout count contradicts itself.
Line 16 says 3 of 4 production-blocking P2 items were fixed, but the bullets below mark four items fixed and Line 21 says four blockers were closed. Please collapse this to one number before publishing the dashboard.
As per coding guidelines, "Keep status claims aligned with evidence in runbooks and smokes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/docs/PRODUCTION_AUDIT_DASHBOARD.md` around lines 16 - 21, The summary
line "P2 Tier 1 triage sweep — 3 of 4" is inconsistent with the four bullets
marked FIXED (P2 `#4` Open-Notebook, P2 `#1` BoTZ, P2 `#8` DoX, P2 `#7` PMOVES.YT) and
the later "4 production-blocking items closed" statement; update the header to
match the evidence (change "3 of 4" to "4 of 4") or, alternatively, if one of
the bullets is incorrect, remove or unmark that specific bullet so the count and
the list align; ensure the phrase "P2 Tier 1 triage sweep" and the later "P2
tracker updated — 4 production-blocking items closed" are consistent.
| - **Trivy CVE overrides** — added pip upgrade steps in Dockerfiles: | ||
| - `archon`: `crawl4ai>=0.8.0` (CVE-2026-26216 RCE), `langchain-core>=1.2.5` (CVE-2025-68664 RCE) | ||
| - `deepresearch`: `ray>=2.52.0` (CVE-2025-62593 RCE), `vllm>=0.14.1` (CVE-2026-22778 RCE) | ||
| - **AB-9 UPDATE** — All 4 self-hosted runners offline as of Mar 9 investigation. Previous "3/4 online" claim was stale. Root causes: `ai-lab-runner` (WSL2, no systemd service installed — process stopped), `ai-lab-win` (no Windows service — process stopped), `hotfix-runner` + `vps-runner` (remote machines, not accessible from dev workstation). WSL2 systemd now enabled (`/etc/wsl.conf`). Local runners require manual restart via `svc.sh install` (WSL) or interactive `run.cmd` (Windows). GHCR builds targeting `[self-hosted, Linux, X64, vps]` remain blocked until VPS runner is restored. |
There was a problem hiding this comment.
AB-9 is reported with multiple conflicting runner states.
The latest-changes section says all four self-hosted runners are offline, the executive summary says 1/4 is online, and the AB-9 blocker row is now regressed/offline again. Please normalize every AB-9 reference to the same snapshot; right now the dashboard is self-contradictory.
As per coding guidelines, "Keep status claims aligned with evidence in runbooks and smokes."
Also applies to: 374-375, 499-499
🧰 Tools
🪛 LanguageTool
[style] ~30-~30: Consider using “inaccessible” to avoid wordiness.
Context: ...unner+vps-runner` (remote machines, not accessible from dev workstation). WSL2 systemd now...
(NOT_ABLE_PREMIUM)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/docs/PRODUCTION_AUDIT_DASHBOARD.md` at line 30, Normalize all AB-9
status mentions to a single authoritative snapshot: pick the correct current
state for AB-9 (e.g., "All 4 self-hosted runners offline" or "1/4 online"), then
update every occurrence—the "latest-changes" paragraph, the "executive summary",
and the "AB-9 blocker row"—so they all state the same status and supporting
evidence (mention affected runners `ai-lab-runner`, `ai-lab-win`,
`hotfix-runner`, `vps-runner` and the required restart commands `svc.sh
install`/`run.cmd`); ensure GHCR build impact (`[self-hosted, Linux, X64, vps]`)
is consistently described and align the runbook/smoke evidence text with that
chosen snapshot.
- Dashboard: normalize runner status to "0/4 offline" (was contradictory) - Dashboard: clarify P2 count "15 open" as pre-triage snapshot - Dashboard: fix "3 of 4" → "4 of 4" P2 items verified - Dashboard: AB-9 blocker detail REGRESSED (was stale RESOLVED) - Dashboard: Docker Bench row reflects AB-9 regression - Dockerfiles: pin exact CVE versions (>=→==) for crawl4ai, langchain-core, ray, vllm - BuildKit migration plan: add archival banner (implemented in PR #838) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(security): validate Hi-RAG video_id against allowlist regex Add _SAFE_VID_RE.match(video_id) check on video IDs extracted from Hi-RAG search results before passing to supa_get(). Prevents query injection via crafted video_id values. Closes P2 #7. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): pin CVE-patched versions for archon + deepresearch Add post-install pip overrides for 4 Trivy-flagged CVEs: - archon: crawl4ai>=0.8.0 (CVE-2026-26216), langchain-core>=1.2.5 (CVE-2025-68664) - deepresearch: ray>=2.52.0 (CVE-2025-62593), vllm>=0.14.1 (CVE-2026-22778) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): resolve 2 CodeQL alerts in chrome extension - options.js: Replace innerHTML template literal with DOM API (textContent) to eliminate XSS vector - mock-server.js: Guard routes[key] lookup with Object.hasOwn() to prevent prototype chain access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(audit): close 4 P2 production-blockers + refresh dashboard - P2 tracker: Mark items #1, #4, #7, #8 as FIXED with verification dates - Dashboard: Add triage sweep entry, update stale PRs to MERGED, document CodeQL and Trivy fixes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(audit): reconcile P2 tracker — close 7 stale P1 findings All 7 reported P1 submodule issues from Phase C audit (2026-02-16) verified already fixed on PMOVES.AI-Edition-Hardened branches. Added individual verification entries with evidence paths to Closed Issues table. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(audit): refresh dashboard — all P1 submodule issues resolved Update executive summary and latest changes to reflect tracker reconciliation: all 7 Phase C P1 submodule findings verified fixed on Hardened branches. Add changelog entry with evidence summary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(tools): add living document reconciliation script Checks and updates dashboard commit SHA/date metadata and flags stale P2 tracker items whose submodules have advanced. Supports --check (CI-safe read-only), --update (write metadata), and --json output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * build(make): add docs-reconcile Make targets and preflight integration Adds docs-reconcile, docs-reconcile-check, docs-reconcile-json targets. Integrates non-blocking docs-reconcile-check into audit-layers-static. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(skills): add /docs:reconcile skill command Provides CLI-invocable skill for living document reconciliation with check, update, and JSON modes. Cross-links audit-layers and sign-trail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(context): add Living Document Maintenance guidance to CLAUDE.md Directs agents to run docs-reconcile after audit/security work or submodule gitlink updates. Lists the two living documents and rules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(review): resolve 7 CodeRabbit findings across PRs #839/#840 - Dashboard: normalize runner status to "0/4 offline" (was contradictory) - Dashboard: clarify P2 count "15 open" as pre-triage snapshot - Dashboard: fix "3 of 4" → "4 of 4" P2 items verified - Dashboard: AB-9 blocker detail REGRESSED (was stale RESOLVED) - Dashboard: Docker Bench row reflects AB-9 regression - Dockerfiles: pin exact CVE versions (>=→==) for crawl4ai, langchain-core, ray, vllm - BuildKit migration plan: add archival banner (implemented in PR #838) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Shaela Bello <slbello@uncg.edu> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Changes
_SAFE_VID_REbefore Supabase queryTest plan
git diffeach commit verifies single-concern changes_SAFE_VID_RErejects non-alphanumeric video IDs in yt search path🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Security