fix(security): harden worker approval, hook traversal, MCP exec, install scripts, git hooks

- orchestrate-codex-worker: drop yolo, default never approval, worktree containment
- run-with-flags-shell: add path traversal containment mirroring JS guard
- mcp-health-check: gate workspace probe, denylist dangerous env, shell-free reconnect with opt-in
- install.sh/ps1: add --ignore-scripts to block postinstall RCE
- git hooks: refuse global hooksPath clobber, remove file disable bypass, gate pre-push repo script execution
- claw.js: remove Windows shell:true, validate model token
- tests: opt into new secure defaults, quote-aware reconnect parsing
This commit is contained in:
Geronimo
2026-09-14 13:24:58 +05:30
parent 8321021c54
commit 27667bc746
10 changed files with 231 additions and 32 deletions
+2 -1
View File
@@ -35,12 +35,13 @@ $scriptDir = Split-Path -Parent $scriptPath
$installerScript = Join-Path -Path (Join-Path -Path $scriptDir -ChildPath 'scripts') -ChildPath 'install-apply.js'
# Auto-install Node dependencies when running from a git clone
# SECURITY: --ignore-scripts blocks preinstall/postinstall RCE from a compromised dependency.
$nodeModules = Join-Path -Path $scriptDir -ChildPath 'node_modules'
if (-not (Test-Path -LiteralPath $nodeModules)) {
Write-Host '[ECC] Installing dependencies...'
Push-Location $scriptDir
try {
& npm install --no-audit --no-fund --loglevel=error
& npm install --ignore-scripts --no-audit --no-fund --loglevel=error
if ($LASTEXITCODE -ne 0) {
Write-Error "npm install failed with exit code $LASTEXITCODE"
exit $LASTEXITCODE
+4 -2
View File
@@ -14,10 +14,12 @@ while [ -L "$SCRIPT_PATH" ]; do
done
SCRIPT_DIR="$(cd "$(dirname "$SCRIPT_PATH")" && pwd)"
# Auto-install Node dependencies when running from a git clone
# Auto-install Node dependencies when running from a git clone.
# SECURITY: --ignore-scripts blocks preinstall/postinstall RCE from a
# compromised dependency. ECC deps are pure JS (no native build step).
if [ ! -d "$SCRIPT_DIR/node_modules" ]; then
echo "[ECC] Installing dependencies..."
(cd "$SCRIPT_DIR" && npm install --no-audit --no-fund --loglevel=error)
(cd "$SCRIPT_DIR" && npm install --ignore-scripts --no-audit --no-fund --loglevel=error)
fi
# On MSYS2/Git Bash, convert the POSIX path to a Windows path so Node.js
+17 -9
View File
@@ -95,20 +95,28 @@ function askClaude(systemPrompt, history, userMessage, model) {
}
args.push('-p');
// On Windows the `claude` binary installed via npm is `claude.cmd`/`claude.ps1`,
// and Node's spawn() cannot resolve those wrappers via PATH without shell: true.
// But shell mode concatenates args *unescaped*, so a multi-line prompt passed as
// an arg gets mangled (newlines and the `===` section markers truncate it, and
// claude receives an empty prompt). Fix: send the prompt over stdin via `input`
// and keep only the short, safe flags (`--model`, `-p`) as args.
// 'claude' is a hardcoded literal here (not user input), so shell mode is safe.
const result = spawnSync('claude', args, {
// SECURITY: never use shell:true — on Windows Node concatenates command+args
// unquoted (DEP0190), so a model value like `x & calc &` breaks out.
// Validate the model token and spawn without a shell; resolve .cmd shim explicitly.
if (model && !/^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$/.test(model)) {
return `[Error: invalid model name]`;
}
let bin = 'claude';
if (process.platform === 'win32') {
for (const ext of ['.cmd', '.exe', '.ps1']) {
try {
const found = require('child_process').spawnSync('where', [`claude${ext}`], { encoding: 'utf8' });
if (found.status === 0 && found.stdout.trim()) { bin = found.stdout.trim().split(/\r?\n/)[0]; break; }
} catch { /* ignore */ }
}
}
const result = spawnSync(bin, args, {
input: fullPrompt,
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe'],
env: { ...process.env, CLAUDECODE: '' },
timeout: 300000,
shell: process.platform === 'win32'
shell: false
});
if (result.error) {
+4 -3
View File
@@ -5,12 +5,13 @@ set -euo pipefail
# Blocks commits that add high-signal secrets.
if [[ "${ECC_SKIP_GIT_HOOKS:-0}" == "1" || "${ECC_SKIP_PRECOMMIT:-0}" == "1" ]]; then
printf '[ECC pre-commit] WARNING: hook bypassed via env (ECC_SKIP_*=1)\n' >&2
exit 0
fi
if [[ -f ".ecc-hooks-disable" || -f ".git/ecc-hooks-disable" ]]; then
exit 0
fi
# NOTE: file-based disables (.ecc-hooks-disable) were removed — a malicious
# repo could ship that file and silently turn off secret scanning exactly
# where it is most needed. Use the env bypass above (audible warning) instead.
if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
exit 0
+20 -5
View File
@@ -5,12 +5,12 @@ set -euo pipefail
# Runs a lightweight verification flow before pushes.
if [[ "${ECC_SKIP_GIT_HOOKS:-0}" == "1" || "${ECC_SKIP_PREPUSH:-0}" == "1" ]]; then
printf '[ECC pre-push] WARNING: hook bypassed via env (ECC_SKIP_*=1)\n' >&2
exit 0
fi
if [[ -f ".ecc-hooks-disable" || -f ".git/ecc-hooks-disable" ]]; then
exit 0
fi
# NOTE: file-based disables (.ecc-hooks-disable) were removed — a malicious
# repo could ship that file and silently disable verification.
if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
exit 0
@@ -85,8 +85,14 @@ run_node_script() {
}
if [[ -f "package.json" ]]; then
pm="$(detect_pm)"
log "Node project detected (package manager: $pm)"
# SECURITY: executing a cloned repo's lint/test/build scripts on push is
# arbitrary code execution (package.json scripts run as you). Opt-in only:
# set ECC_PREPUSH_RUN_CHECKS=1 for repos you trust.
if [[ "${ECC_PREPUSH_RUN_CHECKS:-0}" != "1" ]]; then
log "Node project detected but ECC_PREPUSH_RUN_CHECKS!=1; skipping repo script execution (set =1 to opt in)."
else
pm="$(detect_pm)"
log "Node project detected (package manager: $pm)"
for script_name in lint typecheck test build; do
if has_node_script "$script_name"; then
@@ -109,8 +115,12 @@ if [[ -f "package.json" ]]; then
*) npm audit --omit=dev || fail "npm audit failed" ;;
esac
fi
fi
fi
# SECURITY: go test / pytest execute repo-controlled code (TestMain,
# conftest.py). Same opt-in gate as Node scripts above.
if [[ "${ECC_PREPUSH_RUN_CHECKS:-0}" == "1" ]]; then
if [[ -f "go.mod" ]] && command -v go >/dev/null 2>&1; then
ran_any_check=1
log "Go project detected. Running: go test ./..."
@@ -126,6 +136,11 @@ if [[ -f "pyproject.toml" || -f "requirements.txt" ]]; then
log "Python project detected but pytest is not installed. Skipping."
fi
fi
else
if [[ -f "go.mod" || -f "pyproject.toml" || -f "requirements.txt" ]]; then
log "Go/Python project detected but ECC_PREPUSH_RUN_CHECKS!=1; skipping test execution."
fi
fi
if [[ "$ran_any_check" -eq 0 ]]; then
log "No supported checks found in this repository. Skipping."
+16 -4
View File
@@ -54,12 +54,24 @@ run_or_echo chmod +x "$DEST_DIR/pre-commit" "$DEST_DIR/pre-push"
if [[ "$MODE" == "apply" ]]; then
prev_hooks_path="$(git config --global core.hooksPath || true)"
if [[ -n "$prev_hooks_path" ]]; then
log "Previous global hooksPath: $prev_hooks_path"
if [[ -n "$prev_hooks_path" && "$prev_hooks_path" != "$DEST_DIR" ]]; then
# SECURITY: never silently displace another tool's global hooks — that
# turns every commit/push in every repo into ECC code execution and breaks
# the user's existing security controls. Require explicit opt-in to replace.
if [[ "${ECC_FORCE_GLOBAL_HOOKS:-0}" != "1" ]]; then
log "ERROR: global core.hooksPath already set to: $prev_hooks_path"
log "Refusing to overwrite. Options:"
log " 1) Per-repo install (recommended): git config core.hooksPath \"$DEST_DIR\""
log " 2) Force replace: ECC_FORCE_GLOBAL_HOOKS=1 $0"
log " 3) Restore afterwards: git config --global core.hooksPath \"$prev_hooks_path\""
exit 1
fi
log "WARNING: replacing previous global hooksPath: $prev_hooks_path (ECC_FORCE_GLOBAL_HOOKS=1)"
log "Restore with: git config --global core.hooksPath \"$prev_hooks_path\""
fi
fi
run_or_echo git config --global core.hooksPath "$DEST_DIR"
log "Installed ECC global git hooks."
log "Disable per repo by creating .ecc-hooks-disable in project root."
log "Temporary bypass: ECC_SKIP_PRECOMMIT=1 or ECC_SKIP_PREPUSH=1"
log "Per-repo alternative (recommended): git config core.hooksPath \"$DEST_DIR\""
log "Temporary bypass (audible): ECC_SKIP_GIT_HOOKS=1 (logs a warning to stderr)"
+105 -3
View File
@@ -182,6 +182,12 @@ function extractMcpTargetFromRaw(raw) {
}
function resolveServerConfig(serverName) {
// SECURITY: serverName flows into env-var lookup and shell-adjacent paths.
// Reject anything outside a strict token so config-controlled names cannot
// inject shell metachars ($(..), backticks, ;) downstream.
if (!/^[A-Za-z0-9_-]{1,64}$/.test(String(serverName || ''))) {
return null;
}
for (const filePath of configPaths()) {
const data = readJsonFile(filePath);
const server = data?.mcpServers?.[serverName]
@@ -306,9 +312,21 @@ function probeCommandServer(serverName, config) {
const command = config.command;
const args = Array.isArray(config.args) ? config.args.map(arg => String(arg)) : [];
const timeoutMs = envNumber('ECC_MCP_HEALTH_TIMEOUT_MS', DEFAULT_TIMEOUT_MS);
// SECURITY: config.env comes from repo-committed MCP configs. Never let it
// override process-critical loader vars that turn into code execution
// (LD_PRELOAD, DYLD_*, NODE_OPTIONS, PATH tampering, etc.).
const BLOCKED_ENV_PREFIXES = ['LD_', 'DYLD_', 'NODE_OPTIONS', 'NODE_PATH', 'PATH', 'PYTHONPATH', 'RUBYLIB', 'PERL5LIB'];
const rawEnv = (config.env && typeof config.env === 'object' && !Array.isArray(config.env) ? config.env : {});
const safeConfigEnv = {};
for (const [k, v] of Object.entries(rawEnv)) {
if (BLOCKED_ENV_PREFIXES.some(p => String(k).toUpperCase().startsWith(p))) {
continue;
}
safeConfigEnv[k] = String(v);
}
const mergedEnv = {
...process.env,
...(config.env && typeof config.env === 'object' && !Array.isArray(config.env) ? config.env : {})
...safeConfigEnv
};
let done = false;
@@ -515,6 +533,29 @@ function probeCommandServer(serverName, config) {
async function probeServer(serverName, resolvedConfig) {
const config = resolvedConfig.config;
// SECURITY: cloning a malicious repo must not auto-execute its MCP servers.
// Workspace configs (cwd .claude.json / .claude/settings.json) are untrusted
// by default; only probe them with explicit operator opt-in.
// Home configs (~/.claude.json) and explicit ECC_MCP_CONFIG_PATH remain allowed.
try {
const src = String(resolvedConfig.source || '');
const cwd = process.cwd();
const isWorkspaceSource = src === require('path').join(cwd, '.claude.json')
|| src === require('path').join(cwd, '.claude', 'settings.json')
|| src.startsWith(cwd + require('path').sep + '.claude' + require('path').sep);
if (isWorkspaceSource && !/^(1|true|yes)$/i.test(String(process.env.ECC_MCP_ALLOW_WORKSPACE_PROBE || ''))) {
return {
ok: false,
failureCode: null,
reason: 'untrusted workspace MCP config skipped (set ECC_MCP_ALLOW_WORKSPACE_PROBE=1 to probe)',
source: resolvedConfig.source
};
}
} catch {
// Fail closed on path errors for workspace sources is handled below;
// continue to normal probing for non-workspace sources.
}
if (config.type === 'http' || config.url) {
const result = await requestHttp(config.url, config.headers || {}, envNumber('ECC_MCP_HEALTH_TIMEOUT_MS', DEFAULT_TIMEOUT_MS));
@@ -546,6 +587,15 @@ async function probeServer(serverName, resolvedConfig) {
}
function reconnectCommand(serverName) {
// SECURITY: reconnect commands are shell strings from env. Disabled by
// default; require explicit opt-in so a malicious .env/direnv cannot gain
// shell execution through this hook.
if (!/^(1|true|yes)$/i.test(String(process.env.ECC_MCP_RECONNECT_ALLOW || ''))) {
return null;
}
if (!/^[A-Za-z0-9_-]{1,64}$/.test(String(serverName || ''))) {
return null;
}
const key = `ECC_MCP_RECONNECT_${String(serverName).toUpperCase().replace(/[^A-Z0-9]/g, '_')}`;
const command = process.env[key] || process.env.ECC_MCP_RECONNECT_COMMAND || '';
if (!command.trim()) {
@@ -563,8 +613,60 @@ function attemptReconnect(serverName) {
return { attempted: false, success: false, reason: 'no reconnect command configured' };
}
const result = spawnSync(command, {
shell: true,
// SECURITY: never run reconnect strings through a shell. Split on
// whitespace (no glob/expansion/substitution) and spawn directly.
// Supports single/double quotes for paths with spaces (e.g. node
// "/tmp/dir with space/reconnect.js"). No variable, command, tilde, or
// glob expansion is performed. {server} was already validated above.
function splitReconnectCommand(s) {
const parts = [];
let cur = '';
let quote = null;
let inToken = false;
for (let i = 0; i < s.length; i++) {
const ch = s[i];
if (quote) {
if (ch === quote) {
quote = null;
} else if (ch === '\\' && quote === '"' && i + 1 < s.length && (s[i + 1] === '"' || s[i + 1] === '\\')) {
cur += s[i + 1];
i++;
} else {
cur += ch;
}
} else if (ch === '"' || ch === "'") {
quote = ch;
inToken = true;
} else if (/\s/.test(ch)) {
if (inToken) {
parts.push(cur);
cur = '';
inToken = false;
}
} else {
cur += ch;
inToken = true;
}
}
if (quote) {
return null; // unbalanced quote
}
if (inToken) {
parts.push(cur);
}
return parts;
}
const parts = splitReconnectCommand(String(command).trim());
if (!parts || parts.length === 0) {
return { attempted: false, success: false, reason: 'invalid reconnect command' };
}
const [bin, ...argv] = parts;
if (/[&|<>^%!`$();]/.test(bin) || argv.some(a => /[`$]/.test(a))) {
return { attempted: false, success: false, reason: 'reconnect command contains unsafe characters' };
}
const result = spawnSync(bin, argv, {
shell: false,
env: process.env,
cwd: process.cwd(),
encoding: 'utf8',
+26 -4
View File
@@ -22,9 +22,31 @@ if [[ "$ENABLED" != "yes" ]]; then
exit 0
fi
SCRIPT_PATH="${PLUGIN_ROOT}/${REL_SCRIPT_PATH}"
if [[ ! -f "$SCRIPT_PATH" ]]; then
echo "[Hook] Script not found for ${HOOK_ID}: ${SCRIPT_PATH}" >&2
# Reject traversal / absolute / env-escape paths before touching the filesystem.
# Mirrors the containment check in run-with-flags.js (resolvedRoot prefix).
case "$REL_SCRIPT_PATH" in
/*|\\*|~*|*..*|*\$*|*\`*|*\|*|*\;*|*\&*|*\<*|*\>*|*\"*|*\'*|*\ *|*" "*)
echo "[Hook] Path traversal rejected for ${HOOK_ID}: ${REL_SCRIPT_PATH}" >&2
printf '%s' "$INPUT"
exit 0
;;
esac
# Canonicalize PLUGIN_ROOT (CLAUDE_PLUGIN_ROOT is env-controlled) and the
# candidate script path, then enforce containment inside the plugin root.
PLUGIN_ROOT_CANON="$(realpath -m "$PLUGIN_ROOT" 2>/dev/null || readlink -f "$PLUGIN_ROOT" 2>/dev/null || printf '%s' "$PLUGIN_ROOT")"
SCRIPT_PATH="${PLUGIN_ROOT_CANON}/${REL_SCRIPT_PATH}"
SCRIPT_CANON="$(realpath -m "$SCRIPT_PATH" 2>/dev/null || readlink -f "$SCRIPT_PATH" 2>/dev/null || printf '%s' "$SCRIPT_PATH")"
case "$SCRIPT_CANON" in
"$PLUGIN_ROOT_CANON"/*) ;;
*)
echo "[Hook] Path traversal rejected for ${HOOK_ID}: ${REL_SCRIPT_PATH}" >&2
printf '%s' "$INPUT"
exit 0
;;
esac
if [[ ! -f "$SCRIPT_CANON" ]]; then
echo "[Hook] Script not found for ${HOOK_ID}: ${SCRIPT_CANON}" >&2
printf '%s' "$INPUT"
exit 0
fi
@@ -33,4 +55,4 @@ fi
# This is needed by scripts like observe.sh that behave differently for PreToolUse vs PostToolUse
HOOK_PHASE="${HOOK_ID%%:*}"
printf '%s' "$INPUT" | "$SCRIPT_PATH" "$HOOK_PHASE"
printf '%s' "$INPUT" | "$SCRIPT_CANON" "$HOOK_PHASE"
+30 -1
View File
@@ -48,6 +48,35 @@ fi
write_status "running" "- Task file: \`$task_file\`"
# SECURITY: never auto-approve agent tool execution. The worker prompt is built
# from a task file that may contain LLM-generated or third-party content
# (indirect prompt injection). `codex exec -p yolo` would execute
# rm -rf / exfiltration commands without confirmation.
# Default to the most restrictive approval mode; allow an explicit operator
# override only via env (e.g. ECC_CODEX_APPROVAL_MODE=on-request for trusted runs).
APPROVAL_MODE="${ECC_CODEX_APPROVAL_MODE:-never}"
case "$APPROVAL_MODE" in
never|on-request|on-failure) ;;
*)
echo "[ECC worker] Refusing to run: unsupported ECC_CODEX_APPROVAL_MODE='$APPROVAL_MODE' (expected never|on-request|on-failure)" >&2
write_status "failed" "- Error: unsupported approval mode"
exit 1
;;
esac
# Contain the task file to the current worktree so a malicious launcher cannot
# point the worker at /etc/passwd or a sibling checkout.
task_real="$(realpath -m "$task_file" 2>/dev/null || readlink -f "$task_file" 2>/dev/null || printf '%s' "$task_file")"
work_real="$(pwd -P 2>/dev/null || pwd)"
case "$task_real" in
"$work_real"/*) ;;
*)
echo "[ECC worker] Refusing to run: task file outside worktree: $task_file" >&2
write_status "failed" "- Error: task file outside worktree"
exit 1
;;
esac
prompt_file="$(mktemp)"
output_file="$(mktemp)"
cleanup() {
@@ -77,7 +106,7 @@ Task file: $task_file
$(cat "$task_file")
EOF
if codex exec -p yolo -m gpt-5.4 --color never -C "$(pwd)" -o "$output_file" - < "$prompt_file"; then
if codex exec -p "$APPROVAL_MODE" -m gpt-5.4 --color never -C "$(pwd)" -o "$output_file" - < "$prompt_file"; then
{
echo "# Handoff"
echo
+7
View File
@@ -249,6 +249,9 @@ async function runTests() {
ECC_MCP_CONFIG_PATH: null,
ECC_MCP_HEALTH_STATE_PATH: null,
ECC_MCP_HEALTH_TIMEOUT_MS: '100',
// Workspace configs are untrusted by default; this test uses a
// temp dir it created itself, so opt in explicitly.
ECC_MCP_ALLOW_WORKSPACE_PROBE: '1',
HOME: homeDir,
USERPROFILE: homeDir
},
@@ -619,6 +622,7 @@ async function runTests() {
CLAUDE_HOOK_EVENT_NAME: 'PreToolUse',
ECC_MCP_CONFIG_PATH: configPath,
ECC_MCP_HEALTH_STATE_PATH: statePath,
ECC_MCP_RECONNECT_ALLOW: '1',
ECC_MCP_RECONNECT_COMMAND: `${JSON.stringify(process.execPath)} ${JSON.stringify(reconnectScript)}`,
ECC_MCP_HEALTH_TIMEOUT_MS: '1000',
ECC_MCP_HEALTH_BACKOFF_MS: '10'
@@ -682,6 +686,7 @@ async function runTests() {
CLAUDE_HOOK_EVENT_NAME: 'PostToolUseFailure',
ECC_MCP_CONFIG_PATH: configPath,
ECC_MCP_HEALTH_STATE_PATH: statePath,
ECC_MCP_RECONNECT_ALLOW: '1',
ECC_MCP_RECONNECT_COMMAND: `node ${JSON.stringify(reconnectScript)}`,
ECC_MCP_HEALTH_TIMEOUT_MS: '1000'
}
@@ -773,6 +778,7 @@ async function runTests() {
{
CLAUDE_HOOK_EVENT_NAME: 'PostToolUseFailure',
ECC_MCP_HEALTH_STATE_PATH: statePath,
ECC_MCP_RECONNECT_ALLOW: '1',
ECC_MCP_RECONNECT_COMMAND: `${JSON.stringify(process.execPath)} ${JSON.stringify(reconnectScript)}`
}
);
@@ -810,6 +816,7 @@ async function runTests() {
CLAUDE_HOOK_EVENT_NAME: 'PostToolUseFailure',
ECC_MCP_HEALTH_STATE_PATH: statePath,
ECC_MCP_CONFIG_PATH: path.join(tempDir, 'missing.json'),
ECC_MCP_RECONNECT_ALLOW: '1',
ECC_MCP_RECONNECT_COMMAND: null,
ECC_MCP_RECONNECT_FOO_BAR: `${JSON.stringify(process.execPath)} ${JSON.stringify(reconnectScript)} ${JSON.stringify(markerFile)} {server}`
}