Skip to content

Commit

Permalink
fix(ci): semgrep, cov-comment, buf-break-no-fail (#15)
Browse files Browse the repository at this point in the history
* chore(lint): security options in docker compose

* chore(lint): unlock mutex in testutil

* chore(lint): limit value of uint using strconv

* fix(lint): check out old code correctly for diff

In the event of a pull_request_target action, the checkout action by
default does not check out the PR but the base repository. This change
explicitly sets it up to check out the PR's head branch.

* Revert "fix(lint): check out old code correctly"

This reverts commit 4f24ba2.

* chore(lint): split test coverage commenter

See comment on test.yml for explanation.

* chore(ci): trigger go based CIs

delete superfluous comment

* fix(lint): add empty line at end of yaml file

* chore(lint): fix the test-comment workflow

- shell check the script
- add top level permissions

* Revert "chore(lint): unlock mutex in testutil"

This reverts commit a6cdb02. It is said
that the unlocking of the mutex might deviate from the original purpose
of the function. Instead, we are electing to ignore the testutil folder,
which will be sent in the next commit.

* chore(ci): ignore testutil for semgrep

* chore(ci): do not fail if breaking proto changes

* chore(ci): trigger buf-breaking-action check

This commit will be reverted before merge of the PR.

* chore(ci): add comment if buf reports change

* fix(ci): only overwrite prev coverage comment

* fix(ci)(proto): +comment read/write permission

* fix(ci): add comment rw permission to job

* fix(ci): update job name in test-comment

* fix(ci): give `proto.yml` all rw permissions

* fix(ci): split buf-breaking and its comment

As before, the `pull_request` event does not have enough permissions to
make a comment and the `pull_request_target` cannot safely check out the
PR. Hence, we split the commenting process into 2 parts:
 - Run the buf breaking action and do not fail the workflow even if it
   errors
 - In a separate workflow, consume the artifact produced by the previous
   workflow and comment (or append to an existing comment) its contents.
Note that the *-comment.yml workflows are being commented blindly at the
moment because it is not possible to run them until they exist on the
branch.

* fix(ci): if condition shouldn't be always true

* Revert "chore(ci): trigger go based CIs"

This reverts commit 62b3744.

* Revert "chore(ci): trigger buf-breaking-action...

check." This reverts commit 88566fd.
  • Loading branch information
MaxMustermann2 authored Mar 14, 2024
1 parent 3ded4ac commit bdb2207
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 24 deletions.
81 changes: 81 additions & 0 deletions .github/workflows/proto-comment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
name: Comment protobuf breaking action outcome on the pull request

permissions:
# for finding and downloading artifacts.
actions: read
# for commenting on pull requests.
pull-requests: write
# the content of the repo is irrelevant to this workflow.
contents: none

on:
workflow_run:
workflows: ["Protobuf"]
types:
- completed

jobs:
download-artifact-and-comment:
runs-on: ubuntu-latest
if: >
github.event.workflow_run.conclusion == 'success'
steps:
- name: 'Download artifact'
uses: actions/[email protected]
with:
script: |
var artifacts = await github.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: ${{github.event.workflow_run.id }},
});
var matchArtifact = artifacts.data.artifacts.find((artifact) => {
return artifact.name == "result";
});
if (!matchArtifact) {
var core = require('@actions/core');
core.setFailed('Artifact "result" not found.');
return;
}
var download = await github.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
var fs = require('fs');
fs.writeFileSync('${{github.workspace}}/result.zip', Buffer.from(download.data));
- run: unzip result.zip -d result
- name: Read PR number and outcome
run: |
pr_number=$(cat "result/pr_number.txt")
outcome=$(cat "result/outcome.txt")
echo "PR_NUMBER=${pr_number}" >> "$GITHUB_ENV"
echo "OUTCOME=${outcome}" >> "$GITHUB_ENV"
- name: Find comment
id: find-comment
uses: peter-evans/find-comment@v2
with:
issue-number: ${{ env.PR_NUMBER }}
comment-author: 'github-actions[bot]'
body-includes: buf breaking change
- name: Comment status of break-check in the case of failure
if: ${{ env.OUTCOME == 'failure' }}
uses: peter-evans/create-or-update-comment@v3
with:
issue-number: ${{ env.PR_NUMBER }}
comment-id: ${{ steps.find-comment.outputs.comment-id }}
body: |
${{ github.sha }} (${{ github.event.workflow_run.updated_at }}) has a buf breaking change.
View the workflow run: [here](https://github.com/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }})
edit-mode: append
- name: Comment status of break-check in the case of success
if: env.OUTCOME == 'success' && steps.find-comment.outputs.comment-id != ''
uses: peter-evans/create-or-update-comment@v3
with:
issue-number: ${{ env.PR_NUMBER }}
comment-id: ${{ steps.find-comment.outputs.comment-id }}
body: |
${{ github.sha }} (${{ github.event.workflow_run.updated_at }}) has no buf breaking changes.
View the workflow run: [here](https://github.com/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }})
edit-mode: append
20 changes: 19 additions & 1 deletion .github/workflows/proto.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ on:
- "proto/**"

permissions:
contents: read
# for uploading artifacts
contents: write
pull-requests: read

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
build:
Expand Down Expand Up @@ -42,10 +48,22 @@ jobs:
- uses: actions/checkout@v4
- uses: bufbuild/[email protected]
- uses: bufbuild/buf-breaking-action@v1
id: break-check
with:
input: "proto"
# previously, this ran on ref=HEAD~1, which is incorrect as it can
# only be used to compare within a branch. it is designed to run
# on a PR, so it must compare the HEAD of the base branch against
# the PR branch.
against: "https://github.com/${{ github.repository }}.git#branch=${{ github.event.pull_request.base.ref }},subdir=proto"
# do not fail the build if there are breaking changes
continue-on-error: true
- name: Make buf breaking changes outcome as txt file
run: |
mkdir -p ./result/
echo "${{ steps.break-check.outcome }}" > ./result/outcome.txt
echo "${{ github.event.pull_request.number }}" > ./result/pr_number.txt
- uses: actions/upload-artifact@v2
with:
name: result
path: ./result/
71 changes: 71 additions & 0 deletions .github/workflows/test-comment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# This workflow runs after test.yml and comments the test coverage on the pull request.
name: Comment test coverage on the pull request

permissions:
# for finding and downloading artifacts.
actions: read
# for commenting on pull requests.
pull-requests: write
# the content of the repo is irrelevant to this workflow.
contents: none

on:
workflow_run:
workflows: ["Tests"]
types:
- completed

jobs:
download-artifact-and-comment:
runs-on: ubuntu-latest
if: >
github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.conclusion == 'success'
steps:
- name: Download artifact
uses: actions/[email protected]
with:
script: |
var artifacts = await github.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: ${{github.event.workflow_run.id }},
});
var matchArtifact = artifacts.data.artifacts.find((artifact) => {
return artifact.name == "result"
});
if (!matchArtifact) {
var core = require('@actions/core');
core.setFailed('Artifact "result" not found.');
return;
}
var download = await github.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
var fs = require('fs');
fs.writeFileSync('${{github.workspace}}/result.zip', Buffer.from(download.data));
- run: unzip result.zip -d result
- name: Read PR number and coverage
run: |
pr_number=$(cat "result/pr_number.txt")
coverage=$(cat "result/coverage.txt")
echo "PR_NUMBER=${pr_number}" >> "$GITHUB_ENV"
echo "COVERAGE=${coverage}" >> "$GITHUB_ENV"
- name: Find comment
id: find-comment
uses: peter-evans/find-comment@v2
with:
issue-number: ${{ env.PR_NUMBER }}
comment-author: 'github-actions[bot]'
body-includes: Coverage as of
- name: Comment coverage on PR
uses: peter-evans/create-or-update-comment@v3
with:
issue-number: ${{ env.PR_NUMBER }}
comment-id: ${{ steps.find-comment.outputs.comment-id }}
body: |
Coverage as of ${{ github.sha }}: ${{ env.COVERAGE }}%
edit-mode: append
35 changes: 18 additions & 17 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# This workflow runs on every push and pull request to the repository.
# It then calculates the unit test coverage and checks if it's above a certain threshold.
# this information is passed on to another workflow as artifacts for commenting on the PR.
# This is because the `pull_request` event does not have the commenting permissions.
# We could switch to `pull_request_target` which does have them, however, it
# opens a security hole. See:
# https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
name: Tests
on:
# for write permission, use pull_request_target and not pull_request.
pull_request_target:
pull_request:
push:
branches:
- develop
Expand All @@ -10,8 +16,9 @@ on:
- release/**

permissions:
# for uploading artifacts
contents: write
pull-requests: write
pull-requests: read

# Automatically cancel run if another commit to the same ref is detected.
concurrency:
Expand All @@ -30,7 +37,6 @@ jobs:
- uses: technote-space/[email protected]
with:
PATTERNS: |
**/**.sol
**/**.go
go.mod
go.sum
Expand All @@ -47,19 +53,14 @@ jobs:
# TODO: increase this threshold with time to 80
threshold-total: 10
if: env.GIT_DIFF
- name: Find comment
id: find-comment
uses: peter-evans/find-comment@v2
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: 'github-actions[bot]'
- name: Generate artifact for PR
run: |
mkdir -p ./result/
echo "${{ steps.output-coverage.outputs.total-coverage }}" > ./result/coverage.txt
echo "${{ github.event.pull_request.number }}" > ./result/pr_number.txt
if: env.GIT_DIFF && github.event_name == 'pull_request'
- name: Comment coverage on PR
uses: peter-evans/create-or-update-comment@v3
- uses: actions/upload-artifact@v2
with:
issue-number: ${{ github.event.pull_request.number }}
comment-id: ${{ steps.find-comment.outputs.comment-id }}
body: |
Coverage as of ${{ github.sha }}: ${{ steps.output-coverage.outputs.total-coverage }}%
edit-mode: append
name: result
path: ./result/
if: env.GIT_DIFF && github.event_name == 'pull_request'
1 change: 1 addition & 0 deletions .semgrepignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ vendor/
# Common test paths
test/
tests/
testutil/
*_test.go
*.pb.gw.go
*.pb.go
Expand Down
12 changes: 8 additions & 4 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ services:
exocorenode0:
container_name: exocorenode0
image: "exocore/node"
read_only: true
environment:
- DEBUG=1
- ID=0
- LOG=${LOG:-exocore.log}
cap_add:
- SYS_PTRACE
security_opt:
- seccomp:unconfined
- no-new-privileges:true
ports:
- "26656-26657:26656-26657"
- "1317:1317"
Expand All @@ -28,14 +29,15 @@ services:
exocorenode1:
container_name: exocorenode1
image: "exocore/node"
read_only: true
environment:
- DEBUG=0
- ID=1
- LOG=${LOG:-exocore.log}
cap_add:
- SYS_PTRACE
security_opt:
- seccomp:unconfined
- no-new-privileges:true
ports:
- "26666-26667:26656-26657"
- "1318:1317"
Expand All @@ -52,14 +54,15 @@ services:
exocorenode2:
container_name: exocorenode2
image: "exocore/node"
read_only: true
environment:
- DEBUG=0
- ID=2
- LOG=${LOG:-exocore.log}
cap_add:
- SYS_PTRACE
security_opt:
- seccomp:unconfined
- no-new-privileges:true
ports:
- "26676-26677:26656-26657"
- "1319:1317"
Expand All @@ -76,14 +79,15 @@ services:
exocorenode3:
container_name: exocorenode3
image: "exocore/node"
read_only: true
environment:
- DEBUG=0
- ID=3
- LOG=${LOG:-exocore.log}
cap_add:
- SYS_PTRACE
security_opt:
- seccomp:unconfined
- no-new-privileges:true
ports:
- "26686-26687:26656-26657"
- "1320:1317"
Expand Down
4 changes: 2 additions & 2 deletions x/restaking_assets_manage/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func RegisterClientChain() *cobra.Command {
if err != nil {
return errorsmod.Wrap(restakingtype.ErrCliCmdInputArg, fmt.Sprintf("error arg is:%v", args[2]))
}
addressLength, err := strconv.ParseUint(args[3], 10, 64)
addressLength, err := strconv.ParseUint(args[3], 10, 32)
if err != nil {
return errorsmod.Wrap(restakingtype.ErrCliCmdInputArg, fmt.Sprintf("error arg is:%v", args[3]))
}
Expand Down Expand Up @@ -105,7 +105,7 @@ func RegisterAsset() *cobra.Command {
if err != nil {
return errorsmod.Wrap(restakingtype.ErrCliCmdInputArg, fmt.Sprintf("error arg is:%v", args[5]))
}
decimal, err := strconv.ParseUint(args[6], 10, 64)
decimal, err := strconv.ParseUint(args[6], 10, 32)
if err != nil {
return errorsmod.Wrap(restakingtype.ErrCliCmdInputArg, fmt.Sprintf("error arg is:%v", args[6]))
}
Expand Down

0 comments on commit bdb2207

Please sign in to comment.