-
Notifications
You must be signed in to change notification settings - Fork 934
feat: add CI check for HTTPS links in Cargo.toml #8386
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
base: unstable
Are you sure you want to change the base?
feat: add CI check for HTTPS links in Cargo.toml #8386
Conversation
This script checks for insecure HTTP links in Cargo.toml files and ensures all git dependencies use HTTPS instead.
|
|
|
Thanks for the PR. Could you please rebase to the |
chong-he
left a comment
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.
Looks pretty good to me, tested and it's working locally. Left some comments, and also need to rebase to unstable
| @@ -0,0 +1,43 @@ | |||
| #!/bin/bash | |||
|
|
|||
| # Check for insecure HTTP links in Cargo.toml files | |||
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.
This file has permission issue when I tested locally. This is also caught in the CI: https://github.com/sigp/lighthouse/actions/runs/19207968175/job/55140383694?pr=8386
You need to make the file executable
.github/workflows/test-suite.yml
Outdated
| - name: Run cargo audit | ||
| run: make audit-CI | ||
| - name: Check for HTTPS links in Cargo.toml | ||
| run: ./scripts/ci/check-https-links.sh |
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.
We can put this in Makefile, similar to other checks, for example, the mdlint check:
Lines 238 to 241 in e3ee7fe
| # Check for markdown files | |
| mdlint: | |
| ./scripts/mdlint.sh | |
and then replace with make https-links here. This is easier if we want to run locally, though this may not be run locally frequently
Co-authored-by: chonghe <[email protected]>
Co-authored-by: chonghe <[email protected]>
Co-authored-by: chonghe <[email protected]>
.github/workflows/test-suite.yml
Outdated
| run: make audit-CI | ||
| - name: Check for HTTPS links in Cargo.toml | ||
| run: | | ||
| chmod +x ./scripts/ci/check-https-links.sh |
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.
I don't think we need to run chmod here
You can update the script file permission and push that file, that would be cleaner
chong-he
left a comment
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.

Issue Addressed
#8106
Proposed Changes
Added a CI check to detect insecure HTTP links in Cargo.toml files.
scripts/ci/check-https-links.shto scan all Cargo.toml files for HTTP linkscheck-codejob in CI workflowAdditional Info
_