fix(skill-comply): redact operator home path from compliance reports - #2731
fix(skill-comply): redact operator home path from compliance reports#2731diazMelgarejo wants to merge 4 commits into
Conversation
_parse_stream_json() persisted raw tool_input/tool_response content into ObservationEvents that grade() scores and generate_report() writes to results/<skill>.md -- a report meant to be shared and reviewed. --add-dir restricts the agent's additional accessible directory to the sandbox (SANDBOX_BASE = /tmp/skill-comply-sandbox), but that doesn't stop the agent's own tool calls (a Bash command using ~ expansion, a scenario setup_commands entry referencing a dotfile) from emitting the operator's home directory into tool_input/tool_response -- which then lands verbatim, truncated but not sanitized, in the written report. Adds _redact_home_path(), pure stdlib (Path.home()), applied to both input_str and output_str before they're stored on the ObservationEvent. Scoped deliberately to the home directory only -- grade() needs real tool-call semantics for LLM-based compliance classification, so truncating/stripping content the way a pure logging hook could isn't an option here; only the operator-identifying path component needs to go. New TestParseStreamJsonRedactsHomePath class in skills/skill-comply/tests/test_runner.py (3 tests) -- full file now 10/10 passing, up from 7/7. Confirmed tests/test_invariant_runner.py (the sandbox-execution security tests from affaan-m#2149) still passes clean, 4/4. Fixes affaan-m#2730
|
Cross-ref: sibling fix in agentic-stack for the same-shaped bug (raw tool-call payload into a durable, shareable artifact): codejunkie99/agentic-stack#67 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe runner now recursively redacts the operator’s home-directory path before serializing and truncating recorded tool inputs and outputs. Tests cover nested values, platform-specific paths, truncation order, and unaffected paths. ChangesHome Path Redaction
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@skills/skill-comply/scripts/runner.py`:
- Around line 125-138: Update _redact_home_path to replace only complete
home-directory path components, not arbitrary substring matches; preserve valid
paths such as /home/alice/report.txt while leaving embedded-prefix and sibling
paths like /tmp/home/alice/report.txt and /home/alice-old/report.txt unchanged.
Add regression tests covering both cases.
- Line 173: Update the tool input/output serialization flow around
_redact_home_path so string values are redacted before truncation and JSON
encoding. Apply sanitization to the raw string content first, serialize the
redacted value, then truncate that representation to the existing limit while
preserving non-string handling and current output structure.
In `@skills/skill-comply/tests/test_runner.py`:
- Around line 168-180: Update the home-path tests, including
test_input_home_path_redacted, test_output_home_path_redacted, and
test_paths_outside_home_untouched, to accept monkeypatch, replace Path.home()
with a fixed non-root fixture, and derive the outside-home path from a sibling
directory. Preserve the existing assertions for redacting home paths and leaving
outside-home paths unchanged.
- Line 12: Update TestParseStreamJsonRedactsHomePath by adding `@pytest.mark.unit`
and annotate each of its three test methods—test_input_home_path_redacted,
test_output_home_path_redacted, and test_paths_outside_home_untouched—with ->
None. Do not add marker registration.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 286d4473-5d4b-4231-b6e7-a6e085a67a13
📒 Files selected for processing (2)
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Delegate complex features to a planner agent, architectural decisions to an architect agent, modified code to a code-reviewer agent, and security-sensitive work to a security-reviewer agent; use parallel agents for independent operations.
Never compromise security; validate all inputs and prevent hardcoded secrets, injection, XSS, CSRF, authentication or authorization failures, sensitive error leakage, and missing rate limits.
Never hardcode secrets; use environment variables or a secret manager, validate required secrets at startup, and rotate exposed secrets immediately.
Always create new objects and never mutate existing ones.
Plan complex features before implementation, identifying dependencies, risks, and phases.
Prefer many small, focused files; keep functions under 50 lines, files under 800 lines where practical, avoid nesting deeper than four levels, and use readable, well-named identifiers.
Handle errors at every level, provide user-friendly UI messages, log detailed server-side context, and never silently swallow errors.
Validate all user input at system boundaries using schema-based validation; fail fast with clear messages and never trust external data.
Required tests include unit tests, integration tests for APIs and database operations, and end-to-end tests for critical user flows.
Follow the mandatory TDD cycle: write a failing test, implement the minimum passing solution, then refactor and verify coverage.
Use a consistent API response envelope containing a success indicator, data payload, error message, and pagination metadata.
Encapsulate data access behind a repository interface with operations such asfindAll,findById,create,update, anddelete; business logic must depend on the abstraction rather than storage details.
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
skills/**
📄 CodeRabbit inference engine (AGENTS.md)
Treat
skills/as the canonical workflow surface; add new workflow contributions there first.
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
skills/**/scripts/**
⚙️ CodeRabbit configuration file
skills/**/scripts/**: Review generated or imported scripts as untrusted-input tooling. Flag RCE, path traversal, network fetches without validation, and writes outside the expected workspace.
Files:
skills/skill-comply/scripts/runner.py
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*test*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-testing.md)
**/*test*.{py,pyi}: Use pytest as the testing framework for Python projects
Use pytest.mark for test categorization with markers like@pytest.mark.unitand@pytest.mark.integration
Files:
skills/skill-comply/tests/test_runner.py
🧠 Learnings (1)
📚 Learning: 2026-06-28T09:52:09.015Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1099-1111
Timestamp: 2026-06-28T09:52:09.015Z
Learning: For pytest tests, ensure any marker you use (e.g., `pytest.mark.unit`) is registered in a config file (e.g., `pytest.ini`/`pyproject.toml` under `tool.pytest.ini_options`) or in a `conftest.py` that is reachable from the test module’s directory. If the marker registration `conftest.py` is not discovered for that test path, pytest will emit `PytestUnknownMarkWarning`; in such cases, register the marker globally or add a `conftest.py` within/above the test directory so the marker is known.
Applied to files:
skills/skill-comply/tests/test_runner.py
🪛 ast-grep (0.45.0)
skills/skill-comply/tests/test_runner.py
[info] 188-188: Do not hardcode temporary file or directory names
Context: "/tmp/skill-comply-sandbox/t1/file.txt"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 191-191: Do not hardcode temporary file or directory names
Context: "/tmp/skill-comply-sandbox/t1/file.txt"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
[info] 161-161: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tool_input)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 163-163: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output_text)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
skills/skill-comply/tests/test_runner.py
[error] 189-189: Probable insecure usage of temporary file or directory: "/tmp/skill-comply-sandbox/t1/file.txt"
(S108)
[error] 192-192: Probable insecure usage of temporary file or directory: "/tmp/skill-comply-sandbox/t1/file.txt"
(S108)
🔇 Additional comments (2)
skills/skill-comply/tests/test_runner.py (2)
159-165: LGTM!
187-192: 🎯 Functional Correctness | ⚡ Quick winAlso assert preservation for non-home output.
This test checks only
events[0].input. The output path uses a separate redaction call at Line 197, so a regression in output preservation can pass.Include the same outside path in
output_textand assert thatevents[0].outputremains unchanged.[ suggest_recommended_refactor]
| def _redact_home_path(text: str) -> str: | ||
| """Replace the operator's home directory with a portable placeholder. | ||
|
|
||
| Observations flow into grade() and then into a written report | ||
| (results/<skill>.md) that's meant to be read, diffed, and shared — | ||
| an absolute path bakes the operator's username into every tool call | ||
| that happened to touch anything under $HOME (including the sandbox | ||
| itself, which lives under a tempdir but scenario setup_commands or | ||
| an agent's own tool calls can still reference $HOME directly). | ||
| """ | ||
| home = str(Path.home()) | ||
| if home and home != "/" and home in text: | ||
| return text.replace(home, "~") | ||
| return text |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict redaction to complete home-path matches.
Line 136 checks only whether home is a substring. If home is /home/alice, /tmp/home/alice/report.txt becomes /tmp~/report.txt, and /home/alice-old/report.txt becomes ~-old/report.txt.
Use a boundary-aware or structured-value redaction method. Add regression tests for embedded-prefix and sibling paths.
🤖 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 `@skills/skill-comply/scripts/runner.py` around lines 125 - 138, Update
_redact_home_path to replace only complete home-directory path components, not
arbitrary substring matches; preserve valid paths such as /home/alice/report.txt
while leaving embedded-prefix and sibling paths like /tmp/home/alice/report.txt
and /home/alice-old/report.txt unchanged. Add regression tests covering both
cases.
| import json | ||
| from pathlib import Path | ||
|
|
||
| import pytest |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f '^(pytest\.ini|pyproject\.toml|setup\.cfg|tox\.ini|conftest\.py)$' . \
-x rg -n 'markers|unit' '{}'Repository: affaan-m/ECC
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file =="
fd -t f '^test_runner\.py$' .
echo "== target snippet =="
cat -n skills/skill-comply/tests/test_runner.py | sed -n '1,240p'
echo "== pytest configs =="
fd -t f '^(pytest\.ini|pyproject\.toml|setup\.cfg|tox\.ini|conftest\.py)$' .
echo "== marker references =="
rg -n 'pytest|`@pytest`|markers|unit:|unit)' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200
echo "== python annotations in test file =="
python3 - <<'PY'
import ast
from pathlib import Path
p=Path("skills/skill-comply/tests/test_runner.py")
tree=ast.parse(p.read_text())
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef):
print(f"{p}:{node.lineno}: {node.name} args={[a.arg for a in node.args.args]} returns={ast.unparse(node.returns) if node.returns else '<missing>'}")
print("class decorators:")
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef):
decs=[ast.unparse(d) for d in node.decorator_list]
if decs:
print(f"{node.name}: {decs}")
PYRepository: affaan-m/ECC
Length of output: 10831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== marker references =="
rg -n 'pytest|`@pytest`|markers|unit:|unit\)' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200
echo "== pyproject pytest configs =="
sed -n '1,220p' pyproject.toml | sed -n '/pytest\|tool\.pytest\|addopts\|asyncio_mode\|markers/p'
sed -n '1,220p' skills/skill-comply/pyproject.toml | sed -n '/pytest\|tool\.pytest\|addopts\|asyncio_mode\|markers/p'
echo "== test_runner pytest markers and annotations =="
python3 - <<'PY'
import ast
from pathlib import Path
p = Path("skills/skill-comply/tests/test_runner.py")
tree = ast.parse(p.read_text())
for cls in [cls for cls in tree.body if isinstance(cls, ast.ClassDef)]:
decs = [ast.unparse(d) for d in cls.decorator_list]
has_unit = any("pytest.mark.unit" in d for d in decs)
print(f"class {cls.name}: decorators={decs}; pytest.mark.unit={has_unit}")
for node in cls.body:
if isinstance(node, ast.FunctionDef):
ret = ast.unparse(node.returns) if node.returns else "<missing>"
is_test = node.name.startswith("test_")
methods = [node.name]
for sub in node.body:
if isinstance(sub, (ast.FunctionDef, ast.Lambda, ast.AsyncFunctionDef)):
methods += [sub.name]
print(f" def {node.name}: return={ret}; test={is_test}")
PYRepository: affaan-m/ECC
Length of output: 22294
Add pytest unit markers and return type annotations.
Mark TestParseStreamJsonRedactsHomePath with @pytest.mark.unit and add -> None to its three test methods (test_input_home_path_redacted, test_output_home_path_redacted, test_paths_outside_home_untouched). unit is already registered in the root pytest configuration, so no marker registration is needed.
🤖 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 `@skills/skill-comply/tests/test_runner.py` at line 12, Update
TestParseStreamJsonRedactsHomePath by adding `@pytest.mark.unit` and annotate each
of its three test methods—test_input_home_path_redacted,
test_output_home_path_redacted, and test_paths_outside_home_untouched—with ->
None. Do not add marker registration.
Sources: Coding guidelines, Learnings
haelyra
left a comment
haelyra
left a comment
There was a problem hiding this comment.
Thank you for reporting and fixing this privacy boundary. I completed the maintainer patch with recursive pre-serialization redaction, boundary-aware POSIX and Windows path handling, and nested/truncation regression coverage, then refreshed it on current main. The skill-comply tests, invariants, lint, and type checks pass. Approving this exact head for hosted CI.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@skills/skill-comply/scripts/runner.py`:
- Around line 142-156: Update _redact_home_paths to redact string dictionary
keys as well as values, ensuring paths in JSON object keys are replaced. Broaden
right_boundary in _redact_home_path to recognize any delimiter that cannot
continue a path, including “|”, while preserving the existing path-suffix
exclusions. Add regression coverage for delimiter-terminated paths and home
paths used as dictionary keys.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 0f1ab2b7-5702-4a47-a2d5-41a3f966b724
📒 Files selected for processing (2)
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
skills/**
📄 CodeRabbit inference engine (AGENTS.md)
Treat
skills/as the canonical workflow surface; add new workflow contributions there first.
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
skills/**/scripts/**
⚙️ CodeRabbit configuration file
skills/**/scripts/**: Review generated or imported scripts as untrusted-input tooling. Flag RCE, path traversal, network fetches without validation, and writes outside the expected workspace.
Files:
skills/skill-comply/scripts/runner.py
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
skills/skill-comply/scripts/runner.pyskills/skill-comply/tests/test_runner.py
**/*test*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-testing.md)
**/*test*.{py,pyi}: Use pytest as the testing framework for Python projects
Use pytest.mark for test categorization with markers like@pytest.mark.unitand@pytest.mark.integration
Files:
skills/skill-comply/tests/test_runner.py
🧠 Learnings (1)
📚 Learning: 2026-06-28T09:52:09.015Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1099-1111
Timestamp: 2026-06-28T09:52:09.015Z
Learning: For pytest tests, ensure any marker you use (e.g., `pytest.mark.unit`) is registered in a config file (e.g., `pytest.ini`/`pyproject.toml` under `tool.pytest.ini_options`) or in a `conftest.py` that is reachable from the test module’s directory. If the marker registration `conftest.py` is not discovered for that test path, pytest will emit `PytestUnknownMarkWarning`; in such cases, register the marker globally or add a `conftest.py` within/above the test directory so the marker is known.
Applied to files:
skills/skill-comply/tests/test_runner.py
🪛 ast-grep (0.45.1)
skills/skill-comply/scripts/runner.py
[warning] 143-146: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(
rf"(?<![\w.~+-]){home_pattern}{right_boundary}",
flags,
)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[info] 165-165: use jsonify instead of json.dumps for JSON output
Context: json.dumps(redacted)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
skills/skill-comply/tests/test_runner.py
[info] 159-159: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tool_input)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 161-161: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output_content)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 219-219: Do not hardcode temporary file or directory names
Context: "/tmp/home/alice/report.txt"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🔇 Additional comments (1)
skills/skill-comply/tests/test_runner.py (1)
169-171: Add the required pytest marker.
TestParseStreamJsonRedactsHomePathstill has no@pytest.mark.unit.As per coding guidelines, “Use pytest.mark for test categorization.” Based on learnings, verify that the marker is registered in a discovered pytest configuration.
#!/bin/bash set -euo pipefail fd -t f '^(pytest\.ini|pyproject\.toml|setup\.cfg|tox\.ini|conftest\.py)$' . -0 | xargs -0 -r rg -n 'markers|unit' || true sed -n '140,175p' skills/skill-comply/tests/test_runner.pySources: Coding guidelines, Learnings
| right_boundary = r"(?=$|[\\/]|[\s\"'`,;:)}\]])" | ||
| flags = re.IGNORECASE if re.match(r"^[A-Za-z]:[\\/]", home) else 0 | ||
| pattern = re.compile( | ||
| rf"(?<![\w.~+-]){home_pattern}{right_boundary}", | ||
| flags, | ||
| ) | ||
| return pattern.sub("~", text) | ||
|
|
||
|
|
||
| def _redact_home_paths(value: object) -> object: | ||
| """Return a copy with home paths redacted from every string leaf.""" | ||
| if isinstance(value, str): | ||
| return _redact_home_path(value) | ||
| if isinstance(value, dict): | ||
| return {key: _redact_home_paths(item) for key, item in value.items()} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact dictionary keys and all delimiter-terminated home paths.
Line 156 redacts dictionary values but preserves string keys. json.dumps() then stores a key such as "/home/alice/secret.txt" in the report.
Line 142 does not accept delimiters such as |. A command containing /home/alice| also stores the raw home path.
Redact string keys and use a boundary that accepts any non-path-suffix delimiter. Add regressions for both cases.
Proposed fix
- right_boundary = r"(?=$|[\\/]|[\s\"'`,;:)}\]])"
+ right_boundary = r"(?=$|[^\w.~+-])"
...
if isinstance(value, dict):
- return {key: _redact_home_paths(item) for key, item in value.items()}
+ return {
+ _redact_home_path(key) if isinstance(key, str) else key:
+ _redact_home_paths(item)
+ for key, item in value.items()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| right_boundary = r"(?=$|[\\/]|[\s\"'`,;:)}\]])" | |
| flags = re.IGNORECASE if re.match(r"^[A-Za-z]:[\\/]", home) else 0 | |
| pattern = re.compile( | |
| rf"(?<![\w.~+-]){home_pattern}{right_boundary}", | |
| flags, | |
| ) | |
| return pattern.sub("~", text) | |
| def _redact_home_paths(value: object) -> object: | |
| """Return a copy with home paths redacted from every string leaf.""" | |
| if isinstance(value, str): | |
| return _redact_home_path(value) | |
| if isinstance(value, dict): | |
| return {key: _redact_home_paths(item) for key, item in value.items()} | |
| right_boundary = r"(?=$|[^\w.~+-])" | |
| flags = re.IGNORECASE if re.match(r"^[A-Za-z]:[\\/]", home) else 0 | |
| pattern = re.compile( | |
| rf"(?<![\w.~+-]){home_pattern}{right_boundary}", | |
| flags, | |
| ) | |
| return pattern.sub("~", text) | |
| def _redact_home_paths(value: object) -> object: | |
| """Return a copy with home paths redacted from every string leaf.""" | |
| if isinstance(value, str): | |
| return _redact_home_path(value) | |
| if isinstance(value, dict): | |
| return { | |
| _redact_home_path(key) if isinstance(key, str) else key: | |
| _redact_home_paths(item) | |
| for key, item in value.items() | |
| } |
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 143-146: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(
rf"(?<![\w.~+-]){home_pattern}{right_boundary}",
flags,
)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 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 `@skills/skill-comply/scripts/runner.py` around lines 142 - 156, Update
_redact_home_paths to redact string dictionary keys as well as values, ensuring
paths in JSON object keys are replaced. Broaden right_boundary in
_redact_home_path to recognize any delimiter that cannot continue a path,
including “|”, while preserving the existing path-suffix exclusions. Add
regression coverage for delimiter-terminated paths and home paths used as
dictionary keys.
2 participants
Fixes #2730.
What
skills/skill-comply/scripts/runner.py's_parse_stream_json()persistsraw, unredacted
tool_input/tool_responsecontent into theObservationEvents thatgrade()scores andgenerate_report()writes toresults/<skill>.md— a report meant to be read and shared.--add-dirrestricts the compliance-test agent's additional accessibledirectory to the sandbox, but doesn't stop the agent's own tool calls (a
Bashcommand using~expansion, a scenariosetup_commandsentryreferencing a dotfile) from emitting the operator's home directory into
tool_input/tool_response— which then lands verbatim, truncated butnot sanitized, in the written report.
Fix
Small
_redact_home_path()helper, pure stdlib (Path.home()), applied toboth
input_strandoutput_strright before they're stored on theObservationEvent. Scoped deliberately to the home directory only —grade()needs the actual tool-call semantics to do LLM-based complianceclassification, so stripping/truncating content the way a pure logging
hook could isn't an option here; only the operator-identifying path
component needs to go.
Testing
New
TestParseStreamJsonRedactsHomePathclass inskills/skill-comply/tests/test_runner.py— 3 tests (input redacted,output redacted, non-home paths like the sandbox itself left untouched).
Also confirmed
tests/test_invariant_runner.py(the sandbox-executionsecurity regression tests from #2149) still passes clean, 4/4 — this PR
doesn't touch subprocess/sandboxing logic, only the observation-recording
path.