mirror of
https://github.com/leonvanzyl/autocoder.git
synced 2026-01-29 22:02:05 +00:00
fix: improve path matching and org config validation
Changes: - Support path patterns without ./ prefix (e.g., 'scripts/test.sh') - Reject non-string or empty command names in org config - Add 8 new test cases (5 for path patterns, 3 for validation) Details: - matches_pattern() now treats any pattern with '/' as a path pattern - load_org_config() validates that cmd['name'] is a non-empty string - All 148 unit tests + 9 integration tests passing Security hardening: Prevents invalid command names from reaching pattern matching logic, reducing attack surface.
This commit is contained in:
@@ -20,7 +20,7 @@ This directory contains example configuration files for controlling which bash c
|
|||||||
|
|
||||||
When you create a new project with AutoCoder, it automatically creates:
|
When you create a new project with AutoCoder, it automatically creates:
|
||||||
|
|
||||||
```
|
```text
|
||||||
my-project/
|
my-project/
|
||||||
.autocoder/
|
.autocoder/
|
||||||
allowed_commands.yaml ← Automatically created from template
|
allowed_commands.yaml ← Automatically created from template
|
||||||
@@ -119,7 +119,7 @@ blocked_commands:
|
|||||||
|
|
||||||
When the agent tries to run a command, the system checks in this order:
|
When the agent tries to run a command, the system checks in this order:
|
||||||
|
|
||||||
```
|
```text
|
||||||
┌─────────────────────────────────────────────────────┐
|
┌─────────────────────────────────────────────────────┐
|
||||||
│ 1. HARDCODED BLOCKLIST (highest priority) │
|
│ 1. HARDCODED BLOCKLIST (highest priority) │
|
||||||
│ sudo, dd, shutdown, reboot, chown, etc. │
|
│ sudo, dd, shutdown, reboot, chown, etc. │
|
||||||
@@ -501,12 +501,12 @@ commands:
|
|||||||
**5. Check the agent output:**
|
**5. Check the agent output:**
|
||||||
|
|
||||||
The agent will show security hook messages like:
|
The agent will show security hook messages like:
|
||||||
```
|
```text
|
||||||
Command 'sudo' is blocked at organization level and cannot be approved.
|
Command 'sudo' is blocked at organization level and cannot be approved.
|
||||||
```
|
```
|
||||||
|
|
||||||
Or:
|
Or:
|
||||||
```
|
```text
|
||||||
Command 'wget' is not allowed.
|
Command 'wget' is not allowed.
|
||||||
To allow this command:
|
To allow this command:
|
||||||
1. Add to .autocoder/allowed_commands.yaml for this project, OR
|
1. Add to .autocoder/allowed_commands.yaml for this project, OR
|
||||||
|
|||||||
@@ -387,8 +387,8 @@ def matches_pattern(command: str, pattern: str) -> bool:
|
|||||||
return False
|
return False
|
||||||
return command.startswith(prefix)
|
return command.startswith(prefix)
|
||||||
|
|
||||||
# Local script paths (./scripts/build.sh matches build.sh)
|
# Path patterns (./scripts/build.sh, scripts/test.sh, etc.)
|
||||||
if pattern.startswith("./") or pattern.startswith("../"):
|
if "/" in pattern:
|
||||||
# Extract the script name from the pattern
|
# Extract the script name from the pattern
|
||||||
pattern_name = os.path.basename(pattern)
|
pattern_name = os.path.basename(pattern)
|
||||||
return command == pattern or command == pattern_name or command.endswith("/" + pattern_name)
|
return command == pattern or command == pattern_name or command.endswith("/" + pattern_name)
|
||||||
@@ -442,6 +442,9 @@ def load_org_config() -> Optional[dict]:
|
|||||||
return None
|
return None
|
||||||
if "name" not in cmd:
|
if "name" not in cmd:
|
||||||
return None
|
return None
|
||||||
|
# Validate that name is a non-empty string
|
||||||
|
if not isinstance(cmd["name"], str) or cmd["name"].strip() == "":
|
||||||
|
return None
|
||||||
|
|
||||||
# Validate blocked_commands if present
|
# Validate blocked_commands if present
|
||||||
if "blocked_commands" in config:
|
if "blocked_commands" in config:
|
||||||
|
|||||||
@@ -183,13 +183,20 @@ def test_pattern_matching():
|
|||||||
("sudo", "*", False, "bare wildcard doesn't match sudo"),
|
("sudo", "*", False, "bare wildcard doesn't match sudo"),
|
||||||
("anything", "*", False, "bare wildcard doesn't match anything"),
|
("anything", "*", False, "bare wildcard doesn't match anything"),
|
||||||
|
|
||||||
# Local script paths
|
# Local script paths (with ./ prefix)
|
||||||
("build.sh", "./scripts/build.sh", True, "script name matches path"),
|
("build.sh", "./scripts/build.sh", True, "script name matches path"),
|
||||||
("./scripts/build.sh", "./scripts/build.sh", True, "exact script path"),
|
("./scripts/build.sh", "./scripts/build.sh", True, "exact script path"),
|
||||||
("scripts/build.sh", "./scripts/build.sh", True, "relative script path"),
|
("scripts/build.sh", "./scripts/build.sh", True, "relative script path"),
|
||||||
("/abs/path/scripts/build.sh", "./scripts/build.sh", True, "absolute path matches"),
|
("/abs/path/scripts/build.sh", "./scripts/build.sh", True, "absolute path matches"),
|
||||||
("test.sh", "./scripts/build.sh", False, "different script name"),
|
("test.sh", "./scripts/build.sh", False, "different script name"),
|
||||||
|
|
||||||
|
# Path patterns (without ./ prefix - new behavior)
|
||||||
|
("test.sh", "scripts/test.sh", True, "script name matches path pattern"),
|
||||||
|
("scripts/test.sh", "scripts/test.sh", True, "exact path pattern match"),
|
||||||
|
("/abs/path/scripts/test.sh", "scripts/test.sh", True, "absolute path matches pattern"),
|
||||||
|
("build.sh", "scripts/test.sh", False, "different script name in pattern"),
|
||||||
|
("integration.test.js", "tests/integration.test.js", True, "script with dots matches"),
|
||||||
|
|
||||||
# Non-matches
|
# Non-matches
|
||||||
("go", "swift*", False, "go doesn't match swift*"),
|
("go", "swift*", False, "go doesn't match swift*"),
|
||||||
("rustc", "swift*", False, "rustc doesn't match swift*"),
|
("rustc", "swift*", False, "rustc doesn't match swift*"),
|
||||||
@@ -453,6 +460,51 @@ blocked_commands:
|
|||||||
print(" FAIL: Missing org config returns None")
|
print(" FAIL: Missing org config returns None")
|
||||||
failed += 1
|
failed += 1
|
||||||
|
|
||||||
|
# Test 3: Non-string command name is rejected
|
||||||
|
org_config_path.write_text("""version: 1
|
||||||
|
allowed_commands:
|
||||||
|
- name: 123
|
||||||
|
description: Invalid numeric name
|
||||||
|
""")
|
||||||
|
config = load_org_config()
|
||||||
|
if config is None:
|
||||||
|
print(" PASS: Non-string command name rejected")
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
print(" FAIL: Non-string command name rejected")
|
||||||
|
print(f" Got: {config}")
|
||||||
|
failed += 1
|
||||||
|
|
||||||
|
# Test 4: Empty command name is rejected
|
||||||
|
org_config_path.write_text("""version: 1
|
||||||
|
allowed_commands:
|
||||||
|
- name: ""
|
||||||
|
description: Empty name
|
||||||
|
""")
|
||||||
|
config = load_org_config()
|
||||||
|
if config is None:
|
||||||
|
print(" PASS: Empty command name rejected")
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
print(" FAIL: Empty command name rejected")
|
||||||
|
print(f" Got: {config}")
|
||||||
|
failed += 1
|
||||||
|
|
||||||
|
# Test 5: Whitespace-only command name is rejected
|
||||||
|
org_config_path.write_text("""version: 1
|
||||||
|
allowed_commands:
|
||||||
|
- name: " "
|
||||||
|
description: Whitespace name
|
||||||
|
""")
|
||||||
|
config = load_org_config()
|
||||||
|
if config is None:
|
||||||
|
print(" PASS: Whitespace-only command name rejected")
|
||||||
|
passed += 1
|
||||||
|
else:
|
||||||
|
print(" FAIL: Whitespace-only command name rejected")
|
||||||
|
print(f" Got: {config}")
|
||||||
|
failed += 1
|
||||||
|
|
||||||
# Restore HOME
|
# Restore HOME
|
||||||
os.environ["HOME"] = str(original_home)
|
os.environ["HOME"] = str(original_home)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user