ci: pin actions to commit SHAs (supply-chain); add yara-scan feature build/test job - #14
Merged
Merged
Conversation
…build/test job Closes the CI gap where the yara-scan feature was never built in CI.
Contributor
Reviewer's GuidePins all GitHub Actions in CI and release workflows to specific commit SHAs for supply-chain hardening, and adds a dedicated CI job to build, test, and lint the Sequence diagram for the new yara-scan CI jobsequenceDiagram
participant CI_workflow
participant actions_checkout
participant dtolnay_rust_toolchain
participant Swatinem_rust_cache
participant apt_get
participant cargo
CI_workflow->>actions_checkout: uses actions/checkout@3d3c42e5...
CI_workflow->>dtolnay_rust_toolchain: uses dtolnay/rust-toolchain@4cda84d5...
CI_workflow->>Swatinem_rust_cache: uses Swatinem/rust-cache@e18b4977...
CI_workflow->>apt_get: run sudo apt-get update
CI_workflow->>apt_get: run sudo apt-get install -y clang libclang-dev
CI_workflow->>cargo: run cargo build --release --features yara-scan --verbose
CI_workflow->>cargo: run cargo test --features yara-scan --verbose
CI_workflow->>cargo: run cargo clippy --all-targets --features yara-scan -- -D warnings
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
yarajob largely duplicates the setup steps from other jobs; consider extracting common steps into a reusable workflow or composite action to keep CI configuration DRY and easier to maintain. - The
apt-get update && apt-get installstep in theyarajob will run on every workflow execution; if this becomes a bottleneck, you might want to look into using a prebuilt image or a dedicated setup-clang action to avoid repeated package installs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `yara` job largely duplicates the setup steps from other jobs; consider extracting common steps into a reusable workflow or composite action to keep CI configuration DRY and easier to maintain.
- The `apt-get update && apt-get install` step in the `yara` job will run on every workflow execution; if this becomes a bottleneck, you might want to look into using a prebuilt image or a dedicated setup-clang action to avoid repeated package installs.
## Individual Comments
### Comment 1
<location path=".github/workflows/ci.yml" line_range="58-59" />
<code_context>
+ - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2
+ # bindgen needs libclang; the vendored libyara compiles from source with the
+ # runner's C toolchain (build-essential is preinstalled on ubuntu-latest).
+ - name: Install clang/libclang for the vendored yara build
+ run: sudo apt-get update && sudo apt-get install -y clang libclang-dev
+ - run: cargo build --release --features yara-scan --verbose
+ - run: cargo test --features yara-scan --verbose
</code_context>
<issue_to_address>
**suggestion (performance):** Tighten the apt-get install to avoid pulling unnecessary packages and speed up the job.
Please switch to `sudo apt-get update && sudo apt-get install -y --no-install-recommends clang libclang-dev` so we don’t pull in recommended packages unnecessarily while still meeting bindgen/libyara’s requirements.
```suggestion
- name: Install clang/libclang for the vendored yara build
run: sudo apt-get update && sudo apt-get install -y --no-install-recommends clang libclang-dev
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+58
to
+59
| - name: Install clang/libclang for the vendored yara build | ||
| run: sudo apt-get update && sudo apt-get install -y clang libclang-dev |
Contributor
There was a problem hiding this comment.
suggestion (performance): Tighten the apt-get install to avoid pulling unnecessary packages and speed up the job.
Please switch to sudo apt-get update && sudo apt-get install -y --no-install-recommends clang libclang-dev so we don’t pull in recommended packages unnecessarily while still meeting bindgen/libyara’s requirements.
Suggested change
| - name: Install clang/libclang for the vendored yara build | |
| run: sudo apt-get update && sudo apt-get install -y clang libclang-dev | |
| - name: Install clang/libclang for the vendored yara build | |
| run: sudo apt-get update && sudo apt-get install -y --no-install-recommends clang libclang-dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the CI gap where the yara-scan feature was never built in CI.
Summary by Sourcery
Pin GitHub Actions in CI and release workflows to specific commit SHAs and add CI coverage for the
yara-scanfeature, including build, test, and lint steps.Build:
CI:
yara-scanCI job that installs clang/libclang and runs build, tests, and clippy with theyara-scanfeature enabled.