feat(#989): add GHA workflow command injection to review security dimensions

[fullsend-ai-coder]

The review agent's security checklist listed generic injection types (SQL, command, LDAP, path traversal) but omitted GitHub Actions workflow command injection. This caused a false safety assertion on PR #764 where unsanitized variables in ::error title=...:: were missed. GHA workflow command injection is a distinct attack class where unsanitized values in ::error::, ::warning::, ::set-output::, etc. can inject arbitrary workflow commands.

Updated two files to cover this gap:

• code-review SKILL.md: Added GHA workflow command injection to the
  Security dimension's injection checklist with specific commands to
  audit, escape sequences to check, and a verification instruction
  requiring all interpolated variables to be checked individually.
• pr-review security sub-agent: Added GHA workflow command injection
  to the owned injection types and added a dedicated paragraph with
  the same audit guidance.

Note: make lint could not run due to Go module cache permission errors in the sandbox (infrastructure issue, not code-related). Changes are markdown-only and do not affect Go compilation.

---

Closes #989

Post-script verification

• Branch is not main/master (agent/989-add-gha-injection-security-dim)
• Secret scan passed (gitleaks — e83e1db71bb99552ceab7e5c02e92cc86c4b0148..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

Site preview

Preview: https://8105748f-site.fullsend-ai.workers.dev

Commit: ff65fee4a36ec0725d33c13ddafd0e9de42ccc19

[fullsend-ai-review]

🤖 Review · Started 6:01 PM UTC
Commit: d0ac11b · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [technical-accuracy] internal/scaffold/fullsend-repo/skills/code-review/SKILL.md:105 — The GHA workflow command list omits ::save-state::, which was disabled alongside ::set-env:: in November 2020 for the same injection vulnerability and is similarly re-enabled by ACTIONS_ALLOW_UNSECURE_COMMANDS=true. A reviewer following this guidance might miss ::save-state:: injection in legacy workflows. ::debug:: is also omitted, though it is lower risk.

• [internal-consistency] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md:20 — The same ::save-state:: omission applies to the security sub-agent's GHA guidance block. Both files are now consistent with each other (prior cross-file inconsistency finding is resolved), but they share this completeness gap.

Previous run

Review

Findings

Low

• [technical-accuracy] internal/scaffold/fullsend-repo/skills/code-review/SKILL.md:105 — The guidance lists ::set-output:: and ::set-env:: alongside active workflow commands without noting that both were deprecated/disabled by GitHub (set-env disabled Nov 2020, set-output deprecated Oct 2022). While mentioning them is defensible for legacy codebases or environments with ACTIONS_ALLOW_UNSECURE_COMMANDS=true, a brief parenthetical noting their deprecation status would make the guidance more precise and prevent reviewers from wasting effort searching for commands that modern workflows no longer use.

• [internal-consistency] internal/scaffold/fullsend-repo/skills/code-review/SKILL.md:101 — The summary bullet Injection vulnerabilities: SQL, command, LDAP, path traversal. was not updated to include GHA injection inline, while the security.md sub-agent's equivalent parenthetical was updated to include it. The SKILL.md adds a separate detailed bullet below the summary line instead, which is a valid approach but creates a minor inconsistency between the two files' enumeration styles.

Info

• [list-item-placement] internal/scaffold/fullsend-repo/skills/code-review/SKILL.md:102 — The detailed GHA injection bullet is placed between the simple single-line bullets and the multi-paragraph "Permission manifest changes" section. Consider placing it after "Permission manifest changes" to maintain the pattern of simple bullets first, then detailed guidance — though the current placement adjacent to the generic injection bullet is also logically coherent.

• [documentation-formatting] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md:11 — The new standalone **GHA workflow command injection:** section establishes a new pattern in sub-agent files (previously only "Own:" and "Do not own:" sections existed). This is reasonable for specialized guidance but other injection types may benefit from similar treatment for consistency.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:01 PM UTC · Completed 6:08 PM UTC
Commit: d0ac11b · View workflow run →

[ben-alkov]

/fs-fix

[fullsend-ai-coder]

🤖 Fix · Started 6:48 PM UTC
Commit: d0ac11b · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 2 of 4 review findings. Added deprecation notes for ::set-output:: and ::set-env:: in both files, and added GHA injection to the summary bullet in SKILL.md. Noted disagreement on 2 [info]-level observations that were informational, not actionable.

Fixed (2):

1. ::set-output:: and ::set-env:: listed without deprecation note (internal/scaffold/fullsend-repo/skills/code-review/SKILL.md): Added deprecation parentheticals for ::set-output:: and ::set-env::, noting they remain active when ACTIONS_ALLOW_UNSECURE_COMMANDS=true. Applied same fix to security.md sub-agent for consistency.
2. Summary injection bullet missing GHA injection inline (internal/scaffold/fullsend-repo/skills/code-review/SKILL.md): Added 'GitHub Actions workflow command injection' to the summary injection bullet on line 101 to match the inline enumeration style used in security.md.

Disagreed (2):

1. Detailed GHA injection bullet placement between simple and multi-paragraph sections: This is an [info]-level observation about ordering, not a requested change. The current placement logically groups injection-related items together.
2. New standalone GHA section establishes new pattern in sub-agent files: This is an [info]-level observation about formatting patterns, not a requested change. The standalone section pattern is appropriate for detailed audit guidance that doesn't fit inline.

Tests: passed

_{Updated by fullsend fix agent}

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 6:48 PM UTC · Completed 6:53 PM UTC
Commit: d0ac11b · View workflow run →

[fullsend-ai-review]

🤖 Review · Started 6:54 PM UTC
Commit: d0ac11b · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:56 PM UTC · Completed 7:03 PM UTC
Commit: d0ac11b · View workflow run →

[fullsend-ai-retro]

🤖 Retro · Started 7:14 PM UTC
Commit: d0ac11b · View workflow run →

[fullsend-ai-retro]

Retro: PR #2035 — Add GHA workflow command injection to review security dimensions

Overall: This workflow executed well. The full pipeline (code → review → fix → re-review → merge) completed in ~1h15m with a single human intervention (/fs-fix). The code agent produced a clean, focused markdown change across two files. The review agent found real (low-severity) issues around deprecation notes and consistency. The fix agent handled them appropriately, fixing 2 and correctly disagreeing with 2 info-level observations.

Timeline

1. Issue Add GHA workflow command injection to code-review security dimensions #989 filed by retro agent on 2026-05-15 (from PR feat(#704): surface agent errors in workflow logs #764 retro identifying missed GHA command injection)
2. Code agent created PR at 17:58 UTC on 2026-06-08 (~24 days later)
3. Review agent approved at 18:08 with 2 low + 2 info findings (deprecation notes, consistency)
4. Human (ben-alkov) triggered /fs-fix at 18:46
5. Fix agent completed at 18:53 — fixed 2/4 findings, disagreed with 2 info-level
6. Re-review approved at 19:03 with 2 new low findings (::save-state:: omission)
7. Human approved and merged at 19:12

Observations

• Sandbox lint failure: The code agent reported make lint couldn't run due to Go module cache permission errors. Since this was a markdown-only change, it had no practical impact, but this would be a problem for Go code changes.
• Unresolved findings merged: The second review found ::save-state:: was missing from the GHA command list. The human chose to merge anyway — a reasonable decision for a low-severity completeness gap. Existing issues #1286 and #1956 already track auto-filing for unresolved findings.
• No proposals for ::save-state:: gap or unresolved findings — these are covered by existing issues.

Proposals filed

• Sandbox: fix Go module cache permissions for markdown-only repos (in fullsend-ai/fullsend)

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:14 PM UTC · Completed 7:20 PM UTC
Commit: d0ac11b · View workflow run →