fix(security): Z890 gap analysis remediation — 4 findings - #950
Conversation
…rvice
Remove `pmoves-chit-default` default from Dockerfile ENV and main.py fallback.
Docker-compose enforces runtime injection via ${CHIT_PROD_PASSPHRASE:?...},
but the Dockerfile default was a security smell if the image ran standalone.
Addresses: Z890 gap analysis Issue #3 (PR #905)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Downgrade dev bypass role from "admin" to "dev" to limit privilege escalation in development mode. Add logger.warning() when auth is bypassed so operators can detect misconfiguration in production logs. Addresses: Z890 gap analysis Issue #4 (PR #926) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_build_nats_url() silently fell back to unauthenticated nats:// when NATS_URL and NATS_USER/NATS_PASSWORD were all unset. Add logger.warning() so operators can detect missing NATS credentials in logs. Addresses: Z890 gap analysis Issue #5 (PR #927) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The /api/audit/summary response included `docsRoot` — an absolute server filesystem path — in the JSON body. This leaks internal directory structure to unauthenticated clients. Remove it from the response payload. Addresses: Z890 gap analysis Issue #7 (PR #922) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughMultiple services receive updates: authentication layer adds development bypass logging, consciousness service shifts CHIT_PROD_PASSPHRASE from build-time to runtime injection with warning checks, flute gateway adds NATS credential logging, and the audit API response removes the docsRoot field from output. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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)
📝 Coding Plan
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 |
Docker Hardening ValidationHardening Validation ReportValidated: Sun Mar 15 19:11:32 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.
🧹 Nitpick comments (1)
pmoves/services/consciousness-service/main.py (1)
40-42: Use central secret loader forCHIT_PROD_PASSPHRASEinstead of direct env read.This segment removes the insecure default (good), but it still bypasses the shared secret-loading path used elsewhere in services.
♻️ Suggested refactor
@@ import os +from services.common.env import get_secret @@ -CHIT_PASSPHRASE = os.environ.get("CHIT_PROD_PASSPHRASE", "") +CHIT_PASSPHRASE = get_secret("CHIT_PROD_PASSPHRASE", "") or "" if not CHIT_PASSPHRASE: logger.warning("CHIT_PROD_PASSPHRASE not set — CGP signing disabled")As per coding guidelines "pmoves/services/**: Prefer central env helpers and *_FILE secret loading paths" and "Flag direct critical-secret reads and plaintext fallbacks."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/services/consciousness-service/main.py` around lines 40 - 42, Replace the direct os.environ.get read for CHIT_PASSPHRASE with the project's central secret loader (e.g., call the shared helper used by other services such as load_secret / secrets.get_secret) so it honors the *_FILE pattern (CHIT_PROD_PASSPHRASE_FILE) and secure file-backed secrets; do not use a plaintext default "", and keep the existing logger.warning when the secret is absent but sourced via the central loader API; update references to the CHIT_PASSPHRASE variable to use the value returned by that loader.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pmoves/services/consciousness-service/main.py`:
- Around line 40-42: Replace the direct os.environ.get read for CHIT_PASSPHRASE
with the project's central secret loader (e.g., call the shared helper used by
other services such as load_secret / secrets.get_secret) so it honors the *_FILE
pattern (CHIT_PROD_PASSPHRASE_FILE) and secure file-backed secrets; do not use a
plaintext default "", and keep the existing logger.warning when the secret is
absent but sourced via the central loader API; update references to the
CHIT_PASSPHRASE variable to use the value returned by that loader.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 020bc0b6-06ff-4e35-a9fe-186e7374225d
📒 Files selected for processing (5)
pmoves/services/cast-tts-gateway/auth.pypmoves/services/consciousness-service/Dockerfilepmoves/services/consciousness-service/main.pypmoves/services/flute-gateway/main.pypmoves/ui/app/api/audit/summary/route.ts
💤 Files with no reviewable changes (1)
- pmoves/ui/app/api/audit/summary/route.ts
All 5 TAC_MODEL_INFRA branches (B/D, A, C, F, E) verified as complete on main — registry seed (1511 lines), persona seed (8 personas), resolution view, readiness check (652 lines, 10 checks), service-model mappings (30+). Add Branch G (Agent Zero bootstrap integration) referencing PR #960. Update production audit dashboard with verification status and merged PR summary (#950-958, #960). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Triaged 7 security issues from the PR gap analysis produced during the Phase A-E sprint cleanup. 3 were already resolved on main, 4 required fixes:
pmoves-chit-defaultfrom Dockerfile + main.pyadmin→dev, added warning log_build_nats_url()falls back to no-authdocsRootabsolute path from/api/audit/summaryresponseChanges (4 commits)
consciousness-service/Dockerfile+main.py— Remove hardcodedpmoves-chit-defaultdefault, add startup warning when unsetcast-tts-gateway/auth.py— Downgrade dev bypass role fromadmintodev, addlogger.warning()when auth bypassedflute-gateway/main.py— Add warning in_build_nats_url()when falling back to unauthenticated connectionui/app/api/audit/summary/route.ts— RemovedocsRoot(absolute server path) from JSON response bodyTest plan
grep -c "pmoves-chit-default" pmoves/services/consciousness-service/Dockerfile→ 0grep '"role"' pmoves/services/cast-tts-gateway/auth.py→ first match is"dev"not"admin"grep "without credentials" pmoves/services/flute-gateway/main.py→ founddocsRootnot present in/api/audit/summaryresponse JSONFollow-up (out of scope)
ownerFromJwtto/api/agents/taxonomy,/api/monitor/stats, etc.)ALLOWED_FORMATSallowlist in a2ui-renderer (CodeQL Update n8n PMOVES.YT workflow for nested video response #170)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores