mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-21 09:08:01 +02:00
Merge pull request #3126 from VarunGore36/fix/reviewer-followups
fix(security): security follow-ups for worker, installer, claw, hooks
This commit is contained in:
Affaan Mustafa
+43
-10
@@ -95,21 +95,54 @@ 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: a model value like `x & calc &` breaks out when Node
|
||||
// concatenates command+args unquoted under cmd.exe (DEP0190), so the model
|
||||
// token is validated and only fixed flags reach the command line.
|
||||
if (model && !/^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$/.test(model)) {
|
||||
return `[Error: invalid model name]`;
|
||||
}
|
||||
// On Windows the `claude` binary is usually a .cmd shim, which Node
|
||||
// >=18.20/20.12 refuses to spawn directly (CVE-2024-27980 mitigation), and
|
||||
// .ps1 shims are not directly executable at all. Resolve a natively
|
||||
// executable target first; only .cmd/.bat go through cmd.exe, using the
|
||||
// same quoted-command-line pattern as scripts/hooks/mcp-health-check.js so
|
||||
// space-containing paths survive as single tokens. .ps1 is never executed
|
||||
// directly — fall through to bare `claude` (pre-change behavior) instead.
|
||||
// cmd.exe expands %NAME% even inside double-quoted strings, so reject
|
||||
// percent-delimited executable paths rather than route them through the shell.
|
||||
function quoteWinToken(token) {
|
||||
if (/%/.test(token)) return null;
|
||||
return /[\s"&|<>^();]/.test(token) ? '"' + token.replace(/"/g, '""') + '"' : token;
|
||||
}
|
||||
let bin = 'claude';
|
||||
let useShell = false;
|
||||
if (process.platform === 'win32') {
|
||||
const { spawnSync: spawnWhere } = require('child_process');
|
||||
for (const ext of ['.exe', '.cmd', '.bat']) {
|
||||
let found = null;
|
||||
try {
|
||||
found = spawnWhere('where', [`claude${ext}`], { encoding: 'utf8' });
|
||||
} catch { /* ignore */ }
|
||||
if (found && found.status === 0 && found.stdout && found.stdout.trim()) {
|
||||
bin = found.stdout.trim().split(/\r?\n/)[0];
|
||||
useShell = /\.(cmd|bat)$/i.test(bin);
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (useShell && quoteWinToken(bin) === null) {
|
||||
useShell = false;
|
||||
}
|
||||
}
|
||||
const spawnOpts = {
|
||||
input: fullPrompt,
|
||||
encoding: 'utf8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: { ...process.env, CLAUDECODE: '' },
|
||||
timeout: 300000,
|
||||
shell: process.platform === 'win32'
|
||||
});
|
||||
};
|
||||
const result = useShell
|
||||
? spawnSync([bin, ...args].map(quoteWinToken).join(' '), { ...spawnOpts, shell: true })
|
||||
: spawnSync(bin, args, { ...spawnOpts, shell: false });
|
||||
|
||||
if (result.error) {
|
||||
return `[Error: ${result.error.message}]`;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
printf '[ECC pre-push] Node project detected but ECC_PREPUSH_RUN_CHECKS!=1; skipping repo script execution (set =1 to opt in).\n' >&2
|
||||
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
|
||||
@@ -98,7 +104,9 @@ if [[ -f "package.json" ]]; then
|
||||
fi
|
||||
done
|
||||
|
||||
fi
|
||||
if [[ "${ECC_PREPUSH_AUDIT:-0}" == "1" ]]; then
|
||||
pm="${pm:-$(detect_pm)}"
|
||||
ran_any_check=1
|
||||
log "Running dependency audit (ECC_PREPUSH_AUDIT=1)"
|
||||
case "$pm" in
|
||||
@@ -111,6 +119,9 @@ if [[ -f "package.json" ]]; then
|
||||
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 ./..."
|
||||
@@ -281,6 +292,11 @@ if [[ -f "pyproject.toml" || -f "requirements.txt" ]]; then
|
||||
log " venv, env, uv, poetry, PATH). Set ECC_PYTEST_CMD to point at it."
|
||||
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
|
||||
|
||||
@@ -41,6 +41,23 @@ log "Mode: $MODE"
|
||||
log "Source hooks: $SOURCE_DIR"
|
||||
log "Global hooks destination: $DEST_DIR"
|
||||
|
||||
prev_hooks_path="$(git config --global core.hooksPath || true)"
|
||||
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
|
||||
|
||||
if [[ -d "$DEST_DIR" ]]; then
|
||||
log "Backing up existing hooks directory to $BACKUP_DIR"
|
||||
run_or_echo mkdir -p "$BACKUP_DIR"
|
||||
@@ -51,15 +68,8 @@ run_or_echo mkdir -p "$DEST_DIR"
|
||||
run_or_echo cp "$SOURCE_DIR/pre-commit" "$DEST_DIR/pre-commit"
|
||||
run_or_echo cp "$SOURCE_DIR/pre-push" "$DEST_DIR/pre-push"
|
||||
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"
|
||||
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)"
|
||||
|
||||
@@ -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,38 @@ 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 home = require('os').homedir();
|
||||
const pathMod = require('path');
|
||||
// A config file in the user's home directory (~/.claude.json or
|
||||
// ~/.claude/settings.json) is always trusted regardless of cwd.
|
||||
const isHomeSource = src === pathMod.join(home, '.claude.json')
|
||||
|| src === pathMod.join(home, '.claude', 'settings.json')
|
||||
|| src.startsWith(pathMod.join(home, '.claude') + pathMod.sep);
|
||||
if (!isHomeSource) {
|
||||
const isWorkspaceSource = src === pathMod.join(cwd, '.claude.json')
|
||||
|| src === pathMod.join(cwd, '.claude', 'settings.json')
|
||||
|| src.startsWith(cwd + pathMod.sep + '.claude' + pathMod.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 +596,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 +622,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',
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -227,7 +227,18 @@ async function main() {
|
||||
// which would interfere with the parent process or cause double execution.
|
||||
let hookModule;
|
||||
const src = fs.readFileSync(scriptPath, 'utf8');
|
||||
const hasRunExport = /\bmodule\.exports\b/.test(src) && /\brun\b/.test(src);
|
||||
// Gate require() on concrete export syntax, not a bare word match: the old
|
||||
// /\bmodule\.exports\b/ && /\brun\b/ test fired on comments, strings, and
|
||||
// unrelated properties, causing require() — and its module-scope side
|
||||
// effects — to run for hooks that export no run(). Still lexical (no parser
|
||||
// dependency), but requires an actual export assignment form.
|
||||
const RUN_EXPORT_PATTERNS = [
|
||||
/module\.exports\s*\.\s*run\s*=/,
|
||||
/exports\s*\.\s*run\s*=/,
|
||||
/module\.exports\s*=\s*\{[^}]*\brun\b/,
|
||||
/module\.exports\s*=\s*(async\s+)?function\s+run\b/,
|
||||
];
|
||||
const hasRunExport = RUN_EXPORT_PATTERNS.some(re => re.test(src));
|
||||
|
||||
if (hasRunExport) {
|
||||
try {
|
||||
|
||||
@@ -131,8 +131,15 @@ function removeOpenedRegularFile(filePath, opened) {
|
||||
function atomicWriteJson(filePath, value) {
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true, mode: 0o700 });
|
||||
const tempPath = `${filePath}.tmp-${process.pid}-${Date.now()}`;
|
||||
fs.writeFileSync(tempPath, `${JSON.stringify(value, null, 2)}\n`, { mode: 0o600 });
|
||||
fs.renameSync(tempPath, filePath);
|
||||
try {
|
||||
fs.writeFileSync(tempPath, `${JSON.stringify(value, null, 2)}\n`, { mode: 0o600 });
|
||||
fs.renameSync(tempPath, filePath);
|
||||
} catch (error) {
|
||||
// A failed write/rename must not leave a .tmp-<pid>-<timestamp> file
|
||||
// beside the canonical state file; repeated failures would accumulate them.
|
||||
fs.rmSync(tempPath, { force: true });
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function readState(statePath) {
|
||||
|
||||
@@ -48,6 +48,38 @@ 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).
|
||||
# Codex profiles (-p) and approval policies (--ask-for-approval) are
|
||||
# independent concepts. SECURITY: default to never approving untrusted
|
||||
# tool execution; operators can override via env.
|
||||
APPROVAL_POLICY="${ECC_CODEX_APPROVAL_POLICY:-never}"
|
||||
case "$APPROVAL_POLICY" in
|
||||
never|on-request|on-failure) ;;
|
||||
*)
|
||||
echo "[ECC worker] Refusing to run: unsupported ECC_CODEX_APPROVAL_POLICY='$APPROVAL_POLICY' (expected never|on-request|on-failure)" >&2
|
||||
write_status "failed" "- Error: unsupported approval policy"
|
||||
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 +109,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 --ask-for-approval "$APPROVAL_POLICY" -m gpt-5.4 --color never -C "$(pwd)" -o "$output_file" - < "$prompt_file"; then
|
||||
{
|
||||
echo "# Handoff"
|
||||
echo
|
||||
|
||||
Reference in New Issue
Block a user