Add cargo audit dependency gate - #100
Conversation
Cargo audit dependency gateIntroduces a Build and CI integration
Dependency updates
GitHub retry observability & token metrics
Test support & tests
Documentation
ValidationCI/local validation reported passing: formatting/lint/audit/tests (including feature permutations), cargo check/test permutations, markdownlint, nixie, Makefile validation, and git diff checks. A Follow-ups and related decisions
WalkthroughAdd Makefile ChangesDependency audit integration
Sequence Diagram(s)sequenceDiagram
participant CI
participant Makefile
participant CargoMetadata
participant WorkspaceRoot
participant CargoAudit
CI->>Makefile: install `cargo-audit` and run `make audit`
Makefile->>CargoMetadata: `cargo metadata --no-deps --format-version 1`
CargoMetadata-->>Makefile: workspace_root + manifest paths
Makefile->>WorkspaceRoot: cd workspace_root
WorkspaceRoot->>CargoAudit: run `cargo audit`
CargoAudit-->>Makefile: exit code
Makefile-->>CI: propagate exit code
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 15❌ Failed checks (15 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
94d3b67 to
862c6d2
Compare
Reviewer's GuideAdds a Rust dependency vulnerability audit gate via Makefile and CI, installs cargo-audit in GitHub Actions, updates Rust dependencies to satisfy the new audit, and documents the new quality gate for maintainers. Sequence diagram for CI dependency audit gatesequenceDiagram
participant GitHubActions
participant Make
participant CargoBinstall
participant CargoAudit
GitHubActions->>CargoBinstall: cargo binstall --no-confirm cargo-audit
CargoBinstall-->>GitHubActions: cargo-audit installed
GitHubActions->>Make: make audit
Make->>Make: audit target
Make->>Make: rust-audit target
Make->>CargoAudit: cargo audit (per Cargo.toml manifest)
CargoAudit-->>Make: Vulnerability report
Make-->>GitHubActions: Audit result (pass/fail)
GitHubActions->>GitHubActions: Continue CI steps if audit passes
Flow diagram for rust-audit Make target over manifestsflowchart TD
A[make audit] --> B[rust-audit]
B --> C[find . -name Cargo.toml]
C --> D{Entry under target/?}
D -- Yes --> E[Prune entry]
D -- No --> F[Set manifest_dir]
F --> G[cd manifest_dir]
G --> H[cargo audit]
H --> I{More Cargo.toml files?}
I -- Yes --> C
I -- No --> J[Exit with aggregated audit result]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
4837094 to
fbadef9
Compare
Add a Makefile audit target that runs cargo audit for Rust manifests and wire CI to install cargo-audit before executing the target. Update vulnerable dependency paths reported by the new gate. Move GitHub JWT signing to the AWS-LC backend so the unresolved rsa advisory is no longer present, and update the lockfile for fixed bytes, rustls-webpki, tar, and time releases.
Replace the local agent and documentation style guidance with the requested upstream memoryd versions so this branch tracks the shared documentation practice updates.
Update the developers guide entry for make audit to show that the targetruns cargo audit for each discovered Cargo manifest via find.
Adjust the run snapshot assertion to match non-coloured tracing output after rebase-era log formatting changes.
Corrected remaining en-GB spelling and documentation mismatches by updating AGENTS and the roadmap-style guide wording.\n\nAdjusted the developers guide find . \
\( -path '*/target/*' \) -prune -o \
-name Cargo.toml -exec sh -c 'set -e; for manifest do \
manifest_dir=$(dirname "$manifest"); \
printf "Auditing Rust manifest %s\n" "$manifest"; \
(cd "$manifest_dir" && /home/leynos/.cargo/bin/cargo audit); \
done' sh {} +
Auditing Rust manifest ./Cargo.toml
Crate: proc-macro-error
Version: 1.0.4
Warning: unmaintained
Title: proc-macro-error is unmaintained
Date: 2024-09-01
ID: RUSTSEC-2024-0370
URL: https://rustsec.org/advisories/RUSTSEC-2024-0370
Dependency tree:
proc-macro-error 1.0.4
└── rstest-bdd-macros 0.5.0
└── podbot 0.1.0 description to match Makefile behavior (target-pruned per-manifest audits), reformatted the table row to satisfy Markdown lint alignment, and kept Crate: proc-macro-error
Version: 1.0.4
Warning: unmaintained
Title: proc-macro-error is unmaintained
Date: 2024-09-01
ID: RUSTSEC-2024-0370
URL: https://rustsec.org/advisories/RUSTSEC-2024-0370
Dependency tree:
proc-macro-error 1.0.4
└── rstest-bdd-macros 0.5.0
└── podbot 0.1.0/ references explicit.
Add the missing quality-gates table caption and renumber later table captions so the developers guide keeps sequential table numbering.
Match Corbusier's Rust audit target by pruning generated dependency and virtual-environment directories before discovering Cargo manifests. Update the developers guide so the documented `make audit` behaviour stays aligned with the Makefile target.
91479ec to
317413d
Compare
|
@coderabbitai Have the following now been resolved?
|
This comment was marked as resolved.
This comment was marked as resolved.
Remove the broad lint-integrity suppression from hosted configuration BDD helpers and drop the stale per-step lint expectations it was masking. Document the Octocrab 0.51.0 JWT signing backend choice, including the AWS-LC security rationale, and add Makefile-level tests for the Rust audit target's manifest iteration, generated-directory pruning, cargo invocation, and failure propagation.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Excess Number of Function Argumentssrc/github/retry_metrics.rs: RateLimitMetrics.rate_limited What lead to degradation?RateLimitMetrics.rate_limited has 5 arguments, max arguments = 4 Why does this problem occur?Functions with many arguments indicate either a) low cohesion where the function has too many responsibilities, or b) a missing abstraction that encapsulates those arguments. How to fix it?Start by investigating the responsibilities of the function. Make sure it doesn't do too many things, in which case it should be split into smaller and more cohesive functions. Consider the refactoring INTRODUCE PARAMETER OBJECT to encapsulate arguments that refer to the same logical concept. |
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== src/github/retry_metrics.rs - rate_limited signature and body ==="
fd -e rs retry_metrics.rs --search-path src --exec cat {}Length of output: 3883 File: Current state of the codegraphThe method body has already been reduced to its absolute minimum — two statements, neither of which can be consolidated further: fn rate_limited(
&self,
req: &http::Request<octocrab::OctoBody>,
status_code: http::StatusCode,
retries_remaining: usize,
waiting_seconds: u64,
) {
RetryEventContext::new(req, status_code, retries_remaining).log_warn(
"Octocrab retry policy is waiting before retrying a GitHub API request",
Some(waiting_seconds),
);
record_octocrab_retry_event("rate_limited", status_code);
}No code change is warranted. The five-argument signature is imposed by the upstream CodeScene suppression message
|
This comment was marked as resolved.
This comment was marked as resolved.
Both `register_counter` and `register_histogram` were inlining the same `key.labels().map(...).collect()` expression to convert metric labels into `Vec<(String, String)>`. Extract it into a private `extract_key_labels` helper so the two registration paths share one definition and future label-shape changes only need editing in one place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse the two `#[test]` cases that asserted whether `warn_token_acquisition_failure` emits the `installation_token.timeout_failures.total` counter into a single `#[rstest]` case-driven test that picks the error kind, elapsed duration, and expected-counter-presence per row. Behaviour and assertion text are preserved on both branches; the body is no longer duplicated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/developers-guide.md`:
- Around line 863-870: The table listing types (rows for `CounterEvent`,
`HistogramEvent`, `RecordingMetrics`, `RecordedCounter`, `RecordedHistogram`) is
missing a caption; add a short caption line immediately after that table block
(directly below the final | `RecordedHistogram` row) describing the table (e.g.,
"Shared test-support types and their purposes") so the table has a caption as
required by the docs guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4121f6d3-bf74-43be-b3a9-61fd27615817
📒 Files selected for processing (9)
docs/developers-guide.mdsrc/github/installation_token_tests.rssrc/github/mod.rssrc/github/retry_metrics.rssrc/github/retry_metrics_tests.rssrc/github/test_support.rssrc/github/tests.rstests/github_retry_metrics.rstests/make_audit_target.rs
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Code Duplicationsrc/github/test_support.rs: What lead to degradation?The module contains 2 functions with similar structure: Recorder.register_counter,Recorder.register_histogram Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
Match the existing _Table N_ caption convention used for Tables 1–3 in this guide; the style guide requires a caption for every table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/developers-guide.md (1)
30-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify that
make auditruns per manifest, not once at the workspace root.The description "run
cargo auditonce there" contradicts the per-manifest execution documented in the PR objectives and the past review comment (marked as addressed in commit 10080d1). Reword the purpose cell to reflect thatcargo auditruns separately for each discovered manifest after pruning generated directories.Triage:
[type:docstyle]Proposed fix
-| `make audit` | `cargo metadata --no-deps --format-version 1 \| python3 -c ...` | Derive workspace root with `python3`; run `cargo audit` once there | +| `make audit` | `cargo metadata --no-deps --format-version 1 \| python3 -c ...` | Derive workspace root with `python3`; run `cargo audit` per discovered manifest |Based on learnings "Use the markdown files within the
docs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/developers-guide.md` at line 30, The table entry for `make audit` is incorrect: update the purpose cell to state that `make audit` derives the workspace root with the python3 helper, prunes generated directories, discovers manifests, and runs `cargo audit` for each discovered manifest (i.e., per-manifest), rather than saying it runs `cargo audit` once at the workspace root; refer to the `make audit` target and `cargo audit` in the docs and mention the manifest-per-manifest behavior and pruning of generated directories.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/developers-guide.md`:
- Line 30: The table entry for `make audit` is incorrect: update the purpose
cell to state that `make audit` derives the workspace root with the python3
helper, prunes generated directories, discovers manifests, and runs `cargo
audit` for each discovered manifest (i.e., per-manifest), rather than saying it
runs `cargo audit` once at the workspace root; refer to the `make audit` target
and `cargo audit` in the docs and mention the manifest-per-manifest behavior and
pruning of generated directories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2dbfdb86-50ea-458b-93f1-b8ce4dac6d5b
📒 Files selected for processing (1)
docs/developers-guide.md
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Code Duplicationsrc/github/test_support.rs: What lead to degradation?The module contains 2 functions with similar structure: Recorder.register_counter,Recorder.register_histogram Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== src/github/test_support.rs (current) ==="
fd -e rs test_support --search-path src --exec cat {}Length of output: 4039 File: This fix has already been applied. The current file contains the fn extract_key_labels(key: &Key) -> Vec<(String, String)> {
key.labels()
.map(|label| (label.key().to_owned(), label.value().to_owned()))
.collect()
}
// register_counter
labels: extract_key_labels(key),
// register_histogram
labels: extract_key_labels(key),The CodeScene diagnostic is anchored to the original diff hunk, which predates this change. No further work is needed. |
|
@coderabbitai The CodeScene diagnostic is anchored to the original diff hunk, which predates this change. No further work is needed. Please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
This branch adds a Cargo audit dependency gate so Podbot fails CI when RustSec
reports vulnerable dependency paths. It mirrors Corbusier's Rust audit approach
with a top-level Makefile target, installs
cargo-auditin CI before the gateruns, and updates dependency selections so the new gate passes without ignoring
vulnerability advisories.
No roadmap task or issue was identified for this branch.
Review walkthrough
auditandrust-audittargets, including the manifest discovery loop that skipstargetoutput.cargo-auditand the newmake auditstep.jsonwebtokenandoctocrab, which removes the unresolvedrsaadvisory path.Validation
make check-fmt: passedmake lint: passedmake audit: passed; it still reports the allowed informationalproc-macro-errorwarning throughrstest-bdd-macrosmake test: passedcargo check --no-default-features: passedcargo test --no-default-features --test cli_feature_gating --test compile_contract: passedcargo test --features internal: passedmake markdownlint: passedmake nixie: passedmbake validate Makefile: passedgit diff --check: passedNotes
make fmtwas attempted, but the Markdown formatter path emitted repository-widepre-existing line-length diagnostics before the unrelated generated changes were
discarded. The narrower validation commands above passed after the final branch
state was restored.
Summary by Sourcery
Add a dependency vulnerability audit gate to the build and CI pipeline and update Rust dependencies to satisfy the new checks.
New Features:
Enhancements:
CI:
Documentation: