cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [security] Command injection risk in scripts/check-quality-gates.py via unvalidated subprocess arguments #7286

New issue

Open

opened 2026-04-10 15:09:25 +00:00 by HAL9000 · 5 comments

HAL9000 commented

2026-04-10 15:09:25 +00:00

Owner

Background

The scripts/check-quality-gates.py quality gate checker constructs subprocess.run() calls using file paths and arguments that are not validated or sanitised before use. While the list form of subprocess.run() prevents shell interpretation of the command itself, it does not prevent argument injection: a path component containing metacharacters or crafted values can still be passed verbatim to the underlying tool (e.g., bandit, vulture, radon), potentially causing those tools to misinterpret arguments, traverse unexpected paths, or execute unintended operations.

The vulnerability affects three functions:

Function	Lines	Tool invoked
`check_security()`	~70–81	`bandit`
`check_dead_code()`	~91–102	`vulture`
`check_complexity()`	~113–124	`radon`

In each case, path strings (e.g., "src/cleveragents", "vulture_whitelist.py", "src/cleveragents/discovery") are passed directly to subprocess without existence checks, type validation, or sanitisation. If any of these values were ever sourced from user input, environment variables, or configuration files, they would constitute a command injection vector.

Current Behaviour

The script passes path strings directly to subprocess calls without:

Validating that paths exist on disk
Checking paths do not contain dangerous characters (e.g., ;, |, &, ..)
Using pathlib.Path for safe path manipulation
Explicitly asserting shell=False (the default, but not stated)

Example from check_dead_code():

result = subprocess.run(
    [
        "vulture",
        "src/cleveragents",
        "vulture_whitelist.py",      # Could be environment-controlled
        "--min-confidence",
        "80",
        "--exclude",
        "src/cleveragents/discovery",  # Path components unvalidated
    ],
    capture_output=True,
    text=True,
    check=False,
)

Expected Behaviour

All paths and arguments passed to subprocess must be validated before use:

Paths must be resolved via pathlib.Path and confirmed to exist.
Paths must not escape the project root (no .. traversal).
Arguments must be validated against an allowlist of safe characters.
shell=False must be stated explicitly for clarity and defence-in-depth.

Acceptance Criteria

All path arguments in check_security(), check_dead_code(), and check_complexity() are validated using pathlib.Path.
A validate_path() helper (or equivalent) is introduced that raises ValueError on dangerous input.
shell=False is explicitly set on all subprocess.run() calls in the script.
No path argument is accepted from an unvalidated external source without sanitisation.
All existing quality gate checks continue to pass after the fix.

Metadata

Branch: bugfix/m8-security-cmd-injection-quality-gates
Commit Message: fix(scripts): validate subprocess path arguments in check-quality-gates.py to prevent command injection
Milestone: v3.7.0
Parent Epic: (orphan — see note below)

Subtasks

Audit all subprocess.run() calls in scripts/check-quality-gates.py for unvalidated path arguments
Implement validate_path(path: str, project_root: Path) -> Path helper with allowlist validation
Refactor check_security() to use validated Path objects
Refactor check_dead_code() to use validated Path objects
Refactor check_complexity() to use validated Path objects
Add explicit shell=False to all subprocess.run() calls
Write BDD scenario (TDD issue will be created separately per bug workflow)
Verify all nox stages pass after fix

Definition of Done

validate_path() helper implemented and covers: non-existent paths, path traversal (..), dangerous metacharacters
All three affected functions refactored to use validated paths
shell=False explicitly present on every subprocess.run() call in the script
No regressions in quality gate output
All nox stages pass
Coverage >= 97%

TDD Note: Per the mandatory bug fix workflow in CONTRIBUTING.md, a Type/Testing issue will be created after this issue is verified. The TDD test will use tags @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before the fix is applied.

Automated by CleverAgents Bot
Supervisor: Bug Hunter | Agent: new-issue-creator

## Background The `scripts/check-quality-gates.py` quality gate checker constructs `subprocess.run()` calls using file paths and arguments that are not validated or sanitised before use. While the list form of `subprocess.run()` prevents shell interpretation of the command itself, it does **not** prevent argument injection: a path component containing metacharacters or crafted values can still be passed verbatim to the underlying tool (e.g., `bandit`, `vulture`, `radon`), potentially causing those tools to misinterpret arguments, traverse unexpected paths, or execute unintended operations. The vulnerability affects three functions: | Function | Lines | Tool invoked | |---|---|---| | `check_security()` | ~70–81 | `bandit` | | `check_dead_code()` | ~91–102 | `vulture` | | `check_complexity()` | ~113–124 | `radon` | In each case, path strings (e.g., `"src/cleveragents"`, `"vulture_whitelist.py"`, `"src/cleveragents/discovery"`) are passed directly to subprocess without existence checks, type validation, or sanitisation. If any of these values were ever sourced from user input, environment variables, or configuration files, they would constitute a command injection vector. ## Current Behaviour The script passes path strings directly to subprocess calls without: - Validating that paths exist on disk - Checking paths do not contain dangerous characters (e.g., `;`, `|`, `&`, `..`) - Using `pathlib.Path` for safe path manipulation - Explicitly asserting `shell=False` (the default, but not stated) Example from `check_dead_code()`: ```python result = subprocess.run( [ "vulture", "src/cleveragents", "vulture_whitelist.py", # Could be environment-controlled "--min-confidence", "80", "--exclude", "src/cleveragents/discovery", # Path components unvalidated ], capture_output=True, text=True, check=False, ) ``` ## Expected Behaviour All paths and arguments passed to subprocess must be validated before use: 1. Paths must be resolved via `pathlib.Path` and confirmed to exist. 2. Paths must not escape the project root (no `..` traversal). 3. Arguments must be validated against an allowlist of safe characters. 4. `shell=False` must be stated explicitly for clarity and defence-in-depth. ## Acceptance Criteria - [ ] All path arguments in `check_security()`, `check_dead_code()`, and `check_complexity()` are validated using `pathlib.Path`. - [ ] A `validate_path()` helper (or equivalent) is introduced that raises `ValueError` on dangerous input. - [ ] `shell=False` is explicitly set on all `subprocess.run()` calls in the script. - [ ] No path argument is accepted from an unvalidated external source without sanitisation. - [ ] All existing quality gate checks continue to pass after the fix. ## Metadata - **Branch**: `bugfix/m8-security-cmd-injection-quality-gates` - **Commit Message**: `fix(scripts): validate subprocess path arguments in check-quality-gates.py to prevent command injection` - **Milestone**: v3.7.0 - **Parent Epic**: *(orphan — see note below)* ## Subtasks - [ ] Audit all `subprocess.run()` calls in `scripts/check-quality-gates.py` for unvalidated path arguments - [ ] Implement `validate_path(path: str, project_root: Path) -> Path` helper with allowlist validation - [ ] Refactor `check_security()` to use validated `Path` objects - [ ] Refactor `check_dead_code()` to use validated `Path` objects - [ ] Refactor `check_complexity()` to use validated `Path` objects - [ ] Add explicit `shell=False` to all `subprocess.run()` calls - [ ] Write BDD scenario (TDD issue will be created separately per bug workflow) - [ ] Verify all `nox` stages pass after fix ## Definition of Done - [ ] `validate_path()` helper implemented and covers: non-existent paths, path traversal (`..`), dangerous metacharacters - [ ] All three affected functions refactored to use validated paths - [ ] `shell=False` explicitly present on every `subprocess.run()` call in the script - [ ] No regressions in quality gate output - [ ] All nox stages pass - [ ] Coverage >= 97% > **TDD Note:** Per the mandatory bug fix workflow in CONTRIBUTING.md, a `Type/Testing` issue will be created after this issue is verified. The TDD test will use tags `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before the fix is applied. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunter | Agent: new-issue-creator

HAL9000 added this to the v3.7.0 milestone

2026-04-10 15:09:31 +00:00

HAL9000 commented

2026-04-10 15:13:59 +00:00

Author

Owner

⚠️ Label Application Required

This issue was created by automation but label application via the API is restricted in this environment. The following labels must be applied manually:

State/Unverified (ID: 846)
Type/Bug (ID: 849)
Priority/Critical (ID: 858)

All three labels are mandatory per CONTRIBUTING.md. The milestone v3.7.0 has already been assigned.

Automated by CleverAgents Bot
Supervisor: Bug Hunter | Agent: new-issue-creator

⚠️ **Label Application Required** This issue was created by automation but label application via the API is restricted in this environment. The following labels must be applied manually: - `State/Unverified` (ID: 846) - `Type/Bug` (ID: 849) - `Priority/Critical` (ID: 858) All three labels are mandatory per CONTRIBUTING.md. The milestone `v3.7.0` has already been assigned. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunter | Agent: new-issue-creator

HAL9000 commented

2026-04-10 15:14:11 +00:00

Author

Owner

🔗 Orphan Issue — Manual Parent Epic Linking Required

No parent Epic was found for this issue during automated creation. Per CONTRIBUTING.md, orphan issues are not permitted — every issue must be linked to a parent Epic using Forgejo's dependency system (child blocks parent).

A maintainer must:

Identify or create an appropriate parent Epic (e.g., a Security Hardening Epic or CI/CD Pipeline Epic)

Create the dependency link so that this issue blocks the parent Epic:

curl -X POST "${FORGEJO_URL}/api/v1/repos/cleveragents/cleveragents-core/issues/7286/blocks" \
  -H "Authorization: token <PAT>" \
  -H "Content-Type: application/json" \
  -d '{"owner": "cleveragents", "repo": "cleveragents-core", "index": <PARENT_EPIC_NUMBER>}'

Related security issues that may share a parent Epic:

#7280 BUG-HUNT: [security] JSON injection vulnerability in opencode-builder.sh
#7214 BUG-HUNT: [security] Path traversal vulnerability in file_tools.py
#7084 BUG-HUNT: [MCP-SECURITY] Path traversal vulnerability in SandboxPathRewriter
#7053 BUG-HUNT: [SECURITY] Code injection vulnerability in LangGraph function execution
#7038 BUG-HUNT: [MCP-SECURITY] Command injection vulnerability in MCPServerConfig validation

Automated by CleverAgents Bot
Supervisor: Bug Hunter | Agent: new-issue-creator

🔗 **Orphan Issue — Manual Parent Epic Linking Required** No parent Epic was found for this issue during automated creation. Per CONTRIBUTING.md, orphan issues are not permitted — every issue must be linked to a parent Epic using Forgejo's dependency system (child **blocks** parent). A maintainer must: 1. Identify or create an appropriate parent Epic (e.g., a Security Hardening Epic or CI/CD Pipeline Epic) 2. Create the dependency link so that this issue **blocks** the parent Epic: ```bash curl -X POST "${FORGEJO_URL}/api/v1/repos/cleveragents/cleveragents-core/issues/7286/blocks" \ -H "Authorization: token <PAT>" \ -H "Content-Type: application/json" \ -d '{"owner": "cleveragents", "repo": "cleveragents-core", "index": <PARENT_EPIC_NUMBER>}' ``` Related security issues that may share a parent Epic: - #7280 BUG-HUNT: [security] JSON injection vulnerability in opencode-builder.sh - #7214 BUG-HUNT: [security] Path traversal vulnerability in file_tools.py - #7084 BUG-HUNT: [MCP-SECURITY] Path traversal vulnerability in SandboxPathRewriter - #7053 BUG-HUNT: [SECURITY] Code injection vulnerability in LangGraph function execution - #7038 BUG-HUNT: [MCP-SECURITY] Command injection vulnerability in MCPServerConfig validation --- **Automated by CleverAgents Bot** Supervisor: Bug Hunter | Agent: new-issue-creator

HAL9000 added the

labels

2026-04-10 18:44:26 +00:00

HAL9000 added

and removed

labels

2026-04-14 06:24:21 +00:00

HAL9000 commented

2026-04-14 06:24:21 +00:00

Author

Owner

✅ Verified — Security bug: command injection risk in check-quality-gates.py. MoSCoW: Must-have. Priority: High.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

✅ **Verified** — Security bug: command injection risk in check-quality-gates.py. MoSCoW: Must-have. Priority: High. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 commented

2026-04-14 06:29:59 +00:00

Author

Owner

✅ Verified — Security bug: command injection risk in check-quality-gates.py. MoSCoW: Must-have. Priority: High.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor