Skip to content

Fix script injection GitHub actions#237

Open
yitingb wants to merge 1 commit into
mainfrom
fix-script-injection-github-actions
Open

Fix script injection GitHub actions#237
yitingb wants to merge 1 commit into
mainfrom
fix-script-injection-github-actions

Conversation

@yitingb
Copy link
Copy Markdown
Member

@yitingb yitingb commented Feb 2, 2026

Issue #, if available:

Description of changes:
This commit fixes a script injection vulnerability in the GitHub Actions workflow by changing how PR number and SHA are extracted. Instead of directly interpolating ${{ github.event.* }} expressions (which can be exploited by malicious input), it now safely reads from the $GITHUB_EVENT_PATH JSON file using jq. The values are still sanitized with tr to strip unwanted characters before being written to files.

Why is this change necessary:
When GitHub Actions expressions like ${{ github.event.pull_request.head.sha }} are interpolated directly into shell scripts or environment variables, an attacker who controls the value (e.g., via a crafted PR branch name or title) could inject malicious shell commands that execute during the workflow run. This is a well-known GitHub Actions vulnerability called "script injection."

By reading from $GITHUB_EVENT_PATH and parsing with jq, the data is treated as a literal string rather than being interpreted by the shell—preventing any embedded commands from executing. This is the recommended mitigation from GitHub's security documentation for handling untrusted input in workflows.
How was this change tested:

Any additional information or context required to review the change:

Checklist:

  • Updated the README if applicable
  • Updated or added new unit tests
  • Updated or added new integration tests
  • Updated or added new end-to-end tests

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@yitingb yitingb force-pushed the fix-script-injection-github-actions branch from fcbe69f to 0bcd1f4 Compare February 2, 2026 22:37
@yitingb yitingb force-pushed the fix-script-injection-github-actions branch from 0bcd1f4 to cb3ec54 Compare February 2, 2026 22:39
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 4, 2026

Unit Tests Coverage Report

File Coverage Lines Branches
All files 77% 84% 71%
com.aws.greengrass.logmanager.services.DiskSpaceManagementService 84% 93% 75%
com.aws.greengrass.logmanager.CloudWatchLogsUploader 90% 97% 83%
com.aws.greengrass.logmanager.LogManagerService 70% 83% 57%
com.aws.greengrass.logmanager.LogManagerService$CurrentProcessingFileInformation 79% 89% 68%
com.aws.greengrass.logmanager.PositionTrackingBufferedReader 86% 87% 86%
com.aws.greengrass.logmanager.CloudWatchAttemptLogsProcessor 88% 89% 86%
com.aws.greengrass.logmanager.util.ConfigUtil 85% 85% 85%
com.aws.greengrass.logmanager.util.SdkClientWrapper 58% 67% 50%
com.aws.greengrass.logmanager.util.CloudWatchClientFactory 36% 36% 0%
com.aws.greengrass.logmanager.model.CloudWatchAttemptLogInformation 100% 100% 0%
com.aws.greengrass.logmanager.model.EventType 100% 100% 0%
com.aws.greengrass.logmanager.model.ProcessingFiles 80% 91% 68%
com.aws.greengrass.logmanager.model.ComponentType 100% 100% 0%
com.aws.greengrass.logmanager.model.LogFile 85% 78% 92%
com.aws.greengrass.logmanager.model.LogFileGroup 70% 75% 65%

Minimum allowed coverage is 65%

Generated by 🐒 cobertura-action against cb3ec54

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 4, 2026

Integration Tests Coverage Report

File Coverage Lines Branches
All files 69% 76% 62%
com.aws.greengrass.logmanager.services.DiskSpaceManagementService 65% 68% 62%
com.aws.greengrass.logmanager.CloudWatchLogsUploader 44% 47% 41%
com.aws.greengrass.logmanager.LogManagerService 85% 94% 76%
com.aws.greengrass.logmanager.LogManagerService$CurrentProcessingFileInformation 46% 55% 37%
com.aws.greengrass.logmanager.PositionTrackingBufferedReader 72% 82% 63%
com.aws.greengrass.logmanager.CloudWatchAttemptLogsProcessor 67% 73% 61%
com.aws.greengrass.logmanager.util.ConfigUtil 66% 61% 71%
com.aws.greengrass.logmanager.util.SdkClientWrapper 17% 17% 0%
com.aws.greengrass.logmanager.util.CloudWatchClientFactory 100% 100% 0%
com.aws.greengrass.logmanager.model.CloudWatchAttemptLogInformation 100% 100% 0%
com.aws.greengrass.logmanager.model.EventType 100% 100% 0%
com.aws.greengrass.logmanager.model.ProcessingFiles 87% 93% 81%
com.aws.greengrass.logmanager.model.ComponentType 100% 100% 0%
com.aws.greengrass.logmanager.model.LogFile 40% 46% 34%
com.aws.greengrass.logmanager.model.LogFileGroup 71% 80% 61%

Minimum allowed coverage is 58%

Generated by 🐒 cobertura-action against cb3ec54

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.

1 participant