From 1d67fff9e0d38d3f15e090a07711e987525868eb Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Sun, 25 Jan 2026 12:08:53 +0100 Subject: [PATCH 1/3] fix: add diagnostic warnings for config loading failures (#91) When config files have errors, users had no way to know why their settings weren't being applied. Added logging.warning() calls to diagnose: - Empty config files - Missing 'version' field - Invalid structure (not a dict) - Invalid command entries - Exceeding 100 command limit - YAML parse errors - File read errors Also added .resolve() to project path to handle symlinks correctly. Fixes: leonvanzyl/autocoder#91 Co-Authored-By: Claude Opus 4.5 --- security.py | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/security.py b/security.py index 6bb0036..cefbf3d 100644 --- a/security.py +++ b/security.py @@ -499,36 +499,45 @@ 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 @@ -550,7 +559,11 @@ def load_org_config() -> Optional[dict]: 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 @@ -564,7 +577,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 @@ -574,31 +587,39 @@ 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 @@ -620,7 +641,11 @@ def load_project_commands(project_dir: Path) -> Optional[dict]: 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 From 06c0bf4fd385af9a87492b45ab76cb372fb0ff02 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Tue, 27 Jan 2026 07:00:16 +0100 Subject: [PATCH 2/3] fix: add diagnostic warnings for pkill_processes validation failures Per CodeRabbit feedback, add logger.warning calls when pkill_processes validation fails in both load_org_config and load_project_commands. Co-Authored-By: Claude Opus 4.5 --- security.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/security.py b/security.py index cefbf3d..024ad04 100644 --- a/security.py +++ b/security.py @@ -544,15 +544,18 @@ def load_org_config() -> Optional[dict]: 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 @@ -626,15 +629,18 @@ def load_project_commands(project_dir: Path) -> Optional[dict]: 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 From 0a46eda5e894049d2c3c5aabc7898adbf94d552b Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Tue, 27 Jan 2026 21:21:50 +0100 Subject: [PATCH 3/3] test: add empty command name validation test for project config Adds a test case to verify that empty command names are rejected in project-level allowed_commands.yaml, matching the behavior already tested for org-level config. Updates test count to 163. Addresses review feedback from leonvanzyl. Co-Authored-By: Claude Opus 4.5 --- test_security.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test_security.py b/test_security.py index e8576f2..d8cb256 100644 --- a/test_security.py +++ b/test_security.py @@ -455,6 +455,21 @@ 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