Skip to content

Conversation

@dylan-smith
Copy link
Contributor

@dylan-smith dylan-smith commented Jun 25, 2025

To improve the security of the Integration Tests build I added explicit workflow permissions, and I required the person who queues it to specify the SHA of the PR in addition to the PR number.

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

Copilot AI review requested due to automatic review settings June 25, 2025 04:40
Copy link
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

This PR enhances security for the Integration Tests workflow by adding explicit permissions and verifying the PR head SHA.

  • Added a required sha input to the workflow dispatch.
  • Introduced a permissions block with least-privilege settings.
  • Added a “Check SHA” step to validate the provided SHA matches the PR head.
Comments suppressed due to low confidence (3)

.github/workflows/integration-tests.yml:9

  • Input name is defined as sha (lowercase) but later referenced as ${{ github.event.inputs.SHA }} (uppercase). This mismatch means the check will always compare against an empty value—use ${{ github.event.inputs.sha }} consistently.
      sha:

.github/workflows/integration-tests.yml:31

  • The run block mixes Bash syntax (backticks) with PowerShell commands (Write-Output, if ($...)). Specify shell: bash or convert fully to PowerShell to ensure commands execute correctly.
    - name: Check SHA

.github/workflows/integration-tests.yml:33

  • [nitpick] Rather than invoking git rev-parse, consider using ${{ github.event.inputs.sha }} or github.event.pull_request.head.sha directly from the GitHub context to simplify the workflow.
        prsha=`git rev-parse origin/pull/${{ github.event.inputs.pr_number }}/head | awk '{ print $1 }'`

@github-actions
Copy link

github-actions bot commented Jun 25, 2025

Unit Test Results

  1 files    1 suites   21s ⏱️
887 tests 887 ✅ 0 💤 0 ❌
888 runs  888 ✅ 0 💤 0 ❌

Results for commit d410a25.

♻️ This comment has been updated with latest results.

@dylan-smith dylan-smith requested a review from a team June 25, 2025 04:57
Copy link
Member

@dpmex4527 dpmex4527 left a comment

Choose a reason for hiding this comment

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

Changes look good! Thanks for adding SHA validations for enhanced security when running our integration tests!

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
bbs2gh 82% 76% 669
Octoshift 87% 76% 1410
gei 81% 73% 596
ado2gh 84% 78% 631
Summary 84% (7228 / 8564) 76% (1691 / 2238) 3306

@dylan-smith dylan-smith merged commit 7345e5c into main Jun 25, 2025
33 of 48 checks passed
@dylan-smith dylan-smith deleted the dylan-smith/test-build-security branch June 25, 2025 16:29
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.

3 participants