updates to dev container, security workflow and deny.toml#20
updates to dev container, security workflow and deny.toml#20kelleyblackmore wants to merge 3 commits into
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Pull request overview
This PR introduces security tooling and infrastructure improvements for the Rust project, including a comprehensive security workflow, dependency policy configuration, and an optimized development container setup.
Key Changes:
- Adds a new GitHub Actions security workflow with multiple security scanning jobs (audit, policy checking, SBOM generation, secret scanning, and vulnerability scanning)
- Introduces cargo-deny configuration to enforce license policies and dependency management rules
- Optimizes the devcontainer by switching to a dedicated Rust image, removing redundant feature installation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/security.yml | New comprehensive security workflow with 7 jobs covering code quality checks, dependency auditing, SBOM generation, secret scanning, and filesystem vulnerability scanning |
| deny.toml | New cargo-deny configuration file defining allowed licenses (MIT, Apache-2.0, BSD variants, ISC, Unicode-DFS-2016) and dependency policy rules |
| .devcontainer/devcontainer.json | Optimized container setup by switching from universal:2 to rust:1-bookworm image and removing redundant Rust feature configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
The permissions setting here is redundant since the workflow already defines contents: read at the top level (line 11). Job-level permissions override top-level permissions, so this doesn't add security value and can be removed for simplicity. Only jobs that need different permissions (like trivy-fs which needs security-events: write) should explicitly set them.
| rust-checks: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust (stable) | ||
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| components: rustfmt, clippy | ||
|
|
||
| - name: Rust cache | ||
| uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - name: fmt (fail if changed) | ||
| run: cargo fmt --all -- --check | ||
|
|
||
| - name: clippy | ||
| run: cargo clippy --all-targets --all-features -- -D warnings | ||
|
|
||
| - name: test | ||
| run: cargo test --all --all-features | ||
|
|
There was a problem hiding this comment.
The rust-checks job duplicates functionality already present in the existing test.yml workflow. Both workflows run cargo fmt, clippy, and test on pull requests and pushes to main. This duplication increases CI runtime and maintenance burden. Consider removing this job from the security workflow and relying on the test workflow for these checks, or consolidate the workflows if security-specific context is needed for these checks.
| rust-checks: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| - name: Install Rust (stable) | |
| uses: dtolnay/rust-toolchain@stable | |
| with: | |
| components: rustfmt, clippy | |
| - name: Rust cache | |
| uses: Swatinem/rust-cache@v2 | |
| - name: fmt (fail if changed) | |
| run: cargo fmt --all -- --check | |
| - name: clippy | |
| run: cargo clippy --all-targets --all-features -- -D warnings | |
| - name: test | |
| run: cargo test --all --all-features |
| steps: | ||
| - name: Check all jobs status | ||
| run: | | ||
| if [[ "${{ contains(needs.*.result, 'failure') }}" == "true" ]]; then |
There was a problem hiding this comment.
The condition syntax here may not work as expected. The GitHub Actions expression contains(needs.*.result, 'failure') returns a boolean (true/false), but you're comparing it as a string "true" in bash. The double evaluation (GitHub Actions expression to string, then bash string comparison) is redundant. Consider using a simpler approach with the GitHub Actions if conditional directly on the needs context, or use a more standard pattern like checking the JSON array of results.
| if [[ "${{ contains(needs.*.result, 'failure') }}" == "true" ]]; then | |
| if ${{ contains(needs.*.result, 'failure') }}; then |
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
The permissions setting here is redundant since the workflow already defines contents: read at the top level (line 11). Job-level permissions override top-level permissions, so this doesn't add security value and can be removed for simplicity. Only jobs that need different permissions (like trivy-fs which needs security-events: write) should explicitly set them.
| permissions: | |
| contents: read |
No description provided.