Skip to content

fix(security): fix remaining template injection in protectAuditLabels workflow#1984

Open
mirooon wants to merge 1 commit into
mainfrom
fix/protect-audit-labels-template-injection
Open

fix(security): fix remaining template injection in protectAuditLabels workflow#1984
mirooon wants to merge 1 commit into
mainfrom
fix/protect-audit-labels-template-injection

Conversation

@mirooon

@mirooon mirooon commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Why did I implement it this way?

Follow-up to Aikido's auto-fix PR #1728, which was closed because its specific changes were already applied to the repo. Two earlier steps in protectAuditLabels.yml still contained direct ${{ github.* }} references inside run: scripts, leaving them vulnerable to template injection. This PR moves them into env: blocks, matching the pattern already used in the "Protect Audit Labels" step.

Fixed references:

  • EVENT_ACTION="${{ github.event.action }}" (inline shell assignment, "Check if event requires audit label protection" step)
  • "${{ github.actor }}" and ${{ github.actor }} ("Check for authorized actor" step)

Checklist before requesting a review

  • I have performed a self-review of my code
  • This pull request is as small as possible and only tackles one problem
  • I have added tests that cover the functionality / test the bug
  • For new facets: I have checked all points from this list: https://www.notion.so/lifi/New-Facet-Contract-Checklist-157f0ff14ac78095a2b8f999d655622e
  • I have updated any required documentation
  • I have run /pr-ready (local CodeRabbit) on this branch and resolved (or explicitly documented) all findings

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

… workflow

Move inline ${{ github.event.action }} and ${{ github.actor }} references
in the first two steps into env: blocks, matching the pattern already used
in the third step.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lifi-action-bot lifi-action-bot marked this pull request as draft June 25, 2026 16:57
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dd77b393-d8e6-4c7b-926f-be0ad6211e1d

📥 Commits

Reviewing files that changed from the base of the PR and between 7f717cb and a346823.

📒 Files selected for processing (1)
  • .github/workflows/protectAuditLabels.yml

Walkthrough

The workflow now passes EVENT_ACTION and GITHUB_ACTOR into step environment variables for the audit-label protection checks, and the shell scripts read those values from the environment.

Changes

Audit-label protection workflow

Layer / File(s) Summary
Step env variables for event and actor checks
.github/workflows/protectAuditLabels.yml
The event-action check and authorized-actor check now use step-level env values for EVENT_ACTION and GITHUB_ACTOR instead of inline shell assignment or direct ${{ github.actor }} references.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • lifinance/contracts#1746: Also refactors .github/workflows/protectAuditLabels.yml to move GitHub context values into step env variables for the audit-label protection logic.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the security fix in protectAuditLabels workflow.
Description check ✅ Passed The description matches the template well and explains the change; only the Linear task section is omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/protect-audit-labels-template-injection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mirooon mirooon marked this pull request as ready for review June 25, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants