fix(slm): remove write-capable executables from ALLOWED_EXECUTABLES (#3450)

[mrveiss]

Summary

• Removes apt, yum, dnf, rpm (package install/remove) from ALLOWED_EXECUTABLES entirely — no argument-level enforcement can make these safe enough for an admin API endpoint
• Removes wget, curl (arbitrary file write, exfiltration, POST to internal services) from ALLOWED_EXECUTABLES entirely
• Removes nmap (network scanner with --script exploit support) from ALLOWED_EXECUTABLES entirely
• Adds _GIT_ALLOWED_SUBCOMMANDS frozenset; _validate_command now enforces read-only git subcommands (status, log, diff, show, branch, tag, remote, describe, shortlog, rev-parse, ls-files, ls-remote, stash) — write subcommands (clone, push, fetch, commit, reset, clean, config, etc.) return HTTP 400
• find: _validate_command rejects any command where a token is -exec or -execdir — prevents arbitrary command chaining through find
• Fixes the false inline comment # Git (read-only operations are enforced at argument level by callers) — enforcement is now in _validate_command itself

Closes #3450

Test plan

• apt install nginx / yum install httpd / dnf install vim / rpm -ivh pkg.rpm return HTTP 400 (not permitted)
• wget http://example.com/file / curl -o /tmp/f http://example.com / nmap -sV 10.0.0.1 return HTTP 400 (not permitted)
• git status / git log --oneline -10 / git diff HEAD~1 / git show HEAD / git branch -a / git remote -v pass validation
• git clone ... / git push / git pull / git commit / git reset --hard / git config return HTTP 400 (not permitted)
• git (no subcommand) returns HTTP 400
• find /tmp -exec bash {} ; / find / -execdir rm {} ; return HTTP 400 (not permitted)
• find /var/log -name '*.log' -mtime -1 passes validation

🤖 Generated with Claude Code

[mrveiss]

Code review

Found 4 issues:

1. PR targets main instead of the required Dev_new_gui (CLAUDE.md says "Branch target: Dev_new_gui for all PRs unless told otherwise")

   AutoBot-AI/autobot-slm-backend/api/nodes_execution.py

   Lines 1 to 5 in 7eedab7

   	# AutoBot - AI-Powered Automation Platform
   	# Copyright (c) 2025 mrveiss
   	# Author: mrveiss
   	"""
   	Node Remote Execution API

2. NodeExecuteRequest is missing a language field — dag_executor.py sends {"command": ..., "language": ..., "timeout": ...} but the model only has command + timeout. Pydantic silently drops the language field, making it impossible to add language-aware execution later without a breaking change, and masking the caller mismatch today.

   AutoBot-AI/autobot-slm-backend/api/nodes_execution.py

   Lines 144 to 157 in 7eedab7

   	# Extract the bare executable name (strip any leading path components
   	# so that e.g. /bin/ls still matches "ls").
   	executable = Path(tokens[0]).name
   	if executable not in ALLOWED_EXECUTABLES:
   	logger.warning(
   	"Command rejected — executable %r not in allowlist", executable
   	)
   	raise HTTPException(
   	status_code=status.HTTP_400_BAD_REQUEST,
   	detail=(
   	f"Command rejected: executable {executable!r} is not permitted. "
   	"Contact an administrator to extend the allowlist."

3. dpkg is in ALLOWED_EXECUTABLES with no argument guard — dpkg -i package.deb, dpkg --purge, and dpkg --configure are write operations that pass validation. The PR removes apt/rpm for the same reason but leaves dpkg unguarded, reintroducing package write capability.

   AutoBot-AI/autobot-slm-backend/api/nodes_execution.py

   Lines 82 to 92 in 7eedab7

   	"head",
   	"tail",
   	"find",
   	"stat",
   	"file",
   	# Package management (query-only helpers — apt/yum/dnf/rpm excluded)
   	"dpkg",
   	# AutoBot-specific helpers
   	"autobot-status",
   	"autobot-health",
   	# Git (read-only subcommands enforced at argument level — see

4. execute_on_node (~80 lines) and _run_via_ssh (~70 lines) exceed the mandatory ≤30-line function limit (CLAUDE.md → docs/developer/CLAUDE_RULES.md). Since this is a new file, these should be decomposed now: extract _validate_git_subcommand, _validate_find_args, _build_ssh_cmd, and _dispatch_execution as separate helpers.

   AutoBot-AI/autobot-slm-backend/api/nodes_execution.py

   Lines 275 to 430 in 7eedab7

   	),
   	details={
   	"job_id": job_id,
   	"command": command,
   	"acting_user": acting_user,
   	"exit_code": exit_code,
   	"duration_ms": duration_ms,
   	},
   	)
   	db.add(event)
   	await db.commit()
   	_SSH_KEY_PATH = os.environ.get("SLM_SSH_KEY", "/home/autobot/.ssh/autobot_key") # noqa: ssot-path
   	_SSH_KNOWN_HOSTS_PATH = os.environ.get(
   	"SLM_SSH_KNOWN_HOSTS", "/home/autobot/.ssh/known_hosts"
   	)
   	_LOCAL_ADDRESSES = {"127.0.0.1", "::1", "localhost"}
   	try:
   	_LOCAL_ADDRESSES.add(socket.gethostbyname(socket.gethostname()))
   	except OSError:
   	pass
   	def _is_local_ip(ip: str) -> bool:
   	"""Return True if *ip* resolves to this host."""
   	return ip in _LOCAL_ADDRESSES
   	async def _run_command(tokens: list[str], timeout: int) -> tuple[int, str, str]:
   	"""Execute a pre-tokenised command locally; return (exit_code, stdout, stderr).
   	Uses shell=False (exec list form) — the tokens come from shlex.split() of
   	an allowlist-validated command, so no shell interpretation occurs.
   	"""
   	proc = await asyncio.create_subprocess_exec(
   	*tokens,
   	stdout=asyncio.subprocess.PIPE,
   	stderr=asyncio.subprocess.PIPE,
   	)
   	try:
   	raw_out, raw_err = await asyncio.wait_for(
   	proc.communicate(), timeout=float(timeout)
   	)
   	except asyncio.TimeoutError:
   	proc.kill()
   	await proc.communicate()
   	return 124, "", f"Execution timed out after {timeout}s"
   	return (
   	proc.returncode if proc.returncode is not None else 1,
   	raw_out.decode("utf-8", errors="replace"),
   	raw_err.decode("utf-8", errors="replace"),
   	)
   	async def _run_via_ssh(
   	ip: str,
   	ssh_user: str,
   	ssh_port: int,
   	tokens: list[str],
   	timeout: int,
   	) -> tuple[int, str, str]:
   	"""Execute a pre-tokenised command on *ip* via SSH.
   	Uses known_hosts verification (StrictHostKeyChecking=yes) when a
   	known_hosts file exists, falling back to 'accept-new' for first contact
   	rather than the previous insecure 'no' (#3421).
   	"""
   	known_hosts_path = Path(_SSH_KNOWN_HOSTS_PATH)
   	if known_hosts_path.exists():
   	host_key_checking = "yes"
   	known_hosts_file = str(known_hosts_path)
   	else:
   	# Accept and persist the key on first connection; never silently
   	# accept a changed key (this is safer than StrictHostKeyChecking=no).
   	host_key_checking = "accept-new"
   	known_hosts_file = "/dev/null"
   	logger.warning(
   	"known_hosts file not found at %s — using accept-new for %s",
   	_SSH_KNOWN_HOSTS_PATH,
   	ip,
   	)
   	cmd = [
   	"ssh",
   	"-p", str(ssh_port),
   	"-o", f"StrictHostKeyChecking={host_key_checking}",
   	"-o", f"UserKnownHostsFile={known_hosts_file}",
   	"-o", "BatchMode=yes",
   	"-o", f"ConnectTimeout={min(timeout, 30)}",
   	]
   	if Path(_SSH_KEY_PATH).exists():
   	cmd.extend(["-i", _SSH_KEY_PATH])
   	cmd.append(f"{ssh_user}@{ip}")
   	# Pass the command tokens as individual arguments to avoid any shell
   	# interpretation on the remote side.
   	cmd.extend(tokens)
   	proc = await asyncio.create_subprocess_exec(
   	*cmd,
   	stdout=asyncio.subprocess.PIPE,
   	stderr=asyncio.subprocess.PIPE,
   	)
   	try:
   	raw_out, raw_err = await asyncio.wait_for(
   	proc.communicate(), timeout=float(timeout)
   	)
   	except asyncio.TimeoutError:
   	proc.kill()
   	await proc.communicate()
   	return 124, "", f"SSH execution timed out after {timeout}s"
   	return (
   	proc.returncode if proc.returncode is not None else 1,
   	raw_out.decode("utf-8", errors="replace"),
   	raw_err.decode("utf-8", errors="replace"),
   	)
   	# ---------------------------------------------------------------------------
   	# Route
   	# ---------------------------------------------------------------------------
   	@router.post(
   	"/{node_id}/execute",
   	response_model=NodeExecuteResponse,
   	summary="Execute an allowlisted command on a fleet node",
   	)
   	async def execute_on_node(
   	node_id: str,
   	body: NodeExecuteRequest,
   	db: AsyncSession = Depends(get_db),
   	current_user: dict = Depends(require_admin),
   	) -> NodeExecuteResponse:
   	"""Run *body.command* on the node identified by *node_id*.
   	The node must be ONLINE. *body.command* is tokenised with shlex.split()
   	and the first token (executable name) must be present in
   	ALLOWED_EXECUTABLES — any other command is rejected with HTTP 400.
   	Additional argument-level guards enforce read-only use for git and block
   	find -exec. See _validate_command for details (#3450).
   	Admin privileges are required (require_admin dependency).
   	Local nodes (manager host) execute via subprocess with shell=False;
   	remote nodes execute via SSH using the SLM key (SLM_SSH_KEY env var,
   	default /home/autobot/.ssh/autobot_key) and known_hosts verification
   	(SLM_SSH_KNOWN_HOSTS env var, default /home/autobot/.ssh/known_hosts).
   	All executions are audit-logged including the full command and acting user.
   	"""
   	acting_user: str = current_user.get("sub", "unknown")
   	executable = _validate_command(body.command)

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[mrveiss]

Code review

Found 2 issues:

1. dpkg is in ALLOWED_EXECUTABLES with comment "query-only helpers" but has no argument-level guard. dpkg -i pkg.deb, dpkg --purge pkg, and dpkg --unpack are all write-capable and pass _validate_command unchecked. This directly contradicts the PR's goal of removing write-capable package managers (apt/yum/dnf/rpm were removed, but dpkg is the underlying tool they all call).

AutoBot-AI/autobot-slm-backend/api/nodes_execution.py

Lines 86 to 91 in 7eedab7

	"file",
	# Package management (query-only helpers — apt/yum/dnf/rpm excluded)
	"dpkg",
	# AutoBot-specific helpers
	"autobot-status",
	"autobot-health",

2. git stash is in _GIT_ALLOWED_SUBCOMMANDS with comment # read-only usage: stash list / stash show, but _validate_command only checks tokens[1] against the frozenset — it does not validate tokens[2]. Commands like git stash drop, git stash pop, and git stash clear all pass the guard and mutate repository state. The stated read-only intent is not enforced.

AutoBot-AI/autobot-slm-backend/api/nodes_execution.py

Lines 112 to 118 in 7eedab7

	"ls-files",
	"ls-remote",
	"stash", # read-only usage: stash list / stash show
	}
	)

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[github-actions]

AutoBot Phase Validation Results

System Maturity: 0.0%

Phase Status:

Recommendations:

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!