Replace eval/exec in dependent value resolution with safe resolver

[fthsrbst]

Module YAML content reached eval() during dependent-value resolution (find_and_replace_dependent_values in core/lib/base.py, replace_dependent_response in core/utils/common.py) and exec() built via .format() for responsetime conditions. Since \S+ captures arbitrary Python source, any module YAML could execute code (the PoC in #1651 runs __import__ before failing). Module files are shared data, so data-that-executes-code is worth closing.

What changed

• New fail-closed resolver in nettacker/core/utils/common.py: a strict finder regex accepts only <root>[<int>|'<key'>|"key"]... chains, an AST walk re-validates every slice as an int/str constant (bools rejected), and resolution is a plain index-path walk. Anything else is never evaluated.
• Both eval sites in base.py rewritten on top of it; the two globals().update(locals()) calls are gone.
• replace_dependent_response uses the same resolver with the original " ".join(...) rendering and error fallback.
• The responsetime branch uses an operator map instead of exec.

Compatibility

All 15 unique dependent_on_temp_event/response_dependent expressions shipped under nettacker/modules resolve to identical values before and after (verified expression-by-expression), including mid-string contexts in subdomain.yaml, waf.yaml, log4j_cve_2021_44228.yaml and geoserver_cve_2024_36401.yaml. A per-module test now asserts every dependent-value expression in shipped YAML stays inside the safe grammar.

Fixes #1651

Tests

• New tests/core/lib/test_base.py: shipped shapes, error fallback, hostile-payload inertness, frame-local leak regression.
• Resolver unit tests + rejected-form parametrization (issue payload, attribute access, name/binop/call subscripts, wrong root).
• All six responsetime operators truth-table tested; bad operator and non-numeric threshold covered.
• pytest tests/: 511 passed; the 3 failures reproduce identically on a clean checkout under macOS and are unrelated.

Commits are DCO-signed.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: aea5646a-c9b9-463f-a50a-e13721ed69c9

📥 Commits

Reviewing files that changed from the base of the PR and between c4a221b and 3c0acf7.

📒 Files selected for processing (7)

• nettacker/core/lib/base.py
• nettacker/core/lib/http.py
• nettacker/core/utils/common.py
• tests/core/lib/test_base.py
• tests/core/lib/test_user_added_http_headers.py
• tests/core/utils/test_common.py
• tests/test_yaml_schema_and_regex.py

---

Summary by CodeRabbit

• Bug Fixes

  • Improved dependent-value substitution across nested dictionaries, lists, and embedded text.
  • Unsupported or unresolved expressions now remain unchanged or use a safe fallback instead of being executed.
  • Response-time conditions now handle supported operators and invalid thresholds safely.

• Security

  • Prevented unsafe expression evaluation and potential code execution through dependent expressions.

• Tests

  • Added coverage for nested substitutions, invalid expressions, fallback behavior, response-time comparisons, and YAML validation.

Walkthrough

The change removes dynamic evaluation from dependent-value and response-time handling. It adds restricted AST-based expression resolution, recursive nested-value replacement, explicit response-time operator mapping, and tests for valid, invalid, and hostile inputs.

Changes

Safe dependency and response handling

Layer / File(s)	Summary
Restricted dependent-expression resolution nettacker/core/utils/common.py, tests/core/utils/test_common.py, tests/test_yaml_schema_and_regex.py	Dependent expressions now allow restricted integer and quoted-string indexing. Unsupported syntax and unresolved values use defined fallback behavior.
Nested dependent-value replacement nettacker/core/lib/base.py, tests/core/lib/test_base.py	BaseEngine replaces dependent expressions in strings and recursively processes dictionaries and lists without dynamic evaluation.
Validated response-time conditions nettacker/core/lib/http.py, tests/core/lib/test_user_added_http_headers.py	Response-time conditions use explicit comparison functions and numeric thresholds. Tests cover supported operators, boundaries, invalid operators, and malformed thresholds.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: arkid15r, securestep9

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Contribution validation failed:

• issue_assignee: The linked issue must be assigned to the PR author before this PR can be merged.

• commit_signature: One or more commits are not signed.

  • 3c0acf7

[Copilot]

Pull request overview

This pull request removes unsafe YAML-driven eval()/exec() usage through validated resolution and explicit operator dispatch.

Changes:

• Adds AST-validated dependent-expression resolution.
• Replaces dynamic response-time comparisons with an operator map.
• Adds security, regression, compatibility, grammar, and operator tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/test_yaml_schema_and_regex.py	Validates safe module expression grammar.
tests/core/utils/test_common.py	Tests resolver behavior and hostile inputs.
tests/core/lib/test_user_added_http_headers.py	Tests response-time operators.
tests/core/lib/test_base.py	Tests dependent-value resolution and regressions.
nettacker/core/utils/common.py	Implements safe expression parsing and resolution.
nettacker/core/lib/http.py	Replaces response-time exec() with operator dispatch.
nettacker/core/lib/base.py	Integrates safe dependent-value replacement.

Suppressed comments (2)

nettacker/core/utils/common.py:96

• Using result.replace for each match can corrupt longer chains when one expression is a prefix of another. For example, in root['a'] root['a']['b'], the shorter match is replaced first and turns the second occurrence into <value>['b'], so the longer expression is never resolved. Replace matches in one pass over the original string (optionally caching by match text) so each match is resolved against its original span.

    for expression in sorted(set(find_dependent_expressions(text, root_name))):
        try:
            rendered = renderer(resolve_dependent_value(expression, root_name, root_value))
        except Exception:
            rendered = default

tests/test_yaml_schema_and_regex.py:305

• This matcher can accept only a safe prefix of an unsafe chain. For example, dependent_on_temp_event[0][__import__('os').system('true')] matches dependent_on_temp_event[0]; re.sub then removes that prefix, so the assertion below never sees root[ and the unsafe YAML passes. The per-module guard therefore does not enforce the stated grammar for chained hostile payloads; validate complete occurrences (or reject allowed matches followed by another subscript/attribute) and add this regression case.

DEPENDENT_EXPRESSION_PATTERN = re.compile(
    r"dependent_on_temp_event(?:\[-?\d+\]|\['[^']*'\]|\[\"[^\"]*\"\])+"
)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.