feat(runlog): persist structured detection logs (#746)

Co-authored-by: GitHub Ace <githubnext@users.noreply.github.com>
Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>

Author: davidslater

.github/workflows/detection-only.yml

@@ -230,7 +230,7 @@ jobs:
           fi
           # shellcheck disable=SC1003,SC2016,SC2086
           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 \
-            -- /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 && threat-detect --engine copilot --output /tmp/gh-aw/threat-detection/detection_result.json /tmp/gh-aw/threat-detection' 2>&1 | tee -a /tmp/gh-aw/threat-detection/detection.log
+            -- /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
         env:
           AWF_REFLECT_ENABLED: 1
           COPILOT_AGENT_RUNNER_TYPE: STANDALONE
@@ -265,6 +265,7 @@ jobs:
           name: detection
           path: |
             /tmp/gh-aw/threat-detection/detection_result.json
+            /tmp/gh-aw/threat-detection/detection-runlog.jsonl
             /tmp/gh-aw/threat-detection/detection.log
           if-no-files-found: ignore
       - name: Conclude threat detection

.github/workflows/replay-detection.yml

@@ -413,11 +413,17 @@ jobs:
           set -euo pipefail
           result_path="${REPLAY_OUTPUT}/result.json"
           log_path="${REPLAY_OUTPUT}/replay.log"
+          runlog_path="${REPLAY_OUTPUT}/detection-runlog.jsonl"
           if [ "$ENGINE" = "copilot" ] && [ -n "${GH_AW_COPILOT_TOKEN}" ]; then
             export GH_TOKEN="$GH_AW_COPILOT_TOKEN"
             export GITHUB_TOKEN="$GH_AW_COPILOT_TOKEN"
           fi
           args=(--engine "$ENGINE" --output "$result_path")
+          if "$DETECTOR_BIN" --help 2>&1 | grep -q -- "-log-file"; then
+            args+=(--log-file "$runlog_path")
+          else
+            echo "::warning::Selected detector does not support --log-file; structured replay diagnostics will not be available."
+          fi
           if [ -n "$MODEL" ]; then
             args+=(--model "$MODEL")
           fi
@@ -462,8 +468,8 @@ jobs:
             exit "$run_status"
           fi
 
-      - name: Extract original detection result
-        if: ${{ inputs.original_detection_artifact != '' }}
+      - name: Extract original detection diagnostics
+        if: ${{ always() && inputs.original_detection_artifact != '' }}
         run: |
           set -euo pipefail
           python3 - <<'PY'
@@ -472,7 +478,11 @@ jobs:
           import re
 
           root = pathlib.Path(__import__('os').environ['REPLAY_DOWNLOADS']) / 'original-detection'
-          output = pathlib.Path(__import__('os').environ['REPLAY_OUTPUT']) / 'original-result.json'
+          output_root = pathlib.Path(__import__('os').environ['REPLAY_OUTPUT'])
+          output = output_root / 'original-result.json'
+          runlogs = list(root.rglob('detection-runlog.jsonl'))
+          if runlogs:
+              (output_root / 'original-detection-runlog.jsonl').write_bytes(runlogs[0].read_bytes())
           candidates = list(root.rglob('detection.log')) + list(root.rglob('*.log'))
           marker = re.compile(r'THREAT_DETECTION_RESULT:(\{.*\})')
           for path in candidates:
@@ -582,6 +592,8 @@ jobs:
             ${{ runner.temp }}/gh-aw-replay/output/result.json
             ${{ runner.temp }}/gh-aw-replay/output/comparison.json
             ${{ runner.temp }}/gh-aw-replay/output/original-result.json
+            ${{ runner.temp }}/gh-aw-replay/output/detection-runlog.jsonl
+            ${{ runner.temp }}/gh-aw-replay/output/original-detection-runlog.jsonl
             ${{ runner.temp }}/gh-aw-replay/output/replay.log
             ${{ runner.temp }}/gh-aw-replay/output/awf-replay-config.json
           if-no-files-found: ignore

README.md

@@ -62,7 +62,7 @@ threat-detect [flags] <artifacts-dir>
 - `--custom-prompt` — Additional detection instructions appended to the prompt. Overrides `CUSTOM_PROMPT`
 - `--custom-prompt-file` — Path to a file with additional detection instructions. Takes precedence over `--custom-prompt` and `CUSTOM_PROMPT`
 - `--output` — Path to write JSON result (defaults to stdout)
-- `--log-file` — Path to write structured JSONL run logs (one JSON object per line). Env: `THREAT_DETECTION_LOG_FILE`
+- `--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`
 - `--retries` — Retries for malformed detection outputs. Default: `1` (env: `THREAT_DETECTION_RETRIES`)
 - `--version` — Print version and exit
 
@@ -115,10 +115,12 @@ engine/config failures surface as a step failure. See spec TD-21a.
 
 #### JSONL run logs (`--log-file`)
 
-Pass `--log-file <path>` (or set `THREAT_DETECTION_LOG_FILE`) to record a
-structured trace of the run as [JSON Lines](https://jsonlines.org/): one JSON
-object per line, created fresh (truncating any existing file) with `0600`
-permissions. Every record starts with `time` (RFC 3339), `level`
+Pass `--log-file <path>` (or set `THREAT_DETECTION_LOG_FILE`) to choose where to
+record a structured trace of the run. When `--output` is set without an explicit
+log path, the detector writes `detection-runlog.jsonl` in the output file's
+directory. The log uses [JSON Lines](https://jsonlines.org/): one JSON object per
+line, created fresh (truncating any existing file) with `0600` permissions. Every
+record starts with `time` (RFC 3339), `level`
 (`info`/`error`), and `event`, followed by event-specific fields. Emitted
 events include `run_start`, `artifacts_loaded`, `prompt_built`,
 `attempt_start`/`attempt_recorded`/`attempt_no_verdict`, `verdict`,
@@ -245,7 +247,7 @@ be available on the runner where the binary runs.
 
 ### Replay workflow
 
-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, logs, replay result, and original-result comparison when available.
+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`.
 
 Replay uses the dispatching repository's `GITHUB_TOKEN`; no extra replay token is required. The selected source run must be accessible to that token.
 

cmd/threat-detect/logfile_test.go

@@ -96,6 +96,36 @@ func TestRunWritesJSONLLog(t *testing.T) {
 	}
 }
 
+func TestRunDefaultsLogBesideOutput(t *testing.T) {
+	artifactsDir := t.TempDir()
+	outputDir := t.TempDir()
+	outputPath := filepath.Join(outputDir, "result.json")
+	logPath := filepath.Join(outputDir, "detection-runlog.jsonl")
+	copilotMarker := filepath.Join(t.TempDir(), "copilot-called")
+	sinkJSON := `{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}`
+	fakeBinDir := writeFakeCopilotWithSink(t, copilotMarker, sinkJSON, 0)
+
+	code := runWithTestArgs(t, []string{
+		"threat-detect",
+		"-output", outputPath,
+		artifactsDir,
+	}, map[string]string{
+		"PATH":                      fakeBinDir + string(os.PathListSeparator) + os.Getenv("PATH"),
+		"THREAT_DETECTION_LOG_FILE": "",
+	})
+
+	if code != exitSafe {
+		t.Fatalf("run() exit code = %d, want %d", code, exitSafe)
+	}
+	records := readJSONLRecords(t, logPath)
+	if findRecord(records, "run_start") == nil {
+		t.Fatalf("expected default log file to receive records: %#v", records)
+	}
+	if status := findRecord(records, "status"); status == nil || status["reason"] != reasonResultRecorded {
+		t.Fatalf("expected result_recorded status, got %#v", status)
+	}
+}
+
 func TestRunUsesLogFileEnvVar(t *testing.T) {
 	artifactsDir := t.TempDir()
 	outputPath := filepath.Join(t.TempDir(), "result.json")
@@ -152,6 +182,62 @@ func TestRunRejectsLogFileCollidingWithOutput(t *testing.T) {
 	}
 }
 
+func TestRunRejectsDefaultLogCollidingThroughDanglingOutputSymlink(t *testing.T) {
+	artifactsDir := t.TempDir()
+	outputDir := t.TempDir()
+	outputPath := filepath.Join(outputDir, "result.json")
+	logPath := filepath.Join(outputDir, "detection-runlog.jsonl")
+	if err := os.Symlink(filepath.Base(logPath), outputPath); err != nil {
+		t.Fatalf("creating dangling output symlink: %v", err)
+	}
+
+	code, stderr := runWithTestArgsCapture(t, []string{
+		"threat-detect",
+		"-output", outputPath,
+		artifactsDir,
+	}, map[string]string{"THREAT_DETECTION_LOG_FILE": ""})
+
+	if code != exitError {
+		t.Fatalf("run() exit code = %d, want %d", code, exitError)
+	}
+	if !strings.Contains(stderr, "must not point to the same file") {
+		t.Fatalf("stderr missing collision error, got:\n%s", stderr)
+	}
+	if _, err := os.Stat(logPath); !os.IsNotExist(err) {
+		t.Fatalf("expected no log file to be written, stat err = %v", err)
+	}
+}
+
+func TestRunRejectsCollisionWithParentTraversalAfterSymlink(t *testing.T) {
+	artifactsDir := t.TempDir()
+	root := t.TempDir()
+	targetDir := filepath.Join(root, "target")
+	childDir := filepath.Join(targetDir, "child")
+	if err := os.MkdirAll(childDir, 0o755); err != nil {
+		t.Fatalf("creating symlink target: %v", err)
+	}
+	linkPath := filepath.Join(root, "link")
+	if err := os.Symlink(childDir, linkPath); err != nil {
+		t.Fatalf("creating directory symlink: %v", err)
+	}
+	outputPath := linkPath + string(os.PathSeparator) + ".." + string(os.PathSeparator) + "result.json"
+	logPath := filepath.Join(targetDir, "result.json")
+
+	code, stderr := runWithTestArgsCapture(t, []string{
+		"threat-detect",
+		"-output", outputPath,
+		"-log-file", logPath,
+		artifactsDir,
+	}, nil)
+
+	if code != exitError {
+		t.Fatalf("run() exit code = %d, want %d", code, exitError)
+	}
+	if !strings.Contains(stderr, "must not point to the same file") {
+		t.Fatalf("stderr missing collision error, got:\n%s", stderr)
+	}
+}
+
 func TestRunRejectsUnopenableLogFile(t *testing.T) {
 	artifactsDir := t.TempDir()
 	// Parent directory does not exist, so runlog.Open must fail.

cmd/threat-detect/main.go

@@ -24,6 +24,7 @@ import (
 	"os/signal"
 	"path/filepath"
 	"strconv"
+	"strings"
 
 	"github.com/github/gh-aw-threat-detection/pkg/artifacts"
 	"github.com/github/gh-aw-threat-detection/pkg/detector"
@@ -148,6 +149,13 @@ func run() (code int) {
 	// standalone detector honors the model gh-aw configured for detection.
 	model = engine.ResolveModel(engineID, model)
 
+	// File-based integrations expect diagnostics to survive beside the result.
+	// An explicit flag or environment variable still takes precedence.
+	if logFile == "" && outputJSON != "" {
+		dir, _ := filepath.Split(outputJSON)
+		logFile = dir + "detection-runlog.jsonl"
+	}
+
 	// Reject a --log-file that collides with --output: they are opened and
 	// truncated independently, so sharing an inode would interleave the JSONL
 	// trace and the result JSON and corrupt both while still reporting success.
@@ -163,8 +171,8 @@ func run() (code int) {
 		}
 	}
 
-	// Open the JSONL run log if requested. A failure here is a config error:
-	// the caller explicitly asked for logs and should learn they were not written.
+	// Open the JSONL run log when configured or derived. A failure here is a
+	// config error because file-based integrations require the diagnostic sink.
 	if logFile != "" {
 		l, err := runlog.Open(logFile)
 		if err != nil {
@@ -430,21 +438,58 @@ func samePath(a, b string) (bool, error) {
 	return false, nil
 }
 
-// resolvePath returns an absolute, symlink-resolved path. When the target does
-// not yet exist, it resolves the deepest existing ancestor directory and rejoins
-// the remaining components so not-yet-created siblings still compare correctly.
+// resolvePath returns an absolute, symlink-resolved path. Components are
+// processed in filesystem order so ".." after a symlink applies to the symlink
+// target, matching how the operating system resolves the path.
 func resolvePath(p string) (string, error) {
-	abs, err := filepath.Abs(p)
-	if err != nil {
-		return "", err
-	}
-	if resolved, err := filepath.EvalSymlinks(abs); err == nil {
-		return resolved, nil
-	}
-	dir, base := filepath.Split(abs)
-	dir = filepath.Clean(dir)
-	if resolvedDir, err := filepath.EvalSymlinks(dir); err == nil {
-		return filepath.Join(resolvedDir, base), nil
+	if !filepath.IsAbs(p) {
+		wd, err := os.Getwd()
+		if err != nil {
+			return "", err
+		}
+		p = wd + string(os.PathSeparator) + p
+	}
+	volume := filepath.VolumeName(p)
+	resolved := volume + string(os.PathSeparator)
+	pending := splitPathComponents(strings.TrimPrefix(p, volume))
+	symlinks := 0
+
+	for len(pending) > 0 {
+		component := pending[0]
+		pending = pending[1:]
+		switch component {
+		case ".":
+			continue
+		case "..":
+			resolved = filepath.Dir(resolved)
+			continue
+		}
+
+		candidate := filepath.Join(resolved, component)
+		if info, err := os.Lstat(candidate); err == nil && info.Mode()&os.ModeSymlink != 0 {
+			symlinks++
+			if symlinks > 255 {
+				return "", fmt.Errorf("resolving %q: too many symlinks", p)
+			}
+			target, err := os.Readlink(candidate)
+			if err != nil {
+				return "", fmt.Errorf("reading symlink %q: %w", candidate, err)
+			}
+			if filepath.IsAbs(target) {
+				volume = filepath.VolumeName(target)
+				resolved = volume + string(os.PathSeparator)
+				target = strings.TrimPrefix(target, volume)
+			}
+			pending = append(splitPathComponents(target), pending...)
+			continue
+		}
+		resolved = candidate
 	}
-	return filepath.Clean(abs), nil
+	return filepath.Clean(resolved), nil
+}
+
+func splitPathComponents(p string) []string {
+	return strings.FieldsFunc(p, func(r rune) bool {
+		return r <= 255 && os.IsPathSeparator(uint8(r))
+	})
 }

specs/threat-detection-spec.md

@@ -180,13 +180,15 @@ detector MUST emit a non-fatal `ERR_VALIDATION` warning and continue.
 
 **TD-20a**: The detector MUST support writing a structured run log in JSON Lines
 (JSONL) format to a file via the `--log-file` flag (also configurable through the
-`THREAT_DETECTION_LOG_FILE` environment variable). When enabled, the detector MUST
-write one JSON object per line, each containing at least the `time`, `level`, and
-`event` keys, and MUST record a terminal `status` event whose `reason` and `exit`
-fields carry the same reason string and exit code as the stderr status line. The
-run log is an additive observability
-sink: it MUST NOT alter the result JSON contract (TD-08) or the exit codes (TD-21).
-A failure to open the log file MUST be treated as a configuration error.
+`THREAT_DETECTION_LOG_FILE` environment variable). When `--output` is set and no
+log path is explicitly configured, the detector MUST write the run log as
+`detection-runlog.jsonl` in the result file's directory. When enabled, the
+detector MUST write one JSON object per line, each containing at least the `time`,
+`level`, and `event` keys, and MUST record a terminal `status` event whose
+`reason` and `exit` fields carry the same reason string and exit code as the
+stderr status line. The run log is an additive observability sink: it MUST NOT
+alter the result JSON contract (TD-08) or the exit codes (TD-21). A failure to
+open the log file MUST be treated as a configuration error.
 
 **TD-20b**: The detector MUST provide a `conclude` subcommand that reads a structured
 result file written by a prior detection run and emits the host-side job-output

specs/usage-spec.md

@@ -106,8 +106,10 @@ engine CLI.
 verdict payload and MUST read it from the location it configured (stdout, or the
 `--output` path, or the `detection_result.json` sink used by `conclude`).
 
-**U-09**: The host SHOULD enable the structured JSONL run log via `--log-file`
-(or `THREAT_DETECTION_LOG_FILE`) for observability (per TD-20a), and MUST NOT
+**U-09**: The host SHOULD retain the structured JSONL run log for observability
+(per TD-20a). The host MAY configure its path via `--log-file` (or
+`THREAT_DETECTION_LOG_FILE`); otherwise, when `--output` is set, the detector
+writes `detection-runlog.jsonl` in the result file's directory. The host MUST NOT
 point `--log-file` and `--output` at the same path.
 
 ### 3.1 Flags
@@ -122,7 +124,7 @@ versions.
 | `--model <name>` | Override the engine model (see U-13) |
 | `--prompt-template <path>` | Override the embedded default prompt |
 | `--output <path>` | Write the JSON result to a file instead of stdout |
-| `--log-file <path>` | Write structured JSONL run logs |
+| `--log-file <path>` | Write structured JSONL run logs (defaults beside `--output` to `detection-runlog.jsonl`) |
 | `--retries <n>` | Retries for malformed detection outputs (default `1`) |
 | `--version` | Print version and exit |
 
@@ -259,7 +261,9 @@ require the newest stable release MUST re-pin explicitly.
 **U-27**: A host MAY re-run detection against artifacts from a prior run for
 diagnostics. When it does, it MUST normalize the downloaded artifacts into the
 input contract of Section 3 before invoking the detector, and SHOULD retain the
-detector's JSONL run log (per TD-20a) and structured result for comparison.
+detector's `detection-runlog.jsonl` run log (per TD-20a) and structured result
+for comparison. When the source detection artifact contains a run log, the replay
+host SHOULD retain it separately from the replay run log.
 
 **U-28**: Replay MUST NOT require credentials beyond those already needed to read
 the source run's artifacts and to authenticate the selected engine.