fix(security): replace bare eval() with simpleeval in recipe conditions

[rysweet]

Summary

• Replace unsafe eval() in Step.evaluate_condition() with simpleeval.EvalWithCompoundTypes to eliminate arbitrary code execution risk from recipe condition strings
• Add simpleeval>=0.9.13 to project dependencies
• Add security regression tests verifying __import__() and open() are blocked by the new evaluator

Closes #3485

Changes

File	Change
src/amplihack/recipes/models.py	Replace eval() with EvalWithCompoundTypes, remove # noqa: S307
pyproject.toml	Add simpleeval>=0.9.13 dependency
uv.lock	Lock file updated (simpleeval v1.0.7 added)
tests/unit/recipes/test_models.py	Add 3 security tests (test_in_operator, test_import_blocked, test_open_blocked); consolidate imports

Test plan

• All 24 existing + new tests pass (pytest tests/unit/recipes/test_models.py — 24/24)
• Boolean coercion (True -> 'true') still works
• Numeric comparisons (count >= 4) still work
• is None checks still work
• in operator works ('API' in description)
• __import__('os').system(...) is blocked (returns True via default-on-error)
• open('/etc/passwd').read() is blocked
• amplihack-memory-lib version unchanged at v0.2.0 in uv.lock
• Pre-commit hooks pass (ruff, ruff-format, pyright, detect-secrets)

🤖 Generated with Claude Code

[github-actions]

Repo Guardian - Passed

All 4 changed files are durable, reusable project artifacts. No ephemeral content or temporary scripts were detected.

File	Assessment
pyproject.toml	✅ Dependency configuration — durable
src/amplihack/recipes/models.py	✅ Source code — durable security fix
tests/unit/recipes/test_models.py	✅ Unit tests — durable test coverage
uv.lock	✅ Dependency lock file — durable

Generated by Repo Guardian for issue #3486 · ◷

[rysweet]

PR #3486 Validation Report

1. Scope Check: PASS

Files changed (exactly 4, as expected):

• pyproject.toml — adds simpleeval>=0.9.13 dependency (+1 line)
• src/amplihack/recipes/models.py — replaces eval() with EvalWithCompoundTypes (+5/-3 lines)
• tests/unit/recipes/test_models.py — adds 3 security tests, consolidates imports (+18/-4 lines)
• uv.lock — locks simpleeval v1.0.7 (+11 lines)

amplihack-memory-lib check: NOT modified in uv.lock diff (confirmed via git diff origin/main...origin/fix/eval-security-3485 -- uv.lock | grep amplihack-memory-lib — empty output).

No unrelated changes detected.

2. QA-Team Outside-In Tests: PASS (13/13)

Two gadugi YAML scenarios written and executed via companion pytest file:

Scenario 1: Recipe conditions still evaluate correctly (simpleeval-condition-evaluation.yaml)

• Boolean coercion (True -> 'true' string comparison)
• Numeric comparison (count >= 4)
• String in operator ('API' in description)
• Compound expression (force == 'true' and count >= 2)
• Source verification: EvalWithCompoundTypes present, bare eval() pattern removed, noqa: S307 removed

Scenario 2: Malicious eval attempts blocked (simpleeval-malicious-eval-blocked.yaml)

• __import__('os').system('echo pwned') — blocked (returns True via default-on-error)
• open('/etc/passwd').read() — blocked
• ().__class__.__bases__[0].__subclasses__() — blocked
• Canary file-write payload — confirmed file NOT created (no arbitrary code execution)
• Source verification: old __builtins__ sandbox removed

Execution evidence:

tests/gadugi/test_simpleeval_scenarios.py — 13 passed in 0.09s
tests/unit/recipes/test_models.py — 24 passed in 0.11s

3. Quality Audit (3 Cycles)

Cycle 1 — SEEK: Reviewed the diff for correctness and security properties.

• EvalWithCompoundTypes is the correct simpleeval class for compound types (dicts, lists)
• names=eval_ctx correctly passes the context namespace
• evaluator.eval(self.condition.strip()) correctly evaluates the expression
• Error handling preserved: except Exception catches simpleeval errors and defaults to True
• Boolean coercion logic unchanged (pre-processing step before eval)
• No new noqa suppressions added

Cycle 1 — VALIDATE: No issues found. The implementation is a clean drop-in replacement.

Cycle 2 — SEEK: Checked for behavioral differences between eval() and simpleeval.

• eval() with {"__builtins__": {}} was a weak sandbox (bypassed via __subclasses__)
• simpleeval blocks: attribute access on dunder methods, builtins, imports, file I/O
• simpleeval supports: in, not in, and, or, ==, >=, is None, string/numeric/bool operations
• All existing recipe condition patterns in the codebase are supported by simpleeval
• evaluate_condition is called on Step objects; interface (signature, return type, error behavior) is unchanged

Cycle 2 — VALIDATE: No regression risk. simpleeval supports all operators used in recipe conditions.

Cycle 3 — SEEK: Checked dependency and lock file quality.

• simpleeval>=0.9.13 allows any version from 0.9.13+ (locked to 1.0.7)
• simpleeval is a pure-Python package (py3-none-any wheel), no native dependencies
• No version conflicts with existing dependencies
• type: ignore[import-untyped] is appropriate — simpleeval has no py.typed marker

Cycle 3 — VALIDATE: Dependency addition is clean. No issues.

FIX: No fixes needed across all 3 cycles.

4. CI Status: PASS (with expected exception)

Check	Status
Validate Code (CI)	PASS
GitGuardian Security Checks	PASS
Bot Detection	PASS
Claude Code Plugin Test	PASS
Drift Detection	PASS
Repo Guardian	PASS
Root Directory Hygiene	PASS
Version Check	PASS
Dependabot Auto-Merge	PASS
Atlas PR Impact Check	FAIL (pre-existing)

Atlas failure explanation: The Atlas CI reports layers 2 (compile-deps) and 8 (ast-lsp-bindings) as stale because the PR touches pyproject.toml and src/amplihack/recipes/models.py. This is a pre-existing atlas staleness issue (atlas was not rebuilt after #3465 merged). The failure is NOT caused by this PR's logic — it is a documentation/diagram freshness gate, not a code correctness check.

5. Verdict: READY TO MERGE

All merge criteria pass:

• Scope: exactly 4 files, no unrelated changes, no amplihack-memory-lib downgrade
• Outside-in tests: 13/13 pass (condition evaluation + malicious eval blocking)
• Unit tests: 24/24 pass
• Quality audit: 3 cycles, 0 issues found
• CI: all checks green except pre-existing Atlas staleness (not caused by this PR)

[github-actions]

PR Triage Report

Risk: Medium | Priority: High | Security Fix

Assessment

• Type: Security fix — replaces unsafe eval() with simpleeval in recipe condition evaluation
• Scope: 4 files — models.py, pyproject.toml, uv.lock, tests/unit/recipes/test_models.py
• CI Status: Passing — Validate Code ✅, GitGuardian ✅ (Atlas PR Impact Check non-blocking)
• Test Coverage: 24 tests pass including 3 new security regression tests

Findings

• Closes a real arbitrary code execution risk in Step.evaluate_condition()
• simpleeval>=0.9.13 is a well-established library with active maintenance
• Security regression tests confirm __import__() and open() calls are blocked
• All existing condition patterns preserved: boolean coercion, numeric comparisons, is None, in operator

Recommendation

🔒 Security review then merge. This closes a genuine code execution vulnerability. The fix is conservative (simpleeval is well-known), tests verify both the safety boundary and functional compatibility, and the change is minimal in scope.

Generated by PR Triage Agent · ◷