Conversation
| - name: Mask secrets | ||
| run: | | ||
| while IFS='=' read -r key value; do | ||
| [[ -z "$key" || "$key" == \#* || -z "$value" ]] && continue | ||
| echo "::add-mask::$value" | ||
| done < .env |
There was a problem hiding this comment.
This is a especial addition to the CI jobs that uses secrets that avoid leak those secret in some logs by error
There was a problem hiding this comment.
Doesn't Github upload the tarball of an action as an artifact that's publicly available, including the secrets in the .env? Also, we don't need the PRIVATE_KEY for verification.
There was a problem hiding this comment.
Good question 🤔, then that would be something to fix...
There was a problem hiding this comment.
Unless you specify a file upload on the job the files are normally lost.
The private key is accessible to any job in this repository that calls the Google cloud login step whether we put it in an environment file or not, it doesn't really matter
wischli
left a comment
wischli
left a comment
There was a problem hiding this comment.
How can I test this action before we merge it? Definitely need @gpmayorga eyes on this before merging.
It would be great to have an error reporting, either to slack or by creating a Github issue in this repo in case this action fails. Did you look into that?
| submodules: recursive | ||
|
|
||
| - name: Checkout src/ from deployed version | ||
| run: git checkout ${{ inputs.source_version }} -- src/ |
There was a problem hiding this comment.
AFAICT, we also need to include lib/ and foundry.toml. Did you test this manually?
There was a problem hiding this comment.
Yes, I already tested this.
Note the repo that is downloaded is from main, but later we add the src from the tag to be able to do forge verify-contract with that abi. As long as src compiles in its own, we're fine (at least by now)
| - name: Mask secrets | ||
| run: | | ||
| while IFS='=' read -r key value; do | ||
| [[ -z "$key" || "$key" == \#* || -z "$value" ]] && continue | ||
| echo "::add-mask::$value" | ||
| done < .env |
There was a problem hiding this comment.
Doesn't Github upload the tarball of an action as an artifact that's publicly available, including the secrets in the .env? Also, we don't need the PRIVATE_KEY for verification.
We can not test this until it's merged (GitHub limitations). But what I did is to modify the trigger to push trigger, and I could saw all jobs worked correctly, verifying it.
This CI is triggered by a specific action. So the person who trigger this will see all their jobs passing/failing, and they will evaluate consecutively. I don't think we need further error reporting (at least by now) |
There was a problem hiding this comment.
I read the script and I'm not sure I understand why we need to package the entire repo, upload it and then download it within the same job?
A much easier approach without having to deal with upload/download would be:
- Setup -> establish the matrix (nothing else)
- Verify ->
git checkout $COMMIT -- /srcthen do what you need to do for each enviornment.
|
Coverage after merging ci-verifier into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
3 participants
Product requirements
Design notes
script/VerifyFactoryContracts.s.solscriptenv/*.jsonfiles