fix(library): correct fail-open, payload-leak, and manifest defects in built-in rails - #2257
Open
Pouyanpi wants to merge 10 commits into
Open
fix(library): correct fail-open, payload-leak, and manifest defects in built-in rails#2257Pouyanpi wants to merge 10 commits into
Pouyanpi wants to merge 10 commits into
Conversation
_load_rules returned None on yara.SyntaxError, and the caller treats None as 'no rules configured' and returns a non-injection outcome. A single malformed rule therefore silently disabled injection detection for every bot message, logging an error nobody reads while the guardrail reported clean. Raise ValueError instead, matching the four existing config-error sites in the same module. The runtime turns the raise into an internal-error response, so unchecked model output no longer reaches the caller. test_malformed_inline_yara_rule_fails_gracefully asserted the fail-open behavior directly, so it is renamed and inverted: it now asserts the model output does not reach the caller. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Three raise sites interpolated response.text or a caught exception's str into the error message. Clavata echoes the evaluated user or bot text back in its job results, so the checked content reached logs, tracebacks, and any error reporting downstream. This is the pattern the rail contribution guide forbids, in the rail it names as the HTTP archetype. Keep the status code, which is the actionable part, and drop the bodies. The validation error becomes a count and the catch-all reports the exception type name instead of its message. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
_retrying_http_client passed random_value=lambda: 1.0, which pins the full jitter draw to its cap. Every F5 backoff was therefore the full delay and every client retried in lockstep, which is what jitter exists to prevent. clavata uses the default. The lambda was there to make one test's delay assertions deterministic; that belongs in the test. RetryingHTTPClient's random_value default was bound at definition time, so a test could not patch it. Make it late-bound (None resolves to random.random at construction) and have the one test that asserts exact delays pin the draw itself. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
…ements The manifest's RailSpec had no requirements block at all, so a rail that is a model judge and imports fast_langdetect declared neither. Peer model rails (topic_safety, llama_guard) declare a ModelRequirement, and fast-langdetect already ships in the multilingual extra. This also matters because the rail contribution guide names content_safety as the model-backed archetype to copy, so an empty requirements block propagates. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
…n messages Both Colang 1 flows built their exception message from $policy and $text, neither of which is bound in those flows: they read $user_message and $bot_message and never define $policy. No test executes either branch, so the broken interpolation shipped. Use the fixed messages the Colang 2 flows already use for the same two surfaces. The parameterized Colang 2 flow does bind both, but interpolating $text put the checked user or bot text into an exception message, which the telemetry rule forbids. Keep the policy, drop the text. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
_CLAVATA_API_KEY captured os.environ at module import, so a key set after import (which is what monkeypatch.setenv does in tests, and what any runtime credential refresh does) was invisible on that path. The ClavataClient constructor already reads the environment correctly, leaving the module global as a stale second source of the same secret. Read the environment in AuthHeader.to_headers instead and drop the global. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Both dialect files sent ContentSafetyCheckOuputException, missing the 't' in Output, while the input counterparts spell theirs correctly and docs/configure-rails/exceptions.mdx documents ContentSafetyCheckOutputException. Anything filtering on the documented name never fired. This changes a public event name, so it is a compatibility change, but the shipped name never matched its documentation. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
…g 1 flows The Colang 2 flows take $model as a flow parameter; Colang 1 has no parameterized flows, so both v1 exception messages interpolated a variable that is never bound. Name the flow without the model in the v1 messages. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
A ClavataPluginValueError from get_labels was caught and turned into labels=None with no record, which silently switches the decision from label matching to whole-policy matching. The fallback is intentional, but an operator debugging why a label rule never fires had no signal. Log it at debug. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
…metadata Both rails built outcome metadata with dict(result), and both result TypedDicts carry the full checked text. Outcome metadata reaches ExecutedAction.return_value in the processing log and any tracing exporter, so every scanned user message, bot message, or retrieved chunk was copied into observability output. polygraf deliberately excludes values for this reason. Build the metadata explicitly from the decision fields instead. No flow reads the text key; injection detection's transform still takes its rewrite from the result, not from metadata. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Pouyanpi
added
the
status: triaged
github-actions
Bot
added
status: needs triage
Contributor
📝 WalkthroughWalkthroughThe changes add a default retry jitter provider, improve Clavata fallback and error handling, declare content-safety requirements, correct exception names, remove checked text from detection metadata, and fail closed on malformed YARA rules. ChangesRetry and rail hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_regex_detection.py (1)
690-691: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the regex outcome implementation in the metadata test.
The test constructs
RailOutcomeobjects directly and only checksis_blocked. It passes even if_regex_outcomestill includestext. Call_regex_outcomeordetect_regex_pattern, then assert that metadata contains onlyis_match,detections, andsource.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_regex_detection.py` around lines 690 - 691, Update the metadata test around the directly constructed RailOutcome fixtures to exercise the actual regex outcome path via _regex_outcome or detect_regex_pattern. Assert that the resulting metadata contains exactly is_match, detections, and source, while preserving the existing blocked/allowed behavior checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nemoguardrails/library/clavata/request.py`:
- Line 66: Update ClavataClient.__init__ to store only an explicitly provided
api_key and stop copying CLAVATA_API_KEY into self.api_key. Ensure _get_headers
passes that value to AuthHeader.to_headers so the method resolves the current
environment key on every request, including after key rotation.
In `@nemoguardrails/library/injection_detection/actions.py`:
- Around line 198-201: Update the Raises documentation for _load_rules to state
that invalid YARA rules and compilation failures are raised as ValueError,
replacing the outdated yara.SyntaxError entry. Keep the existing exception
conversion and caller behavior unchanged.
- Around line 62-68: Update the RailOutcome.metadata documentation in
nemoguardrails/library/injection_detection/actions.py (lines 62-68) to remove
text and document is_injection, detections, and action; update
nemoguardrails/library/regex/actions.py (lines 33-35) to remove text and
document is_match, detections, and source. No downstream metadata consumer
changes are needed.
---
Nitpick comments:
In `@tests/test_regex_detection.py`:
- Around line 690-691: Update the metadata test around the directly constructed
RailOutcome fixtures to exercise the actual regex outcome path via
_regex_outcome or detect_regex_pattern. Assert that the resulting metadata
contains exactly is_match, detections, and source, while preserving the existing
blocked/allowed behavior checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 95a303d8-1360-413d-8c00-14bc3a766eda
📒 Files selected for processing (14)
nemoguardrails/http/retry.pynemoguardrails/library/clavata/actions.pynemoguardrails/library/clavata/flows.conemoguardrails/library/clavata/flows.v1.conemoguardrails/library/clavata/request.pynemoguardrails/library/content_safety/flows.conemoguardrails/library/content_safety/flows.v1.conemoguardrails/library/content_safety/rail.pynemoguardrails/library/f5/actions.pynemoguardrails/library/injection_detection/actions.pynemoguardrails/library/regex/actions.pytests/test_f5_guardrails.pytests/test_injection_detection.pytests/test_regex_detection.py
💤 Files with no reviewable changes (1)
- nemoguardrails/library/f5/actions.py
| Converts the auth token into request headers. | ||
| """ | ||
| api_key = self.api_key or _CLAVATA_API_KEY | ||
| api_key = self.api_key or os.environ.get("CLAVATA_API_KEY") |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not cache the environment key in ClavataClient.
AuthHeader.to_headers now reads CLAVATA_API_KEY, but ClavataClient.__init__ still stores the environment value in self.api_key at Line 160. _get_headers passes that cached value at Line 171, so the fallback at Line 66 is not reached after a key rotation. A reused client sends the old credential.
Store only an explicitly supplied api_key in ClavataClient. Let AuthHeader.to_headers resolve the environment variable for each request. This conflicts with the PR objective that the key is read at call time.
Proposed fix
- self.api_key = api_key or os.environ.get("CLAVATA_API_KEY")
- if self.api_key is None:
- raise ClavataPluginConfigurationError(...)
+ self.api_key = api_key🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nemoguardrails/library/clavata/request.py` at line 66, Update
ClavataClient.__init__ to store only an explicitly provided api_key and stop
copying CLAVATA_API_KEY into self.api_key. Ensure _get_headers passes that value
to AuthHeader.to_headers so the method resolves the current environment key on
every request, including after key rotation.
Comment on lines
+62
to
+68
| # the checked text is deliberately excluded: outcome metadata reaches | ||
| # processing logs and tracing exporters | ||
| metadata = { | ||
| "is_injection": result["is_injection"], | ||
| "detections": result["detections"], | ||
| "action": action_option, | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'metadata.*text|_injection_detection_outcome|_regex_outcome' nemoguardrails tests --glob '*.py'Repository: NVIDIA-NeMo/Guardrails
Length of output: 31707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository guidance ---'
find nemoguardrails -name AGENTS.md -print -exec cat {} \;
printf '%s\n' '--- injection detection implementation and tests ---'
sed -n '1,125p' nemoguardrails/library/injection_detection/actions.py
sed -n '1,145p' tests/test_injection_detection.py
printf '%s\n' '--- regex implementation and related tests/docs ---'
sed -n '1,125p' nemoguardrails/library/regex/actions.py
rg -n -C 4 'regex_detection|_regex_outcome|is_match|detections.*source|source.*detections' nemoguardrails tests docs README.md --glob '*.py' --glob '*.md' 2>/dev/null || true
printf '%s\n' '--- exact metadata text consumers ---'
rg -n -C 3 'metadata\s*\[\s*["'\'']text["'\'']\s*\]|metadata\.get\(\s*["'\'']text["'\'']|["'\'']text["'\'']\s*:\s*.*metadata|metadata.*text' nemoguardrails tests --glob '*.py' || true
printf '%s\n' '--- current diff for the two implementations ---'
git diff -- nemoguardrails/library/injection_detection/actions.py nemoguardrails/library/regex/actions.pyRepository: NVIDIA-NeMo/Guardrails
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- injection action documentation ---'
rg -n -A35 -B5 'def (detect|check).*injection|InjectionDetectionResult|injection detection' \
nemoguardrails/library/injection_detection/actions.py
printf '%s\n' '--- regex tests and metadata assertions ---'
rg -n -C 5 'detect_regex_pattern|metadata|is_match|detections|source' \
tests/test_regex_detection.py tests/test_runtime_flow_gate_equivalence.py \
tests/recorded --glob '*.py' | head -n 260
printf '%s\n' '--- focused diff status ---'
git diff --stat -- nemoguardrails/library/injection_detection/actions.py nemoguardrails/library/regex/actions.py tests
git diff --unified=15 -- nemoguardrails/library/injection_detection/actions.py nemoguardrails/library/regex/actions.py
printf '%s\n' '--- AST-based metadata consumer and documentation scan ---'
python3 - <<'PY'
import ast
from pathlib import Path
roots = [Path("nemoguardrails"), Path("tests")]
for root in roots:
for path in root.rglob("*.py"):
try:
tree = ast.parse(path.read_text())
except Exception:
continue
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef) and node.name in {
"detect_regex_pattern",
"_injection_detection_outcome",
"_regex_outcome",
}:
print(f"{path}:{node.lineno} {node.name}")
print(" docstring:", repr(ast.get_docstring(node)))
if isinstance(node, ast.Subscript):
target = node.value
if isinstance(target, ast.Name) and target.id == "metadata":
sl = node.slice
if isinstance(sl, ast.Constant) and sl.value == "text":
print(f"{path}:{node.lineno} metadata['text'] read")
if isinstance(node, ast.Call):
for kw in node.keywords:
if kw.arg == "metadata" and isinstance(kw.value, ast.Dict):
keys = [
k.value for k in kw.value.keys
if isinstance(k, ast.Constant) and isinstance(k.value, str)
]
if "text" in keys:
print(f"{path}:{node.lineno} metadata literal keys={keys}")
PYRepository: NVIDIA-NeMo/Guardrails
Length of output: 32083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- public action docstrings ---'
sed -n '300,350p' nemoguardrails/library/injection_detection/actions.py
sed -n '42,66p' nemoguardrails/library/regex/actions.py
rg -n -F 'The original text that was checked' . --glob '*.py' --glob '*.md' || true
printf '%s\n' '--- equivalence helper and injection/regex cases ---'
sed -n '840,930p' tests/test_runtime_flow_gate_equivalence.py
sed -n '1140,1180p' tests/test_runtime_flow_gate_equivalence.py
sed -n '1428,1490p' tests/test_runtime_flow_gate_equivalence.py
rg -n -A35 -B10 'assert .*metadata|expected_outcome|actual_outcome|RailOutcome' \
tests/test_runtime_flow_gate_equivalence.py | tail -n 180
printf '%s\n' '--- metadata reads in runtime and tests ---'
python3 - <<'PY'
import ast
from pathlib import Path
for path in [*Path("nemoguardrails").rglob("*.py"), *Path("tests").rglob("*.py")]:
try:
tree = ast.parse(path.read_text())
except Exception:
continue
for node in ast.walk(tree):
if not isinstance(node, ast.Subscript):
continue
key = node.slice
is_text = isinstance(key, ast.Constant) and key.value == "text"
if not is_text:
continue
base = ast.unparse(node.value)
if "metadata" in base or "outcome" in base or "result" in base:
print(f"{path}:{node.lineno}: {base}['text']")
PYRepository: NVIDIA-NeMo/Guardrails
Length of output: 16631
Update the RailOutcome.metadata documentation.
Remove text from the injection_detection and detect_regex_pattern return documentation. Document the actual fields: is_injection, detections, and action for injection detection; is_match, detections, and source for regex detection. No downstream metadata["text"] consumer requires a code change.
📍 Affects 2 files
nemoguardrails/library/injection_detection/actions.py#L62-L68(this comment)nemoguardrails/library/regex/actions.py#L33-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nemoguardrails/library/injection_detection/actions.py` around lines 62 - 68,
Update the RailOutcome.metadata documentation in
nemoguardrails/library/injection_detection/actions.py (lines 62-68) to remove
text and document is_injection, detections, and action; update
nemoguardrails/library/regex/actions.py (lines 33-35) to remove text and
document is_match, detections, and source. No downstream metadata consumer
changes are needed.
Source: Coding guidelines
Comment on lines
198
to
+201
| except yara.SyntaxError as e: | ||
| msg = f"Failed to initialize injection detection due to configuration or YARA rule error: YARA compilation failed: {e}" | ||
| log.error(msg) | ||
| return None | ||
| raise ValueError(msg) from e |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'yara\.SyntaxError|_load_rules|injection_detection' nemoguardrails tests --glob '*.py'Repository: NVIDIA-NeMo/Guardrails
Length of output: 49424
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- AGENTS.md files ---'
find nemoguardrails -name AGENTS.md -print -exec cat {} \;
printf '%s\n' '--- actions.py documentation and exception flow ---'
sed -n '70,215p' nemoguardrails/library/injection_detection/actions.py
sed -n '315,355p' nemoguardrails/library/injection_detection/actions.py
printf '%s\n' '--- exception handling and documentation references ---'
rg -n -C 4 'except (ValueError|yara\.SyntaxError)|raises:|Raises:|YARA compilation|_load_rules\(' nemoguardrails tests --glob '*.py'Repository: NVIDIA-NeMo/Guardrails
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused injection tests ---'
sed -n '700,770p' tests/test_injection_detection.py
printf '%s\n' '--- action-dispatch files ---'
fd -t f -i 'action.*dispatcher|dispatcher.*action' nemoguardrails
printf '%s\n' '--- injection action references outside its implementation ---'
rg -n 'library\.injection_detection\.actions|actions:injection_detection|_load_rules|yara\.SyntaxError' nemoguardrails tests --glob '*.py' \
| rg -v 'nemoguardrails/library/injection_detection/actions\.py'
printf '%s\n' '--- targeted exception handlers in the runtime ---'
rg -n -C 3 'execute_action|run_action|dispatch.*action|action.*exception|except Exception|except ValueError' nemoguardrails/actions nemoguardrails/colang nemoguardrails/rails --glob '*.py' \
| head -n 240
printf '%s\n' '--- AST-based documentation and raise verifier ---'
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("nemoguardrails/library/injection_detection/actions.py")
tree = ast.parse(path.read_text())
functions = {
node.name: node
for node in ast.walk(tree)
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
}
for name in ("_load_rules", "injection_detection"):
node = functions[name]
doc = ast.get_docstring(node) or ""
raises = [n for n in ast.walk(node) if isinstance(n, ast.Raise)]
print(f"{name}: documented_yara_syntax_error={'yara.SyntaxError' in doc}, "
f"documented_value_error={'ValueError' in doc}, raise_count={len(raises)}")
load = functions["_load_rules"]
for node in ast.walk(load):
if isinstance(node, ast.ExceptHandler):
print("load_rules_handler:", ast.unparse(node.type) if node.type else "bare")
for child in ast.walk(node):
if isinstance(child, ast.Raise):
print("load_rules_raise:", ast.unparse(child.exc) if child.exc else "bare")
PYRepository: NVIDIA-NeMo/Guardrails
Length of output: 23636
Document YARA compilation failures as ValueError.
_load_rules converts yara.SyntaxError to ValueError, but its Raises section still lists yara.SyntaxError. Clarify that injection_detection raises ValueError for invalid YARA rules. Callers handle the failure generically and do not require yara.SyntaxError handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nemoguardrails/library/injection_detection/actions.py` around lines 198 -
201, Update the Raises documentation for _load_rules to state that invalid YARA
rules and compilation failures are raised as ValueError, replacing the outdated
yara.SyntaxError entry. Keep the existing exception conversion and caller
behavior unchanged.
Source: Coding guidelines
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Pouyanpi
added
status: triaged
Contributor
Greptile SummaryThis PR hardens several built-in rails against fail-open behavior and sensitive-content leakage while correcting retry, manifest, and cross-dialect flow defects.
|
| Filename | Overview |
|---|---|
| nemoguardrails/library/injection_detection/actions.py | Fails closed on YARA compilation errors and removes checked text from outcome metadata; the return docstring still advertises the removed field. |
| nemoguardrails/library/clavata/request.py | Resolves credentials at client construction and redacts vendor response and validation details from exceptions. |
| nemoguardrails/library/content_safety/rail.py | Declares the required content-safety model and multilingual installation extra as manifest metadata. |
| nemoguardrails/http/retry.py | Late-binds the default jitter source while preserving explicit injectable randomness. |
| nemoguardrails/library/regex/actions.py | Removes checked text from outcome metadata but leaves the documented return shape stale. |
| nemoguardrails/library/content_safety/flows.co | Corrects the output exception event spelling for Colang 2.x. |
| nemoguardrails/library/content_safety/flows.v1.co | Corrects exception events and removes references to unbound model variables in Colang 1.0. |
Prompt To Fix All With AI
### Issue 1
nemoguardrails/library/regex/actions.py:35
**Metadata docs retain removed text**
The regex and injection-detection outcome helpers now omit checked text, but their action docstrings still advertise `metadata["text"]`; custom consumers following that documented return shape can access a nonexistent field, and generated documentation continues to promise sensitive data that is deliberately no longer returned.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(regex,injection-detection): keep the..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| metadata["source"] = source | ||
| # the checked text is deliberately excluded: outcome metadata reaches | ||
| # processing logs and tracing exporters | ||
| metadata = {"is_match": result["is_match"], "detections": result["detections"], "source": source} |
Contributor
There was a problem hiding this comment.
The regex and injection-detection outcome helpers now omit checked text, but their action docstrings still advertise metadata["text"]; custom consumers following that documented return shape can access a nonexistent field, and generated documentation continues to promise sensitive data that is deliberately no longer returned.
Knowledge Base Used: Library Rails
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/regex/actions.py
Line: 35
Comment:
**Metadata docs retain removed text**
The regex and injection-detection outcome helpers now omit checked text, but their action docstrings still advertise `metadata["text"]`; custom consumers following that documented return shape can access a nonexistent field, and generated documentation continues to promise sensitive data that is deliberately no longer returned.
**Knowledge Base Used:** [Library Rails](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia-nemo/guardrails/-/docs/library-rails.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Ten fixes to built-in rails, found by auditing the rails that the rail
contribution guide holds up as exemplars. Each is independent; the two that
matter most:
injection_detectionfailed open on a malformed YARA rule._load_rulesreturned
Noneonyara.SyntaxError, and the caller treatsNoneas "norules configured" and allows. A single bad rule silently disabled injection
detection for every bot message. It now raises, matching the four existing
config-error sites in the same module.
clavataput vendor response bodies into exception messages. Clavataechoes the evaluated text back in its responses, so checked content reached
logs and tracebacks. Status codes are kept, bodies dropped.
The rest:
f5had retry jitter disabled in production (random_value=lambda: 1.0);content_safetydeclared no requirements at all despite being a modeljudge that imports
fast_langdetect;clavatacached the API key in a moduleglobal at import time; both dialects of
content_safetysent a misspelledexception event; Colang 1 flows in
clavataandcontent_safetyinterpolatedvariables that are never bound;
regexandinjection_detectioncopied thefull checked text into outcome metadata, which reaches the processing log and
tracing exporters.
Behavior changes worth flagging
ContentSafetyCheckOuputExceptionis renamed toContentSafetyCheckOutputException. This is a public event name. Theshipped spelling never matched
docs/configure-rails/exceptions.mdx, whichdocuments the corrected name, so anything filtering on the documented name
has never fired.
injection_detectionnow raises on a malformed rule instead of allowing.test_malformed_inline_yara_rule_fails_gracefullyasserted the old fail-openbehavior; it is renamed and inverted.
RetryingHTTPClient.random_valueis now late-bound (Noneresolves torandom.randomat construction) so tests can pin the jitter draw instead ofproduction disabling it.
Nothing skipped.
AI Assistance
Checklist
Summary by CodeRabbit
Bug Fixes
Security & Privacy