Skip to content

fix: add diagnostic warnings for config loading failures #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
leonvanzyl merged 3 commits into from
Jan 29, 2026
+56 −10
Merged

fix: add diagnostic warnings for config loading failures #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d67fff
fix: add diagnostic warnings for config loading failures (#91)
cabana8471-arch Jan 25, 2026
06c0bf4
fix: add diagnostic warnings for pkill_processes validation failures
cabana8471-arch Jan 27, 2026
0a46eda
test: add empty command name validation test for project config
cabana8471-arch Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions security.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,58 +499,74 @@ def load_org_config() -> Optional[dict]:
config = yaml.safe_load(f)

if not config:
logger.warning(f"Org config at {config_path} is empty")
return None

# Validate structure
if not isinstance(config, dict):
logger.warning(f"Org config at {config_path} must be a YAML dictionary")
return None

if "version" not in config:
logger.warning(f"Org config at {config_path} missing required 'version' field")
return None

# Validate allowed_commands if present
if "allowed_commands" in config:
allowed = config["allowed_commands"]
if not isinstance(allowed, list):
logger.warning(f"Org config at {config_path}: 'allowed_commands' must be a list")
return None
for cmd in allowed:
for i, cmd in enumerate(allowed):
if not isinstance(cmd, dict):
logger.warning(f"Org config at {config_path}: allowed_commands[{i}] must be a dict")
return None
if "name" not in cmd:
logger.warning(f"Org config at {config_path}: allowed_commands[{i}] missing 'name'")
return None
# Validate that name is a non-empty string
if not isinstance(cmd["name"], str) or cmd["name"].strip() == "":
logger.warning(f"Org config at {config_path}: allowed_commands[{i}] has invalid 'name'")
return None

# Validate blocked_commands if present
if "blocked_commands" in config:
blocked = config["blocked_commands"]
if not isinstance(blocked, list):
logger.warning(f"Org config at {config_path}: 'blocked_commands' must be a list")
return None
for cmd in blocked:
for i, cmd in enumerate(blocked):
if not isinstance(cmd, str):
logger.warning(f"Org config at {config_path}: blocked_commands[{i}] must be a string")
return None

# Validate pkill_processes if present
if "pkill_processes" in config:
processes = config["pkill_processes"]
if not isinstance(processes, list):
logger.warning(f"Org config at {config_path}: 'pkill_processes' must be a list")
return None
# Normalize and validate each process name against safe pattern
normalized = []
for proc in processes:
for i, proc in enumerate(processes):
if not isinstance(proc, str):
logger.warning(f"Org config at {config_path}: pkill_processes[{i}] must be a string")
return None
proc = proc.strip()
# Block empty strings and regex metacharacters
if not proc or not VALID_PROCESS_NAME_PATTERN.fullmatch(proc):
logger.warning(f"Org config at {config_path}: pkill_processes[{i}] has invalid value '{proc}'")
return None
normalized.append(proc)
config["pkill_processes"] = normalized

return config

except (yaml.YAMLError, IOError, OSError):
except yaml.YAMLError as e:
logger.warning(f"Failed to parse org config at {config_path}: {e}")
return None
except (IOError, OSError) as e:
logger.warning(f"Failed to read org config at {config_path}: {e}")
return None


Expand All @@ -564,7 +580,7 @@ def load_project_commands(project_dir: Path) -> Optional[dict]:
Returns:
Dict with parsed YAML config, or None if file doesn't exist or is invalid
"""
config_path = project_dir / ".autocoder" / "allowed_commands.yaml"
config_path = project_dir.resolve() / ".autocoder" / "allowed_commands.yaml"

if not config_path.exists():
return None
Expand All @@ -574,53 +590,68 @@ def load_project_commands(project_dir: Path) -> Optional[dict]:
config = yaml.safe_load(f)

if not config:
logger.warning(f"Project config at {config_path} is empty")
return None

# Validate structure
if not isinstance(config, dict):
logger.warning(f"Project config at {config_path} must be a YAML dictionary")
return None

if "version" not in config:
logger.warning(f"Project config at {config_path} missing required 'version' field")
return None

commands = config.get("commands", [])
if not isinstance(commands, list):
logger.warning(f"Project config at {config_path}: 'commands' must be a list")
return None

# Enforce 100 command limit
if len(commands) > 100:
logger.warning(f"Project config at {config_path} exceeds 100 command limit ({len(commands)} commands)")
return None

# Validate each command entry
for cmd in commands:
for i, cmd in enumerate(commands):
if not isinstance(cmd, dict):
logger.warning(f"Project config at {config_path}: commands[{i}] must be a dict")
return None
if "name" not in cmd:
logger.warning(f"Project config at {config_path}: commands[{i}] missing 'name'")
return None
# Validate name is a string
if not isinstance(cmd["name"], str):
# Validate name is a non-empty string
if not isinstance(cmd["name"], str) or cmd["name"].strip() == "":
logger.warning(f"Project config at {config_path}: commands[{i}] has invalid 'name'")
return None

# Validate pkill_processes if present
if "pkill_processes" in config:
processes = config["pkill_processes"]
if not isinstance(processes, list):
logger.warning(f"Project config at {config_path}: 'pkill_processes' must be a list")
return None
# Normalize and validate each process name against safe pattern
normalized = []
for proc in processes:
for i, proc in enumerate(processes):
if not isinstance(proc, str):
logger.warning(f"Project config at {config_path}: pkill_processes[{i}] must be a string")
return None
proc = proc.strip()
# Block empty strings and regex metacharacters
if not proc or not VALID_PROCESS_NAME_PATTERN.fullmatch(proc):
logger.warning(f"Project config at {config_path}: pkill_processes[{i}] has invalid value '{proc}'")
return None
normalized.append(proc)
config["pkill_processes"] = normalized

return config

except (yaml.YAMLError, IOError, OSError):
except yaml.YAMLError as e:
logger.warning(f"Failed to parse project config at {config_path}: {e}")
return None
except (IOError, OSError) as e:
logger.warning(f"Failed to read project config at {config_path}: {e}")
return None


Expand Down
15 changes: 15 additions & 0 deletions test_security.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,21 @@ def test_project_commands():
print(" FAIL: Non-allowed command 'rustc' should be blocked")
failed += 1

# Test 4: Empty command name is rejected
config_path.write_text("""version: 1
commands:
- name: ""
description: Empty name should be rejected
""")
result = load_project_commands(project_dir)
if result is None:
print(" PASS: Empty command name rejected in project config")
passed += 1
else:
print(" FAIL: Empty command name should be rejected in project config")
print(f" Got: {result}")
failed += 1

return passed, failed


Expand Down
Loading