Open
Description
Emergent Requirement - Problem
When reviewing PRs that are supposed to resolve CodeQL alerts, there is a lack of documentation on how to verify that the code changes resolve the alert. This expends developer resources and may allow improper code reviews.
Issue you discovered this emergent requirement in
- Resolve codeql alert 51 6622 #6651
- Refactor codeql.yml file #6691
- Note: Encountered while reviewing PRs, not fixing an issue
Date discovered
4/19/24
Did you have to do something temporarily
- YES
- NO
Who was involved
@gaylem, @t-will-gillis, @marioantonini, @Dartis4
What happens if this is not addressed
- Reviewers may spend much more time figuring out how to verify that CodeQL alerts are resolved (or are blocked entirely)
- Senior contributors must dedicate more time and attention to helping confused contributors
- Alternatively, reviewers approve code changes without properly testing
Resources
- Research and Provide a Demo of Unit Test Implementation for HfLA Repo #2249
- @Dartis4 is exploring the possibility of overhauling
hackforla
's testing framework, which may resolve this issue as an aside. However, this appears to be far from implementation and deployment at this time.
- @Dartis4 is exploring the possibility of overhauling
Recommended Action Items
- Make a new issue
- Discuss with team
- Let a Team Lead know
Potential solutions [draft]
Verify that the code changes flip the target CodeQL alert from 'Open' to 'Fixed'
- Capture CodeQL status on personal fork before code changes
a. Navigate to personal fork remote repo
b. From fork, create new branch according to following naming scheme:contributorUsername-issueBranchName
. This is to match the name of the branch created later in Step 2a.
c. Set issue branch as default: Settings > General > Default branch > Switch > select newly created issue branch
d. Run workflow from branch: Actions > CodeQL Scan > Run workflow > Use workflow from 'Branch: [name of the branch]' > Run workflow
e. Find target alert: Security > Code scanning > (use filters/details like filename to find the same alert)- Note: Alert numbers will not be the same across
hackforla
repo and personal fork
- Note: Alert numbers will not be the same across
- Integrate proposed code changes into branch to verify CodeQL resolution after code changes
a. Create branch locally and pull proposed code changes from PR (refer to commandline instructions in Step 3 of 'How to Review Pull Requests')
b. Modify.github/workflow/codeql.yml
: Underon:
>push:
, setbranches: ['name of the branch']
c. git add, commit, push. This should push the code changes to the branch that was previously created in Step 1, making the two branches one.
d. At this point, a new workflow run should be queued to be run. Check under Actions > CodeQL Scan
e. Once workflow finishes, check that the CodeQL alert has been closed: Security > Code scanning > Closed
Other common problems encountered when reviewing CodeQL PRs
Great documentation exists for the following, but contributors would benefit from this documentation being more obviously accessible
- Personal access tokens (Tip 7 from Hack for LA's GitHub Actions)
- Not having PATs set up causes failed workflows that may be confusing to reviewers
- Project board: duplicate from
hackforla
repo (Tip 6 from Hack for LA's GitHub Actions)- Not having classic project board set up causes 'Issue Trigger' workflow to fail
- Note: Classic project boards (like
hackforla
's) are being sunsetted and duplication is no longer possible until the project is migrated to the new Projects format.
Metadata
Assignees
Labels
Type
Projects
Status
New Issue Approval