From 86f0140ee0d59cdc923b7ffa87dbcb33928f0179 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 16:58:20 -0400 Subject: [PATCH] docs(logging): add prohibited-fields section Extends the Fabro logging strategy with an explicit prohibited-fields table covering the Run Files Changed endpoint's sensitive surface: diff_contents, per-changed-file file_path values, raw git_stderr, and credential-ish strings. Each entry pairs the prohibition with a concrete cardinality-bounded alternative, so future handlers have a precedent to follow rather than rediscovering the rule. The Run Files handler (Unit 5) already emits exactly the allowlisted field set (run_id, file_count, bytes_total, duration_ms, truncated, binary_count, sensitive_count, symlink_count, submodule_count); this change makes the policy enforceable for other endpoints. Refs plan docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) --- docs-internal/logging-strategy.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs-internal/logging-strategy.md b/docs-internal/logging-strategy.md index 08d6a8a09..5450ae19e 100644 --- a/docs-internal/logging-strategy.md +++ b/docs-internal/logging-strategy.md @@ -180,3 +180,16 @@ The domain event enums (`AgentEvent`, `PipelineEvent`, `ExecutionEnvEvent`) each - **Detached UX belongs in events, not stderr.** If an attached user needs to see the message later via `fabro attach` or `fabro logs`, emit a workflow event (for example `RunNotice`) and let tracing capture the developer-oriented copy separately. - **Wrapper variants are no-ops.** When one event enum wraps another (`PipelineEvent::Agent` wraps `AgentEvent`, `AgentEvent::SubAgentEvent` wraps a child `AgentEvent`), the wrapper's `trace()` arm is `{}` because the inner event was already traced at its origin. This prevents double-logging. - **Streaming noise variants are no-ops.** `TextDelta` and `ToolCallOutputDelta` produce no log output — per-token events would flood the logs even at DEBUG level. + +## Prohibited Fields + +Some field values carry real or latent sensitivity and must not appear in `tracing` events under any level. When the underlying information is genuinely useful for observability, emit a cardinality-bounded summary (count, size, truncation flag) instead of the raw value. + +| Field | Why it's prohibited | Emit instead | +|-------|---------------------|--------------| +| `diff_contents` | File contents from a user workspace may include secrets, PII, or copyrighted code. | `bytes_total`, `file_count`, `truncated` counters | +| `file_path` (for changed-file paths in the Run Files endpoint specifically) | Leaks workspace structure; combined with public run IDs can expose layout of private repos. | `file_count`, aggregate counts bucketed by `binary`, `sensitive`, `symlink`, `submodule` | +| `git_stderr` | Raw git output for untrusted workspaces may include path-shaped secrets (e.g. `~/.ssh/id_rsa_work`) and terminal control sequences. | A short categorized reason (`"timeout"`, `"bad_revision"`, `"unknown_object"`) derived from stderr, never the stderr itself | +| Credential-ish strings (`api_key`, `bearer_token`, `cookie`, `session_id`, …) | Exfiltration risk. | Emit `has_credentials: true` or a fingerprint (`token_last4`) only when debugging is the only option | + +These prohibitions apply to every level (ERROR through TRACE). If an error path genuinely needs raw output for triage, route it through an authenticated support channel — not the default tracing subscriber.