Skip to content

Conversation

sachinh-amazon
Copy link
Contributor

@sachinh-amazon sachinh-amazon commented Sep 30, 2025

Description of changes:

This PR fixes the following issues in the Security Scanning workflow.

  1. AccessDenied error when assuming IAM role from a PR: GitHub does not allow PRs to access repo secrets if the pull_request trigger is used for invoking a GitHub Action. This is because GitHub considers this unsafe as with the pull_request trigger, the GitHub Action's .yaml file that's used to invoke the workflow is from the PR branch. The trigger has now been updated to pull_request_target. With pull_request_target as the trigger, the GitHub Action's .yaml file will be from the base branch of the PR. This is more secure as it doesn't allow untrusted code to invoke a workflow and will also grant access to GitHub repo secrets.
  2. ./scripts/security-scan.sh: Permission denied error: For each branch scan, the security-scan.sh file is always downloaded from the main branch to ensure latest changes in the script are used for running the scan. After downloading, the file needs "execute" permissions. This is fixed by updating file permissions using sudo chmod +x.
  3. Regex for fetching upstream branches for scanning: The previous regex also considered 1.95.x branch for security scanning. The regex has been fixed, it will now consider main and <digit>.<digit> branches and will no longer consider *.*.x branches as eligible for scanning.
  4. Use an environment variable for accessing head_ref as the branch name can be set by an external contributor and can be used to mount an injection attack. This is per a recommendation from CodeQL.

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

@sachinh-amazon sachinh-amazon requested a review from a team as a code owner September 30, 2025 13:05
@sachinh-amazon sachinh-amazon changed the title Fix issues for the Security Scan Workflow Fix issues in the Security Scan Workflow Sep 30, 2025
@sachinh-amazon sachinh-amazon changed the title Fix issues in the Security Scan Workflow Fix issues for the Security Scan Workflow Sep 30, 2025

- name: Determine branches for PR events
id: determine-pr-branches
if: github.event_name == 'pull_request'
Copy link
Contributor

Choose a reason for hiding this comment

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

should this condition be changed if trigger were changed to from pull_request to pull_request_target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Yes it indeed needs to be changed. I will fix it.

@sachinh-amazon sachinh-amazon merged commit d7cd961 into main Oct 1, 2025
3 checks passed
@sachinh-amazon sachinh-amazon deleted the sachinh-security-scan branch October 1, 2025 12:31
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