Add result link to triage result

[fu-zhao-loggia]

No description provided.

[claude]

CI Triage PR #12 — Review: Add result link to triage report

Overview

This PR adds Shiplight Cloud v2 (Nova) enrichment to the triage workflow: a new Extract Nova run ID step extracts a run ID from the downloaded report-data.json artifact, passes it to the triage agent, and the agent appends per-test Nova links to /tmp/triage.md. Supporting changes include a new scrub-agent-output.py script that redacts LLM/API tokens from agent output files, a scrub sentinel gate on the triage-context artifact upload, and an optional debug-artifacts upload path with a public-repo guard. Documentation is expanded in README.md and a new SECURITY.md.

The injection-hardening design (numeric-only ID at two independent layers, env-only token passing, GITHUB_OUTPUT via printf not interpolation) is well-executed. The supply-chain pinning is consistent with the existing pattern. The overall threat model in SECURITY.md and inline comments is unusually thorough.

Two medium-severity issues require changes before merge.

---

MEDIUM — Scrub sentinel uses hashFiles() outside GITHUB_WORKSPACE, likely always returns ''

Files: .github/workflows/triage.yml:204, action.yml:126

if: always() && hashFiles('/tmp/.scrub-succeeded') != ''

GitHub Actions hashFiles() is documented to only resolve patterns inside GITHUB_WORKSPACE. /tmp is outside the workspace, so hashFiles('/tmp/.scrub-succeeded') will almost certainly return '' on all GitHub-hosted runners. If so:

• The condition is always false, the triage-context artifact is never uploaded
• The autofix job's Download triage context step (which is not conditional) fails on every run
• The entire autofix pipeline silently breaks

A reliable alternative is to write the sentinel into the workspace and check it there, or — more idiomatically — avoid the cross-step/cross-action sentinel pattern entirely and instead use the scrub composite step's outcome:

# In action.yml: give the scrub step an id
- name: Scrub secrets from agent output
  id: scrub
  if: always()
  ...

# In triage.yml: check step outcome directly
if: always() && steps.<run-triage-agent-step-id>.scrub.outcome == 'success'

Since the scrub step lives inside the composite action, its outcome is not directly visible in the caller. The simplest fix is to write the sentinel into $GITHUB_WORKSPACE (always accessible):

# in action.yml scrub step
touch "$GITHUB_WORKSPACE/.scrub-succeeded"

# in triage.yml
if: always() && hashFiles('.scrub-succeeded') != ''

Add a rm -f "$GITHUB_WORKSPACE/.scrub-succeeded" at the start of the job to avoid reuse on self-hosted runners.

---

MEDIUM — Stale sentinel on self-hosted runners allows artifact upload to bypass a failed scrub

Files: action.yml:126–130, .github/workflows/triage.yml:200–214

On self-hosted (non-ephemeral) runners, /tmp/.scrub-succeeded written by a prior job persists across runs. If the scrub Python script fails in a later run (exits 1), the bash wrapper calls exit 1, marking the scrub step failed. But the artifact upload step uses if: always() && hashFiles('/tmp/.scrub-succeeded') != ''. With always() in the condition and a stale sentinel, the upload condition evaluates to true and the artifact uploads with unredacted secrets.

This is partially addressed by fixing the MEDIUM issue above (using a workspace sentinel auto-cleaned at job start). The fix is the same: write the sentinel to a workspace path that is guaranteed fresh each run, and explicitly remove it at job start.

---

LOW — Debug-artifacts guard fails open when GITHUB_REPOSITORY_VISIBILITY is empty

File: .github/workflows/triage.yml:221–224

if [ "${GITHUB_REPOSITORY_VISIBILITY:-}" = "public" ]; then
  exit 1
fi

This guard only blocks when the env var is "public". If GITHUB_REPOSITORY_VISIBILITY is unset (e.g., an older or misconfigured self-hosted runner), the check evaluates to "" = "public" → false → the guard exits 0, allowing the debug upload.

A fail-safe pattern blocks unless visibility is explicitly confirmed:

case "${GITHUB_REPOSITORY_VISIBILITY:-}" in
  private|internal) ;;
  public)  echo "::error::..."; exit 1 ;;
  *)       echo "::error::GITHUB_REPOSITORY_VISIBILITY is unset or unrecognised; refusing debug upload."; exit 1 ;;
esac

---

LOW — head -1 in Nova run ID extraction is non-deterministic for multi-URL artifacts

File: .github/workflows/triage.yml:149–150

nova_run_id="$(grep -oE 'nova\.shiplight\.ai/run-results/[0-9]+' "$report_json" 2>/dev/null \
  | grep -oE '[0-9]+$' | head -1 || true)"

If report-data.json contains multiple nova.shiplight.ai/run-results/<id> URLs (e.g. a merged or multi-suite artifact), head -1 picks the first match without warning. Consider using sort -u to assert uniqueness, or emit a warning when more than one distinct ID is found.

---

LOW — Scrub sentinel does not distinguish "scrubbed" from "nothing to scrub"

File: scripts/scrub-agent-output.py:35–36

if not TOKENS:
    sys.exit(0)

When no tokens are configured, the script exits 0, and the sentinel is written, signalling "scrub OK." A future operator adding a debug upload might assume the sentinel implies scrub coverage, when in fact it implies nothing was checked. This is a documentation/design note rather than a bug, but worth a comment clarifying that the sentinel means "scrub step ran without error" rather than "all secrets were verified redacted."

---

Positive observations

• Numeric-only Nova run ID validation at two independent layers (grep chain and build-triage-prompt.sh) is correct defense-in-depth against heredoc/GITHUB_OUTPUT injection.
• SHIPLIGHT_API_TOKEN is correctly isolated to the agent-run step only, absent from the prompt-build step env — no token in the prompt context.
• Binary replace (not regex) in scrub-agent-output.py correctly handles tokens with regex metacharacters.
• Atomic scrub write via temp-file + os.replace() prevents partial overwrites.
• Supply-chain pinning is consistent (actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02).
• The extra_env scrub gap is correctly documented in both SECURITY.md and the Python script's maintenance comment.
• LLM untrusted-data instruction in the Nova enrichment prompt is appropriate.