fix(security): harden dashboard-web — loopback-only bind + Host/Origin guard (#2506)#2518
fix(security): harden dashboard-web — loopback-only bind + Host/Origin guard (#2506)#2518pangerlkr wants to merge 2 commits into
Conversation
…Origin guard Fixes gap 1 from issue affaan-m#2506. - Bind to process.env.ECC_DASHBOARD_HOST || '127.0.0.1' instead of all interfaces (0.0.0.0/::/default) - Wire loopback-guard.js: reject requests with a disallowed Host header (421) to block DNS-rebinding attacks - Wire loopback-guard.js: reject cross-origin requests with a disallowed Origin header (403) - Pass HOST to server.listen() so the bind address matches the guard allowlist Gap 2 (agent-data-home path containment) will follow in a separate commit.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe dashboard server now binds to a configurable host and validates Host and Origin headers. Agent data home resolution now enforces containment within a trusted user-home root. ChangesDashboard and path-safety hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant dashboardWeb as dashboard-web.js
participant DashboardRoute
Client->>dashboardWeb: Send request with Host and optional Origin
dashboardWeb->>dashboardWeb: Validate Host and Origin
dashboardWeb->>DashboardRoute: Route valid request
DashboardRoute-->>Client: Return JSON or HTML
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c96b953c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const PORT = parsePort(process.argv[2] || process.env.ECC_DASHBOARD_PORT || '3456'); | ||
| const ROOT = path.resolve(__dirname, '..'); | ||
| const HOST = process.env.ECC_DASHBOARD_HOST || '127.0.0.1'; | ||
| const ALLOWED = buildAllowedHostnames(HOST); |
There was a problem hiding this comment.
When ECC_DASHBOARD_HOST=0.0.0.0 is used for the documented LAN/container access case, this allowlist only includes loopback names plus the literal 0.0.0.0. A browser connecting via the machine or container address sends a Host header such as 192.168.1.10:3456 or host.docker.internal:3456, so the new guard returns 421 and the dashboard is unusable unless the client spoofs Host. Separate the bind address from the accepted public hostnames, or provide an explicit allowed-hosts setting for non-loopback binds.
Useful? React with 👍 / 👎.
|
| Filename | Overview |
|---|---|
| scripts/dashboard-web.js | Adds loopback-only default binding plus Host and Origin checks before serving dashboard routes. |
| scripts/lib/agent-data-home.js | Adds a trusted-root containment check before returning a project-configured agent data home. |
Reviews (2): Last reviewed commit: "fix(#2506): enforce path containment in ..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
Hardens the scripts/dashboard-web.js local dashboard server by defaulting to loopback-only binding and adding Host/Origin validation to mitigate DNS-rebinding and cross-origin access risks.
Changes:
- Default bind host changed to
127.0.0.1viaECC_DASHBOARD_HOSToverride. - Added Host header allowlist validation (returns
421on mismatch). - Added Origin allowlist validation when an Origin header is present (returns
403on mismatch).
|
|
||
| const server = http.createServer((req, res) => { | ||
| const url = new URL(req.url, 'http://localhost'); | ||
| if (!isAllowedHostHeader(req.headers['host'], ALLOWED)) { res.writeHead(421, { 'Content-Type': 'text/plain' }); return res.end('421 Misdirected Request'); } if (req.headers['origin'] && !isAllowedOrigin(req.headers['origin'], ALLOWED)) { res.writeHead(403, { 'Content-Type': 'text/plain' }); return res.end('403 Forbidden'); } const url = new URL(req.url, 'http://localhost'); |
| server.listen(PORT, HOST, () => { | ||
| console.log(`\n ECC Capabilities → http://localhost:${PORT}\n`); | ||
| try { const { spawn } = require('child_process'); const p = process.platform; const c = p === 'darwin' ? 'open' : p === 'win32' ? 'start' : 'xdg-open'; if (c === 'start') spawn('cmd', ['/c', 'start', `http://localhost:${PORT}`], { stdio: 'ignore' }); else spawn(c, [`http://localhost:${PORT}`], { stdio: 'ignore' }); } catch { /* best-effort auto-open */ } | ||
| }); |
| if (url.pathname === '/api/data') { | ||
| res.writeHead(200, { 'Content-Type': 'application/json' }); | ||
| return res.end(JSON.stringify({ agents: loadAgents(), skills: loadSkills(), commands: loadCommands(), rules: loadRules(), mcps: loadMcps(), hooks: loadHooks() })); | ||
|
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const PORT = parsePort(process.argv[2] || process.env.ECC_DASHBOARD_PORT || '3456'); | ||
| const ROOT = path.resolve(__dirname, '..'); | ||
| const HOST = process.env.ECC_DASHBOARD_HOST || '127.0.0.1'; | ||
| const ALLOWED = buildAllowedHostnames(HOST); |
There was a problem hiding this comment.
When ECC_DASHBOARD_HOST=0.0.0.0 is used for LAN or container access, the server listens on all interfaces but the allowlist only adds the literal host 0.0.0.0. A remote browser sends Host: <machine-ip>:3456 or a DNS name instead, so the new Host guard returns 421 and the documented external access mode does not work.
Rule Used: Treat CLI inputs, URLs, file paths, and subprocess... (source)
| if (require.main === module) { | ||
| server.listen(PORT, () => { | ||
| server.listen(PORT, HOST, () => { | ||
| console.log(`\n ECC Capabilities → http://localhost:${PORT}\n`); |
There was a problem hiding this comment.
When ECC_DASHBOARD_HOST is set to a LAN IP or another explicit host, the server binds there but still prints and opens http://localhost:${PORT}. In remote or container workflows, that URL points at the viewer's own machine rather than the dashboard listener, so the configured access path appears broken even when the server started correctly.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…ia assertWithinTrustedRoot Fixes gap 2 from issue affaan-m#2506. readProjectConfigAt() reads agentDataHome from /.cursor/ecc-agent-data.json and passes it through expandHomePath(), but never validated that the resolved path stays within a safe root. An attacker-controlled config file could redirect session-data writes to arbitrary locations via absolute paths or path traversal. This commit: - Imports assertWithinTrustedRoot from path-safety.js (same pattern used elsewhere) - After expandHomePath(), calls assertWithinTrustedRoot(resolved, getHomeDirFromEnv(), 'agent-data-home write') to enforce that any configured path stays within the user home subtree - If the path escapes the trusted root, assertWithinTrustedRoot throws, the surrounding try/catch in readProjectConfigAt returns null (safe fallback), and ECC falls back to its default agent-data-home resolution
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/dashboard-web.js`:
- Around line 24-25: Validate and normalize ECC_DASHBOARD_HOST to supported
loopback host forms before passing it to buildAllowedHostnames or server.listen,
rejecting malformed or unresolvable values with a clear configuration error. Add
an explicit error handler to the server.listen startup flow so bind failures are
reported cleanly instead of becoming unhandled errors.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b634c3a-8775-41a6-a771-2559e7b77330
📒 Files selected for processing (1)
scripts/dashboard-web.js
📜 Review details
⏰ Context from checks skipped due to timeout. (28)
- GitHub Check: Greptile Review
- GitHub Check: Test (ubuntu-latest, Node 18.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 18.x, bun)
- GitHub Check: Test (windows-latest, Node 22.x, npm)
- GitHub Check: Test (windows-latest, Node 18.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 20.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 22.x, yarn)
- GitHub Check: Test (macos-latest, Node 22.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 22.x, npm)
- GitHub Check: Test (windows-latest, Node 20.x, yarn)
- GitHub Check: Test (windows-latest, Node 22.x, yarn)
- GitHub Check: Test (ubuntu-latest, Node 18.x, pnpm)
- GitHub Check: Test (windows-latest, Node 18.x, yarn)
- GitHub Check: Test (windows-latest, Node 18.x, npm)
- GitHub Check: Test (macos-latest, Node 18.x, npm)
- GitHub Check: Test (macos-latest, Node 18.x, yarn)
- GitHub Check: Test (ubuntu-latest, Node 18.x, yarn)
- GitHub Check: Test (windows-latest, Node 22.x, pnpm)
- GitHub Check: Test (macos-latest, Node 22.x, npm)
- GitHub Check: Test (macos-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, yarn)
- GitHub Check: Test (windows-latest, Node 20.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, npm)
- GitHub Check: Test (windows-latest, Node 20.x, pnpm)
- GitHub Check: Coverage
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Implement XSS prevention by sanitizing HTML output
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
scripts/dashboard-web.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
**/*.{ts,tsx,js,jsx}: Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation
Use async/await with try-catch for error handling in TypeScript/JavaScript
Use Zod for schema-based input validation in TypeScript/JavaScript
No console.log statements in production code; use proper logging libraries instead
**/*.{ts,tsx,js,jsx}: Auto-format JavaScript/TypeScript files using Prettier after edit
Warn aboutconsole.logstatements in edited files
Check all modified files forconsole.logstatements before session ends
**/*.{ts,tsx,js,jsx}: Use the ApiResponse interface pattern with generic type parameter:interface ApiResponse<T> { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }
Implement custom React hooks following the pattern: export a named function with use prefix, generic type parameters, and proper useEffect cleanup for side effects
**/*.{ts,tsx,js,jsx}: Never hardcode secrets; always use environment variables for sensitive credentials like API keys
Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are metUse Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript
Files:
scripts/dashboard-web.js
{package.json,*.config.js,scripts/**/*.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Package manager detection should support npm, pnpm, yarn, and bun, with configuration via CLAUDE_PACKAGE_MANAGER environment variable or project config.
Files:
scripts/dashboard-web.js
scripts/**/*.js
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure cross-platform support for Windows, macOS, and Linux via Node.js scripts in the scripts/ directory.
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Always create new objects and never mutate in place; return new copies instead
Keep files between 200–400 lines typical, with a maximum of 800 lines
Extract helpers when a file exceeds 200 lines
Handle errors explicitly at every level; never swallow errors silently
Validate all user input before processing; use schema-based validation where available
Never trust external data (API responses, file content, query params); always validate
All user inputs must be validated and sanitized
Error messages must be scrubbed of sensitive internals
Use readable, well-named identifiers in all code
Keep functions under 50 lines
Keep files under 800 lines
Avoid nesting deeper than 4 levels
Implement comprehensive error handling in all code
Do not hardcode values; use constants or environment configuration instead
Do not use in-place mutation; always return new objects or state
Files:
scripts/dashboard-web.js
**/*.{js,ts,jsx,tsx,json,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not hardcode secrets, API keys, passwords, or tokens
Files:
scripts/dashboard-web.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use parameterized queries for all database writes (no string interpolation)
Auth/authz must be checked server-side for every sensitive path
Rate limiting must be applied to all public endpoints
Files:
scripts/dashboard-web.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
HTML output must be sanitized where applicable
Files:
scripts/dashboard-web.js
**/*.{js,ts,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Required environment variables must be validated at startup
Files:
scripts/dashboard-web.js
{scripts,bin}/**
⚙️ CodeRabbit configuration file
{scripts,bin}/**: Focus on command injection, unsafe subprocess usage, path traversal, SSRF, secret exposure, and missing tests for new CLI behavior.
Files:
scripts/dashboard-web.js
🔇 Additional comments (1)
scripts/dashboard-web.js (1)
15-15: LGTM!Also applies to: 762-762
| const HOST = process.env.ECC_DASHBOARD_HOST || '127.0.0.1'; | ||
| const ALLOWED = buildAllowedHostnames(HOST); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "ECC_DASHBOARD_HOST|server\.listen|server\.on\(['\"]error" scripts/dashboard-web.jsRepository: affaan-m/ECC
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' scripts/dashboard-web.js
printf '\n---\n'
sed -n '730,800p' scripts/dashboard-web.jsRepository: affaan-m/ECC
Length of output: 19793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' scripts/lib/loopback-guard.jsRepository: affaan-m/ECC
Length of output: 1816
Validate ECC_DASHBOARD_HOST before using it and add a bind error handler.
buildAllowedHostnames() accepts any string, and server.listen(PORT, HOST, ...) has no error listener. A malformed or unresolvable host will surface as an unhandled startup failure instead of a clear configuration error. Normalize/validate supported loopback host forms before building ALLOWED, and handle listen() errors explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/dashboard-web.js` around lines 24 - 25, Validate and normalize
ECC_DASHBOARD_HOST to supported loopback host forms before passing it to
buildAllowedHostnames or server.listen, rejecting malformed or unresolvable
values with a clear configuration error. Add an explicit error handler to the
server.listen startup flow so bind failures are reported cleanly instead of
becoming unhandled errors.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lib/agent-data-home.js`:
- Line 108: Update readProjectConfigAt to return the canonical path produced by
assertWithinTrustedRoot rather than discarding that helper’s return value.
Preserve the existing expandHomePath and trusted-root validation flow while
ensuring subsequent writes use the canonicalized path.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9546f527-ecdb-41cf-9e4f-635f6b4f3652
📒 Files selected for processing (1)
scripts/lib/agent-data-home.js
📜 Review details
⏰ Context from checks skipped due to timeout. (32)
- GitHub Check: Test (windows-latest, Node 20.x, pnpm)
- GitHub Check: Test (windows-latest, Node 22.x, npm)
- GitHub Check: Test (windows-latest, Node 22.x, pnpm)
- GitHub Check: Test (macos-latest, Node 18.x, pnpm)
- GitHub Check: Test (windows-latest, Node 22.x, yarn)
- GitHub Check: Test (macos-latest, Node 20.x, pnpm)
- GitHub Check: Test (macos-latest, Node 20.x, bun)
- GitHub Check: Test (macos-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 18.x, yarn)
- GitHub Check: Test (ubuntu-latest, Node 20.x, pnpm)
- GitHub Check: Test (macos-latest, Node 22.x, npm)
- GitHub Check: Test (windows-latest, Node 18.x, pnpm)
- GitHub Check: Test (windows-latest, Node 20.x, yarn)
- GitHub Check: Test (windows-latest, Node 18.x, yarn)
- GitHub Check: Test (windows-latest, Node 18.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, yarn)
- GitHub Check: Test (ubuntu-latest, Node 18.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 18.x, pnpm)
- GitHub Check: Test (windows-latest, Node 20.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 18.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, npm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, yarn)
- GitHub Check: Test (ubuntu-latest, Node 22.x, bun)
- GitHub Check: Coverage
- GitHub Check: Python Tests
- GitHub Check: Validate Components
- GitHub Check: Security Scan
- GitHub Check: Lint
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Implement XSS prevention by sanitizing HTML output
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
scripts/lib/agent-data-home.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
**/*.{ts,tsx,js,jsx}: Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation
Use async/await with try-catch for error handling in TypeScript/JavaScript
Use Zod for schema-based input validation in TypeScript/JavaScript
No console.log statements in production code; use proper logging libraries instead
**/*.{ts,tsx,js,jsx}: Auto-format JavaScript/TypeScript files using Prettier after edit
Warn aboutconsole.logstatements in edited files
Check all modified files forconsole.logstatements before session ends
**/*.{ts,tsx,js,jsx}: Use the ApiResponse interface pattern with generic type parameter:interface ApiResponse<T> { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }
Implement custom React hooks following the pattern: export a named function with use prefix, generic type parameters, and proper useEffect cleanup for side effects
**/*.{ts,tsx,js,jsx}: Never hardcode secrets; always use environment variables for sensitive credentials like API keys
Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are metUse Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript
Files:
scripts/lib/agent-data-home.js
{package.json,*.config.js,scripts/**/*.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Package manager detection should support npm, pnpm, yarn, and bun, with configuration via CLAUDE_PACKAGE_MANAGER environment variable or project config.
Files:
scripts/lib/agent-data-home.js
scripts/**/*.js
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure cross-platform support for Windows, macOS, and Linux via Node.js scripts in the scripts/ directory.
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Always create new objects and never mutate in place; return new copies instead
Keep files between 200–400 lines typical, with a maximum of 800 lines
Extract helpers when a file exceeds 200 lines
Handle errors explicitly at every level; never swallow errors silently
Validate all user input before processing; use schema-based validation where available
Never trust external data (API responses, file content, query params); always validate
All user inputs must be validated and sanitized
Error messages must be scrubbed of sensitive internals
Use readable, well-named identifiers in all code
Keep functions under 50 lines
Keep files under 800 lines
Avoid nesting deeper than 4 levels
Implement comprehensive error handling in all code
Do not hardcode values; use constants or environment configuration instead
Do not use in-place mutation; always return new objects or state
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,jsx,tsx,json,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not hardcode secrets, API keys, passwords, or tokens
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use parameterized queries for all database writes (no string interpolation)
Auth/authz must be checked server-side for every sensitive path
Rate limiting must be applied to all public endpoints
Files:
scripts/lib/agent-data-home.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
HTML output must be sanitized where applicable
Files:
scripts/lib/agent-data-home.js
**/*.{js,ts,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Required environment variables must be validated at startup
Files:
scripts/lib/agent-data-home.js
{scripts,bin}/**
⚙️ CodeRabbit configuration file
{scripts,bin}/**: Focus on command injection, unsafe subprocess usage, path traversal, SSRF, secret exposure, and missing tests for new CLI behavior.
Files:
scripts/lib/agent-data-home.js
🔇 Additional comments (1)
scripts/lib/agent-data-home.js (1)
17-17: LGTM!
| if (typeof candidate !== 'string' || !candidate.trim()) return null; | ||
| const projectRoot = resolveProjectRootFromConfigPath(configPath); | ||
| return expandHomePath(candidate, projectRoot); | ||
| const resolved = expandHomePath(candidate, projectRoot); assertWithinTrustedRoot(resolved, getHomeDirFromEnv(), 'agent-data-home write'); return resolved; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== file map ==\n'
git ls-files 'scripts/lib/agent-data-home.js' 'scripts/lib/*.js' | sed -n '1,120p'
printf '\n== outline ==\n'
ast-grep outline scripts/lib/agent-data-home.js --view expanded || true
printf '\n== relevant source ==\n'
nl -ba scripts/lib/agent-data-home.js | sed -n '1,220p'
printf '\n== search for helper definitions/usages ==\n'
rg -n "assertWithinTrustedRoot|expandHomePath|getHomeDirFromEnv|agent-data-home write" scripts/lib scripts -g '*.js' -A 3 -B 3Repository: affaan-m/ECC
Length of output: 4911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('scripts/lib/agent-data-home.js')
print(p.exists(), p.stat().st_size if p.exists() else None)
PYRepository: affaan-m/ECC
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== agent-data-home.js (selected ranges) ==\n'
sed -n '1,240p' scripts/lib/agent-data-home.js
printf '\n== path-safety.js (selected ranges) ==\n'
sed -n '1,240p' scripts/lib/path-safety.js
printf '\n== usages of resolveAgentDataHome / ensureAgentDataHomeEnv / assertWithinTrustedRoot ==\n'
rg -n "resolveAgentDataHome|ensureAgentDataHomeEnv|assertWithinTrustedRoot" scripts/lib scripts -g '*.js' -A 2 -B 2Repository: affaan-m/ECC
Length of output: 22859
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [Path('scripts/lib/agent-data-home.js'), Path('scripts/lib/path-safety.js')]:
print(f'## {p}')
print('\n'.join(f'{i+1:4d}: {line}' for i, line in enumerate(p.read_text().splitlines())))
print()
PYRepository: affaan-m/ECC
Length of output: 11033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for file in ['scripts/lib/agent-data-home.js', 'scripts/lib/path-safety.js']:
p = Path(file)
print(f'===== {file} =====')
for i, line in enumerate(p.read_text().splitlines(), start=1):
if i <= 240:
print(f'{i:4d}: {line}')
print()
PY
printf '===== usages =====\n'
rg -n "resolveAgentDataHome|ensureAgentDataHomeEnv|assertWithinTrustedRoot" scripts/lib scripts -g '*.js' -A 3 -B 3Repository: affaan-m/ECC
Length of output: 29032
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(file, start=None, end=None):
p = Path(file)
print(f'===== {file} =====')
lines = p.read_text().splitlines()
for i, line in enumerate(lines, start=1):
if start is not None and i < start:
continue
if end is not None and i > end:
break
print(f'{i:4d}: {line}')
print()
show('scripts/lib/agent-data-home.js', 1, 240)
show('scripts/lib/path-safety.js', 1, 220)
PY
printf '===== symbol usages =====\n'
rg -n "assertWithinTrustedRoot|resolveAgentDataHome|ensureAgentDataHomeEnv" scripts/lib -g '*.js' -A 4 -B 4Repository: affaan-m/ECC
Length of output: 20720
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('scripts/lib/path-safety.js')
print('exists:', p.exists(), 'size:', p.stat().st_size if p.exists() else None)
if p.exists():
text = p.read_text().splitlines()
for i, line in enumerate(text, start=1):
if 'assertWithinTrustedRoot' in line or 'realpath' in line or 'resolve(' in line:
print(f'{i:4d}: {line}')
PY
printf '\n===== all call sites =====\n'
rg -n "assertWithinTrustedRoot\(" scripts/lib -g '*.js' -A 2 -B 2Repository: affaan-m/ECC
Length of output: 2957
🌐 Web query:
"affaan-m/ECC scripts/lib/agent-data-home.js assertWithinTrustedRoot"
💡 Result:
The function assertWithinTrustedRoot is a security-related validation utility found within the affaan-m/ECC (Everything Claude Code) repository [1][2]. It is primarily implemented in or used in conjunction with scripts located in scripts/lib/, such as agent-data-home.js, to ensure that file system operations or path resolutions remain within designated "trusted" boundaries [3][4]. In the context of the ECC agent harness, these trust boundaries are critical for preventing unauthorized file access or directory escapes [5][6]. The implementation generally performs the following: 1. Path Canonicalization: It resolves the given path (often using realpath techniques) to its absolute, canonical form, ensuring all symlinks are expanded [7]. 2. Boundary Validation: It compares the resolved path against a defined trusted root directory [7]. 3. Security Enforcement: If the path is determined to be outside the trusted root, the function typically triggers an error or a validation failure, adhering to a "fail-closed" security posture [5][7]. This utility is part of a broader set of security controls within the ECC ecosystem—which includes input validation, origin allowlisting, and trust-check adapters—designed to protect the agent harness from malicious or accidental directory traversal attacks during operations like agent data management or tool execution [5][8][9][3].
Citations:
- 1: https://github.com/affaan-m/ECC/blob/HEAD/README.md
- 2: http://github.com/affaan-m/everything-claude-code
- 3: feat: Cursor-independent ECC memory via ECC_AGENT_DATA_HOME #2066
- 4: feat: Cursor-independent ECC memory via ECC_AGENT_DATA_HOME #2066
- 5: https://github.com/affaan-m/ECC/blob/ebf0f135bba6be99fb173f9f494efe29a589515e/the-security-guide.md
- 6: https://github.com/affaan-m/ECC/blob/main/SECURITY.md
- 7: https://github.com/vercel-labs/just-bash/blob/305c833a/src/fs/real-fs-utils.ts
- 8: fix(security): add host/origin allowlist + validate git refs + quote workflow input #2185
- 9: 1d72dfb
🌐 Web query:
"\"assertWithinTrustedRoot\" \"agent-data-home\""
💡 Result:
The terms "assertWithinTrustedRoot" and "agent-data-home" are associated with filesystem security and path validation in software environments, particularly within agent-based or data-handling toolchains [1][2]. assertWithinTrustedRoot This appears to be a specific security function or pattern used to prevent "root-scatter" or path traversal attacks [1][2]. - It functions as a validation mechanism that ensures a given target path is strictly contained within a designated "trusted" root directory [2]. - In implementations (such as the qmdc crate or general Rust path-handling utilities), it typically canonicalizes both the root and the target path before comparison to ensure accuracy [2]. - If the target path is found to escape the defined root, the operation fails (often returning an error such as OutOfRoot), acting as a "fail-closed" security invariant [2]. agent-data-home This term refers to a directory path used for local persistent storage and configuration within agent frameworks [3][4][5]. - It is often configurable via environment variables (e.g., to support multi-harness isolation, where different agent sessions or instances require separate data spaces) [3][6][4]. - While "agent data home" can refer to generic directory configurations in various tools (such as Elastic Agent [5]), in the context of recent agent-development tooling (like Everything Claude Code or related CLI tools), it is used to manage persistent hooks, session metrics, learned skills, and other stateful data across multiple environment configurations [3][6][4]. Relationship The two concepts are often used together in secure agent architectures: "agent-data-home" defines the base directory for an agent's data, and "assertWithinTrustedRoot" (or similar path-containment logic) is applied to ensure that the agent does not perform operations or write files outside of its assigned sandbox or data home, thereby preventing unauthorized access to the host filesystem [1][2].
Citations:
- 1: https://docs.rs/trusty-common/latest/src/trusty_common/data_dir.rs.html
- 2: https://docs.rs/qmdc/latest/qmdc/core/index_seam/fn.assert_within_root.html
- 3: https://github.com/affaan-m/ecc
- 4: https://www.datacamp.com/tutorial/everything-claude-code
- 5: https://discuss.elastic.co/t/errors-with-filebeat-when-trying-to-integrate-any-windows-integration-logs-with-the-agent/380768
- 6: https://github.com/severinzhong/agent-data-cli/releases/tag/v0.2.0
Return the canonicalized path from assertWithinTrustedRoot in readProjectConfigAt.
resolved is validated, but the helper’s realpath is dropped, so later writes can still follow the original symlinked path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/lib/agent-data-home.js` at line 108, Update readProjectConfigAt to
return the canonical path produced by assertWithinTrustedRoot rather than
discarding that helper’s return value. Preserve the existing expandHomePath and
trusted-root validation flow while ensuring subsequent writes use the
canonicalized path.
|
Local review from Codex automation: I would request changes before merging this PR. |
daltino
left a comment
daltino
left a comment
There was a problem hiding this comment.
This PR strengthens security for the dashboard by restricting the bind to loopback-only (127.0.0.1) and adding Host/Origin validation mechanisms. It introduces helper functions to handle hostname safety and origin checks, aligning with secure server practices. Changes are concise, follow existing structure, and appropriately enhance safety.
Closes gap 1 of #2506.
Problem
scripts/dashboard-web.jswas binding to all interfaces (0.0.0.0 / :: / OS default) with no Host or Origin validation, making the dashboard reachable from any network interface and vulnerable to DNS-rebinding attacks from local malicious pages.What Changed
1. Loopback-only bind by default
Users who need LAN/container access can set
ECC_DASHBOARD_HOST=0.0.0.0; the default posture is now localhost-only.2. Host header guard (DNS-rebinding protection)
Blocks requests where the Host header doesn't match the known loopback hostnames, consistent with the pattern already used in
plan-canvas/server.js.3. Origin guard (cross-origin protection)
Rejects cross-origin requests from disallowed origins.
Gap 2
agent-data-home.jspath-containment hardening (the second gap in #2506) will follow in a separate commit on this branch.Testing
node scripts/dashboard-web.jsstill starts and serves onhttp://localhost:3456Host: attacker.com) now receive 421ECC_DASHBOARD_HOST=0.0.0.0 node scripts/dashboard-web.jsstill works for users who need it