-
Notifications
You must be signed in to change notification settings - Fork 2
[CERT-9592] Vyper setup #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Certora Run Started (Fail to Start)
Certora Run Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this 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 adds Vyper support to the action pipeline by introducing a new vyper-version
input parameter that allows users to install specific versions of the Vyper compiler.
- Adds
vyper-version
input parameter for specifying Vyper installation version - Implements Vyper installation logic using UV tool manager
- Updates documentation to reflect the new Vyper configuration option
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
action.yml | Adds vyper-version input and installation step with version handling logic |
README.md | Documents the new vyper-version parameter in EVM-specific inputs section |
.github/workflows/main.yml | Demonstrates usage by setting vyper-version to "latest" in test workflow |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if [ '${{ inputs.vyper-version }}' == 'latest' ]; then | ||
VYPER_PACKAGE="vyper" | ||
else | ||
VYPER_PACKAGE="vyper==${{ inputs.vyper-version }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shell script is vulnerable to command injection through the vyper-version input. The input should be properly quoted and validated to prevent malicious commands from being executed.
if [ '${{ inputs.vyper-version }}' == 'latest' ]; then | |
VYPER_PACKAGE="vyper" | |
else | |
VYPER_PACKAGE="vyper==${{ inputs.vyper-version }}" | |
# Validate vyper-version input: allow "latest" or version strings like "0.3.10", "0.3.10-beta" | |
if [[ '${{ inputs.vyper-version }}' == "latest" ]]; then | |
VYPER_PACKAGE="vyper" | |
elif [[ '${{ inputs.vyper-version }}' =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9]+)?$ ]]; then | |
VYPER_PACKAGE="vyper==${{ inputs.vyper-version }}" | |
else | |
echo "::error title=Invalid Vyper version::The vyper-version input ('${{ inputs.vyper-version }}') is invalid. Only 'latest' or version strings like '0.3.10' are allowed." | |
exit 1 |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It got some point here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's irrelevant, as this is a direct user input. If someone wants to destroy their own CI, there is very little we can do about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, everything is passed as an escaped string, thus it does not really matter what is there, as uv would just try to install a package with that name and fail.
VYPER_PACKAGE="vyper==${{ inputs.vyper-version }}" | ||
fi | ||
echo "Installing Vyper version: $VYPER_PACKAGE" | ||
uv tool install "$VYPER_PACKAGE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The installation step lacks error handling. If the Vyper installation fails, users won't get clear feedback about what went wrong. Consider adding error checking and meaningful error messages.
uv tool install "$VYPER_PACKAGE" | |
if ! uv tool install "$VYPER_PACKAGE"; then | |
echo "::error title=Vyper Installation Failed::Failed to install Vyper package '$VYPER_PACKAGE'. Please check the version and network connectivity, and try again." | |
exit 1 | |
fi |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verification Results
- Group ID: ee98d771-bfbb-4863-9787-ef11e0d03ef9
Job | Result | SANITY_FAILED | VIOLATED | Link |
---|---|---|---|---|
Default.conf | ❌ | 2 | 1 | Link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving although adding a parser to the Vyper version might be beneficial
vyper-version: | ||
required: false | ||
description: |- | ||
The version of Vyper to install. Can be latest, or a specific version like 0.3.3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version of Vyper to install. Can be latest, or a specific version like 0.3.3. | |
The version of Vyper to install. Can be `latest`, or a specific version like 0.3.3. |
if [ '${{ inputs.vyper-version }}' == 'latest' ]; then | ||
VYPER_PACKAGE="vyper" | ||
else | ||
VYPER_PACKAGE="vyper==${{ inputs.vyper-version }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It got some point here
Certora Run Started (Fail to Start)
Certora Run Summary
|
🚀 Pull Request Overview
This PR adds a Vyper setup into our pipeline.
📜 Tests Checklist
Before submitting this PR, ensure that all tests pass and meet the following conditions: