fix(aqua): improve security feature detection#7385
Conversation
Two fixes for aqua security info: 1. Default security features to enabled when config is present (unless explicitly disabled with `enabled: false`). Previously used `unwrap_or(false)` which meant features weren't shown unless explicitly enabled. 2. Check version_overrides for security features, not just the base package. Many packages define security features (like SLSA, cosign, checksums) only in version-specific overrides. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves security feature detection for Aqua packages by checking version-specific overrides in addition to base package configuration, and by defaulting security features to enabled when their configuration is present (unless explicitly set to enabled: false).
Key Changes:
- Security feature detection now scans both base package and version overrides
- Default behavior changed from opt-in (
enabled: true) to opt-out (enabled: false) - Made
version_overridesfield public to enable this cross-version security feature checking
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/backend/aqua.rs | Refactored security feature detection to check all package versions and default features to enabled |
| crates/aqua-registry/src/types.rs | Made version_overrides field public to support checking security features across versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let all_pkgs: Vec<&AquaPackage> = | ||
| std::iter::once(&pkg).chain(pkg.version_overrides.iter()).collect(); |
There was a problem hiding this comment.
The collection into a Vec is unnecessary since the iterator is only used for reading. Consider removing .collect() and using the iterator directly in the subsequent checks, or storing it as let all_pkgs = std::iter::once(&pkg).chain(pkg.version_overrides.iter()); if you need to reuse it multiple times.
| if all_pkgs.iter().any(|p| { | ||
| p.github_artifact_attestations | ||
| .as_ref() | ||
| .is_some_and(|a| a.enabled.unwrap_or(true)) | ||
| }) { |
There was a problem hiding this comment.
The default behavior change from unwrap_or(false) to unwrap_or(true) is a significant semantic shift that could affect existing configurations. This means packages that previously had security features disabled by default will now have them enabled. Consider whether this breaking change aligns with security best practices and user expectations, especially for features that may have performance implications or require additional setup.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| let algorithm = all_pkgs | ||
| .iter() | ||
| .filter_map(|p| p.checksum.as_ref()) | ||
| .find_map(|c| c.algorithm.as_ref().map(|a| a.to_string())); |
There was a problem hiding this comment.
Bug: Metadata extracted from disabled configurations
The code determines if a feature is enabled by checking if ANY configuration has it enabled, but extracts metadata (algorithm, signer_workflow, public_key) from the FIRST configuration that has the field, without filtering by enabled status. If the base package has a disabled config with metadata X and a version override has an enabled config with metadata Y, the displayed metadata will incorrectly show X from the disabled config. The filter_map chain needs to also filter by enabled status before extracting metadata.
Additional Locations (2)
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.12.11 x -- echo |
21.6 ± 0.7 | 20.0 | 27.0 | 1.00 |
mise x -- echo |
22.2 ± 0.7 | 20.2 | 25.6 | 1.03 ± 0.05 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.12.11 env |
21.0 ± 0.6 | 19.4 | 22.9 | 1.00 |
mise env |
21.8 ± 1.1 | 19.4 | 39.8 | 1.04 ± 0.06 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.12.11 hook-env |
21.1 ± 0.6 | 19.7 | 23.3 | 1.00 |
mise hook-env |
22.0 ± 0.6 | 20.1 | 23.7 | 1.04 ± 0.04 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.12.11 ls |
18.8 ± 0.6 | 17.3 | 20.7 | 1.00 |
mise ls |
19.9 ± 0.8 | 17.4 | 24.1 | 1.06 ± 0.06 |
xtasks/test/perf
| Command | mise-2025.12.11 | mise | Variance |
|---|---|---|---|
| install (cached) | 115ms | 115ms | +0% |
| ls (cached) | 70ms | 70ms | +0% |
| bin-paths (cached) | 76ms | 77ms | -1% |
| task-ls (cached) | 293ms | -87% |
Summary
enabled: falseBefore this fix,
mise tool aqua:suzuki-shunsuke/ghcpshowedSecurity: [none]even though the package has SLSA, cosign, and checksum configured.After:
{ "security": [ {"type": "checksum", "algorithm": "sha256"}, {"type": "slsa"}, {"type": "cosign"} ] }Test plan
mise tool aqua:suzuki-shunsuke/ghcpnow shows SLSA, cosign, and checksummise tool aqua:cli/clishows checksum and github_attestations🤖 Generated with Claude Code
Note
Aggregate security features across base package and
version_overrides, defaulting to enabled when present; exposeversion_overridespublicly.src/backend/aqua.rs):version_overrides.checksum,slsa_provenance,minisign, andgithub_artifact_attestations(incl. nestedcosign) as enabled by default when config exists unless explicitly disabled.algorithm, attestationsigner_workflow, minisignpublic_key).crates/aqua-registry/src/types.rs):AquaPackage.version_overridespublic to enable backend access.Written by Cursor Bugbot for commit d4c4d4f. This will update automatically on new commits. Configure here.