diff --git a/plugins/violin_guard/handlers/base.py b/plugins/violin_guard/handlers/base.py index 86f833d..b8fcc63 100644 --- a/plugins/violin_guard/handlers/base.py +++ b/plugins/violin_guard/handlers/base.py @@ -4,6 +4,7 @@ from __future__ import annotations import json import logging +from datetime import UTC, datetime from functools import wraps from pathlib import Path from typing import Any @@ -47,8 +48,43 @@ def _result(r) -> dict[str, list[str]]: return {"errors": r.errors, "warnings": r.warnings, "infos": r.infos} +def _log_guard_friction(eng_dir: Path, result, command: str) -> None: + """Append a framework_feedback.md row when the guard blocks or reviews. + + Only writes when state/framework_feedback.md already exists — the + benchmark runner creates it at engagement init. Real engagements without + the file are untouched. Recording here means friction is captured at the + moment it happens, with zero agent bookkeeping, so the agent never has to + reconstruct what was blocked from memory at the end of the run. + """ + feedback = eng_dir / "state" / "framework_feedback.md" + if not feedback.exists(): + return + rows = [("Guard Block", err) for err in result.errors] + [ + ("Guard Review", warn) for warn in result.warnings + ] + if not rows: + return + existing = feedback.read_text(encoding="utf-8", errors="replace") + now = datetime.now(UTC).strftime("%Y-%m-%d %H:%M:%S UTC") + lines = [] + for category, issue in rows: + safe = str(issue).replace("|", "\\|").replace("\n", " ").strip() + if safe in existing: # avoid spam from repeated identical failures + continue + lines.append( + f"| {now} | {category} | {safe} | " + f"command blocked: {command[:100]} | " + f"fix the command or the guard inputs (phase/hypothesis/scope) |" + ) + if not lines: + return + with feedback.open("a", encoding="utf-8") as fh: + fh.write("\n".join(lines) + "\n") + + def _check_command_internal(a) -> cmd_module.CheckResult: - return cmd_module.check_command( + result = cmd_module.check_command( CheckCommandArgs( command=a.get("command", ""), phase=a.get("phase", ""), @@ -58,6 +94,13 @@ def _check_command_internal(a) -> cmd_module.CheckResult: session_id=a.get("session_id"), ) ) + try: + eng_path = state.resolve_eng_dir(a.get("eng_dir", "")) + except Exception: # noqa: BLE001 — logging must never break the gate + eng_path = None + if eng_path is not None and (result.errors or result.warnings): + _log_guard_friction(eng_path, result, a.get("command", "")) + return result def _call(fn, args, **kwargs) -> Any: diff --git a/skills/pentest/SKILL.md b/skills/pentest/SKILL.md index e0c6ba3..cdb21af 100644 --- a/skills/pentest/SKILL.md +++ b/skills/pentest/SKILL.md @@ -204,6 +204,7 @@ This prevents re-running scans, missing cross-phase patterns, and losing the inv > - **Complete commands only**: a batch `commands` array must contain only complete shell commands — never labels, descriptions, or prose. An entry that is not an executable command is a contract violation. > - **Fix and rerun**: after any nonzero exit or stderr traceback, fix the command and rerun it before closing the phase — a failed command is not a result. > - **Review without skill override**: when reviewing a completed batch via `violin_review_batch`, omit the optional `skill` argument so the active execution binding is used. +> - **Record as you go — never defer state writes**: after EVERY `violin_review_batch`, immediately update in the same turn: (1) the hypothesis board (`violin_record_hypothesis` — status, test response, runtime evidence), (2) the coverage matrix when a new endpoint/method/param cell is exercised, and (3) PTT task status (`violin_record_ptt`). Do NOT batch state writes until closeout, and NEVER reconstruct what you tested from conversation memory after compression — memory reconstruction is how false positives are born. State files (`hypotheses.md`, `state/coverage-matrix.yaml`, `state/ptt.md`) are the only source of truth; if you cannot write the state at the moment of the result, you did not produce a result. | Subcommand / Tool | Type | Action | Parameters | |---|---|---|---| diff --git a/tests/guard/test_auto_friction_log.py b/tests/guard/test_auto_friction_log.py new file mode 100644 index 0000000..7fe4a7a --- /dev/null +++ b/tests/guard/test_auto_friction_log.py @@ -0,0 +1,78 @@ +"""Tests for automatic guard-friction logging (framework feedback as it happens).""" + +from __future__ import annotations + +from pathlib import Path + +from plugins.violin_guard.command import CheckResult +from plugins.violin_guard.handlers.base import _log_guard_friction + + +def _feedback_file(eng_dir: Path) -> Path: + return eng_dir / "state" / "framework_feedback.md" + + +def test_log_guard_friction_appends_block_row(tmp_path: Path) -> None: + eng_dir = tmp_path / "eng" + (eng_dir / "state").mkdir(parents=True) + feedback = _feedback_file(eng_dir) + feedback.write_text( + "# Violin Framework Feedback & Friction Log\n\n| Timestamp | Category | Issue | Impact | Prevention |\n|---|---|---|---|---|\n", + encoding="utf-8", + ) + result = CheckResult() + result.add_error("destructive filesystem deletion (rm -rf) is blocked") + _log_guard_friction(eng_dir, result, "rm -rf /") + text = feedback.read_text(encoding="utf-8") + assert "Guard Block" in text + assert "rm -rf" in text + assert "destructive filesystem deletion" in text + + +def test_log_guard_friction_noop_without_file(tmp_path: Path) -> None: + """Non-benchmark engagements (no framework_feedback.md) are untouched.""" + eng_dir = tmp_path / "eng" + (eng_dir / "state").mkdir(parents=True) + result = CheckResult() + result.add_error("some block") + _log_guard_friction(eng_dir, result, "cmd") + assert not _feedback_file(eng_dir).exists() + + +def test_log_guard_friction_noop_without_errors(tmp_path: Path) -> None: + eng_dir = tmp_path / "eng" + (eng_dir / "state").mkdir(parents=True) + feedback = _feedback_file(eng_dir) + feedback.write_text("header\n", encoding="utf-8") + result = CheckResult() + result.add_info("all good") + _log_guard_friction(eng_dir, result, "ok-cmd") + assert "Guard" not in feedback.read_text(encoding="utf-8") + + +def test_log_guard_friction_dedupes_identical_rows(tmp_path: Path) -> None: + eng_dir = tmp_path / "eng" + (eng_dir / "state").mkdir(parents=True) + feedback = _feedback_file(eng_dir) + feedback.write_text( + "| Timestamp | Category | Issue | Impact | Prevention |\n|---|---|---|---|---|\n", + encoding="utf-8", + ) + result = CheckResult() + result.add_error("same issue twice") + _log_guard_friction(eng_dir, result, "cmd") + _log_guard_friction(eng_dir, result, "cmd") + assert feedback.read_text(encoding="utf-8").count("same issue twice") == 1 + + +def test_log_guard_friction_escapes_pipes(tmp_path: Path) -> None: + eng_dir = tmp_path / "eng" + (eng_dir / "state").mkdir(parents=True) + feedback = _feedback_file(eng_dir) + feedback.write_text("header\n", encoding="utf-8") + result = CheckResult() + result.add_error("a | b | c") + _log_guard_friction(eng_dir, result, "cmd") + text = feedback.read_text(encoding="utf-8") + # the pipe inside the issue must not split the table row into extra cells + assert text.count("| a \\| b \\| c |") == 1