fix(cli): Use `yaml.safe_load` exclusively in `_load_config_text` to prevent unsafe deserialization #3748

New issue

Open

opened 2026-04-05 22:26:19 +00:00 by freemo · 0 comments

freemo commented

2026-04-05 22:26:19 +00:00

Owner

Metadata

Branch: fix/security-safe-deserialization-load-config-text
Commit Message: fix(cli): use yaml.safe_load exclusively in _load_config_text to prevent unsafe deserialization
Milestone: (none — see backlog note below)
Parent Epic: #362

Bug Report

Severity Assessment

Impact: A maliciously crafted JSON file could potentially cause a denial of service by consuming excessive memory or CPU.
Likelihood: Low — requires a malicious actor to supply a crafted config file.
Priority: Low

Location

File: src/cleveragents/cli/commands/actor.py
Function: _load_config_text
Lines: 279–308

Description

The _load_config_text function currently attempts json.loads first, then falls back to yaml.safe_load. While json.loads is generally safe, a maliciously crafted JSON payload could cause a denial of service by consuming excessive memory or CPU. Since yaml.safe_load is a strict superset parser that handles both YAML and JSON safely, it should be used as the sole parser for all config file types, eliminating the json.loads path entirely.

Evidence

def _load_config_text(config_path: Path | None) -> tuple[str, dict[str, Any]] | None:
    ...
    text = path.read_text()
    try:
        data = json.loads(text)          # ← unnecessary; yaml.safe_load handles JSON too
    except json.JSONDecodeError:
        try:
            data = yaml.safe_load(text)  # ← should be the only parser
        except Exception as exc:         # pragma: no cover - defensive
            raise typer.BadParameter(f"Failed to parse config: {exc}") from exc

Expected Behavior

_load_config_text uses yaml.safe_load as the sole parser for both JSON and YAML config files, removing the json.loads code path.

Suggested Fix

Replace the try/except json.loads → yaml.safe_load fallback chain with a single yaml.safe_load call:

text = path.read_text()
try:
    data = yaml.safe_load(text)
except Exception as exc:
    raise typer.BadParameter(f"Failed to parse config: {exc}") from exc

Subtasks

Audit _load_config_text in src/cleveragents/cli/commands/actor.py (lines 279–308) and confirm yaml.safe_load correctly parses all supported JSON and YAML config formats
Remove the json.loads code path; replace with a single yaml.safe_load call
Remove the now-unused import json if it is no longer referenced elsewhere in the module
Update existing Behave unit tests in features/ to cover the updated parsing logic (JSON-as-YAML and YAML inputs)
Add a Behave scenario verifying that a malformed config raises typer.BadParameter with a descriptive message
Verify Robot Framework integration tests in robot/ still pass (or update as needed)
Run nox to confirm all quality gates pass

Definition of Done

_load_config_text uses yaml.safe_load exclusively — no json.loads code path remains
All existing unit and integration tests pass with the updated implementation
New Behave unit test scenarios cover JSON-formatted YAML, standard YAML, and malformed input
import json removed from module if no longer used
All nox stages pass
Coverage >= 97%

Backlog note: This issue was discovered during autonomous operation
on milestone NONE. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Automated by CleverAgents Bot
Supervisor: Acting on behalf of: Bug Hunter | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/security-safe-deserialization-load-config-text` - **Commit Message**: `fix(cli): use yaml.safe_load exclusively in _load_config_text to prevent unsafe deserialization` - **Milestone**: *(none — see backlog note below)* - **Parent Epic**: #362 ## Bug Report ### Severity Assessment - **Impact**: A maliciously crafted JSON file could potentially cause a denial of service by consuming excessive memory or CPU. - **Likelihood**: Low — requires a malicious actor to supply a crafted config file. - **Priority**: Low ### Location - **File**: `src/cleveragents/cli/commands/actor.py` - **Function**: `_load_config_text` - **Lines**: 279–308 ### Description The `_load_config_text` function currently attempts `json.loads` first, then falls back to `yaml.safe_load`. While `json.loads` is generally safe, a maliciously crafted JSON payload could cause a denial of service by consuming excessive memory or CPU. Since `yaml.safe_load` is a strict superset parser that handles both YAML and JSON safely, it should be used as the sole parser for all config file types, eliminating the `json.loads` path entirely. ### Evidence ```python def _load_config_text(config_path: Path | None) -> tuple[str, dict[str, Any]] | None: ... text = path.read_text() try: data = json.loads(text) # ← unnecessary; yaml.safe_load handles JSON too except json.JSONDecodeError: try: data = yaml.safe_load(text) # ← should be the only parser except Exception as exc: # pragma: no cover - defensive raise typer.BadParameter(f"Failed to parse config: {exc}") from exc ``` ### Expected Behavior `_load_config_text` uses `yaml.safe_load` as the sole parser for both JSON and YAML config files, removing the `json.loads` code path. ### Suggested Fix Replace the try/except `json.loads` → `yaml.safe_load` fallback chain with a single `yaml.safe_load` call: ```python text = path.read_text() try: data = yaml.safe_load(text) except Exception as exc: raise typer.BadParameter(f"Failed to parse config: {exc}") from exc ``` --- ## Subtasks - [ ] Audit `_load_config_text` in `src/cleveragents/cli/commands/actor.py` (lines 279–308) and confirm `yaml.safe_load` correctly parses all supported JSON and YAML config formats - [ ] Remove the `json.loads` code path; replace with a single `yaml.safe_load` call - [ ] Remove the now-unused `import json` if it is no longer referenced elsewhere in the module - [ ] Update existing Behave unit tests in `features/` to cover the updated parsing logic (JSON-as-YAML and YAML inputs) - [ ] Add a Behave scenario verifying that a malformed config raises `typer.BadParameter` with a descriptive message - [ ] Verify Robot Framework integration tests in `robot/` still pass (or update as needed) - [ ] Run `nox` to confirm all quality gates pass ## Definition of Done - [ ] `_load_config_text` uses `yaml.safe_load` exclusively — no `json.loads` code path remains - [ ] All existing unit and integration tests pass with the updated implementation - [ ] New Behave unit test scenarios cover JSON-formatted YAML, standard YAML, and malformed input - [ ] `import json` removed from module if no longer used - [ ] All nox stages pass - [ ] Coverage >= 97% --- > **Backlog note:** This issue was discovered during autonomous operation > on milestone NONE. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: Acting on behalf of: Bug Hunter | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-05 22:26:26 +00:00

#362 Epic: Security & Safety Hardening