mirror of
https://github.com/leonvanzyl/autocoder.git
synced 2026-01-30 06:12:06 +00:00
security: validate pkill process names against safe character set
Address CodeRabbit security feedback - restrict pkill_processes entries to alphanumeric names with dots, underscores, and hyphens only. This prevents potential exploitation through regex metacharacters like '.*' being registered as process names. Changes: - Added VALID_PROCESS_NAME_PATTERN regex constant - Updated both org and project config validation to: - Normalize (trim whitespace) process names - Reject names with regex metacharacters - Reject names with spaces - Added 3 new tests for regex validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
25
security.py
25
security.py
@@ -7,12 +7,17 @@ Uses an allowlist approach - only explicitly permitted commands can run.
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
import shlex
|
import shlex
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Optional
|
from typing import Optional
|
||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
|
# Regex pattern for valid pkill process names (no regex metacharacters allowed)
|
||||||
|
# Matches alphanumeric names with dots, underscores, and hyphens
|
||||||
|
VALID_PROCESS_NAME_PATTERN = re.compile(r"^[A-Za-z0-9._-]+$")
|
||||||
|
|
||||||
# Allowed commands for development tasks
|
# Allowed commands for development tasks
|
||||||
# Minimal set needed for the autonomous coding demo
|
# Minimal set needed for the autonomous coding demo
|
||||||
ALLOWED_COMMANDS = {
|
ALLOWED_COMMANDS = {
|
||||||
@@ -474,9 +479,17 @@ def load_org_config() -> Optional[dict]:
|
|||||||
processes = config["pkill_processes"]
|
processes = config["pkill_processes"]
|
||||||
if not isinstance(processes, list):
|
if not isinstance(processes, list):
|
||||||
return None
|
return None
|
||||||
|
# Normalize and validate each process name against safe pattern
|
||||||
|
normalized = []
|
||||||
for proc in processes:
|
for proc in processes:
|
||||||
if not isinstance(proc, str) or proc.strip() == "":
|
if not isinstance(proc, str):
|
||||||
return None
|
return None
|
||||||
|
proc = proc.strip()
|
||||||
|
# Block empty strings and regex metacharacters
|
||||||
|
if not proc or not VALID_PROCESS_NAME_PATTERN.fullmatch(proc):
|
||||||
|
return None
|
||||||
|
normalized.append(proc)
|
||||||
|
config["pkill_processes"] = normalized
|
||||||
|
|
||||||
return config
|
return config
|
||||||
|
|
||||||
@@ -536,9 +549,17 @@ def load_project_commands(project_dir: Path) -> Optional[dict]:
|
|||||||
processes = config["pkill_processes"]
|
processes = config["pkill_processes"]
|
||||||
if not isinstance(processes, list):
|
if not isinstance(processes, list):
|
||||||
return None
|
return None
|
||||||
|
# Normalize and validate each process name against safe pattern
|
||||||
|
normalized = []
|
||||||
for proc in processes:
|
for proc in processes:
|
||||||
if not isinstance(proc, str) or proc.strip() == "":
|
if not isinstance(proc, str):
|
||||||
return None
|
return None
|
||||||
|
proc = proc.strip()
|
||||||
|
# Block empty strings and regex metacharacters
|
||||||
|
if not proc or not VALID_PROCESS_NAME_PATTERN.fullmatch(proc):
|
||||||
|
return None
|
||||||
|
normalized.append(proc)
|
||||||
|
config["pkill_processes"] = normalized
|
||||||
|
|
||||||
return config
|
return config
|
||||||
|
|
||||||
|
|||||||
@@ -809,6 +809,73 @@ pkill_processes:
|
|||||||
print(f" FAIL: pkill python should be allowed with org config: {result}")
|
print(f" FAIL: pkill python should be allowed with org config: {result}")
|
||||||
failed += 1
|
failed += 1
|
||||||
|
|
||||||
|
# Test 9: Regex metacharacters should be rejected in pkill_processes
|
||||||
|
with tempfile.TemporaryDirectory() as tmphome:
|
||||||
|
with tempfile.TemporaryDirectory() as tmpproject:
|
||||||
|
with temporary_home(tmphome):
|
||||||
|
org_dir = Path(tmphome) / ".autocoder"
|
||||||
|
org_dir.mkdir()
|
||||||
|
org_config_path = org_dir / "config.yaml"
|
||||||
|
|
||||||
|
# Try to register a regex pattern (should be rejected)
|
||||||
|
org_config_path.write_text("""version: 1
|
||||||
|
pkill_processes:
|
||||||
|
- ".*"
|
||||||
|
""")
|
||||||
|
|
||||||
|
config = load_org_config()
|
||||||
|
if config is None:
|
||||||
|
print(" PASS: Regex pattern '.*' rejected in pkill_processes")
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
print(" FAIL: Regex pattern '.*' should be rejected")
|
||||||
|
failed += 1
|
||||||
|
|
||||||
|
# Test 10: Valid process names with dots/underscores/hyphens should be accepted
|
||||||
|
with tempfile.TemporaryDirectory() as tmphome:
|
||||||
|
with tempfile.TemporaryDirectory() as tmpproject:
|
||||||
|
with temporary_home(tmphome):
|
||||||
|
org_dir = Path(tmphome) / ".autocoder"
|
||||||
|
org_dir.mkdir()
|
||||||
|
org_config_path = org_dir / "config.yaml"
|
||||||
|
|
||||||
|
# Valid names with special chars
|
||||||
|
org_config_path.write_text("""version: 1
|
||||||
|
pkill_processes:
|
||||||
|
- my-app
|
||||||
|
- app_server
|
||||||
|
- node.js
|
||||||
|
""")
|
||||||
|
|
||||||
|
config = load_org_config()
|
||||||
|
if config is not None and config.get("pkill_processes") == ["my-app", "app_server", "node.js"]:
|
||||||
|
print(" PASS: Valid process names with dots/underscores/hyphens accepted")
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
print(f" FAIL: Valid process names should be accepted: {config}")
|
||||||
|
failed += 1
|
||||||
|
|
||||||
|
# Test 11: Names with spaces should be rejected
|
||||||
|
with tempfile.TemporaryDirectory() as tmphome:
|
||||||
|
with tempfile.TemporaryDirectory() as tmpproject:
|
||||||
|
with temporary_home(tmphome):
|
||||||
|
org_dir = Path(tmphome) / ".autocoder"
|
||||||
|
org_dir.mkdir()
|
||||||
|
org_config_path = org_dir / "config.yaml"
|
||||||
|
|
||||||
|
org_config_path.write_text("""version: 1
|
||||||
|
pkill_processes:
|
||||||
|
- "my app"
|
||||||
|
""")
|
||||||
|
|
||||||
|
config = load_org_config()
|
||||||
|
if config is None:
|
||||||
|
print(" PASS: Process name with space rejected")
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
print(" FAIL: Process name with space should be rejected")
|
||||||
|
failed += 1
|
||||||
|
|
||||||
return passed, failed
|
return passed, failed
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user