Skip to content

fix(ci): prevent template literal injection in repo-health workflow#865

Closed
diberry wants to merge 1 commit intodevfrom
squad/769-fix-template-injection
Closed

fix(ci): prevent template literal injection in repo-health workflow#865
diberry wants to merge 1 commit intodevfrom
squad/769-fix-template-injection

Conversation

@diberry
Copy link
Copy Markdown
Collaborator

@diberry diberry commented Apr 5, 2026

Closes #769 - fixes P2 security finding from RETRO audit.

Problem

squad-repo-health.yml injects step outputs directly into actions/github-script script: blocks via JS template literals. If output contains backticks or template expressions, they get interpreted as code.

Fix

  • Template literal injection (primary): Moved all 3 step output references to env: blocks. Scripts now read process.env.STEP_RESULT (with ?? '' fallback) - treated as data, not code.
  • Heredoc delimiter injection (secondary): Replaced all 4 static EOF delimiters with unique uuidgen-based delimiters to prevent early termination.

Changes

  • .github/workflows/squad-repo-health.yml (1 file, 21 insertions, 11 deletions)
  • No changes to scripts/repo-health-comment.mjs (contract unchanged)

Replace direct steps output interpolation inside actions/github-script
script blocks with env: + process.env pattern. This prevents PR output
containing backticks or template expressions from being interpreted as
JavaScript code.

Also hardens heredoc delimiters in GITHUB_OUTPUT by using unique
uuidgen-based delimiters instead of static EOF, preventing output
injection via early delimiter termination.

Changes:
- 3x github-script steps: moved step output to env block, read via
  process.env.STEP_RESULT with nullish coalescing fallback
- 4x run steps: replaced static EOF with EOF_uuidgen

Closes #769

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 5, 2026 19:29
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 5, 2026

🟢 Impact Analysis — PR #865

Risk tier: 🟢 LOW

📊 Summary

Metric Count
Files changed 1
Files added 0
Files modified 1
Files deleted 0
Modules touched 1

🎯 Risk Factors

  • 1 files changed (≤5 → LOW)
  • 1 module(s) touched (≤1 → LOW)

📦 Modules Affected

ci-workflows (1 file)
  • .github/workflows/squad-repo-health.yml

This report is generated automatically for every PR. See #733 for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 5, 2026

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 9ade4d2

⚠️ 2 item(s) to address before review

Status Check Details
Single commit 1 commit — clean history
Not in draft Ready for review
Branch up to date Up to date with dev
Copilot review No Copilot review yet — it may still be processing
Changeset present No source files changed — changeset not required
Scope clean No .squad/ or docs/proposals/ files
No merge conflicts No merge conflicts
Copilot threads resolved No Copilot review threads
CI passing 17 check(s) still running

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 5, 2026

🏗️ Architectural Review

⚠️ Architectural review: 1 info.

Severity Category Finding Files
ℹ️ info template-sync Template files changed in .github/workflows/ but not in other template locations. If these templates should stay in sync, consider updating the others too. Changed: .github/workflows/, Unchanged: templates/, .squad-templates/, packages/squad-cli/templates/

Automated architectural review — informational only.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens the Repo Health GitHub Actions workflow against template-literal injection in actions/github-script by ensuring step outputs are treated as data (env vars) rather than interpolated into executable script text.

Changes:

  • Move step output consumption in actions/github-script from ${{ ... }} template-literal interpolation to env: + process.env.
  • Replace static EOF heredoc delimiters used for multiline $GITHUB_OUTPUT values with per-step UUID-based delimiters.

Comment on lines +44 to +47
DELIM="EOF_$(uuidgen)"
echo "result<<${DELIM}" >> $GITHUB_OUTPUT
echo "$OUTPUT" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
echo "${DELIM}" >> $GITHUB_OUTPUT
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step now relies on uuidgen being available on the runner to generate a unique heredoc delimiter. GitHub-hosted runners typically have it, but it’s not guaranteed by the workflow itself; consider using a more universally available source (e.g., /proc/sys/kernel/random/uuid) or explicitly installing the required package to avoid workflow failures. Also consider using printf instead of echo when writing $OUTPUT to $GITHUB_OUTPUT, since echo can treat certain leading values (e.g. -n, -e) as options and mangle the captured output.

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +102
DELIM="EOF_$(uuidgen)"
echo "result<<${DELIM}" >> $GITHUB_OUTPUT
echo "$OUTPUT" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
echo "${DELIM}" >> $GITHUB_OUTPUT
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step now relies on uuidgen being available on the runner to generate a unique heredoc delimiter. GitHub-hosted runners typically have it, but it’s not guaranteed by the workflow itself; consider using a more universally available source (e.g., /proc/sys/kernel/random/uuid) or explicitly installing the required package to avoid workflow failures. Also consider using printf instead of echo when writing $OUTPUT to $GITHUB_OUTPUT, since echo can treat certain leading values (e.g. -n, -e) as options and mangle the captured output.

Copilot uses AI. Check for mistakes.
Comment on lines +139 to +142
DELIM="EOF_$(uuidgen)"
echo "result<<${DELIM}" >> $GITHUB_OUTPUT
echo "$OUTPUT" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
echo "${DELIM}" >> $GITHUB_OUTPUT
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step now relies on uuidgen being available on the runner to generate a unique heredoc delimiter. GitHub-hosted runners typically have it, but it’s not guaranteed by the workflow itself; consider using a more universally available source (e.g., /proc/sys/kernel/random/uuid) or explicitly installing the required package to avoid workflow failures. Also consider using printf instead of echo when writing $OUTPUT to $GITHUB_OUTPUT, since echo can treat certain leading values (e.g. -n, -e) as options and mangle the captured output.

Copilot uses AI. Check for mistakes.
Comment on lines +179 to +182
DELIM="EOF_$(uuidgen)"
echo "result<<${DELIM}" >> $GITHUB_OUTPUT
echo "$OUTPUT" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
echo "${DELIM}" >> $GITHUB_OUTPUT
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step now relies on uuidgen being available on the runner to generate a unique heredoc delimiter. GitHub-hosted runners typically have it, but it’s not guaranteed by the workflow itself; consider using a more universally available source (e.g., /proc/sys/kernel/random/uuid) or explicitly installing the required package to avoid workflow failures. Also consider using printf instead of echo when writing $OUTPUT to $GITHUB_OUTPUT, since echo can treat certain leading values (e.g. -n, -e) as options and mangle the captured output.

Copilot uses AI. Check for mistakes.
@diberry
Copy link
Copy Markdown
Collaborator Author

diberry commented Apr 6, 2026

Closing — superseded by #873 which rewrote the same repo-health.yml file and is now merged into dev.

@diberry diberry closed this Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants