Add compliance CI checks and scripts#1453
Conversation
…tiple files - Added SPDX license identifier (GPL-3.0-or-later) to various shell scripts and TypeScript files. - Updated license field in Cargo.toml files for multiple packages to GPL-3.0-or-later. - Set publish flag to false in Cargo.toml files for several packages to prevent accidental publishing. - Introduced new compliance scripts for cargo audit, cargo deny, gitleaks, and others to enhance project security and compliance checks.
WalkthroughThis PR introduces comprehensive compliance and security scanning tooling including a new GitHub Actions compliance workflow, Semgrep rules for detecting debug macros in runtime code, configuration for cargo-deny, gitleaks, Grype, and ScanCode tools, plus SPDX license headers to shell scripts and updated license metadata across Cargo.toml files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🪛 Biome (2.1.2)integration-tests/package.json[error] 20-20: The key pnpm was already declared. This where a duplicated key was declared again. If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored. (lint/suspicious/noDuplicateObjectKeys) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
integration-tests/scripts/build-zeitgeist-spec.sh (1)
1-2: Standardize shebang for consistency.This script uses
#!/bin/bashwhile all other shell scripts in the repository use#!/usr/bin/env bash. The latter is more portable and aligns with the project's convention.Consider updating to:
-#!/bin/bash +#!/usr/bin/env bashThis ensures consistency across all shell scripts and improves portability.
scripts/compliance/cargo_audit.sh (1)
8-13: Extractrequire_cmdto a shared utility to reduce duplication.The
require_cmdhelper is duplicated across 7+ compliance scripts (cargo_audit.sh, cargo_deny.sh, gitleaks.sh, node_checks.sh, sbom.sh, scancode.sh, semgrep.sh). Consider sourcing it from a shared utility file or library to follow DRY principles.Example approach—create
scripts/compliance/lib.sh:#!/usr/bin/env bash # SPDX-License-Identifier: GPL-3.0-or-later require_cmd() { if ! command -v "$1" >/dev/null 2>&1; then echo "Missing required command '$1'. Install it and retry." >&2 exit 127 fi }Then source it in each script:
source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh"This maintains consistency and makes future updates simpler.
scripts/compliance/scancode.sh (1)
8-13: Extractrequire_cmdto shared utility (duplicate across scripts).As noted in cargo_audit.sh review, this function is duplicated across multiple compliance scripts. Consider refactoring to a shared utility.
scripts/compliance/sbom.sh (1)
8-13: Extractrequire_cmdto shared utility (duplicate across scripts).As noted in previous reviews, this function is duplicated across 7+ compliance scripts. Recommend refactoring to a shared sourced utility.
scripts/compliance/node_checks.sh (1)
8-19: Good Node.js compliance checks, but extract the duplicatedrequire_cmdfunction.The
require_cmdfunction (lines 8–13) is identical across all compliance scripts. Extract it to a shared utility file (e.g.,scripts/compliance/common.sh) and source it from each script to reduce duplication and maintenance burden.scripts/compliance/semgrep.sh (1)
8-24: Config validation is good, but resolve code duplication and verify config file.The config existence check (lines 17–20) is excellent defensive programming. However:
- Extract the duplicated
require_cmdfunction (lines 8–13) to a shared utility.- Verify that
.github/semgrep-rules.ymlis committed in this PR.scripts/compliance/gitleaks.sh (1)
8-17: LGTM, but extract the duplicatedrequire_cmdfunction.The
--redactflag is good practice for security tool output in CI. However, consolidate therequire_cmdfunction to a shared utility across all compliance scripts.deny.toml (1)
3-22: Extensive advisory ignore list warrants tracking.The 18 ignored RUSTSEC advisories suggest dependencies with known vulnerabilities. Consider documenting justifications for each ignore or creating issues to track remediation timelines for critical ones.
.github/workflows/compliance.yml (1)
16-20: Consider refactoring repeated conditional logic.The same if-condition is repeated across all five jobs. For maintainability, consider extracting this to a reusable workflow input or defining it as an environment-level setting.
Also applies to: 35-39, 51-55, 72-76, 88-92
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockintegration-tests/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (73)
.github/semgrep-rules.yml(1 hunks).github/workflows/compliance.yml(1 hunks).github/workflows/integration-tests.yml(4 hunks).github/workflows/scancode.yml(1 hunks).gitignore(1 hunks).gitleaks.toml(1 hunks).grype.yaml(1 hunks)Cargo.toml(1 hunks)deny.toml(1 hunks)integration-tests/package.json(2 hunks)integration-tests/scripts/build-battery-station-spec.sh(1 hunks)integration-tests/scripts/build-node-configuration.sh(1 hunks)integration-tests/scripts/build-node.sh(1 hunks)integration-tests/scripts/build-zeitgeist-spec.sh(1 hunks)integration-tests/scripts/deploy-zombienet.sh(1 hunks)integration-tests/tests/common-tests.ts(1 hunks)macros/Cargo.toml(1 hunks)node/Cargo.toml(1 hunks)primitives/Cargo.toml(1 hunks)runtime/battery-station/Cargo.toml(1 hunks)runtime/common/Cargo.toml(1 hunks)runtime/zeitgeist/Cargo.toml(1 hunks)scripts/benchmarks/configuration.sh(1 hunks)scripts/benchmarks/quick_check.sh(1 hunks)scripts/benchmarks/run_benchmarks.sh(1 hunks)scripts/build-parachain-spec.sh(1 hunks)scripts/compliance/cargo_audit.sh(1 hunks)scripts/compliance/cargo_deny.sh(1 hunks)scripts/compliance/gitleaks.sh(1 hunks)scripts/compliance/node_checks.sh(1 hunks)scripts/compliance/run_all.sh(1 hunks)scripts/compliance/sbom.sh(1 hunks)scripts/compliance/scancode.sh(1 hunks)scripts/compliance/semgrep.sh(1 hunks)scripts/docker-hub-publish.sh(1 hunks)scripts/init.sh(1 hunks)scripts/parachain/bootstrap.sh(1 hunks)scripts/parachain/local.sh(1 hunks)scripts/parachain/testing-network-commons.sh(1 hunks)scripts/runtime-upgrade/test_runtime_upgrade.sh(1 hunks)scripts/tests/all-sequencial.sh(1 hunks)scripts/tests/aux-functions.sh(1 hunks)scripts/tests/clippy.sh(1 hunks)scripts/tests/coverage.sh(1 hunks)scripts/tests/format.sh(1 hunks)scripts/tests/fuzz.sh(1 hunks)scripts/tests/parachain.sh(1 hunks)scripts/tests/standalone.sh(1 hunks)scripts/tests/test_parachain.sh(1 hunks)scripts/tests/test_standalone.sh(1 hunks)scripts/update-copyright.sh(1 hunks)zrml/authorized/Cargo.toml(1 hunks)zrml/combinatorial-tokens/Cargo.toml(1 hunks)zrml/combinatorial-tokens/fuzz/Cargo.toml(1 hunks)zrml/court/Cargo.toml(1 hunks)zrml/futarchy/Cargo.toml(1 hunks)zrml/futarchy/fuzz/Cargo.toml(1 hunks)zrml/global-disputes/Cargo.toml(1 hunks)zrml/hybrid-router/Cargo.toml(1 hunks)zrml/market-commons/Cargo.toml(1 hunks)zrml/neo-swaps/Cargo.toml(1 hunks)zrml/neo-swaps/fuzz/Cargo.toml(1 hunks)zrml/orderbook/Cargo.toml(1 hunks)zrml/orderbook/fuzz/Cargo.toml(1 hunks)zrml/parimutuel/Cargo.toml(1 hunks)zrml/prediction-markets/Cargo.toml(1 hunks)zrml/prediction-markets/fuzz/Cargo.toml(1 hunks)zrml/prediction-markets/runtime-api/Cargo.toml(1 hunks)zrml/styx/Cargo.toml(1 hunks)zrml/swaps/Cargo.toml(1 hunks)zrml/swaps/fuzz/Cargo.toml(1 hunks)zrml/swaps/rpc/Cargo.toml(1 hunks)zrml/swaps/runtime-api/Cargo.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
scripts/compliance/scancode.sh (6)
scripts/compliance/cargo_audit.sh (1)
require_cmd(8-13)scripts/compliance/cargo_deny.sh (1)
require_cmd(8-13)scripts/compliance/gitleaks.sh (1)
require_cmd(8-13)scripts/compliance/node_checks.sh (1)
require_cmd(8-13)scripts/compliance/sbom.sh (1)
require_cmd(8-13)scripts/compliance/semgrep.sh (1)
require_cmd(8-13)
scripts/compliance/cargo_audit.sh (6)
scripts/compliance/cargo_deny.sh (1)
require_cmd(8-13)scripts/compliance/gitleaks.sh (1)
require_cmd(8-13)scripts/compliance/node_checks.sh (1)
require_cmd(8-13)scripts/compliance/sbom.sh (1)
require_cmd(8-13)scripts/compliance/scancode.sh (1)
require_cmd(8-13)scripts/compliance/semgrep.sh (1)
require_cmd(8-13)
scripts/compliance/cargo_deny.sh (1)
scripts/compliance/cargo_audit.sh (1)
require_cmd(8-13)
scripts/compliance/node_checks.sh (6)
scripts/compliance/cargo_audit.sh (1)
require_cmd(8-13)scripts/compliance/cargo_deny.sh (1)
require_cmd(8-13)scripts/compliance/gitleaks.sh (1)
require_cmd(8-13)scripts/compliance/sbom.sh (1)
require_cmd(8-13)scripts/compliance/scancode.sh (1)
require_cmd(8-13)scripts/compliance/semgrep.sh (1)
require_cmd(8-13)
scripts/compliance/sbom.sh (6)
scripts/compliance/cargo_audit.sh (1)
require_cmd(8-13)scripts/compliance/cargo_deny.sh (1)
require_cmd(8-13)scripts/compliance/gitleaks.sh (1)
require_cmd(8-13)scripts/compliance/node_checks.sh (1)
require_cmd(8-13)scripts/compliance/scancode.sh (1)
require_cmd(8-13)scripts/compliance/semgrep.sh (1)
require_cmd(8-13)
🪛 actionlint (1.7.8)
.github/workflows/scancode.yml
7-7: character '^' is invalid for branch and tag names. ref name cannot contain spaces, ~, ^, :, [, ?, *. see man git-check-ref-format for more details. note that regular expression is unavailable. note: filter pattern syntax is explained at https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet
(glob)
🪛 Shellcheck (0.11.0)
scripts/update-copyright.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Test parachain build
- GitHub Check: Test standalone build
- GitHub Check: Quick check benchmarks
- GitHub Check: Fuzz
- GitHub Check: Checks (parachain)
- GitHub Check: Cargo Deny
- GitHub Check: Checks (clippy)
- GitHub Check: Checks (standalone)
- GitHub Check: Format
- GitHub Check: Coverage
- GitHub Check: Summary
🔇 Additional comments (68)
scripts/tests/all-sequencial.sh (1)
2-2: LGTM!The SPDX license identifier is correctly placed after the shebang and uses the proper format. This change has no impact on script functionality and aligns with the compliance objectives of the PR.
scripts/parachain/testing-network-commons.sh (1)
2-2: License header added correctly.The SPDX license identifier is properly formatted as a bash comment and positioned correctly after the shebang, conforming to standard conventions and the PR's compliance objectives.
zrml/swaps/rpc/Cargo.toml (1)
13-15: Metadata updates are consistent and appropriate.The license declaration and publish flag align with the compliance standardization across the workspace and similar updates in related zrml packages.
scripts/tests/format.sh (1)
2-2: License header added correctly.SPDX identifier placement after the shebang follows standard convention and does not impact script functionality.
scripts/docker-hub-publish.sh (1)
2-2: License header added without impact to deployment logic.The SPDX identifier does not affect the docker build and push operations.
scripts/tests/fuzz.sh (1)
2-2: License header added; fuzzing test logic unchanged.The SPDX identifier is purely metadata and does not affect the fuzz test configurations or execution.
scripts/tests/clippy.sh (1)
2-2: License header added; clippy invocation unchanged.The SPDX identifier does not impact the clippy lint configuration or execution.
zrml/orderbook/fuzz/Cargo.toml (1)
17-19: License metadata added consistently with workspace standards.The license declaration aligns with compliance standardization across zrml packages. The publish flag was already set appropriately for a fuzz crate.
scripts/benchmarks/run_benchmarks.sh (1)
2-2: License header added; benchmark orchestration logic unchanged.The SPDX identifier does not impact the benchmark configurations or command execution.
scripts/runtime-upgrade/test_runtime_upgrade.sh (1)
2-2: License header added; runtime upgrade test logic unchanged.The SPDX identifier does not impact the runtime upgrade state export, transformation, or execution.
scripts/tests/aux-functions.sh (1)
2-2: License header properly positioned.The SPDX license identifier is correctly placed after the shebang and follows standard conventions.
zrml/combinatorial-tokens/Cargo.toml (1)
69-71: Verify publish = false doesn't break release workflows.The addition of
publish = falseis appropriate for internal workspace crates. However, confirm that no existing CI/CD pipelines or release workflows depend on publishing these crates to crates.io.scripts/tests/test_parachain.sh (1)
2-2: License header correctly added.Placement and format follow SPDX standards and are consistent with other shell scripts in the repository.
runtime/zeitgeist/Cargo.toml (1)
434-436: Verify publish = false for runtime crate aligns with release strategy.This is a core runtime crate. Ensure that
publish = falsedoesn't conflict with any release workflows that may need to publish or tag this crate.scripts/benchmarks/quick_check.sh (1)
2-2: License header properly added.Placement is correct and format is consistent across the repository's shell scripts.
zrml/market-commons/Cargo.toml (1)
36-38: Metadata update aligns with project-wide licensing effort.The license and publish fields are appropriate for this internal workspace crate. Ensure the
publish = falsesetting is consistent with the project's release strategy across all internal crates.integration-tests/scripts/deploy-zombienet.sh (1)
2-2: License header added in standard location.The SPDX identifier is correctly positioned after the shebang and matches formatting across other repository scripts.
scripts/init.sh (1)
2-2: SPDX license header correctly placed.The GPL-3.0-or-later license identifier is properly positioned after the shebang, following standard SPDX conventions.
scripts/tests/test_standalone.sh (1)
2-2: SPDX license header correctly applied.License identifier properly placed after the shebang.
scripts/build-parachain-spec.sh (1)
2-2: SPDX license header correctly positioned.The license identifier is properly placed immediately after the shebang.
scripts/tests/coverage.sh (1)
2-2: SPDX license header correctly added.The license identifier is properly positioned after the shebang.
scripts/tests/standalone.sh (1)
2-2: SPDX license header correctly placed.The license identifier is properly positioned immediately after the shebang.
scripts/parachain/local.sh (1)
2-2: SPDX license header correctly positioned.The license identifier is properly placed immediately after the shebang.
zrml/neo-swaps/Cargo.toml (1)
111-113: Metadata fields correctly added to package manifest.The
licenseandpublishfields are appropriate additions to standardize package metadata across the workspace. The GPL-3.0-or-later identifier is consistent with the repository's licensing approach.scripts/tests/parachain.sh (1)
2-2: SPDX license header correctly added.The license identifier is properly formatted and positioned. This aligns with compliance best practices and the PR's licensing standardization goals.
scripts/benchmarks/configuration.sh (1)
2-2: SPDX license header correctly added.Consistent with other script updates in the PR. No impact on script functionality.
integration-tests/scripts/build-battery-station-spec.sh (1)
2-2: SPDX license header correctly added.Consistent pattern across integration test scripts. No functional impact.
runtime/common/Cargo.toml (1)
87-87: Metadata additions are appropriate for an internal runtime crate.Adding
licenseandpublish = falsestandardizes compliance declarations and prevents accidental publication to crates.io. This is the correct strategy for shared runtime libraries in a monorepo.Also applies to: 89-89
node/Cargo.toml (1)
192-192: Verify thatpublish = falseis intentional for the end-user node binary.The zeitgeist-node is an end-user-facing binary. Confirm that preventing publication to crates.io aligns with distribution strategy (e.g., it's only distributed via GitHub releases or Docker containers, not as a crate). If the binary should be publishable independently, remove
publish = false.Also applies to: 194-194
scripts/parachain/bootstrap.sh (1)
2-2: SPDX license header correctly added.Consistent with other bootstrap and setup scripts. No impact on functionality.
macros/Cargo.toml (1)
8-8: License metadata appropriately added.Macros crate includes license but not
publish = false, which is appropriate if this utility crate is intended for independent publication. Consistent with compliance standardization.zrml/combinatorial-tokens/fuzz/Cargo.toml (1)
33-33: License metadata appropriately added to fuzz target.Fuzz harness correctly includes license. The existing
publish = falseis appropriate and remains in place. Consistent with licensing standardization across the workspace.zrml/orderbook/Cargo.toml (1)
59-61: LGTM! License and publishing metadata correctly added.The GPL-3.0-or-later license identifier is valid, and setting
publish = falseappropriately prevents accidental publishing of this internal package.integration-tests/tests/common-tests.ts (1)
1-1: LGTM! SPDX license header correctly added.The license header follows the standard SPDX format and is appropriately placed at the top of the file.
integration-tests/scripts/build-node.sh (1)
2-2: LGTM! SPDX license header correctly added.The license header follows the standard format and is correctly positioned after the shebang.
zrml/neo-swaps/fuzz/Cargo.toml (1)
33-33: LGTM! License metadata correctly added.The GPL-3.0-or-later license identifier is valid and aligns with the repository-wide licensing standardization.
zrml/global-disputes/Cargo.toml (1)
49-51: LGTM! License and publishing metadata correctly added.The license identifier is valid, and the
publish = falsesetting appropriately prevents accidental publication of this internal crate.zrml/parimutuel/Cargo.toml (1)
48-50: LGTM! License and publishing metadata correctly added.The metadata additions are correct and consistent with the repository-wide compliance updates.
integration-tests/scripts/build-node-configuration.sh (1)
2-2: LGTM! SPDX license header correctly added.The license header is correctly formatted and positioned after the shebang.
zrml/swaps/runtime-api/Cargo.toml (1)
20-22: LGTM! License and publishing metadata correctly added.The metadata additions are properly formatted and consistent with the repository-wide compliance updates.
zrml/court/Cargo.toml (1)
51-51: License and publishing metadata align with PR objectives.Adding GPL-3.0-or-later and disabling publication is consistent with the standardized licensing and compliance strategy across the workspace.
Also applies to: 53-53
zrml/swaps/fuzz/Cargo.toml (1)
68-68: License metadata addition is consistent with workspace standardization.The GPL-3.0-or-later license aligns with PR objectives. The publish = false setting was already present.
runtime/battery-station/Cargo.toml (1)
442-442: Runtime package metadata updated consistently with licensing standardization.Disabling publication for battery-station-runtime is appropriate, and the GPL-3.0-or-later license aligns with the workspace policy.
Also applies to: 444-444
.gitignore (1)
162-165: Compliance artifacts properly excluded from version control.Adding sbom.json to .gitignore is appropriate, as SBOM files are generated outputs from compliance tooling rather than source artifacts.
primitives/Cargo.toml (1)
42-42: Primitives package metadata aligned with workspace licensing standards.Adding GPL-3.0-or-later and disabling publication is consistent with other workspace packages and PR objectives.
Also applies to: 44-44
zrml/futarchy/fuzz/Cargo.toml (1)
21-21: Fuzz crate license metadata is consistent with workspace standards.GPL-3.0-or-later license and existing publish = false setting align with the standardized licensing approach.
zrml/futarchy/Cargo.toml (1)
64-64: Futarchy pallet metadata follows workspace licensing standardization.GPL-3.0-or-later license and publish = false setting are consistent with the standardized compliance approach applied across zrml crates.
Also applies to: 66-66
integration-tests/package.json (1)
6-6: ws security fix verified; pnpm overrides are part of coordinated stable2409 dependency update.ws 8.17.1 fixes CVE-2024-37890, a Denial‑of‑Service vulnerability triggered by requests with a large number of HTTP headers. The upgrade is justified.
The pnpm.overrides section pins 13 transitive dependencies as part of the "Update dependencies to stable2409" effort (commit 28e6eaf). These pinned versions were coordinated and tested together, as evidenced by the significant pnpm-lock.yaml update (1003 lines). While adding an inline comment above the overrides block explaining this rationale would improve code clarity, the pinned versions are locked in the lock file and should maintain stability.
zrml/styx/Cargo.toml (1)
39-41: Metadata additions align with licensing and publishing controls.The new
licenseandpublishfields are consistent with the PR's compliance objectives and match the pattern applied across other zrml crates.zrml/authorized/Cargo.toml (1)
41-43: Metadata additions consistent across crates.License and publish metadata align with the pattern across the zrml workspace.
zrml/swaps/Cargo.toml (1)
67-69: Consistent licensing and publish control metadata.scripts/compliance/cargo_audit.sh (1)
15-18: Script implementation is solid.The validation and execution logic is clean. The
--jsonflag ensures structured output for potential CI integration.zrml/hybrid-router/Cargo.toml (1)
99-101: Metadata changes follow established pattern.scripts/compliance/scancode.sh (1)
17-32: Well-designed configuration with sensible defaults.The script handles process count intelligently (
nprocfallback), provides customizable OUTPUT, and applies comprehensive ignore patterns for typical build and test directories. Pretty-printed JSON output is appropriate for artifact generation.scripts/compliance/sbom.sh (2)
15-21: SBOM generation and scanning integration is well-structured.The dual invocation (syft → generate SBOM, then grype → scan) is the standard pattern. Configuration integration with
.grype.yamlis correct. The--fail-on mediumthreshold ensures CI catches moderate-severity vulnerabilities.
20-21: Exclude path is valid and intentional—no action required.The
./scripts/check-license/.venvpath refers to a legitimate Python project within the repository (scripts/check-license/containssetup.pyandrequirements.txt). The exclude is a precautionary best practice to prevent syft from scanning virtual environment dependencies when the.venvdirectory is created during local setup or CI execution. This path is not a leftover from another project but rather proper configuration for accurate SBOM generation.Cargo.toml (1)
5-5: LGTM!The license update to "GPL-3.0-or-later" is appropriate and follows SPDX best practices, providing flexibility for future GPL version compliance.
zrml/prediction-markets/fuzz/Cargo.toml (1)
18-20: LGTM!The license addition and
publish = falsesetting are appropriate for a fuzzing test crate.scripts/compliance/run_all.sh (1)
1-13: Robust orchestration script with good error handling.The
set -euo pipefailand safe directory resolution ensure the compliance suite fails fast on any check failure. However, verify that all seven called scripts (especiallysbom.shandscancode.sh) exist and are executable in the repository..gitleaks.toml (1)
1-15: Verify allowlist entries contain only test data.The gitleaks allowlist entries reference test and configuration files. Please verify that the allowlisted files (especially the JSON chain specs in
node/res/) and YAML configs contain only non-sensitive, public test data and not actual credentials..github/workflows/integration-tests.yml (1)
83-83: Verify GitHub Actions version pinning is intentional.All
actions/download-artifactuses have been updated fromv4tov4.1.8. Please confirm:
- This version exists and is stable.
- Whether this pin is due to a specific bug fix or security patch that should be documented.
Also applies to: 89-89, 140-140, 146-146, 196-196, 251-251
zrml/prediction-markets/runtime-api/Cargo.toml (1)
17-19: Metadata additions are appropriate.The license and publish fields align with the broader repository licensing strategy and help prevent accidental crate publishing.
.github/semgrep-rules.yml (1)
1-20: Semgrep rules are well-configured.The rules appropriately target debug and print statements in runtime and pallet code with reasonable severity levels. The patterns and path filters are correct.
.github/workflows/scancode.yml (1)
1-24: Workflow structure and steps are correct.The scancode installation, execution, and artifact upload follow GitHub Actions best practices with appropriate error handling via pip.
zrml/prediction-markets/Cargo.toml (1)
96-98: Metadata additions follow repository policy.The license and publish fields consistently apply the licensing and publishing restrictions across the codebase.
deny.toml (1)
34-88: Configuration structure is sound.The license allowlist, crate-specific clarifications, and source whitelisting are well-organized and handle edge cases appropriately. The confidence threshold and multiple-versions warnings align with cargo-deny best practices.
scripts/compliance/cargo_deny.sh (1)
1-58: Well-structured and robust compliance script.The script properly handles environment setup, dependency validation, optional flags, and logging. The separation of full and error logs provides good visibility, and user guidance messages are helpful.
.github/workflows/compliance.yml (1)
1-110: Comprehensive compliance workflow is well-structured.The workflow appropriately triggers on PR labels and push to main, covers multiple compliance domains (cargo, semgrep, node, secrets, SBOM), and uses modern actions with appropriate runtime settings.
…pdate license-checker and audit commands
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/compliance.yml (2)
16-20: Reduce duplication of conditional logic across jobs.The
ifcondition is repeated verbatim across all five jobs, violating DRY and making maintenance difficult if the logic needs to change. Consider extracting into workflow inputs or a reusable workflow.Additionally, the multiline conditional formatting is hard to parse. Clarify operator precedence with explicit grouping:
if: | (github.event_name == 'pull_request' && (contains(github.event.pull_request.labels.*.name, 's:review-needed') || contains(github.event.pull_request.labels.*.name, 's:accepted'))) || github.event_name == 'push'If you refactor using a reusable workflow, you could centralize this logic:
# .github/workflows/compliance.yml jobs: compliance-matrix: uses: ./.github/workflows/compliance-jobs.ymlThen move individual jobs into
.github/workflows/compliance-jobs.ymlas separate called jobs.Also applies to: 35-39, 51-55, 84-89, 117-121
127-140:.grype.yamlexists; consider pinning tool versions for reproducibility.The
.grype.yamlconfiguration file is present at the repository root with a properly structured allowlist for known vulnerabilities. However, usingsyft-version: latestandgrype-version: latest(lines 132, 139) remains a reproducibility concern—different CI runs may use different tool versions, potentially yielding inconsistent results.Recommendation: Pin versions explicitly (e.g.,
syft-version: "0.95.0",grype-version: "0.67.0") to ensure consistent scanning behavior across runs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/compliance.yml(1 hunks)scripts/update-copyright.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/update-copyright.sh
[error] 2-2: The shebang must be on the first line. Delete blanks and move comments.
(SC1128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Test parachain build
- GitHub Check: Checks (clippy)
- GitHub Check: Checks (standalone)
- GitHub Check: Coverage
- GitHub Check: Coverage
- GitHub Check: Test parachain build
- GitHub Check: Checks (clippy)
- GitHub Check: Fuzz
- GitHub Check: Checks (standalone)
- GitHub Check: Checks (parachain)
- GitHub Check: Quick check benchmarks
- GitHub Check: Test standalone build
- GitHub Check: Summary
🔇 Additional comments (3)
.github/workflows/compliance.yml (3)
140-140: Verify thatfail-build: falsealigns with project security policy.The SBOM vulnerability scan will not fail the build if vulnerabilities are found (line 140:
fail-build: false). Depending on your security posture, this may allow vulnerable dependencies to be merged.Ensure this is intentional and aligns with project compliance requirements. If blocking PRs on vulnerabilities is desired, consider
fail-build: true.
47-47: All referenced configuration files exist and are present in the repository. No action needed.
63-79: Cache path verified—no action needed.The
integration-tests/pnpm-lock.yamlfile exists at the specified path, so the cache step in the Node Licenses job will function correctly.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1453 +/- ##
=======================================
Coverage 93.32% 93.32%
=======================================
Files 181 181
Lines 34769 34769
=======================================
Hits 32448 32448
Misses 2321 2321
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…eaks installation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/compliance.yml (2)
3-20: Condition duplication across jobs reduces maintainability.The conditional logic (lines 16-20, 35-39, 51-55, 104-108) is repeated identically across four jobs. This violates DRY and makes future updates to the triggering logic error-prone.
Consider refactoring to use GitHub Actions'
envoroutputsto centralize this condition. Alternatively, extract into a reusable workflow or use a custom action that evaluates the condition once:env: SHOULD_RUN: | github.event_name == 'pull_request' && (contains(github.event.pull_request.labels.*.name, 's:review-needed') || contains(github.event.pull_request.labels.*.name, 's:accepted')) || github.event_name == 'push' jobs: cargo-deny: if: env.SHOULD_RUN == 'true' # ... rest of jobHowever, note that
envcontext is not directly accessible inifconditions, so you would need a setup job that outputs this value or use GitHub's reusable workflow pattern.
81-100: Gitleaks binary download relies on OS/arch detection without verification.The gitleaks binary is downloaded dynamically using
uname -sanduname -m(line 96-97), but there is no checksum verification or retry logic. If the asset naming convention changes or the OS/architecture mapping is unexpected, the download could silently fail or extract incorrectly.Consider:
- Adding checksum verification post-download
- Using a dedicated GitHub Action (e.g.,
gitleaks-action) if available, which likely handles versioning and verification- Adding explicit error handling or validation that the binary is executable
Alternative safer approach:
- name: Install gitleaks uses: gitleaks/gitleaks-action@v2This delegates binary management to the action maintainers. If this action doesn't exist or doesn't meet your needs, add checksums to the manual approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/compliance.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Test parachain build
- GitHub Check: Fuzz
- GitHub Check: Checks (standalone)
- GitHub Check: Checks (clippy)
- GitHub Check: Checks (parachain)
- GitHub Check: Quick check benchmarks
- GitHub Check: Format
- GitHub Check: Test standalone build
- GitHub Check: Coverage
- GitHub Check: Summary
🔇 Additional comments (5)
.github/workflows/compliance.yml (5)
102-127: Verify intent offail-build: falsein vulnerability scan.The sbom-vuln job scans for vulnerabilities but sets
fail-build: false(line 127), meaning security issues won't block the build. This appears to be an informational scan only.Confirm whether this is intentional:
- Is this a temporary measure pending full remediation?
- Should critical/high vulnerabilities fail the build instead?
- Is there a separate process that consumes these scan results downstream?
If vulnerabilities should block the build, change to
fail-build: trueand handle failures in a dedicated task or exception process.
16-20: PR compliance checks require explicit labels; runs for all main branch pushes.The workflow only triggers compliance checks on PRs with
s:review-neededors:acceptedlabels, but runs unconditionally on push to main. This creates an asymmetry where:
- PRs without these labels skip compliance checks (by design or oversight?)
- Main branch pushes always run checks
Confirm this is intentional. If all PRs should be checked regardless of label, consider:
on: pull_request: types: [opened, synchronize, reopened, labeled] push: branches: [main]And simplify the job conditions to remove label-filtering for PRs (keep only push check).
Also applies to: 35-39, 51-55, 104-108
49-79: Node-licenses job structure is sound; verify integration-tests directory stability.The job is well-configured with pinned Node (20) and pnpm (9) versions, frozen lockfile, and prod-only audits. The
working-directory: integration-testssetting is appropriate.Ensure the
integration-tests/pnpm-lock.yamlpath is stable and the directory structure won't change unexpectedly. If integration-tests is optional or conditionally built, consider guarding this job with an existence check.
33-47: Semgrep job references external rules file; verify rules exist and are maintained.The job uses
.github/semgrep-rules.yml(line 47). Ensure this file:
- Exists and is tracked in the repository
- Is regularly reviewed and updated for new patterns
- Aligns with your team's security/code quality standards
This approach is sound; just verify the referenced config file is present and kept up-to-date as part of PR reviews.
9-11: Concurrency strategy is appropriate for this workflow.The concurrency group correctly uses
github.refto serialize runs per branch and cancels in-progress runs, preventing redundant compliance checks. This is good practice.
…e detection and error handling
…hitecture detection
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/compliance.yml (1)
16-20: Standardize conditional logic formatting across jobs.The conditional logic varies in formatting: some jobs (e.g., gitleaks at line 84) use explicit outer parentheses, while others (e.g., cargo-deny at line 16) omit them. This affects readability. For consistency and clarity, apply the explicit parentheses pattern to all jobs.
Example: Update cargo-deny, semgrep, and node-licenses to match gitleaks formatting:
- if: | - github.event_name == 'pull_request' && - (contains(github.event.pull_request.labels.*.name, 's:review-needed') || - contains(github.event.pull_request.labels.*.name, 's:accepted')) || - github.event_name == 'push' + if: | + ( + github.event_name == 'pull_request' && + (contains(github.event.pull_request.labels.*.name, 's:review-needed') || + contains(github.event.pull_request.labels.*.name, 's:accepted')) + ) || + github.event_name == 'push'Also applies to: 35-39, 51-55, 84-89, 108-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/compliance.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Coverage
- GitHub Check: Summary
🔇 Additional comments (3)
.github/workflows/compliance.yml (3)
131-131: Clarify intent offail-build: falsesetting.The sbom-vuln job sets
fail-build: false, which allows the build to pass even if vulnerabilities are detected. Confirm whether this is intentional or if vulnerabilities should fail the build.If vulnerabilities should block merges, update to:
- fail-build: false + fail-build: true
126-127: No action required. The.grype.yamlfile exists and is properly configured.Verification confirms that
.grype.yamlexists at the repository root with valid YAML configuration. The environment variable reference in the workflow will correctly resolve to this file.Likely an incorrect or invalid review comment.
57-73: Directory structure and lockfile verified—no action needed.The
integration-testsdirectory andpnpm-lock.yamlfile exist at the expected locations. The workflow configuration correctly references these withdefaults.run.working-directory: integration-testsandcache-dependency-path: integration-tests/pnpm-lock.yaml. The setup is valid.
…sion retrieval and architecture support
What does it do?
What important points should reviewers know?
Is there something left for follow-up PRs?
What alternative implementations were considered?
Are there relevant PRs or issues?
References
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.