ci: harden pr.yaml + chaos workflow

[bdimitrov-netzine]

Findings from a full workflow audit (actionlint 1.7.12 reports the whole .github/workflows tree clean; these are hardening/correctness gaps it doesn't flag).

Fixes in this PR

1. pr.yaml: cap the Trivy comment output — latent failure
The forge path caps its comment at tail -c 60000 (GitHub's 65536-char comment-body limit), but the Trivy path did a raw cat trivy-results.txt >> $GITHUB_OUTPUT. A PR with many HIGH/CRITICAL findings produces a table over the limit → the sticky-comment POST 422s and the step fails. Now capped the same way.

2. pr.yaml: add top-level permissions: contents: read
The fmt and tests jobs declared no permissions and inherited the repo-default GITHUB_TOKEN scope. Every other workflow already pins least privilege. The two commenting jobs keep their own pull-requests: write override.

3. pr.yaml: add concurrency with cancel-in-progress
No cancellation meant every push/rebase stacked another full build+test run — amplified by the "require branch up-to-date" rule. Now a new push supersedes the in-flight run. (CodeQL already does this; the push/tag build workflows intentionally don't.)

4. nightly-chaos.yml: route the dispatch input through env:
... --test-threads 1 ${{ github.event.inputs.test }} templated a user input straight into the shell — a script-injection surface (low severity: dispatch requires write access, and it's the only workflow doing this vs the env-var pattern build-docker/promote-docker use). Now passed as env: TEST and referenced as $TEST (data, not code). Left unquoted so empty still means "run all scenarios".

Deliberately not changed (per review decision)

• Clippy is installed in 5 jobs but never run — left as-is.
• promote-docker.yml's "fail if target tag exists" guard stays commented (overwrite-on-re-promote kept).
• Draft-PR skip stays commented (full CI runs on drafts).

[github-actions]

Claude finished @bdimitrov-netzine's task in 1m 44s —— View job

---

PR Review

• Gather context
• Read changed workflow files and diff
• Review each fix for correctness and completeness
• Post findings

---

All four changes are correct and well-motivated. A few observations worth noting before merge:

---

Fix 1 — Trivy output cap (pr.yaml line 135)

Correct. Consistent with the forge path at line 239. One semantic note: tail -c 60000 preserves the last 60 KB, which for a Trivy table means the column headers and first findings are dropped when truncation kicks in. head -c 60000 would preserve the header rows instead. Since this is a sticky comment meant for quick triage, the beginning of the output is arguably more useful. That said, matching the forge precedent has its own value — this is a judgment call, not a bug.

---

Fix 2 — Top-level permissions: contents: read (pr.yaml lines 11–12)

Correct. fmt and tests jobs had no explicit permissions and were implicitly inheriting the repo default. The two commenting jobs (security-scan, contracts-tests) both already carry their own permissions: blocks with contents: read + pull-requests: write, so they are unaffected — job-level permissions override the top-level entirely in GitHub Actions.

---

Fix 3 — concurrency block (pr.yaml lines 16–18)

Correct. The group key pr-${{ github.ref }} matches the pattern CodeQL already uses (codeql-${{ github.ref }}). One interaction worth being aware of: pr.yaml also has a workflow_dispatch trigger (line 3). In that case github.ref resolves to the branch ref (e.g. refs/heads/feature-branch) rather than the PR merge ref (refs/pull/N/merge), so manual dispatches on a branch and in-flight PR runs on that same branch get different concurrency keys and won't cancel each other — this is correct behavior. Two manual dispatches on the same branch would cancel each other, which is also fine.

---

Fix 4 — Script injection in nightly-chaos.yml (lines 70–74)

Correct. The env: indirection (data path) vs template interpolation (code path) is the right fix. The intentional lack of quoting on $TEST is correctly reasoned: an empty env var expands to nothing, which passes no filter to cargo test and runs all scenarios. If it were quoted ("$TEST"), an empty dispatch would pass "" as a positional argument to the test binary, likely causing an error. The comment in the diff documents this clearly.

One minor note: $TEST is still subject to word-splitting if the test name contained spaces. That can't happen with Rust test names (they use :: as separators), so this is a non-issue in practice.

---

Summary

No blocking issues. The PR is a clean hardening pass — the injection fix and the GITHUB_OUTPUT cap are the two most impactful changes. The tail vs head question on truncation is the only open style point; raising it in case you want to revisit consistency with the forge path before merging.

[github-actions]

Security Scan - Code

Severity: HIGH, CRITICAL

No vulnerabilities found

View scan results

Report Summary

┌─────────────────────────────────────────────┬────────────┬─────────────────┬───────────────────┐
│                   Target                    │    Type    │ Vulnerabilities │ Misconfigurations │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ Cargo.lock                                  │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/Cargo.lock      │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ crates/testing/fuzz-targets/fuzz/Cargo.lock │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/state-sum/Cargo.lock                    │   cargo    │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/tps/package-lock.json                   │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ rayls-contracts/package-lock.json           │    npm     │        0        │         -         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/chaos-network/Dockerfile                │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-network/Dockerfile               │ dockerfile │        -        │         0         │
├─────────────────────────────────────────────┼────────────┼─────────────────┼───────────────────┤
│ etc/docker-replay/Dockerfile                │ dockerfile │        -        │         0         │
└─────────────────────────────────────────────┴────────────┴─────────────────┴───────────────────┘
Legend:
- '-': Not scanned
- '0': Clean (no security findings detected)

[github-actions]

Claude finished @bdimitrov-netzine's task in 1m 6s —— View job

---

PR Review

• Gather context
• Read changed workflow files and diff
• Review each fix for correctness and completeness
• Post findings

---

All four changes are correct and well-motivated. Notes per fix:

---

Fix 1 — Trivy output cap (pr.yaml line 135)

Correct. Consistent with the forge path at line 239. One semantic note: tail -c 60000 preserves the last 60 KB, which for a Trivy table drops the column headers and early findings when truncation kicks in. head -c 60000 would preserve the header rows instead — arguably more useful for triage. That said, matching the existing forge precedent has its own consistency value. Not a blocker, just a judgment call worth being aware of.

---

Fix 2 — Top-level permissions: contents: read (pr.yaml lines 11–12)

Correct. fmt and tests jobs previously had no explicit permissions and were implicitly inheriting the repo-default GITHUB_TOKEN scope. The two commenting jobs (security-scan, contracts-tests) both already carry their own permissions: blocks with contents: read + pull-requests: write (lines 107–109, 176–178), so they are unaffected — job-level permissions fully override the top-level in GitHub Actions.

---

Fix 3 — concurrency block (pr.yaml lines 16–18)

Correct. The group key pr-${{ github.ref }} matches the pattern CodeQL already uses. One interaction worth noting: pr.yaml has a workflow_dispatch trigger (line 3). In that case github.ref resolves to the branch ref (e.g. refs/heads/feature-branch) rather than the PR merge ref, so manual dispatches and in-flight PR runs on the same branch get different concurrency keys and won't cancel each other — this is correct behavior. Two manual dispatches on the same branch would cancel each other, which is also fine.

---

Fix 4 — Script injection in nightly-chaos.yml (lines 70–74)

Correct. Routing the dispatch input through env: TEST (data path) instead of templating it directly into the shell command (code path) is the right fix. The intentional lack of quoting on $TEST is correctly reasoned: an empty env var expands to nothing, so an empty dispatch passes no filter to cargo test and runs all scenarios. Rust test names use :: as separators and cannot contain spaces, so word-splitting on $TEST is a non-issue in practice. The in-code comment explains the unquoted choice clearly.

---

Summary

No blocking issues. The injection fix and the GITHUB_OUTPUT cap are the two most impactful changes; the permissions and concurrency additions are clean hardening with no regressions. The only open style point is tail vs head for the Trivy truncation — worth a quick decision before merging if you want to revisit consistency with the forge path.