mirror of
https://github.com/affaan-m/ECC.git
synced 2026-08-31 04:09:51 +02:00
fix(skill-comply): redact operator home path from compliance reports
_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 #2149) still passes clean, 4/4. Fixes #2730
This commit is contained in:
@@ -122,6 +122,22 @@ def _setup_sandbox(sandbox_dir: Path, scenario: Scenario) -> None:
|
||||
continue
|
||||
|
||||
|
||||
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
|
||||
|
||||
|
||||
def _parse_stream_json(stdout: str) -> list[ObservationEvent]:
|
||||
"""Parse claude -p stream-json output into ObservationEvents.
|
||||
|
||||
@@ -154,7 +170,7 @@ def _parse_stream_json(stdout: str) -> list[ObservationEvent]:
|
||||
)
|
||||
pending[tool_use_id] = {
|
||||
"tool": block.get("name", "unknown"),
|
||||
"input": input_str,
|
||||
"input": _redact_home_path(input_str),
|
||||
"order": event_counter,
|
||||
}
|
||||
event_counter += 1
|
||||
@@ -178,7 +194,7 @@ def _parse_stream_json(stdout: str) -> list[ObservationEvent]:
|
||||
tool=info["tool"],
|
||||
session=msg.get("session_id", "unknown"),
|
||||
input=info["input"],
|
||||
output=output_str,
|
||||
output=_redact_home_path(output_str),
|
||||
))
|
||||
|
||||
for _tool_use_id, info in pending.items():
|
||||
|
||||
@@ -6,9 +6,12 @@ import subprocess
|
||||
from dataclasses import dataclass
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from scripts.runner import _setup_sandbox, run_scenario
|
||||
from scripts.runner import _parse_stream_json, _setup_sandbox, run_scenario
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -143,6 +146,52 @@ class TestRunScenarioMaxTurnsTermination:
|
||||
run_scenario(scenario, model="haiku")
|
||||
|
||||
|
||||
class TestParseStreamJsonRedactsHomePath:
|
||||
"""Observations feed grade() and then a written report (results/<skill>.md) —
|
||||
a raw absolute path bakes the operator's username into every tool call
|
||||
that touched anything under $HOME. --add-dir restricts the sandbox, but
|
||||
scenario setup_commands or the model's own tool calls can still reference
|
||||
$HOME directly (e.g. a Bash command using ~ expansion, or a scenario that
|
||||
legitimately needs to read a dotfile). Redact to a portable placeholder
|
||||
rather than persisting the raw path.
|
||||
"""
|
||||
|
||||
def _stream_json_for(self, tool_input: dict, output_text: str) -> str:
|
||||
return (
|
||||
'{"type":"assistant","message":{"content":[{"type":"tool_use",'
|
||||
'"id":"tu1","name":"Read","input":' + json.dumps(tool_input) + "}]}}\n"
|
||||
'{"type":"user","session_id":"s1","message":{"content":[{"type":'
|
||||
'"tool_result","tool_use_id":"tu1","content":' + json.dumps(output_text) + "}]}}\n"
|
||||
)
|
||||
|
||||
def test_input_home_path_redacted(self):
|
||||
home = str(Path.home())
|
||||
stdout = self._stream_json_for(
|
||||
{"file_path": f"{home}/notes/secrets.env"}, "irrelevant output"
|
||||
)
|
||||
events = _parse_stream_json(stdout)
|
||||
assert len(events) == 1
|
||||
assert home not in events[0].input
|
||||
assert "~/notes/secrets.env" in events[0].input
|
||||
|
||||
def test_output_home_path_redacted(self):
|
||||
home = str(Path.home())
|
||||
stdout = self._stream_json_for(
|
||||
{"file_path": "irrelevant"}, f"wrote to {home}/notes/secrets.env"
|
||||
)
|
||||
events = _parse_stream_json(stdout)
|
||||
assert len(events) == 1
|
||||
assert home not in events[0].output
|
||||
assert "~/notes/secrets.env" in events[0].output
|
||||
|
||||
def test_paths_outside_home_untouched(self):
|
||||
stdout = self._stream_json_for(
|
||||
{"file_path": "/tmp/skill-comply-sandbox/t1/file.txt"}, "ok"
|
||||
)
|
||||
events = _parse_stream_json(stdout)
|
||||
assert "/tmp/skill-comply-sandbox/t1/file.txt" in events[0].input
|
||||
|
||||
|
||||
class TestRunScenarioErrorIncludesStdoutTail:
|
||||
"""Error messages must include stdout tail, not only stderr.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user