fix: prevent PromQL injection in chaos_recommender prometheus queries

[1PoPTRoN]

Type of change

• Refactor
• New feature
• Bug fix
• Optimization

Description

Namespace, scrape_duration, and pod name values are interpolated directly into PromQL query strings via %s in chaos_recommender/prometheus.py. A malicious or malformed value like default"}[1m]) + sum(something_else{x=" could break query syntax or probe data from other namespaces.

This PR adds input validation using regex allowlisting based on Kubernetes and Prometheus naming rules:

• _validate_namespace() — enforces K8s namespace format
• _validate_scrape_duration() — enforces Prometheus duration format (e.g. 5m, 1h)
• _validate_pod_name() — enforces K8s pod name format

Invalid inputs are rejected with a ValueError before reaching any PromQL query.

Documentation

• Is documentation needed for this update?

Checklist before requesting a review

• Ensure the changes and proposed solution have been discussed in the relevant issue and have received acknowledgment from the community or maintainers.
• I have performed a self-review of my code by running krkn and specific scenario
• If it is a core feature, I have added thorough unit tests with above 80% coverage

REQUIRED:

python -m unittest tests.test_chaos_recommender_prometheus -v

test_dots_rejected (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_dots_rejected) ... ok
test_empty_string (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_empty_string) ... ok
test_ends_with_hyphen_rejected (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_ends_with_hyphen_rejected) ... ok
test_injection_closing_brace (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_injection_closing_brace) ... ok
test_injection_special_chars (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_injection_special_chars) ... ok
test_max_length_accepted (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_max_length_accepted) ... ok
test_starts_with_hyphen_rejected (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_starts_with_hyphen_rejected) ... ok
test_too_long_rejected (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_too_long_rejected) ... ok
test_underscores_rejected (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_underscores_rejected) ... ok
test_uppercase_rejected (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_uppercase_rejected) ... ok
test_valid_namespaces (tests.test_chaos_recommender_prometheus.TestValidateNamespace.test_valid_namespaces) ... ok
test_empty_string (tests.test_chaos_recommender_prometheus.TestValidatePodName.test_empty_string) ... ok
test_ends_with_hyphen_rejected (tests.test_chaos_recommender_prometheus.TestValidatePodName.test_ends_with_hyphen_rejected) ... ok
test_injection_attempt (tests.test_chaos_recommender_prometheus.TestValidatePodName.test_injection_attempt) ... ok
test_starts_with_hyphen_rejected (tests.test_chaos_recommender_prometheus.TestValidatePodName.test_starts_with_hyphen_rejected) ... ok
test_uppercase_rejected (tests.test_chaos_recommender_prometheus.TestValidatePodName.test_uppercase_rejected) ... ok
test_valid_pod_names (tests.test_chaos_recommender_prometheus.TestValidatePodName.test_valid_pod_names) ... ok
test_empty_string (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_empty_string) ... ok
test_float_rejected (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_float_rejected) ... ok
test_injection_attempt (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_injection_attempt) ... ok
test_invalid_unit (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_invalid_unit) ... ok
test_missing_number (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_missing_number) ... ok
test_missing_unit (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_missing_unit) ... ok
test_valid_durations (tests.test_chaos_recommender_prometheus.TestValidateScrapeDuration.test_valid_durations) ... ok

----------------------------------------------------------------------
Ran 24 tests in 0.000s

OK

[qodo-code-review]

Review Summary by Qodo

Prevent PromQL injection in chaos_recommender prometheus queries

🐞 Bug fix

Walkthroughs

Description

• Prevents PromQL injection attacks by validating namespace, pod name, and scrape duration inputs
• Adds three validation functions with regex-based allowlisting for Kubernetes and Prometheus naming
  rules
• Integrates validation checks into query functions before string interpolation
• Includes comprehensive unit test suite with 26 test cases covering valid inputs and injection
  attempts

Diagram

flowchart LR
  Input["User Input<br/>namespace/pod/duration"] --> Validate["Validation Functions<br/>_validate_namespace<br/>_validate_pod_name<br/>_validate_scrape_duration"]
  Validate --> Check{"Regex Match<br/>Valid?"}
  Check -->|No| Reject["Raise ValueError"]
  Check -->|Yes| Query["Safe PromQL Query<br/>Interpolation"]
  Query --> Result["Query Execution"]

Loading

File Changes

1. krkn/chaos_recommender/prometheus.py 🐞 Bug fix +38/-0

Add input validation to prevent PromQL injection

• Added three regex patterns for validating Kubernetes namespace, pod name, and Prometheus duration
 formats
• Implemented _validate_namespace(), _validate_scrape_duration(), and _validate_pod_name()
 validation functions
• Integrated validation calls in get_node_capacity() for pod names and
 fetch_utilization_from_prometheus() for scrape duration and namespaces
• Validation occurs before any PromQL query string interpolation to prevent injection attacks

krkn/chaos_recommender/prometheus.py

---

2. tests/test_chaos_recommender_prometheus.py 🧪 Tests +146/-0

Add comprehensive test suite for PromQL injection prevention

• Created new test file with 26 unit tests covering all three validation functions
• Tests validate correct acceptance of valid inputs (namespaces, pod names, durations)
• Tests verify rejection of injection attempts and malformed inputs
• Tests cover edge cases like empty strings, length limits, invalid characters, and special
 characters
• Mocks heavy dependencies to isolate validation logic testing

tests/test_chaos_recommender_prometheus.py

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Global sys.modules pollution 🐞 Bug ☼ Reliability

Description

The new test module mutates sys.modules at import time to insert MagicMocks for
urllib3/pandas/prometheus_api_client, and those mocks persist for the entire test process. This can
break other tests that import the real urllib3 module (or rely on real pandas) later in the same
run.

Code

tests/test_chaos_recommender_prometheus.py[R17-26]

+# Mock heavy dependencies that are not needed for validation tests
+sys.modules.setdefault("prometheus_api_client", MagicMock())
+sys.modules.setdefault("pandas", MagicMock())
+sys.modules.setdefault("urllib3", MagicMock())
+
+from krkn.chaos_recommender.prometheus import (  # noqa: E402
+    _validate_namespace,
+    _validate_pod_name,
+    _validate_scrape_duration,
+)

Evidence

The test file sets sys.modules entries before importing the module under test, which globally
changes import behavior for the rest of the test session. Another test explicitly imports and
patches urllib3.exceptions, which will fail or behave incorrectly if urllib3 was replaced with a
MagicMock earlier.

tests/test_chaos_recommender_prometheus.py[13-26]
tests/test_logging_and_code_quality.py[63-67]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`tests/test_chaos_recommender_prometheus.py` currently inserts MagicMocks into `sys.modules` at module import time. This pollutes the interpreter-wide module cache and can break other tests that expect the real modules (notably `urllib3`).

### Issue Context
The goal is only to avoid importing heavy dependencies while importing `krkn.chaos_recommender.prometheus` for validation tests.

### Fix Focus Areas
- tests/test_chaos_recommender_prometheus.py[13-26]

### Suggested fix
- Replace the top-level `sys.modules.setdefault(...)` calls with a scoped patch around the import of `krkn.chaos_recommender.prometheus`, e.g. `unittest.mock.patch.dict(sys.modules, {...})`, and do the import inside that context.
- Do **not** mock `urllib3` globally (other tests import `urllib3.exceptions`). If you truly need to mock it for this file, do it only inside the scoped patch used during the import of the module under test.
- Example pattern:
 - Use `with patch.dict(sys.modules, {"prometheus_api_client": MagicMock(), "pandas": MagicMock()}):`
 - Then `import importlib; prom = importlib.import_module("krkn.chaos_recommender.prometheus")`
 - Reference `prom._validate_namespace` etc.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Empty namespaces now fatal 🐞 Bug ≡ Correctness

Description

fetch_utilization_from_prometheus() now validates each namespace and raises ValueError on empty
strings, but the config parser can generate empty namespace entries via re.split() and main() only
checks list truthiness. A config like "default," (trailing comma) can now abort execution
unexpectedly.

Code

krkn/chaos_recommender/prometheus.py[R154-157]

+    _validate_scrape_duration(scrape_duration)
+    for ns in namespaces:
+        _validate_namespace(ns)
+

Evidence

Namespace splitting from config uses re.split() without filtering, which can yield empty-string
elements. main() only checks if not namespaces, so a list containing empty strings passes. The new
validation loop then calls _validate_namespace(''), which fails because the namespace regex requires
at least one character.

utils/chaos_recommender/chaos_recommender.py[134-136]
utils/chaos_recommender/chaos_recommender.py[287-290]
krkn/chaos_recommender/prometheus.py[149-157]
krkn/chaos_recommender/prometheus.py[11-24]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Config-driven namespace parsing can produce empty namespace entries (e.g., trailing commas or extra whitespace). With the new namespace validator, those empty entries now raise `ValueError` during `fetch_utilization_from_prometheus`, aborting runs that previously proceeded.

### Issue Context
`read_configuration()` splits the namespaces string but does not remove empty tokens; `main()` only checks `if not namespaces` (list-level), not whether entries are non-empty.

### Fix Focus Areas
- utils/chaos_recommender/chaos_recommender.py[134-136]
- utils/chaos_recommender/chaos_recommender.py[287-290]
- krkn/chaos_recommender/prometheus.py[154-156]

### Suggested fix
- After splitting namespaces in `read_configuration()`, strip and filter empties, e.g.:
 - `namespaces = [ns.strip() for ns in namespaces if ns and ns.strip()]`
- Optionally, strengthen the `main()` validation to reject lists that become empty after filtering (so the error message is clearer: “No namespaces provided”).
- Keep the allowlist validation in `fetch_utilization_from_prometheus()` as the final guard.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[Copilot]

Pull request overview

This PR mitigates PromQL injection risk in the chaos recommender by validating user-provided inputs before interpolating them into Prometheus query strings.

Changes:

• Added allowlist-based validators for namespace, pod name, and scrape duration inputs.
• Integrated validation into chaos_recommender/prometheus.py before constructing PromQL queries.
• Added unit tests covering accepted and rejected inputs (including injection-shaped strings).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
krkn/chaos_recommender/prometheus.py	Calls new validators prior to building PromQL queries (but currently introduces a runtime NameError due to a removed constant).
krkn/chaos_recommender/validators.py	Introduces regex-based input validators used to block unsafe PromQL interpolation.
tests/test_chaos_recommender_prometheus.py	Adds unit tests to verify validators reject malformed/injection-like values.

[1PoPTRoN]

Hey @paigerube14 @chaitanyaenr,
I've opened a PR to address the PromQL injection vulnerability in chaos_recommender/prometheus.py. Namespace, scrape_duration, and pod name values were being interpolated directly into PromQL query strings without any sanitization, a crafted input could break query syntax or probe data from other namespaces. Added regex-based allowlist validators in a separate module along with 24 unit tests. Would appreciate your review when you get a chance, this one's a real security risk so I wanted to make sure it's on your radar!