Skip to content

Conversation

@CreeptoGengar
Copy link

Issue Addressed

#8106

Proposed Changes

Added a CI check to detect insecure HTTP links in Cargo.toml files.

  • Created scripts/ci/check-https-links.sh to scan all Cargo.toml files for HTTP links
  • Integrated the check into the check-code job in CI workflow
  • Prevented insecure HTTP dependencies from being merged

Additional Info

_

This script checks for insecure HTTP links in Cargo.toml files and ensures all git dependencies use HTTPS instead.
@cla-assistant
Copy link

cla-assistant bot commented Nov 9, 2025

CLA assistant check
All committers have signed the CLA.

@cla-assistant
Copy link

cla-assistant bot commented Nov 9, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@chong-he
Copy link
Member

Thanks for the PR. Could you please rebase to the unstable branch?

@chong-he chong-he self-requested a review November 12, 2025 02:24
Copy link
Member

@chong-he chong-he left a 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
Copy link
Member

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

- name: Run cargo audit
run: make audit-CI
- name: Check for HTTPS links in Cargo.toml
run: ./scripts/ci/check-https-links.sh
Copy link
Member

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:

lighthouse/Makefile

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

@chong-he chong-he added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Nov 12, 2025
@CreeptoGengar CreeptoGengar changed the base branch from stable to unstable November 12, 2025 19:16
run: make audit-CI
- name: Check for HTTPS links in Cargo.toml
run: |
chmod +x ./scripts/ci/check-https-links.sh
Copy link
Member

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

Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script file is still not executable
Screenshot from 2025-11-15 08-35-07

You need to chmod and then push the file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra-ci waiting-on-author The reviewer has suggested changes and awaits thier implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants