Skip to content

Commit 206a2af

Browse files
davidslaterGitHub Ace
andauthored
refactor(logging): remove the --log-file JSONL run log (#795)
* refactor(logging): remove the --log-file JSONL run log The detector wrote a structured JSONL run log to a separate artifact, but almost every record duplicated something already on stderr/stdout: the degraded-input annotations, the config/engine/prompt-build errors, the verdict (= the result JSON), and the terminal status line. In `conclude` the duplication was total — every logger call sat next to a `c.info` twin. Drop the sink entirely (flag, THREAT_DETECTION_LOG_FILE, the derived `detection-runlog.jsonl` default, the path-collision checks, and the `pkg/runlog` package) and fold the genuinely log-only diagnostics onto stderr so the job log is the single place a run is diagnosed: - run configuration: version, engine, model, retry budget - the recursive artifact inventory (TD-17b), bounded and sanitized - prompt metadata: byte count and framework-scaffolding detection - the per-attempt trace and whether each recorded a verdict - the engine subprocess argv (stderr previously had only the arg count) - each degraded-input finding's field and required-input classification, which the "::warning::" text alone cannot convey in warn mode Untrusted values stay confined to one physical line and listings stay bounded, so neither model-authored text nor a hostile filename can forge a workflow command or flood the job log. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com> * fix(logging): close diagnostic injection vectors and stale spec refs Review follow-ups on the run-log removal. Two values reached a diagnostic line unescaped: - The engine ID is echoed on the run-start line before engine.New validates it, and Canonical only lowercases, so an arbitrary --engine value flowed through verbatim. Sanitize it (and the version, for consistency) as the model already was. - The engine-invoke line composes its command description from nodeCommand(), which reads GH_AW_NODE_BIN, and emitted it raw. Quote it with %q as argv and the model already are. Both are now covered by tests that fail without the fix. Three spec/doc claims were stale or wrong: - TD-18b still required a `prompt_analysis_degraded` run-log event and TD-22-flags still required workflow context on `prompt_built`; both reference a sink TD-20a now forbids. Reduce each to its job-log requirement. - TD-20a's escaping rule read as covering forwarded engine output, which runCLIEnvWithSink tees verbatim by design. Scope the requirement to detector-authored diagnostics and state that forwarded engine output is a separate, non-attested stream; mirror this in the README. - The README trace showed `kind=prompt` and a `<system>` scaffolding marker; artifactKind emits `file` and `<system>` is never a marker. Restore two guarantees the deleted logfile_test.go was the sole cover for: the TD-18b ERR_VALIDATION warning, and the TD-20a inventory bound, omission label, and path escaping. Finally, retain the source run's detection.log as `original-detection.log` in the replay artifact, which usage-spec U-27 requires and the runlog removal had dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com> --------- Co-authored-by: GitHub Ace <githubnext@users.noreply.github.com> Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
1 parent b6c3ebd commit 206a2af

17 files changed

Lines changed: 475 additions & 1377 deletions

.github/workflows/detection-only.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ jobs:
230230
fi
231231
# shellcheck disable=SC1003,SC2016,SC2086
232232
awf --config "${RUNNER_TEMP}/gh-aw/awf-config.json" --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" ${GH_AW_TOOL_CACHE_MOUNT:+--mount "$GH_AW_TOOL_CACHE_MOUNT"} ${GH_AW_DOCKER_HOST:+--docker-host "$GH_AW_DOCKER_HOST"} --env-all --exclude-env COPILOT_GITHUB_TOKEN --mount /tmp/gh-aw/threat-detection:/tmp/gh-aw/threat-detection:rw --log-level info --skip-pull \
233-
-- /bin/bash -c 'set +o histexpand; : "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true; set -- --engine copilot --output /tmp/gh-aw/threat-detection/detection_result.json; if threat-detect --help 2>&1 | grep -q -- "-log-file"; then set -- "$@" --log-file /tmp/gh-aw/threat-detection/detection-runlog.jsonl; else echo "::warning::Selected detector does not support --log-file; structured detection diagnostics will not be available."; fi; set -- "$@" /tmp/gh-aw/threat-detection; threat-detect "$@"' 2>&1 | tee -a /tmp/gh-aw/threat-detection/detection.log
233+
-- /bin/bash -c 'set +o histexpand; : "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true; set -- --engine copilot --output /tmp/gh-aw/threat-detection/detection_result.json /tmp/gh-aw/threat-detection; threat-detect "$@"' 2>&1 | tee -a /tmp/gh-aw/threat-detection/detection.log
234234
env:
235235
AWF_REFLECT_ENABLED: 1
236236
COPILOT_AGENT_RUNNER_TYPE: STANDALONE
@@ -265,7 +265,6 @@ jobs:
265265
name: detection
266266
path: |
267267
/tmp/gh-aw/threat-detection/detection_result.json
268-
/tmp/gh-aw/threat-detection/detection-runlog.jsonl
269268
/tmp/gh-aw/threat-detection/detection.log
270269
if-no-files-found: ignore
271270
- name: Conclude threat detection

.github/workflows/replay-detection.yml

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -413,17 +413,11 @@ jobs:
413413
set -euo pipefail
414414
result_path="${REPLAY_OUTPUT}/result.json"
415415
log_path="${REPLAY_OUTPUT}/replay.log"
416-
runlog_path="${REPLAY_OUTPUT}/detection-runlog.jsonl"
417416
if [ "$ENGINE" = "copilot" ] && [ -n "${GH_AW_COPILOT_TOKEN}" ]; then
418417
export GH_TOKEN="$GH_AW_COPILOT_TOKEN"
419418
export GITHUB_TOKEN="$GH_AW_COPILOT_TOKEN"
420419
fi
421420
args=(--engine "$ENGINE" --output "$result_path")
422-
if "$DETECTOR_BIN" --help 2>&1 | grep -q -- "-log-file"; then
423-
args+=(--log-file "$runlog_path")
424-
else
425-
echo "::warning::Selected detector does not support --log-file; structured replay diagnostics will not be available."
426-
fi
427421
if [ -n "$MODEL" ]; then
428422
args+=(--model "$MODEL")
429423
fi
@@ -480,10 +474,14 @@ jobs:
480474
root = pathlib.Path(__import__('os').environ['REPLAY_DOWNLOADS']) / 'original-detection'
481475
output_root = pathlib.Path(__import__('os').environ['REPLAY_OUTPUT'])
482476
output = output_root / 'original-result.json'
483-
runlogs = list(root.rglob('detection-runlog.jsonl'))
484-
if runlogs:
485-
(output_root / 'original-detection-runlog.jsonl').write_bytes(runlogs[0].read_bytes())
486477
candidates = list(root.rglob('detection.log')) + list(root.rglob('*.log'))
478+
479+
# Retain the source run's captured detection log alongside the replay's
480+
# own log (per usage-spec U-27), so the two can be compared without
481+
# re-downloading the original run's artifacts.
482+
if candidates:
483+
(output_root / 'original-detection.log').write_bytes(candidates[0].read_bytes())
484+
487485
# Diagnostic-only: parses the *original* gh-aw run's log, which may
488486
# carry the legacy marker wrapped in Markdown emphasis (**/__/*/_).
489487
marker = re.compile(r'THREAT_DETECTION_RESULT:\s*(\{.*\})')
@@ -594,8 +592,7 @@ jobs:
594592
${{ runner.temp }}/gh-aw-replay/output/result.json
595593
${{ runner.temp }}/gh-aw-replay/output/comparison.json
596594
${{ runner.temp }}/gh-aw-replay/output/original-result.json
597-
${{ runner.temp }}/gh-aw-replay/output/detection-runlog.jsonl
598-
${{ runner.temp }}/gh-aw-replay/output/original-detection-runlog.jsonl
595+
${{ runner.temp }}/gh-aw-replay/output/original-detection.log
599596
${{ runner.temp }}/gh-aw-replay/output/replay.log
600597
${{ runner.temp }}/gh-aw-replay/output/awf-replay-config.json
601598
if-no-files-found: ignore

CLAUDE.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ pkg/detector/ Core detection logic
3838
pkg/engine/ AI engine abstraction
3939
├── engine.go copilot/claude/codex CLI adapters; Copilot uses runCLIWithPromptFile, Claude uses runCLI with stdin, Codex passes prompts via codexArgs/runCLIEnv
4040
└── tool.go threat_detection_result wrapper provisioning + result-sink watcher
41-
pkg/runlog/ Structured JSONL run-log writer (--log-file); nil-safe no-op logger
4241
specs/ Normative spec (threat-detection-spec.md)
4342
skills/ Repo-relevant agent skills (console-rendering, error-messages)
4443
scratchpad/ Retained design references inherited from gh-aw
@@ -81,7 +80,6 @@ threat-detect [flags] <artifacts-dir>
8180
- `--model <name>` — model override forwarded to the engine
8281
- `--prompt-template <path>` — override the embedded default
8382
- `--output <path>` — write JSON result (defaults to stdout)
84-
- `--log-file <path>` — write structured JSONL run logs; env: `THREAT_DETECTION_LOG_FILE`
8583
- `--retries` (default `1`) — retries for malformed detection outputs; env: `THREAT_DETECTION_RETRIES`
8684

8785
**Exit codes** (defined in `cmd/threat-detect/main.go`):
@@ -116,7 +114,7 @@ The detector reads the verdict exclusively from the out-of-band result sink. The
116114
└── comment-memory/*.md # optional, inventoried only
117115
```
118116

119-
All files are recursively inventoried in the JSONL run log. The prompt consumes only an allowlisted, size-bounded subset of
117+
All files are recursively inventoried on stderr. The prompt consumes only an allowlisted, size-bounded subset of
120118
`aw_info.json`; unknown fields are ignored and all included values are untrusted.
121119

122120
## Detection Flow

README.md

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ threat-detect [flags] <artifacts-dir>
6262
- `--custom-prompt` — Additional detection instructions appended to the prompt. Overrides `CUSTOM_PROMPT`
6363
- `--custom-prompt-file` — Path to a file with additional detection instructions. Takes precedence over `--custom-prompt` and `CUSTOM_PROMPT`
6464
- `--output` — Path to write JSON result (defaults to stdout)
65-
- `--log-file` — Path to write structured JSONL run logs (one JSON object per line). Env: `THREAT_DETECTION_LOG_FILE`; defaults to `detection-runlog.jsonl` beside `--output`
6665
- `--retries` — Retries for malformed detection outputs. Default: `1` (env: `THREAT_DETECTION_RETRIES`)
6766
- `--version` — Print version and exit
6867

@@ -114,28 +113,36 @@ the step, so warn-mode workflows proceed exactly as they do under `gh-aw`'s nati
114113
engine (which treats a missing verdict as a recoverable `parse_error`). Only genuine
115114
engine/config failures surface as a step failure. See spec TD-21a.
116115

117-
#### JSONL run logs (`--log-file`)
118-
119-
Pass `--log-file <path>` (or set `THREAT_DETECTION_LOG_FILE`) to choose where to
120-
record a structured trace of the run. When `--output` is set without an explicit
121-
log path, the detector writes `detection-runlog.jsonl` in the output file's
122-
directory. The log uses [JSON Lines](https://jsonlines.org/): one JSON object per
123-
line, created fresh (truncating any existing file) with `0600` permissions. Every
124-
record starts with `time` (RFC 3339), `level`
125-
(`info`/`error`), and `event`, followed by event-specific fields. Emitted
126-
events include `run_start`, `artifacts_loaded`, `artifact_degraded`,
127-
`prompt_built`,
128-
`attempt_start`/`attempt_recorded`/`attempt_no_verdict`, `verdict`,
129-
`detection_failed`, and a terminal `status` record carrying the same `reason`
130-
and `exit` code as the stderr status line. The verdict JSON contract
131-
(`--output`) is unchanged; the log file is an additive observability sink.
132-
`--log-file` and `--output` must not resolve to the same file — a collision is
133-
rejected as a configuration error to avoid corrupting both outputs.
134-
135-
```jsonl
136-
{"time":"2026-07-14T18:00:00Z","level":"info","event":"run_start","engine":"copilot","model":"","retries":1,"version":"1.2.3"}
137-
{"time":"2026-07-14T18:00:03Z","level":"info","event":"verdict","has_threats":false,"malicious_patch":false,"prompt_injection":false,"reasons":[],"secret_leak":false}
138-
{"time":"2026-07-14T18:00:03Z","level":"info","event":"status","exit":0,"reason":"result_recorded"}
116+
#### Diagnostics
117+
118+
The detector writes no log artifact of its own. Apart from the result JSON, every
119+
diagnostic goes to stderr, so capturing the step's output (for example
120+
`threat-detect ... 2>&1 | tee detection.log`) preserves the full trace. Reported
121+
on stderr are the resolved run configuration (version, engine, model, retries),
122+
the recursive artifact inventory with per-file size and consumed status, the
123+
prompt metadata (byte count, resolved workflow name/description, custom-prompt
124+
provenance, framework-scaffolding detection), each detection attempt and whether
125+
it recorded a verdict, the engine subprocess invocation and argv, any
126+
`::warning::`/`::error::` annotations for degraded inputs, and the terminal status
127+
line. The rendered prompt itself is never echoed.
128+
129+
Untrusted values interpolated into these detector-authored lines are escaped to a
130+
single physical line and listings are bounded, so neither a model-authored string
131+
nor a hostile filename can forge a workflow command or flood the job log. The
132+
engine subprocess's own stdout/stderr are a separate stream: they are forwarded
133+
verbatim (so harness output and engine errors appear in real time) and are not
134+
detector-attested.
135+
136+
```text
137+
[threat-detect] run start: version=1.2.3 engine=copilot model=(none; using engine default) retries=1
138+
[threat-detect] artifacts loaded: dir=/tmp/gh-aw/threat-detection prompt_bytes=4096 agent_output_bytes=812 patch_files=1 all_primary_inputs_missing=false
139+
[threat-detect] artifact inventory (3 entries):
140+
[threat-detect] aw-prompts/prompt.txt bytes=4096 kind=file consumed=true
141+
[threat-detect] comment-memory/notes.md bytes=128 kind=file consumed=false
142+
[threat-detect] prompt built: prompt_bytes=9241 framework_scaffolding_detected=true framework_scaffolding_markers=<github-context>, <safe-output-tools>
143+
[threat-detect] detection attempt 1 of 2
144+
[threat-detect] attempt 1 recorded a verdict via the threat_detection_result tool
145+
THREAT_DETECTION_STATUS: reason=result_recorded exit=0
139146
```
140147

141148
#### Concluding a run (`conclude`)
@@ -192,15 +199,9 @@ Additional flags:
192199
table above); it is also the source of the diagnostic log statistics and marker
193200
lines. It is consulted for the terminal status reason but never parsed for a
194201
verdict.
195-
- `--log-file <path>` — mirror the conclusion into a JSONL run log (env:
196-
`THREAT_DETECTION_LOG_FILE`), emitting `conclude_start`, `conclude_verdict`,
197-
`conclude_directory_listing`, `conclude_detection_log`, and `conclude_outcome`
198-
events. It must not resolve to the same file as `--result-file` or the
199-
detection log — the log is opened truncating, so a collision would destroy the
200-
input it is meant to describe. Collisions and unopenable log paths are
201-
configuration errors that fail the step.
202-
203-
Diagnostic output is bounded so a pathological run cannot flood the job log, and
202+
203+
Like the detection run, `conclude` writes no separate log artifact; its
204+
diagnostics go to stdout only. Diagnostic output is bounded so a pathological run cannot flood the job log, and
204205
truncation is always labelled rather than passed off as a complete reading.
205206
Untrusted values (model-authored reasons, artifact filenames, detection-log
206207
lines) have control characters escaped so each stays on one line and cannot
@@ -319,7 +320,7 @@ engine runs. Findings about other artifacts stay advisory warnings in both
319320
modes.
320321

321322
Every file below the artifacts directory is recorded with its size and consumed
322-
status in the JSONL `artifacts_loaded` event. Only an allowlisted, size-bounded subset of
323+
status in the artifact inventory printed to stderr. Only an allowlisted, size-bounded subset of
323324
`aw_info.json` is added to the detection prompt, and all of its values are
324325
explicitly treated as untrusted runtime data.
325326

@@ -336,7 +337,7 @@ explicitly treated as untrusted runtime data.
336337

337338
### Replay workflow
338339

339-
Maintainers can manually run **Replay Threat Detection** from the Actions tab to rerun detection against artifacts from a prior workflow run. Provide the source repository and run ID; the workflow downloads the `agent`, `activation`, optional experiment, and optional original `detection` artifacts, normalizes them into the CLI input contract above, runs `threat-detect`, and uploads a sanitized `replay-detection-<run_id>` artifact with the manifest, file inventory, free-form replay log, replay result, and original-result comparison. Detectors that support structured logging also produce `detection-runlog.jsonl`; when available, the source run's structured log is retained separately as `original-detection-runlog.jsonl`.
340+
Maintainers can manually run **Replay Threat Detection** from the Actions tab to rerun detection against artifacts from a prior workflow run. Provide the source repository and run ID; the workflow downloads the `agent`, `activation`, optional experiment, and optional original `detection` artifacts, normalizes them into the CLI input contract above, runs `threat-detect`, and uploads a sanitized `replay-detection-<run_id>` artifact with the manifest, file inventory, free-form replay log, replay result, and original-result comparison.
340341

341342
Replay uses the dispatching repository's `GITHUB_TOKEN`; no extra replay token is required. The selected source run must be accessible to that token.
342343

0 commit comments

Comments
 (0)