mosaic/stack
1
0
Fork 0
Code Issues 10 Pull Requests Actions Packages Projects Releases 2 Wiki Activity
Files
e38aaa950bdbb42e6bfb533af545abeecfb0488d
stack/docs/reports/codebase-review-2026-02-05/01-security-review.md
Jason Woltje 9dfbf8cf61
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
Details
chore: Remove pre-created task files, add review reports 
- Delete docs/tasks.md (let orchestrator bootstrap from scratch)
- Delete docs/claude/task-tracking.md (superseded by universal guide)
- Add codebase review reports for orchestrator to parse

Tests orchestrator's autonomous bootstrap capability.
2026-02-05 15:08:02 -06:00

479 lines
18 KiB
Markdown
Raw Blame History

# Mosaic Stack - Security Review
**Date:** 2026-02-05
**Reviewers:** 3 parallel security analysis agents
**Total Findings:** 95 (12 Critical, 25 High, 39 Medium, 19 Low)
---
## Table of Contents
1. [Critical Findings](#critical-findings)
2. [High Findings](#high-findings)
3. [Medium Findings](#medium-findings)
4. [Low Findings](#low-findings)
5. [Positive Security Observations](#positive-security-observations)
---
## Critical Findings
### SEC-API-1: OIDC Configuration Silently Degrades to Empty Strings
- **File:** `apps/api/src/auth/auth.config.ts:19-21`
- **Impact:** If OIDC env vars are missing, auth starts with broken config instead of failing fast. Discovery URL becomes a relative path.
- **Fix:** Validate OIDC env vars at startup. Throw if enabled but unconfigured.
### SEC-API-2: WorkspaceGuard Swallows Database Errors as Access Denied
- **File:** `apps/api/src/common/guards/workspace.guard.ts:131-150`
- **Impact:** Any Prisma error (connection timeout, pool exhaustion) is reported to users as "You do not have access to this workspace."
- **Fix:** Let database errors propagate as 500s. Only catch Prisma-specific "not found" errors.
### SEC-API-3: PermissionGuard Swallows Database Errors as Null Role
- **File:** `apps/api/src/common/guards/permission.guard.ts:107-128`
- **Impact:** Identical to SEC-API-2. Database failures masquerade as permissions problems.
- **Fix:** Remove broad try-catch or only catch specific Prisma errors.
### SEC-API-4: RLS Context Never Actually Applied in Service Layer
- **File:** All service files (tasks.service.ts, knowledge.service.ts, brain.service.ts, etc.)
- **Impact:** The RLS infrastructure (withWorkspaceContext, setWorkspaceContext) exists but is never used. Tenant isolation relies on application-level WHERE clauses only.
- **Fix:** Wrap service-layer database calls in withWorkspaceContext() transactions, or add automated tests verifying every query includes workspaceId.
### SEC-WEB-1: Open Redirect via Unsanitized OAuth Error Parameter
- **File:** `apps/web/src/app/(auth)/callback/page.tsx:19`
- **Impact:** Attacker can craft URLs with malicious error parameters reflected into router.push().
- **Fix:** Validate error against allowlist of known OAuth error codes. Always encodeURIComponent().
### SEC-WEB-2: WikiLinkRenderer Stored XSS via dangerouslySetInnerHTML
- **File:** `apps/web/src/components/knowledge/WikiLinkRenderer.tsx:30-38`
- **Impact:** parseWikiLinks() only sanitizes wiki-link text. Surrounding HTML passes through raw. Stored XSS via knowledge entry content.
- **Fix:** Sanitize the entire HTML input with DOMPurify BEFORE processing wiki-links.
### SEC-ORCH-1: Secret Scanner Returns False on Scan Errors
- **File:** `apps/orchestrator/src/git/secret-scanner.service.ts:259-268`
- **Impact:** File read errors return `{ hasSecrets: false }`. A file that couldn't be scanned is reported as clean.
- **Fix:** Return explicit error state. Add scanError field to result, or throw so caller can decide.
### SEC-ORCH-2: No Authentication on Orchestrator API Endpoints
- **File:** `apps/orchestrator/src/api/agents/agents.controller.ts:23-214`
- **Impact:** Spawn, kill, kill-all, status endpoints have zero auth. Any network client can drain API credits or kill all agents.
- **Fix:** Add API key guard or JWT authentication. Killswitch endpoints need authorization controls.
### SEC-ORCH-3: Docker Sandbox Disabled by Default
- **File:** `apps/orchestrator/src/config/orchestrator.config.ts:25`
- **Impact:** Agents run directly on the host without container isolation unless explicitly enabled.
- **Fix:** Enable sandbox by default in production. Log prominent warning when disabled.
### SEC-ORCH-4: Unauthenticated Inter-Service Communication
- **File:** `apps/orchestrator/src/coordinator/coordinator-client.service.ts:72-79`
- **Impact:** Orchestrator-to-coordinator communication has no auth. Quality gate responses can be spoofed.
- **Fix:** Add mutual authentication (API key at minimum, mTLS ideally). Enforce HTTPS.
### SEC-ORCH-5: Redis KEYS Command in Production (DoS Risk)
- **File:** `apps/orchestrator/src/valkey/valkey.client.ts:115-127, 185-198`
- **Impact:** KEYS command blocks the entire Redis instance. Unauthenticated list endpoints can DoS Redis.
- **Fix:** Replace with SCAN (non-blocking, cursor-based iteration).
### SEC-ORCH-6: Unsafe Deserialization with Type Assertion
- **File:** `apps/orchestrator/src/valkey/valkey.client.ts:69, 141, 193, 214`
- **Impact:** JSON.parse() + `as Type` with no runtime validation. Corrupted/tampered Redis data propagates silently.
- **Fix:** Add runtime validation using Zod or class-validator after JSON.parse().
---
## High Findings
### SEC-API-5: OpenAI Embedding Service Initialized with Dummy API Key
- **File:** `apps/api/src/knowledge/services/embedding.service.ts:27-35`
- **Fix:** Don't instantiate OpenAI client when key is missing (use null).
### SEC-API-6: Embedding Generation Failures Silently Swallowed
- **File:** `apps/api/src/knowledge/knowledge.service.ts:246-248, 408-412`
- **Fix:** Use structured logger. Track entries missing embeddings.
### SEC-API-7: CSRF Token Not Cryptographically Tied to Session
- **File:** `apps/api/src/common/controllers/csrf.controller.ts:20-34`
- **Fix:** Bind CSRF token to user session via HMAC with server secret.
### SEC-API-8: Rate Limiter Silently Falls Back to In-Memory
- **File:** `apps/api/src/common/throttler/throttler-storage.service.ts:54-60`
- **Fix:** Log at ERROR level. Expose health check for degraded mode.
### SEC-API-9: AdminGuard Uses Workspace Ownership as Admin Check
- **File:** `apps/api/src/auth/guards/admin.guard.ts:33-43`
- **Fix:** Implement proper system admin role (isSystemAdmin on User model).
### SEC-API-10: Auth Route Catch-All Bypasses Guards
- **File:** `apps/api/src/auth/auth.controller.ts:80-84`
- **Fix:** Add rate limiting and logging to auth catch-all.
### SEC-API-11: Federation Uses Hardcoded Default Workspace "default"
- **File:** `apps/api/src/federation/federation.controller.ts:226-239`
- **Fix:** Validate DEFAULT_WORKSPACE_ID is set and is a valid UUID.
### SEC-WEB-3: Missing CSRF Token on Multiple Direct fetch() Calls
- **Files:** `ImportExportActions.tsx:66`, `KanbanBoard.tsx:98`, `ActiveProjectsWidget.tsx:46-76`
- **Fix:** Route all state-changing API calls through apiPost/apiPatch/apiDelete.
### SEC-WEB-4: Mock Data Shipped in Production Code Paths
- **Files:** `federation/connections/page.tsx:49`, `settings/workspaces/page.tsx:11-28`, `teams/page.tsx:21`
- **Fix:** Connect to real APIs, show "Coming Soon", or gate behind NODE_ENV check.
### SEC-WEB-5: Silent Auth Session Check Failure
- **File:** `apps/web/src/lib/auth/auth-context.tsx:21-30`
- **Fix:** Log errors. Distinguish "not logged in" from "backend is down."
### SEC-WEB-6: WebSocket Token Without TLS Verification
- **File:** `apps/web/src/hooks/useWebSocket.ts:70-73`
- **Fix:** Enforce WSS in production. Add connect_error event handling.
### SEC-WEB-7: KanbanBoard Swallows Task Update Errors
- **File:** `apps/web/src/components/kanban/KanbanBoard.tsx:114-117`
- **Fix:** Revert UI state on error. Show notification.
### SEC-WEB-8: ActiveProjectsWidget Silently Drops Non-OK Responses
- **File:** `apps/web/src/components/widgets/ActiveProjectsWidget.tsx:46-58, 72-86`
- **Fix:** Handle non-OK responses explicitly.
### SEC-WEB-9: QuickCaptureWidget Discards User Data
- **File:** `apps/web/src/components/widgets/QuickCaptureWidget.tsx:21-32`
- **Fix:** Connect to API, persist to localStorage, or disable with "Coming Soon".
### SEC-WEB-10: Inconsistent API Base URL for Knowledge Graph
- **File:** `apps/web/src/components/mindmap/hooks/useGraphData.ts:122`
- **Fix:** Use same API base URL as rest of application (localhost:3001).
### SEC-WEB-11: Dual Auth Mechanisms (Cookie + Bearer Token)
- **File:** `apps/web/src/components/mindmap/hooks/useGraphData.ts:153-177`
- **Fix:** Standardize on single auth mechanism. Use apiGet/apiPost from client.ts.
### SEC-ORCH-7: Coordinator Loops Silently Swallow All Exceptions
- **File:** `apps/coordinator/src/coordinator.py:85-89, 299-304`
- **Fix:** Add circuit breaker. Track consecutive failures. Expose error counts in health endpoint.
### SEC-ORCH-8: Queue File Corrupted Data Silently Discarded
- **File:** `apps/coordinator/src/queue.py:217-234`
- **Fix:** Log corruption event. Backup corrupted file. Consider SQLite/Redis persistence.
### SEC-ORCH-9: Environment Variable Injection via Docker Container
- **File:** `apps/orchestrator/src/spawner/docker-sandbox.service.ts:87-91`
- **Fix:** Whitelist allowed env var names. Block LD_PRELOAD, PATH, NODE_OPTIONS, etc.
### SEC-ORCH-10: Docker Container Not Properly Isolated
- **File:** `apps/orchestrator/src/spawner/docker-sandbox.service.ts:100-114`
- **Fix:** Add CapDrop: ["ALL"], enable ReadonlyRootfs, NetworkMode: "none" default, PidsLimit.
### SEC-ORCH-11: No Rate Limiting on Orchestrator API
- **File:** `apps/orchestrator/src/api/agents/agents.controller.ts`
- **Fix:** Add @nestjs/throttler. Strict limits on killswitch endpoints.
### SEC-ORCH-12: No Max Concurrent Agents Enforcement
- **File:** `apps/orchestrator/src/spawner/agent-spawner.service.ts:40-74`
- **Fix:** Add configurable limit. Check sessions.size before spawning.
### SEC-ORCH-13: YOLO Mode Bypasses All Quality Gates
- **File:** `apps/orchestrator/src/coordinator/quality-gates.service.ts:222-257`
- **Fix:** Block in production. Add startup warning. Metric counter for bypasses.
### SEC-ORCH-14: Parser Prompt Injection via Issue Body
- **File:** `apps/coordinator/src/parser.py:99-136`
- **Fix:** Sanitize issue body. Use Claude's tool use for structured output. Validate bounds.
### SEC-ORCH-15: Valkey Connection Has No Authentication by Default
- **File:** `apps/orchestrator/src/config/orchestrator.config.ts:8`
- **Fix:** Log warning when VALKEY_PASSWORD not set. Require in production. Add TLS.
---
## Medium Findings
### SEC-API-12: CurrentUser Decorator Returns undefined Without Error
- **File:** `apps/api/src/auth/decorators/current-user.decorator.ts:9-14`
### SEC-API-13: Session Verification Catch-All Returns null
- **File:** `apps/api/src/auth/auth.service.ts:86-92`
### SEC-API-14: WebSocket Token in Query Parameters
- **File:** `apps/api/src/websocket/websocket.gateway.ts:181-184`
### SEC-API-15: Markdown Renderer Uses console.error
- **File:** `apps/api/src/knowledge/utils/markdown.ts:178, 204`
### SEC-API-16: Throttler Guard Uses console.warn
- **File:** `apps/api/src/common/throttler/throttler-api-key.guard.ts:40`
### SEC-API-17: data: URI Scheme Allowed in Markdown
- **File:** `apps/api/src/knowledge/utils/markdown.ts:109-111`
### SEC-API-18: Batch Embedding Failures Silently Continue
- **Files:** `apps/api/src/knowledge/services/ollama-embedding.service.ts:183-189`, `embedding.service.ts:150-157`
### SEC-API-19: BrainService Search Not Length-Validated
- **File:** `apps/api/src/brain/brain.service.ts:316-321, 360-365, 408-413`
### SEC-API-20: Brain Search Limit Parameter Allows Negatives
- **File:** `apps/api/src/brain/brain.controller.ts:74-76`
### SEC-API-21: Semantic/Hybrid Search Body Not Validated via DTO
- **File:** `apps/api/src/knowledge/search.controller.ts:113-149`
### SEC-API-22: Federation Controller Throws Generic Error
- **File:** `apps/api/src/federation/federation.controller.ts:65-207`
### SEC-API-23: OllamaEmbedding isConfigured() Discards Error
- **File:** `apps/api/src/knowledge/services/ollama-embedding.service.ts:48-51`
### SEC-API-24: Global Exception Filter Exposes Error Messages
- **File:** `apps/api/src/filters/global-exception.filter.ts:34-35`
### SEC-WEB-12: ChatInput Silently Swallows Version Fetch
- **File:** `apps/web/src/components/chat/ChatInput.tsx:32-34`
### SEC-WEB-13: MessageList Clipboard Copy Silently Fails
- **File:** `apps/web/src/components/chat/MessageList.tsx:75-78`
### SEC-WEB-14: BackendStatusBanner Is a Non-Functional Stub
- **File:** `apps/web/src/components/chat/BackendStatusBanner.tsx:19-33`
### SEC-WEB-15: Pervasive alert() for Error Feedback (13 instances)
- **Files:** ImportExportActions, WorkspaceSettings, MemberList, InviteMember, TeamSettings, TeamMemberList, settings pages
### SEC-WEB-16: No Content Security Policy Headers
- **File:** `apps/web/next.config.ts`
### SEC-WEB-17: useGraphData Silently Skips Backlink Fetch Errors
- **File:** `apps/web/src/components/mindmap/hooks/useGraphData.ts:305-308`
### SEC-WEB-18: useGraphData fetchStatistics Silently Fails
- **File:** `apps/web/src/components/mindmap/hooks/useGraphData.ts:412-414`
### SEC-WEB-19: Session Expiration Flag Never Resets
- **File:** `apps/web/src/lib/api.ts:5-6, 31-33`
### SEC-WEB-20: BetterAuth URL Not a NEXT_PUBLIC Variable
- **File:** `apps/web/src/lib/auth-client.ts:17-20`
### SEC-WEB-21: No Rate Limiting on Auth Session Refresh
- **File:** `apps/web/src/lib/auth/auth-context.tsx:42-44`
### SEC-WEB-22: Error Boundary Only Logs to Console
- **File:** `apps/web/src/components/error-boundary.tsx:34`
### SEC-WEB-23: ImportExportActions Missing credentials: "include"
- **File:** `apps/web/src/components/knowledge/ImportExportActions.tsx:66-69, 115-117`
### SEC-WEB-24: Authenticated Layout Race Between Load and Redirect
- **File:** `apps/web/src/app/(authenticated)/layout.tsx:18-33`
### SEC-WEB-25: LoginButton No PKCE/State Parameter
- **File:** `apps/web/src/components/auth/LoginButton.tsx:8-11`
### SEC-ORCH-16: Health/Ready Endpoint Always Returns True
- **File:** `apps/orchestrator/src/api/health/health.controller.ts:17-21`
### SEC-ORCH-17: Queue File No Locking (Concurrent Corruption)
- **File:** `apps/coordinator/src/queue.py:210-215`
### SEC-ORCH-18: Shared Package Uses console.warn
- **File:** `packages/shared/src/utils/index.ts:10`
### SEC-ORCH-19: agentId Path Parameter Not Validated as UUID
- **File:** `apps/orchestrator/src/api/agents/agents.controller.ts:78, 136`
### SEC-ORCH-20: Orchestrator Binds to 0.0.0.0 by Default
- **File:** `apps/orchestrator/src/main.ts:14`
### SEC-ORCH-21: BullMQ Connection Missing Password
- **File:** `apps/orchestrator/src/app.module.ts:15-20`
### SEC-ORCH-22: Docker Image Tag Not Validated
- **File:** `apps/orchestrator/src/spawner/docker-sandbox.service.ts:73`
### SEC-ORCH-23: git add "." When No Files Specified
- **File:** `apps/orchestrator/src/git/git-operations.service.ts:89-93`
### SEC-ORCH-24: Coordinator Context Check Returns CONTINUE on Error
- **File:** `apps/coordinator/src/coordinator.py:447-449`
### SEC-ORCH-25: SSRF Protection Incomplete for SSH URLs
- **File:** `apps/orchestrator/src/git/git-validation.util.ts:129-131`
### SEC-ORCH-26: In-Memory Session Store (Unbounded Growth)
- **File:** `apps/orchestrator/src/spawner/agent-spawner.service.ts:19`
### SEC-ORCH-27: unittest.mock Import in Production Code
- **File:** `apps/coordinator/src/quality_orchestrator.py:6`
---
## Low Findings
### SEC-API-25: ValidationPipe forbidNonWhitelisted: false
- **File:** `apps/api/src/main.ts:40`
### SEC-API-26: CORS Allows Requests with No Origin
- **File:** `apps/api/src/main.ts:63-66`
### SEC-API-27: createAuthMiddleware Sets RLS Context Outside Transaction
- **File:** `apps/api/src/lib/db-context.ts:347-358`
### SEC-API-28: MCP Service Uses console.error
- **Files:** `apps/api/src/mcp/mcp-hub.service.ts:164`, `mcp/stdio-transport.ts:42, 133`
### SEC-WEB-26: console.log Statements in Production
- **Files:** `settings/workspaces/page.tsx:56`, `teams/page.tsx:39`
### SEC-WEB-27: Weak Email Validation
- **File:** `apps/web/src/components/workspace/InviteMember.tsx:24-28`
### SEC-WEB-28: Role Cast Without Validation
- **File:** `apps/web/src/components/workspace/MemberList.tsx:91`
### SEC-WEB-29: formatTime Returns Empty String on Error
- **File:** `apps/web/src/components/chat/MessageList.tsx:316-323`
### SEC-WEB-30: JSON.parse Without Type Validation (deserializeMessages)
- **File:** `apps/web/src/hooks/useChat.ts:112-119`
### SEC-WEB-31: JSON.parse Without Validation (ConversationSidebar)
- **File:** `apps/web/src/components/chat/ConversationSidebar.tsx:46-52`
### SEC-WEB-32: No Input Length Limits on Forms
- **Files:** WorkspaceSettings, TeamSettings, InviteMember
### SEC-WEB-33: Mermaid Error Displays Raw Source
- **File:** `apps/web/src/components/mindmap/MermaidViewer.tsx:126-133`
### SEC-WEB-34: No Timeout on API Requests
- **File:** `apps/web/src/lib/api/client.ts`
### SEC-WEB-35: useWorkspaceId Returns null Without Explanation
- **File:** `apps/web/src/lib/hooks/useLayout.ts:225-240`
### SEC-WEB-36: localStorage Data Not Validated After Parse
- **Files:** `useChatOverlay.ts:40`, `useLayout.ts:45`
### SEC-WEB-37: Federation Mock Data Includes Fake Public Keys
- **File:** `apps/web/src/lib/api/federation.ts:202-272`
### SEC-ORCH-28: Valkey Client Has No Connection Timeout
- **File:** `apps/orchestrator/src/valkey/valkey.client.ts:38-44`
### SEC-ORCH-29: No Validation on workItems String Lengths
- **File:** `apps/orchestrator/src/api/agents/dto/spawn-agent.dto.ts:84-87`
### SEC-ORCH-30: Container Name Collision Risk
- **File:** `apps/orchestrator/src/spawner/docker-sandbox.service.ts:94`
---
## Positive Security Observations
1. **No $queryRawUnsafe or $executeRawUnsafe** -- All raw SQL uses Prisma's parameterized tagged template literals
2. **Timing-safe API key comparison** using crypto.timingSafeEqual
3. **Federation private keys encrypted at rest** with AES-256-GCM
4. **HTML sanitization** applied to markdown-rendered content before storage
5. **Zip bomb protection** with file count and size limits
6. **Path traversal prevention** in import service zip handler
7. **Guard chain consistency** (AuthGuard -> WorkspaceGuard -> PermissionGuard)
8. **Input validation via DTOs** with class-validator decorators on most endpoints
9. **Search query sanitization** strips PostgreSQL full-text operators
10. **Log sanitization** utilities redact secrets, tokens, and PII
11. **Git input validation** with whitelist-based branch name and URL validation
12. **Webhook signature verification** using hmac.compare_digest()
13. **State machine transitions** with explicit valid transition maps
Reference in New Issue View Git Blame Copy Permalink