Add Rust dependency audit to Makefile#354
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
SummaryThis PR extends the repository's audit gate to include Rust dependency checks via ChangesAudit infrastructure and dependency checks
Security audit module refactoring
Tests and documentation
Rust roadmap tracking
Validation
NotesNo new execplan document was identified for this branch. Issue WalkthroughSplit repository audits into ChangesUnified Audit and Dependency Maintenance
Sequence Diagram(s)sequenceDiagram
participant Makefile
participant PNPM as pnpm
participant CI as CI
participant CargoBinstall as cargo-binstall
participant CargoAudit as cargo-audit
participant AuditUtils as security/audit-utils.js
participant PackageData as security/audit-package-data.js
participant Registry as npm-registry
Makefile->>PNPM: run workspace audits (pnpm -r --if-present run audit)
Makefile->>PNPM: run pnpm run audit:validate
CI->>CargoBinstall: run cargo binstall --no-confirm cargo-audit@0.22.1
CargoBinstall->>CargoAudit: provide cargo-audit binary
Makefile->>CargoAudit: invoke cargo audit --file Cargo.lock --ignore RUSTSEC-2023-0071
PNPM-->>AuditUtils: stdout/json or retirement error
AuditUtils->>PackageData: collectInstalledPackageVersions(auditIo)
AuditUtils->>Registry: POST bulk advisory query with Abort (fallback)
Registry-->>AuditUtils: bulk advisory payload
AuditUtils->>PackageData: normalizeBulkAdvisories(payload)
Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 warnings, 1 inconclusive)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Split the audit target into frontend and Rust phases, and run `cargo audit` against the workspace lockfile. Install `cargo-audit` in CI before the audit gate so the target is available on runners. Update vulnerable frontend and Rust dependencies to patched releases. Keep the SQLx optional MySQL RSA advisory ignored because this workspace only enables PostgreSQL and RustSec has no fixed RSA release.
Point the CI Rust setup action at the current `leynos/shared-actions` default-branch commit so the workflow uses the latest shared action.
Restore the `openapi-dump` and `er-snapshots` binary entries that were collapsed into duplicate `ingest-osm` targets during conflict resolution. Keep the rebased manifest aligned with `origin/main` while preserving the audit branch changes.
beeeb98 to
1a63868
Compare
Reviewer's GuideAdds a Rust dependency audit phase to the repository’s audit gate, wires it into CI, and updates Rust and frontend dependency versions to clear actionable vulnerabilities while documenting the new behavior. Flow diagram for updated make audit target with Rust dependency auditingflowchart LR
actor_CI[CI or developer]
actor_CI --> make_audit[make audit]
make_audit --> audit_node[audit-node: pnpm -r install && pnpm -r run audit && pnpm run audit:validate]
make_audit --> rust_audit[rust-audit: cargo audit --file Cargo.lock --ignore RUSTSEC-2023-0071]
audit_node --> frontend_deps[Frontend dependencies audited]
rust_audit --> rust_deps[Rust dependencies audited]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @.github/workflows/ci.yml:
- Around line 94-95: Update the "Install cargo-audit" CI step so it installs a
fixed release instead of the latest; replace the current command string "cargo
binstall --no-confirm cargo-audit" with one that pins the package to version
0.22.1 (e.g., using the cargo-binstall package@version form) so the CI
consistently installs cargo-audit@0.22.1.
🪄 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: 379d987d-38f8-44c5-b9ed-2fb95f4fd2c4
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.github/workflows/ci.ymlMakefilebackend/Cargo.tomldocs/repository-structure.mdpackage.json
Add `make audit` to the documented quality gates and note that the local setup expects Corepack to enable `pnpm`. This keeps the developer guide aligned with the repository-structure guidance.
Remove the redundant node audit install, add an explicit `cargo-audit` availability check, and pin the CI install to `cargo-audit@0.22.1`. Thread audit I/O and expiry-date dependencies through the audit helpers so fallible process and network work is explicit at the boundary. Add required `#[expect]` reasons and roadmap links for still-valid lint suppressions. Update the `qs` override to `6.15.2` and refresh the pnpm lockfile to clear the current `GHSA-q8mj-m7cp-5q26` audit violation.
|
@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix: |
Document that `make audit` covers both frontend and Rust dependency checks, and note the `cargo-audit` prerequisite alongside Corepack so the local setup matches the Makefile and CI flow.
This comment was marked as resolved.
This comment was marked as resolved.
Add root Vitest coverage for the `rust-audit` target, including the `cargo-audit` availability guard, command shape, and configured RustSec ignore. Run architecture linting in CI and tighten the remaining lint expectation reasons with concrete roadmap references.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
security/audit-utils.js (1)
1-416: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winDecompose this module to comply with the 400-line limit.
The file is 416 lines, exceeding the 400-line maximum. Extract one of the following into a dedicated module:
- Advisory normalisation helpers (
deriveAdvisoryKey,normalizeBulkAdvisories,addPackageAdvisories, lines 195–265).- Reporting utilities (
formatAdvisoryLine,reportUnexpectedAdvisories,partitionAdvisoriesById, lines 362–415).As per coding guidelines: "No single code file may be longer than 400 lines; long switch statements or dispatch tables should be broken up by feature, and large test data blocks moved to external files."
🤖 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`:
- Line 26: The repository-wide commit-gates list in the developers guide is
missing `make audit`; update the "commit-gates" list (the section that lists
required commands for commits) to include `make audit` alongside the existing
entries so the guide and other quality-gate lists are synchronized; ensure the
entry is added in the same format as the other commands in that list.
🪄 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: 78fadc3e-0258-4fd5-b9a3-e0f430f775de
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/workflows/ci.ymlMakefilebackend/src/inbound/http/schemas.rsbackend/src/outbound/persistence/models.rsbackend/tests/example_data_runs_bdd.rsdocs/developers-guide.mdpackage.jsonscripts/makefile-audit.test.mjssecurity/audit-utils.jssecurity/validate-audit.js
|
@coderabbitai Have the following now been resolved?
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8603
1. Duplicate .PHONY: all clean be fe … audit audit-node rust-audit \
lint-rust … workspace-sync
.PHONY: audit audit-node rust-audit ← remove this line2. # Current (inline check)
rust-audit:
`@command` -v cargo-audit >/dev/null 2>&1 || { echo "Error: cargo-audit is required. Install it with 'cargo binstall --no-confirm cargo-audit@0.22.1'."; exit 1; }
$(CARGO) audit --file Cargo.lock $(CARGO_AUDIT_IGNORES)
# Fix: use the existing macro, keep the install hint in a comment
rust-audit:
$(call ensure_tool,cargo-audit)
# RUSTSEC-2023-0071 is in SQLx's optional MySQL support; this workspace only enables PostgreSQL.
# Install cargo-audit with: cargo binstall --no-confirm cargo-audit@0.22.1
$(CARGO) audit --file Cargo.lock $(CARGO_AUDIT_IGNORES)Remove the duplicate |
Bump `pg-embed-setup-unpriv` to `0.5.1` and refresh `Cargo.lock` so the worker subprocess receives the parent environment in CI. Pin the embedded PostgreSQL runtime version in the Rust test environment to avoid release-listing requests during archive resolution. Extract audit reporting helpers from `security/audit-utils.js` so the shared audit utility module stays below the repository file-size limit.
|
@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. Comment on file import { execFileSync, spawnSync } from 'node:child_process';
export {❌ New issue: Overall Code Complexity |
This comment was marked as resolved.
This comment was marked as resolved.
Add `make audit` to the remaining developer-guide commit-gate references so the required command lists match the repository-wide quality gate set.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai please create a new GitHub issue for the following: Issue 1 — Rust Compiler Lint Integrity
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh issue create \
--repo leynos/wildside \
--title "Remove #[expect(dead_code)] suppressions once roadmap item 3.5.1 lands" \
--assignee leynos \
--body "## Summary
Track and remove \`#[expect(dead_code)]\` suppressions on the following items once the corresponding \`docs/backend-roadmap.md 3.5.1\` work is implemented:
- \`RouteRow\`
- \`ExampleDataRunRow\`
- \`UserRow.updated_at\`
- \`WalkSessionRow.created_at\` and \`WalkSessionRow.updated_at\`
## Context
The suppressions are correctly using \`#[expect(dead_code)]\` (which will cause a compile error automatically once the suppressed item is used), so no action is required before the roadmap items land. This issue exists solely to ensure clean-up is not forgotten when roadmap item 3.5.1 is implemented.
Raised during review of PR `#354` by `@leynos`."Length of output: 924
|
Document audit exception policy responsibilities, route advisory reporting through an injected IO adapter, cover bulk-advisory abort cleanup, and reuse the shared Makefile tool guard for cargo-audit.
|
@coderabbitai please provide an AI agent prompt for addition of a reference to #360 to relevant expectation reasons in the code |
This comment was marked as resolved.
This comment was marked as resolved.
Add the tracked GitHub issue to the roadmap 3.5.1 dead-code expectations for generated schema and persistence timestamp fields. Keep the cleanup obligation traceable from the source without changing runtime behaviour.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 `@scripts/security-audit-helpers.test.mjs`:
- Around line 451-455: The test is swallowing all exceptions from runAuditJson
which hides regressions; update the property test to assert behavior based on
shouldAbort: when shouldAbort is true, assert runAuditJson(auditIo) rejects with
a timeout error matching /Bulk advisory audit timed out after \d+ms at/; when
shouldAbort is false, assert runAuditJson(auditIo) resolves and returns an
object with status: 0 (use expect(...).rejects.toThrow(...) and
expect(...).resolves.toMatchObject(...) respectively) instead of the empty
try/catch around runAuditJson.
🪄 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: 9e1933f1-d453-403f-8915-2634b4bc790a
📒 Files selected for processing (8)
Makefilebackend/src/inbound/http/schemas.rsbackend/src/outbound/persistence/models.rsscripts/makefile-audit.test.mjsscripts/security-audit-helpers.test.mjsscripts/security-audit-reporting.test.mjssecurity/audit-exception-policy.jssecurity/audit-reporting.js
Replace the catch-all in the bulk advisory timeout property test with explicit expectations for each generated case. The aborting path must reject with the timeout message, while the successful path must resolve with a clean audit status.
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. ❌ Failed checks (1 error, 5 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
Update the developer guide to match the current `reportUnexpectedAdvisories` signature and describe the optional stderr adapter used by tests.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai please create new GitHub issues for the following: Testing (Unit And Behavioural) |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh issue create \
--repo leynos/wildside \
--title "Add E2E and behavioural tests for validate-audit.js workflow and Makefile audit targets" \
--assignee leynos \
--body "## Summary
Makefile audit tests exercise only dry-run command strings; no end-to-end or behavioural tests exist for the \`validate-audit.js\` workflow or for actual Makefile target execution.
## Problems
- \`scripts/makefile-audit.test.mjs\` verifies only that expected command strings appear in \`make --dry-run\` output; it does not execute the targets with real tools.
- \`validate-audit.js\` has no tests that exercise the \`assertValidSchema\` and \`assertMitigated\` orchestration paths with real or mock advisories.
- There are no behavioural tests that verify audit execution failure paths (e.g. unexpected advisory detected, schema validation error).
## Acceptance criteria
- Add integration or behavioural tests for the \`validate-audit.js\` orchestration, covering:
- \`assertValidSchema\` with valid and invalid exception-ledger JSON.
- \`assertMitigated\` with a match between advisories and the exception ledger (no error), and with an unmatched advisory (error + non-zero exit).
- Add behavioural tests that execute relevant Makefile targets (\`audit-node\`, \`rust-audit\`) with actual (or suitably shimmed) tool behaviour rather than dry-run string checks.
## Context
Raised during review of PR `#354` by `@leynos`."
echo "---"
gh issue create \
--repo leynos/wildside \
--title "Add E2E snapshot tests for validate-audit.js error output paths" \
--assignee leynos \
--body "## Summary
Snapshot tests for advisory reporting helpers are well-designed, but end-to-end snapshot tests for \`validate-audit.js\` error output are missing despite the PR recommendation.
## Problems
- No snapshot tests exist for the following error paths in \`validate-audit.js\`:
- AJV schema validation errors (malformed \`audit-exceptions.json\`).
- Unexpected advisory detection (\`reportUnexpectedAdvisories\` output).
- Validator vulnerability detection errors (advisory present in audit but not in exception ledger).
## Acceptance criteria
- Add snapshot tests (using Vitest \`toMatchInlineSnapshot\` or \`toMatchSnapshot\`) for \`validate-audit.js\` error paths, covering at minimum:
- Schema validation failure output.
- Unexpected-advisory heading and bullet-line output.
- At least one end-to-end path that drives \`validate-audit.js\` from a controlled advisory fixture through to the final error message captured from \`reportingIo\`.
## Context
Raised during review of PR `#354` by `@leynos`."
echo "---"
gh issue create \
--repo leynos/wildside \
--title "Decouple assertMitigated domain logic from reportUnexpectedAdvisories I/O side effects" \
--assignee leynos \
--body "## Summary
\`assertMitigated\` in \`validate-audit.js\` couples a pure policy decision (determining whether unexpected advisories exist) to infrastructure I/O (\`reportUnexpectedAdvisories\` side effects), violating domain-adapter boundaries already established by the \`policyIo\` pattern in \`audit-exception-policy.js\`.
## Problem
Calling \`reportUnexpectedAdvisories\` directly inside \`assertMitigated\` means:
- The domain function cannot be tested without either suppressing or capturing \`console.error\` output.
- The reporting transport is baked into domain logic rather than injected at the call site.
- The pattern is inconsistent with \`assertNoExpired\`, which accepts a \`policyIo\` adapter.
## Acceptance criteria
- Extract the \`reportUnexpectedAdvisories\` call out of \`assertMitigated\`.
- \`assertMitigated\` should return (or expose) the partition result rather than performing I/O.
- \`validate-audit.js\` invokes \`reportUnexpectedAdvisories\` after \`assertMitigated\` completes, passing an injected \`reportingIo\` adapter if testability is required.
- Update tests to exercise \`assertMitigated\` independently of I/O, and to exercise the reporting step separately.
## Context
Raised during review of PR `#354` by `@leynos`."Length of output: 4262
|
Summary
This branch extends the repository audit gate so Rust dependencies are checked alongside the existing frontend audit workflow. It splits the Makefile target into frontend and Rust phases, installs
cargo-auditin CI before the audit gate, updates vulnerable dependency versions found while exercising the expanded target, updates the CI Rust setup action to the currentleynos/shared-actionsdefault-branch commit, and raises thewsoverride to 8.20.1 so GHSA-58qx-3vcg-4xpx no longer fails the frontend audit.No roadmap task, issue, or execplan was identified for this branch.
Review walkthrough
make auditnow delegates toaudit-nodeandrust-audit, including the scopedRUSTSEC-2023-0071ignore for SQLx's optional MySQL dependency path.cargo-auditand the updatedleynos/shared-actionssetup-rustpin ate4c6b0e200a057edf927c45c298e7ddf229b3934.ws8.20.1 override used by Puppeteer'spuppeteer-coredependency.Validation
make audit: passed;cargo auditreports no unignored vulnerabilities, the frontend audit no longer reports GHSA-58qx-3vcg-4xpx, and the SQLx optional MySQL RSA advisory remains ignored with a documented rationale.make check-fmt: passed after thewsoverride update.make lint: passed after thewsoverride update.make test: passed after thewsoverride update; Rust nextest reported 1195 passed and 4 skipped, followed by passing frontend and workspace tests.pnpm run test: passed as a focused audit-tooling regression check.make fmt: passed.Notes
cargo auditstill reports allowed warnings for unmaintained, unsound, or yanked crates in the dependency graph. This branch addresses the audit violations that cause the target to fail and records the remaining RSA advisory as a scoped ignore because the workspace only enables PostgreSQL support for SQLx.Summary by Sourcery
Add Rust dependency auditing to the repository-wide audit gate and refresh dependencies to satisfy the expanded checks.
New Features:
cargo auditvia the consolidatedmake audittarget.Enhancements:
CI:
cargo-auditin the CI workflow and bump the shared Rust setup action to the latest pinned commit.