Skip to content

chore: add cargo-deny + dependency-review + commitlint workflows#22

Merged
satyakwok merged 2 commits into
mainfrom
chore/add-supply-chain-hardening
May 13, 2026
Merged

chore: add cargo-deny + dependency-review + commitlint workflows#22
satyakwok merged 2 commits into
mainfrom
chore/add-supply-chain-hardening

Conversation

@satyakwok
Copy link
Copy Markdown
Member

@satyakwok satyakwok commented May 13, 2026

Why

sdk-rs publishes to crates.io as `sentrix-chain` — every release is a public artifact, so license + vuln gating on the dep tree matters. Mirrors the supply-chain hygiene already running on `sentrix-labs/sentrix`.

Per the repo hardening matrix (`research/02_repo_hardening_matrix.md`), this is MEDIUM priority — `gitleaks` was already in place; this PR closes the remaining 3 supply-chain gaps.

What

  • `deny.toml` — license allowlist + advisory yank-block + wildcard warning
  • `.github/workflows/cargo-deny.yml` — runs `cargo deny check` on PR + push to main
  • `.github/workflows/dependency-review.yml` — pre-merge diff-only license + vuln gate
  • `.github/workflows/commitlint.yml` + `.commitlintrc.json` — Conventional Commits enforce

`cargo-audit` is intentionally NOT added — `cargo deny check` already scans the RustSec advisory DB through `deny.toml [advisories]`.

Test plan

  • PR check shows cargo-deny + dependency-review + commitlint green

Summary by CodeRabbit

  • Chores
    • Added automated dependency scanning and vulnerability checks to PRs.
    • Implemented commit message format validation to enforce standardized commit types and header length.
    • Added license compliance verification and allowlist handling for project dependencies.
    • Configured dependency security policies and a cargo-deny audit to enforce severity-based checks.

Review Change Stack

Mirrors the supply-chain hygiene already running on sentrix-labs/sentrix.
sdk-rs publishes to crates.io as `sentrix-chain` — every release is a
public artifact, so license + vuln gating on the dep tree matters.

deny.toml: license allowlist + advisory yank-block + wildcard warning.
cargo-deny.yml: workflow that runs `cargo deny check` on every PR + push.
dependency-review.yml: pre-merge diff-only gate; license list mirrors deny.toml.
commitlint.yml + .commitlintrc.json: Conventional Commits enforce.

cargo-audit is intentionally NOT added — `cargo deny check` already scans
the RustSec advisory DB through deny.toml [advisories].
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

This PR adds repository-level enforcement and checks: a Commitlint configuration and workflow to validate commit messages (conventional preset, header max 100, restricted type enum); a deny.toml cargo-deny configuration and a cargo-deny workflow that runs check --all-features; and a dependency-review workflow that runs GitHub's dependency review action on pull requests to main with fail-on-severity set to high.

Sequence Diagram(s)

sequenceDiagram
  participant PR as Pull Request
  participant GH_Actions as GitHub Actions
  participant Runner as Actions runner
  participant Checkout as actions/checkout
  participant CargoDeny as EmbarkStudios/cargo-deny-action

  PR->>GH_Actions: trigger cargo-deny workflow (PR/push/dispatch)
  GH_Actions->>Runner: schedule job on ubuntu-22.04
  Runner->>Checkout: checkout repository
  Runner->>CargoDeny: run `cargo-deny check --all-features` with deny.toml
  CargoDeny-->>Runner: analysis results
  Runner-->>GH_Actions: job outcome
  GH_Actions-->>PR: status/checks
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description is largely complete and provides comprehensive context, though it deviates significantly from the template structure required for this Solidity/Forge-focused repository. Consider clarifying whether the required Forge/contract checks apply to this CI/tooling PR. The template assumes Solidity contracts, but this PR is repo tooling (CI workflows and configs). Confirm scope classification.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: adding three workflows (cargo-deny, dependency-review, commitlint) and their configurations to implement supply-chain hardening.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/add-supply-chain-hardening

Comment @coderabbitai help to get the list of available commands and usage tips.

Both are unmaintained / deprecated advisories with no actual
vulnerability — `paste` (transitive via alloy / sqlx / prost) and
`rustls-pemfile` (transitive via reqwest -> rustls). Mirrors the
ignore list in indexer-rs and the chain repo so policy is consistent
across the org.

Tested locally: `cargo deny --all-features check` → advisories ok.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@deny.toml`:
- Around line 40-45: Update the deny.toml ban policy entries: change the
multiple-versions rule and the wildcards rule from "warn" to "deny" when ready
(i.e., set multiple-versions = "deny" and wildcards = "deny") to prevent
duplicate dependencies and enforce explicit version pinning, while leaving
allow-wildcard-paths = true as-is; ensure you run the dependency check suite
after making these changes to catch any violations and address them before
committing.
- Line 35: Remove the unused "BUSL-1.1" entry from the license allowlist by
deleting the "BUSL-1.1" string from the allowlist array in deny.toml (the entry
shown as "BUSL-1.1",); ensure the allowlist remains valid TOML (commas and
quoting adjusted if necessary) after removal.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f634234d-2b01-4ca5-b519-df7235211f0c

📥 Commits

Reviewing files that changed from the base of the PR and between f389d3c and 9d04017.

📒 Files selected for processing (1)
  • deny.toml

Comment thread deny.toml
"CC0-1.0",
"Zlib",
"0BSD",
"BUSL-1.1",
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if any dependencies use BUSL-1.1 license

cargo deny --all-features list 2>/dev/null | grep -i "busl\|business.source" || echo "No BUSL license found in dependency tree"

Repository: Sentriscloud/sdk-rs

Length of output: 104


Remove unused BUSL-1.1 license from the allowlist.

No dependencies in the tree use BUSL-1.1. While the configuration allows unused licenses without error, this entry in the allowlist serves no purpose and can be safely removed to keep the configuration clean.

🤖 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 `@deny.toml` at line 35, Remove the unused "BUSL-1.1" entry from the license
allowlist by deleting the "BUSL-1.1" string from the allowlist array in
deny.toml (the entry shown as "BUSL-1.1",); ensure the allowlist remains valid
TOML (commas and quoting adjusted if necessary) after removal.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip — kept for forward-compat. sdk-rs is the SDK for Sentrix Chain whose workspace crates are all BUSL-1.1. If a future iteration adds a workspace path-dep on a chain crate (e.g. for shared types between server + client), the allowlist needs BUSL-1.1 already. Removing now and re-adding later = churn. Harmless idle entry.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satyakwok, that's a clear rationale — keeping BUSL-1.1 as a forward-compat entry makes sense given that the chain workspace crates are already BUSL-1.1 licensed, and unused-allowed-license = "allow" means the idle entry is entirely harmless. I'll note this for future reviews.


✏️ Learnings added
Learnt from: satyakwok
Repo: Sentriscloud/sdk-rs PR: 22
File: deny.toml:35-35
Timestamp: 2026-05-13T17:31:26.010Z
Learning: In `deny.toml` of the `sdk-rs` repository (Sentriscloud/sdk-rs), the `BUSL-1.1` entry in the `[licenses] allow` list is intentionally kept for forward-compatibility. `sdk-rs` is the SDK for Sentrix Chain, whose workspace crates are all BUSL-1.1 licensed. If a future iteration adds a path-dep on a chain crate (e.g. for shared types), the allowlist already covers it. The entry is harmless because `unused-allowed-license = "allow"` is set. Do not flag this as an unused/unnecessary entry.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment thread deny.toml
Comment on lines +40 to +45
[bans]
# Multiple major versions tolerated for now; tighten later.
multiple-versions = "warn"
wildcards = "warn"
allow-wildcard-paths = true
highlight = "all"
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider tightening ban policies in future iterations.

The current configuration uses warn for multiple-versions and wildcards, which is reasonable for initial rollout. However, once baseline issues are resolved:

  • Consider changing multiple-versions = "deny" to prevent duplicate dependencies that increase binary size
  • Consider changing wildcards = "deny" to enforce explicit version pinning for reproducible builds

The allow-wildcard-paths = true setting is appropriate as it only affects local path dependencies.

🤖 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 `@deny.toml` around lines 40 - 45, Update the deny.toml ban policy entries:
change the multiple-versions rule and the wildcards rule from "warn" to "deny"
when ready (i.e., set multiple-versions = "deny" and wildcards = "deny") to
prevent duplicate dependencies and enforce explicit version pinning, while
leaving allow-wildcard-paths = true as-is; ensure you run the dependency check
suite after making these changes to catch any violations and address them before
committing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip for now — the comment itself frames this as "in future iterations after baseline issues are resolved". sdk-rs is at v0.1.0-alpha.0 with active migration churn (the sentrix-proto extraction lands in #23). Tightening multiple-versions/wildcards to deny right now would block legitimate transitive duplicates from alloy/leptos ecosystems mid-migration. Revisit at 1.0 stabilisation.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satyakwok — Makes sense. The rationale is solid: locking down multiple-versions/wildcards to deny mid-migration with alloy/leptos in the graph would produce noisy violations that aren't actionable yet. Keeping warn now and revisiting at 1.0 stabilisation is the right call.

Would you like me to open a GitHub issue tagged as a 1.0 milestone task to track the eventual tightening of these ban policies? That way it doesn't fall through the cracks when the migration settles.

@satyakwok satyakwok merged commit df78d2a into main May 13, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant