Skip to content

Conversation

@uvizhe
Copy link
Contributor

@uvizhe uvizhe commented Nov 25, 2025

Issue Addressed

#8106

Proposed Changes

I added insecure-deps target to Makefile and a new step into check-code section of test-suite CI workflow that uses the former.

That bash multiliner is not ideal, I'd prefer a cargo plugin instead but none exists.

I also changed Cargo.toml to test that the new CI check works. Once we see a pipeline fails, I revert the change.

@uvizhe
Copy link
Contributor Author

uvizhe commented Nov 25, 2025

Oops, I did not notice there's already another PR on the same issue. Feel free to close this

Makefile Outdated
Comment on lines 302 to 355
bash -c \
"find -name Cargo.toml \
| xargs grep -P \"git\s?=\s?[\\\"']http:\" \
| tee /dev/tty \
| [ \`wc -l\` == 0 ] || (echo \"Using plain HTTP in dependencies is forbidden\" && false)"

Copy link
Member

Choose a reason for hiding this comment

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

We could add a message to display when no http link is found (the all good case)

@chong-he chong-he added waiting-on-author The reviewer has suggested changes and awaits thier implementation. infra-ci labels Dec 23, 2025
@uvizhe uvizhe force-pushed the check-insecure-deps branch from fc90403 to 0521750 Compare December 23, 2025 09:45
@uvizhe uvizhe force-pushed the check-insecure-deps branch from 0521750 to f70d3ae Compare December 23, 2025 09:49
@uvizhe uvizhe requested a review from chong-he December 23, 2025 10:00
Makefile Outdated
cargo +$(PINNED_NIGHTLY) udeps --tests --all-targets --release --features "$(TEST_FEATURES)"

# Checks dependencies for unencrypted HTTP links
# Tee preserves output. If there are matches, print a message and return 1
Copy link
Member

Choose a reason for hiding this comment

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

This line is not relevant now

Suggested change
# Tee preserves output. If there are matches, print a message and return 1

Makefile Outdated
Comment on lines 348 to 349
.ONESHELL:
SHELL = /bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

These two lines seem not necessary, we can have the command directly (need to tweak a bit the command) to check for http links, and then can error or exit 1 if found

@uvizhe uvizhe requested a review from chong-he December 24, 2025 08:46
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