Add verify audit action #793
Conversation
|
Warning Rate limit exceeded@0xDEnYO has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 46 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new GitHub Actions workflow named Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Coverage ReportLine Coverage: 75.37% (1595 / 2116 lines) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/verifyAudit.yml (1 hunks)
Additional context used
actionlint
.github/workflows/verifyAudit.yml
144-144: shellcheck reported issue in this script: SC2034:warning:9:1: COMMIT_HASHES_FILE appears unused. Verify use (or export if used externally)
(shellcheck)
144-144: shellcheck reported issue in this script: SC2034:warning:10:1: AUDITOR_GIT_HANDLES_FILE appears unused. Verify use (or export if used externally)
(shellcheck)
144-144: shellcheck reported issue in this script: SC2027:warning:122:94: The surrounding quotes actually unquote this. Remove or escape them
(shellcheck)
144-144: shellcheck reported issue in this script: SC2086:info:122:94: Double quote to prevent globbing and word splitting
(shellcheck)
144-144: shellcheck reported issue in this script: SC2086:info:159:19: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (5)
.github/workflows/verifyAudit.yml (5)
1-10: General Overview: Workflow DescriptionThe workflow is well-documented with comments explaining each step, which is crucial for maintainability and understanding the purpose of the workflow. The comments clearly outline the conditions under which labels are assigned and the verification process for audits. This is a good practice as it helps future maintainers or reviewers understand the logic and purpose of the workflow.
15-28: Trigger Conditions and Environment SetupThe workflow is triggered on pull request events such as opened, synchronize, reopened, and ready_for_review, which are appropriate for the audit checks it performs. The environment variables and secrets are well-defined, ensuring that the workflow has all the necessary information to perform its tasks. Using
GITHUB_TOKENwith a specific permission set enhances security by limiting the scope of actions the workflow can perform.
29-72: Detailed Review of Steps: Checking for Protected ContractsThe script checks for changes in protected folders and handles the absence of changes robustly by setting an environment variable to prevent further steps from executing. This is a critical part of the workflow as it determines whether an audit is necessary based on the files changed in the PR. The use of environment variables to pass state between steps is a good practice in GitHub Actions workflows.
363-387: Final Steps: Label Assignment and Failure HandlingThe final steps of the workflow handle the assignment of the "AuditCompleted" label and ensure that the workflow fails correctly if any critical checks do not pass. The use of conditional steps based on the
CONTINUEenvironment variable is a good practice to control the flow of the workflow based on the outcomes of previous steps.
73-138: Label Management LogicThe logic to manage labels based on the audit requirement is implemented effectively. Using conditional checks and the GitHub CLI to manipulate labels ensures that the PR is correctly annotated based on its audit status. The detailed logging and error handling in this section help in debugging and maintaining the workflow. However, ensure that the labels used are correctly configured in the repository to avoid issues during execution.
Verification successful
Labels are correctly configured in the repository
The labels "AuditRequired" and "AuditNotRequired" used in the workflow are present in the repository, ensuring that the label management logic will function as intended. No further action is needed regarding label configuration.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify label configuration in the repository gh label listLength of output: 231
| # KNOWN LIMITATIONS | ||
| # - will only check the last 100 commits for any matches with audit commit hashes | ||
| # - only one audit can be registered per contract (in a specific version) | ||
|
|
There was a problem hiding this comment.
Consideration of Known Limitations
The workflow acknowledges its limitations, such as only checking the last 100 commits and handling one audit per contract version. This transparency is beneficial for users and maintainers to understand the scope and constraints of the workflow. However, it might be useful to explore ways to handle these limitations in future iterations, possibly by integrating more robust handling mechanisms or expanding the scope of the audit checks.
Consider addressing these limitations in future updates to ensure the workflow can handle more complex scenarios.
|
|
||
| ##### make sure that a file exists at the audit report path | ||
| if [ ! -f "$AUDIT_REPORT_PATH" ]; then | ||
| echo -e "\033[31mCould not find an audit report in path $AUDIT_REPORT_PATH for contract "$FILENAME".\033[0m" | ||
| echo -e "\033[31mThis github action cannot complete before the audit report is uploaded to 'audit/reports/'.\033[0m" | ||
| echo -e "\033[31mAborting now.\033[0m" | ||
| echo "CONTINUE=false" >> "$GITHUB_ENV" | ||
| exit 1 | ||
| else | ||
| echo "The audit report for $AUDIT_ID was found in path $AUDIT_REPORT_PATH" | ||
| fi | ||
|
|
||
| ##### make sure that audit log entry contains audit commit hash | ||
| if [ -z "$AUDIT_COMMIT_HASH" ]; then | ||
| echo -e "\033[31mThe audit log entry for file $FILE contains invalid or no 'auditCommitHash' information.\033[0m" | ||
| echo -e "\033[31mThis github action cannot complete before the audit log is complete.\033[0m" | ||
| echo -e "\033[31mAborting now.\033[0m" | ||
| echo "CONTINUE=false" >> "$GITHUB_ENV" | ||
| exit 1 | ||
| else | ||
| echo "The audit log contains the commit hash that was audited in $AUDIT_ID: $AUDIT_COMMIT_HASH" | ||
| fi | ||
| echo -e "\033[32mThe audit log contains all required information for contract $FILE\033[0m" | ||
|
|
||
| echo "now checking if audit commit hash $AUDIT_COMMIT_HASH is associated with PR $PR_NUMBER" | ||
| ##### Fetch the list of commits associated with the PR | ||
| COMMIT_LIST=$(gh pr view "$PR_NUMBER" --json commits --jq '.commits[].oid') | ||
|
|
||
| ##### Check if the target commit is in the list | ||
| if echo "$COMMIT_LIST" | grep -q "$TARGET_COMMIT"; then | ||
| echo -e "\033[32mCommit $AUDIT_COMMIT_HASH is associated with PR #$PR_NUMBER.\033[0m" | ||
| else | ||
| echo -e "\033[31mCommit $AUDIT_COMMIT_HASH is NOT associated with PR #$PR_NUMBER.\033[0m" | ||
| echo "CONTINUE=false" >> "$GITHUB_ENV" | ||
| exit 1 | ||
| fi | ||
|
|
||
|
|
||
| ##### Check if the auditor git handle exists on github | ||
| echo "now checking if the auditor git handle ($AUDITOR_GIT_HANDLE) actually exists" | ||
| if gh api users/$AUDITOR_GIT_HANDLE > /dev/null 2>&1; then | ||
| echo -e "\033[32mA user with handle '$AUDITOR_GIT_HANDLE' exists on GitHub.\033[0m" | ||
| else | ||
| echo -e "\033[31mA user with handle '$AUDITOR_GIT_HANDLE' does not exist on GitHub.\033[0m" | ||
| echo -e "\033[31mPlease fix the audit log before continuing.\033[0m" | ||
| echo -e "\033[31mCheck failed.\033[0m" | ||
| echo "CONTINUE=false" >> "$GITHUB_ENV" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # ##### ----------------------------------------------------------------------------- | ||
| # ##### DISABLED FOR NOW (NEED TO CHECK IF THIS IS COMPATIBLE WITH OUR FLOW) | ||
| # ##### Fetch PR reviews using the GitHub API via gh cli | ||
| # echo "now checking if the auditor ($AUDITOR_GIT_HANDLE) approved this PR ($PR_NUMBER)" | ||
| # REVIEWS=$(gh api repos/lifinance/contracts/pulls/$PR_NUMBER/reviews --jq '.[] | select(.state == "APPROVED") | @json') | ||
|
|
||
| # ##### Check if the output is empty or not valid JSON | ||
| # if [[ -z "$REVIEWS" ]]; then | ||
| # echo "ERROR: No reviews found or failed to fetch reviews for PR #$PR_NUMBER" | ||
| # exit 1 | ||
| # fi | ||
| # ##### Flag to track if the review by the specified person is found | ||
| # FOUND_REVIEW=false | ||
|
|
||
| # ##### Check if the desired reviewer is present among the reviews | ||
| # echo "$REVIEWS" | jq -c '.' | while read -r REVIEW; do | ||
| # AUTHOR=$(echo "$REVIEW" | jq -r '.user.login // empty') | ||
| # STATE=$(echo "$REVIEW" | jq -r '.state // empty') | ||
|
|
||
| # echo "found review by $AUTHOR with state $STATE" | ||
|
|
||
|
|
||
| # ##### Check if the reviewer is the person we're looking for | ||
| # if [ "$AUTHOR" == "$REVIEWER" ]; then | ||
| # echo "Approving review found by $REVIEWER" | ||
| # FOUND_REVIEW=true | ||
| # exit 0 | ||
| # fi | ||
| # done | ||
|
|
||
| # ##### If no matching review was found, exit with an error | ||
| # if [ "$FOUND_REVIEW" == true ]; then | ||
| # echo -e "\033[32mPR $PR_NUMBER has an approving review by $AUDITOR_GIT_HANDLE\033[0m" | ||
| # echo -e "\033[32mCheck passed\033[0m" | ||
| # exit 0 | ||
| # else | ||
| # echo -e "\033[31mERROR: No review found by git user '$AUDITOR_GIT_HANDLE' (= the auditor)\033[0m" | ||
| # echo -e "\033[31mCheck failed\033[0m" | ||
| # echo "CONTINUE=false" >> "$GITHUB_ENV" | ||
| # exit 1 | ||
| # fi | ||
| # ##### ----------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| done <<< "$PROTECTED_CONTRACTS" | ||
|
|
||
| echo -e "\033[32mSuccessfully verified that all contracts in this PRs are audited.\033[0m" | ||
| echo -e "\033[32mCheck passed.\033[0m" | ||
| echo "CONTINUE=true" >> "$GITHUB_ENV" | ||
| echo "Assigning label 'AuditCompleted' next" |
There was a problem hiding this comment.
Audit Log Verification Process
This section is the core of the workflow, where it checks the audit log for each protected contract affected by the PR. The detailed checks for each required piece of information in the audit log are crucial for ensuring the integrity of the audit process. However, there are several shellcheck warnings that need to be addressed to ensure the script is robust and free of common scripting errors.
Address the shellcheck warnings to improve the script's robustness and reliability. Consider using double quotes to prevent globbing and word splitting, and verify the usage of declared variables.
Tools
actionlint
144-144: shellcheck reported issue in this script: SC2034:warning:9:1: COMMIT_HASHES_FILE appears unused. Verify use (or export if used externally)
(shellcheck)
144-144: shellcheck reported issue in this script: SC2034:warning:10:1: AUDITOR_GIT_HANDLES_FILE appears unused. Verify use (or export if used externally)
(shellcheck)
144-144: shellcheck reported issue in this script: SC2027:warning:122:94: The surrounding quotes actually unquote this. Remove or escape them
(shellcheck)
144-144: shellcheck reported issue in this script: SC2086:info:122:94: Double quote to prevent globbing and word splitting
(shellcheck)
144-144: shellcheck reported issue in this script: SC2086:info:159:19: Double quote to prevent globbing and word splitting
(shellcheck)
Which Jira task belongs to this PR?
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)