refactor(harbor): simplify verifier evidence ingestion

Author: dleen

examples/harbor/README.md

@@ -39,7 +39,7 @@ uv run harbor run \
 
 The agent solves the task in `task/`, and Harbor's verifier emits a normalized `reward` plus an `answer_length` metric. The plugin creates `jobs/braintrust-harbor-example/braintrust-sync.json` after synchronization.
 
-With the default `attachments=verifier-details` mode, the verification span and each score also include Harbor's captured `test-stdout.txt`, optional `test-stderr.txt`, and conventional `ctrf.json` output when present. The size-bounded summary and complete credential-scrubbed `verifier-output.json` attachment appear together in the span output. Common authorization headers, credential assignments, and provider token formats are scrubbed automatically; use `redact_patterns` for application-specific secrets.
+With the default `attachments=verifier-details` mode, the verification span and each score also include Harbor's captured `test-stdout.txt`, optional `test-stderr.txt`, and conventional `ctrf.json` output when present. The size-bounded summary and complete redacted `verifier-output.json` attachment appear together in the span output. Structured CTRF fields with sensitive key names use the plugin's standard redaction, and configured `redact_patterns` apply to both structured CTRF strings and raw verifier text.
 
 By default, the plugin uses `Harbor` as the Braintrust project name. Override it with `--plugin-kwarg project_name=example-harbor` or through `.env`:
 

py/src/braintrust/integrations/harbor/plugin.py

@@ -9,7 +9,6 @@
 import json
 import logging
 import os
-import re
 import stat
 from dataclasses import dataclass, field, fields, replace
 from datetime import datetime
@@ -60,14 +59,6 @@
 _PLUGIN_VERSION = "1"
 _MANIFEST_VERSION = 1
 _DEFAULT_PROJECT = "Harbor"
-_VERIFIER_SECRET_ASSIGNMENT = re.compile(
-    r"(?i)(?P<prefix>[\"']?\b(?:[A-Za-z0-9_-]*api[-_]?key[A-Za-z0-9_-]*|token|access[-_]?token|"
-    r"auth[-_]?token|refresh[-_]?token|id[-_]?token|[A-Za-z0-9_-]*(?:secret|password|passwd|credential|"
-    r"authorization|cookie)[A-Za-z0-9_-]*)[\"']?\s*[:=]\s*)"
-    r"(?:[\"'][^\"'\r\n]*[\"']|(?:Bearer|Basic)\s+[^\s,;]+|[^\s,;]+)"
-)
-_VERIFIER_BEARER_SECRET = re.compile(r"(?i)(\bBearer\s+)[A-Za-z0-9._~+/=-]+")
-_VERIFIER_TOKEN_SECRET = re.compile(r"\b(?:sk-|gh[pousr]_|github_pat_|xox[baprs]-)[A-Za-z0-9_-]{8,}\b", re.IGNORECASE)
 
 
 @dataclass
@@ -180,58 +171,18 @@ def _read_bounded_file(path: Path, max_bytes: int) -> tuple[bytes | None, str |
         if before.st_size > max_bytes:
             return None, "attachment size limit"
         flags = os.O_RDONLY | getattr(os, "O_BINARY", 0) | getattr(os, "O_NOFOLLOW", 0) | getattr(os, "O_NONBLOCK", 0)
-        descriptor = os.open(path, flags)
+        with os.fdopen(os.open(path, flags), "rb") as file_obj:
+            opened = os.fstat(file_obj.fileno())
+            if not stat.S_ISREG(opened.st_mode) or (opened.st_dev, opened.st_ino) != (before.st_dev, before.st_ino):
+                return None, "unsafe file type"
+            data = file_obj.read(max_bytes + 1)
     except FileNotFoundError:
         return None, None
     except OSError as exc:
         return None, str(exc)
-    try:
-        opened = os.fstat(descriptor)
-        if not stat.S_ISREG(opened.st_mode) or (opened.st_dev, opened.st_ino) != (before.st_dev, before.st_ino):
-            return None, "unsafe file type"
-        if opened.st_size > max_bytes:
-            return None, "attachment size limit"
-        chunks = []
-        remaining = max_bytes + 1
-        while remaining:
-            chunk = os.read(descriptor, remaining)
-            if not chunk:
-                break
-            chunks.append(chunk)
-            remaining -= len(chunk)
-        data = b"".join(chunks)
-        if len(data) > max_bytes:
-            return None, "attachment size limit"
-        return data, None
-    except OSError as exc:
-        return None, str(exc)
-    finally:
-        os.close(descriptor)
-
-
-def _redact_verifier_value(value: Any) -> tuple[Any, bool]:
-    """Scrub common credential forms from raw logs and nested CTRF strings."""
-    if isinstance(value, str):
-        redacted = _VERIFIER_SECRET_ASSIGNMENT.sub(r"\g<prefix>[REDACTED]", value)
-        redacted = _VERIFIER_BEARER_SECRET.sub(r"\1[REDACTED]", redacted)
-        redacted = _VERIFIER_TOKEN_SECRET.sub("[REDACTED]", redacted)
-        return redacted, redacted != value
-    if isinstance(value, dict):
-        result = {}
-        changed = False
-        for key, child in value.items():
-            result[key], child_changed = _redact_verifier_value(child)
-            changed = changed or child_changed
-        return result, changed
-    if isinstance(value, list):
-        result = []
-        changed = False
-        for child in value:
-            redacted_child, child_changed = _redact_verifier_value(child)
-            result.append(redacted_child)
-            changed = changed or child_changed
-        return result, changed
-    return value, False
+    if opened.st_size > max_bytes or len(data) > max_bytes:
+        return None, "attachment size limit"
+    return data, None
 
 
 def _read_json_summary(entries: list[tuple[str | None, Path]], max_bytes: int) -> tuple[Any, list[str]]:
@@ -340,52 +291,58 @@ def _attachment(
     )
 
 
+_VERIFIER_OUTPUT_FILES = (
+    ("stdout", "test-stdout.txt", False),
+    ("stderr", "test-stderr.txt", False),
+    ("ctrf", "ctrf.json", True),
+)
+
+
+def _decode_verifier_output(data: bytes, parse_json: bool) -> tuple[Any, list[str]]:
+    if not parse_json:
+        return data.decode("utf-8", errors="replace"), []
+    try:
+        return json.loads(data), []
+    except (UnicodeDecodeError, json.JSONDecodeError):
+        return data.decode("utf-8", errors="replace"), ["is not valid JSON"]
+
+
+def _read_verifier_output(
+    path: Path, parse_json: bool, config: PluginConfig, max_bytes: int
+) -> tuple[Any | None, int, list[str]]:
+    data, read_warning = _read_bounded_file(path, max_bytes)
+    if data is None:
+        return None, 0, [] if read_warning is None else [f"omitted: {read_warning}"]
+    if not data:
+        return None, 0, []
+    value, warnings = _decode_verifier_output(data, parse_json)
+    normalized = normalize_json(
+        value,
+        max_bytes=config.max_attachment_bytes,
+        redact_patterns=config.redact_patterns,
+        max_depth=20,
+        redact_absolute_paths=False,
+    )
+    return normalized.value, len(data), [*warnings, *normalized.warnings]
+
+
 def _verifier_output_attachment(result: Any, config: PluginConfig) -> tuple[Attachment | None, Any, list[str]]:
     if config.attachments == "none":
         return None, None, []
-
-    output_files = (
-        ("stdout", "test-stdout.txt", False),
-        ("stderr", "test-stderr.txt", False),
-        ("ctrf", "ctrf.json", True),
-    )
     outputs: list[tuple[str | None, dict[str, Any]]] = []
     warnings: list[str] = []
     total = 0
     for step_name, verifier_dir in verifier_output_paths(result):
         step_output: dict[str, Any] = {}
-        for key, filename, parse_json in output_files:
+        for key, filename, parse_json in _VERIFIER_OUTPUT_FILES:
             path = verifier_dir / filename
             label = _step_label(step_name, path)
             limit = min(config.max_attachment_bytes, config.max_total_attachment_bytes - total)
-            data, read_warning = _read_bounded_file(path, limit)
-            if data is None:
-                if read_warning is not None:
-                    warnings.append(f"{label} omitted: {read_warning}")
-                continue
-            if not data:
-                continue
-            if parse_json:
-                try:
-                    value: Any = json.loads(data)
-                except (UnicodeDecodeError, json.JSONDecodeError):
-                    warnings.append(f"{label} is not valid JSON")
-                    value = data.decode("utf-8", errors="replace")
-            else:
-                value = data.decode("utf-8", errors="replace")
-            value, redacted = _redact_verifier_value(value)
-            if redacted:
-                warnings.append(f"{label}: redacted credential-like text")
-            normalized = normalize_json(
-                value,
-                max_bytes=config.max_attachment_bytes,
-                redact_patterns=config.redact_patterns,
-                max_depth=20,
-                redact_absolute_paths=False,
-            )
-            warnings.extend(f"{label}: {warning}" for warning in normalized.warnings)
-            step_output[key] = normalized.value
-            total += len(data)
+            value, size, file_warnings = _read_verifier_output(path, parse_json, config, limit)
+            warnings.extend(f"{label} {warning}" for warning in file_warnings)
+            if value is not None:
+                step_output[key] = value
+                total += size
         if step_output:
             outputs.append((step_name, step_output))
 
@@ -403,6 +360,33 @@ def _verifier_output_attachment(result: Any, config: PluginConfig) -> tuple[Atta
     )
 
 
+def _remaining_attachment_config(config: PluginConfig, used_bytes: int) -> PluginConfig:
+    return replace(config, max_total_attachment_bytes=max(0, config.max_total_attachment_bytes - used_bytes))
+
+
+def _attachments_size(attachments: dict[str, Attachment]) -> int:
+    return sum(len(attachment.data) for attachment in attachments.values())
+
+
+def _verifier_evidence(result: Any, config: PluginConfig) -> tuple[dict[str, Any], list[str], int]:
+    attachment, summary, warnings = _verifier_output_attachment(result, config)
+    output: dict[str, Any] = {}
+    if summary is not None:
+        output["verifier_output_summary"] = normalize_json(
+            summary,
+            max_bytes=config.max_content_bytes,
+            redact_patterns=config.redact_patterns,
+            redact_absolute_paths=False,
+        ).value
+    if attachment is not None:
+        output["verifier_output"] = attachment
+    return output, warnings, 0 if attachment is None else len(attachment.data)
+
+
+def _output_event(output: dict[str, Any]) -> dict[str, Any]:
+    return {} if not output else {"output": output}
+
+
 class HarborPlugin:
     """Harbor plugin that reconciles final trials into Braintrust experiments."""
 
@@ -846,7 +830,7 @@ def _sync_final_result(self, result: Any) -> None:
             execution_input["extra_instructions"] = extra_instructions
         selected_artifacts, artifact_attachment_warnings = _artifact_attachments(result, self.config)
         metadata["harbor"]["warnings"].extend(artifact_attachment_warnings)
-        attachment_bytes = sum(len(attachment.data) for attachment in selected_artifacts.values())
+        attachment_bytes = _attachments_size(selected_artifacts)
         agent_span = task.start_span(
             name="agent_execution",
             type="task",
@@ -879,30 +863,10 @@ def _sync_final_result(self, result: Any) -> None:
         if selected_artifacts:
             agent_span.log(output={"artifacts": selected_artifacts})
         agent_span.end(end_time=agent_end)
-        verifier_config = replace(
-            self.config,
-            max_total_attachment_bytes=max(0, self.config.max_total_attachment_bytes - attachment_bytes),
-        )
-        verifier_attachment, verifier_summary, verifier_warnings = _verifier_output_attachment(result, verifier_config)
+        verifier_config = _remaining_attachment_config(self.config, attachment_bytes)
+        verifier_output, verifier_warnings, verifier_attachment_bytes = _verifier_evidence(result, verifier_config)
         metadata["harbor"]["warnings"].extend(verifier_warnings)
-        if verifier_attachment is not None:
-            attachment_bytes += len(verifier_attachment.data)
-        bounded_verifier_summary = (
-            None
-            if verifier_summary is None
-            else normalize_json(
-                verifier_summary,
-                max_bytes=self.config.max_content_bytes,
-                redact_patterns=self.config.redact_patterns,
-                redact_absolute_paths=False,
-            ).value
-        )
-        verification_output: dict[str, Any] = {}
-        if bounded_verifier_summary is not None:
-            verification_output["verifier_output_summary"] = bounded_verifier_summary
-        if verifier_attachment is not None:
-            verification_output["verifier_output"] = verifier_attachment
-        verification_event = {} if not verification_output else {"output": verification_output}
+        attachment_bytes += verifier_attachment_bytes
         self._start_phase(
             task,
             result,
@@ -911,7 +875,7 @@ def _sync_final_result(self, result: Any) -> None:
             trial_id,
             root_start,
             root_end,
-            **verification_event,
+            **_output_event(verifier_output),
         )
 
         for step in getattr(result, "step_results", None) or []:
@@ -979,10 +943,7 @@ def _sync_final_result(self, result: Any) -> None:
         task.log(metadata={"harbor": {"warnings": trajectory_warnings}})
         task.end(end_time=root_end)
 
-        details_config = replace(
-            self.config,
-            max_total_attachment_bytes=max(0, self.config.max_total_attachment_bytes - attachment_bytes),
-        )
+        details_config = _remaining_attachment_config(self.config, attachment_bytes)
         details_attachment, details_summary, detail_warnings = _attachment(
             reward_details_paths(result), details_config
         )
@@ -1014,10 +975,7 @@ def _sync_final_result(self, result: Any) -> None:
                 scorer_output["reward_details_summary"] = bounded_details
             if details_attachment is not None:
                 scorer_output["reward_details"] = details_attachment
-            if bounded_verifier_summary is not None:
-                scorer_output["verifier_output_summary"] = bounded_verifier_summary
-            if verifier_attachment is not None:
-                scorer_output["verifier_output"] = verifier_attachment
+            scorer_output.update(verifier_output)
             scorer.log(output=scorer_output, scores={score.name: score.value})
             scorer.end(end_time=root_end)
 

py/src/braintrust/integrations/harbor/test_harbor.py

@@ -500,7 +500,7 @@ def test_verifier_output_attachment_collects_standard_harbor_evidence(tmp_path):
         )
     )
 
-    config = PluginConfig.from_options()
+    config = PluginConfig.from_options(redact_patterns=(r"secret-value|Bearer verifier-secret",))
     attachment, summary, warnings = _verifier_output_attachment(result, config)
 
     assert summary == {
@@ -523,34 +523,27 @@ def test_verifier_output_attachment_collects_standard_harbor_evidence(tmp_path):
     assert attachment is not None
     assert attachment.reference["filename"] == "verifier-output.json"
     assert json.loads(attachment.data) == summary
-    assert any("redacted credential-like text" in warning for warning in warnings)
+    assert warnings == []
 
 
-def test_verifier_output_attachment_handles_invalid_utf8_and_scrubs_common_credentials(tmp_path):
+def test_verifier_output_attachment_handles_invalid_utf8_and_configured_redaction(tmp_path):
     result = _trial_result(tmp_path, "trial-1", "task-a")
     verifier_dir = tmp_path / "trial-1" / "verifier"
     verifier_dir.mkdir(parents=True)
-    (verifier_dir / "test-stdout.txt").write_text(
-        "Authorization: Bearer header-secret\n"
-        "OPENAI_API_KEY=sk-example123456\n"
-        "github=ghp_abcdefghijklmnopqrstuvwxyz\n"
-        "fine_grained=github_pat_abcdefghijklmnopqrstuvwxyz\n"
-    )
+    (verifier_dir / "test-stdout.txt").write_text("Authorization: Bearer header-secret\n")
     (verifier_dir / "ctrf.json").write_bytes(b"\xff token=opaque-secret\n")
 
-    attachment, summary, warnings = _verifier_output_attachment(result, PluginConfig.from_options())
+    attachment, summary, warnings = _verifier_output_attachment(
+        result,
+        PluginConfig.from_options(redact_patterns=(r"(?:header|opaque)-secret",)),
+    )
 
     assert attachment is not None
     assert summary == {
-        "stdout": (
-            "Authorization: [REDACTED]\nOPENAI_API_KEY=[REDACTED]\ngithub=[REDACTED]\nfine_grained=[REDACTED]\n"
-        ),
+        "stdout": "Authorization: Bearer [REDACTED]\n",
         "ctrf": "\ufffd token=[REDACTED]\n",
     }
-    assert "ghp_" not in summary["stdout"]
-    assert "github_pat_" not in summary["stdout"]
     assert any("ctrf.json is not valid JSON" in warning for warning in warnings)
-    assert sum("redacted credential-like text" in warning for warning in warnings) == 2
 
 
 def test_verifier_output_attachment_rejects_oversized_file_before_reading_it(tmp_path, monkeypatch):