chore(ci): add OSSF and CI best practices#132
Conversation
📝 WalkthroughThis PR contains no changes to core source code, public APIs, binary format handling, or semantic-classification implementation/logic. The modifications are repository-level: CI/CD, automation, security, documentation, and project governance. Architectural/implementation impact
CI/CD & automation
Security & contributor policies
Tooling & repository configuration
Notes on semantic classification accuracy, binary formats, and APIs
Commit-level fix
Summary: repository governance, CI, security, and documentation improvements only — no functional or API changes to the codebase, binary handling, or semantic-classification logic. WalkthroughAdds CI/workflow and automation changes, new security and governance docs/configs, Mergify rules, changelog configuration, and minor repository housekeeping updates. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 📃 Configuration Change RequirementsWonderful, this rule succeeded.Mergify configuration change
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 CI must passWonderful, this rule succeeded.All CI checks must pass. This protection prevents manual merges that bypass the merge queue.
🟢 Do not merge outdated PRsWonderful, this rule succeeded.Make sure PRs are within 10 commits of the base branch before merging
|
🧪 CI InsightsHere's what we observed from your CI run for 44d76a1. 🟢 All jobs passed!But CI Insights is watching 👀 |
- Add .mergify.yml with 3-tier PR strategy: maintainer+lgtm, dependabot auto-approve, and external PRs requiring maintainer approval - Add merge protections: conventional commits, CI checks, staleness - Add cliff.toml for changelog generation via git-cliff - Remove the `changes` path-filter job from CI (gameable by pushing a docs-only commit after code changes to skip tests) - Remove the `ci-gate` aggregator job (mergify references checks directly) - All CI jobs now run unconditionally on every PR - Update dependabot config with rebase-strategy disabled Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
- Add OSSF Scorecard workflow for supply-chain security analysis - Add SECURITY.md with vulnerability reporting, scope, safe harbor, and PGP key - Update CONTRIBUTING.md with DCO sign-off requirements, code review criteria, and project governance section - Rewrite AGENTS.md OSSF section for Rust/Stringy (was Go-specific copy) - Fix MSRV reference in CONTRIBUTING.md (1.85 -> 1.91) Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
unclesp1d3r
force-pushed
the
feature/add_CI_bestpractices
branch
from
ab7c997 to
d49beb6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
coderabbitai
bot
added
documentation
- Replace emoji shortcodes with ASCII text in SECURITY.md (ASCII-only rule) - Remove unreachable pull_request condition in scorecard workflow
There was a problem hiding this comment.
Pull request overview
This PR raises the repository’s automation and governance baseline by tightening CI execution rules, adding supply-chain/security automation (OSSF Scorecard), and documenting contributor/security expectations.
Changes:
- Make CI run unconditionally by removing path-based job filtering and removing the aggregate gate job.
- Add/adjust automation and governance tooling: OSSF Scorecard workflow, Mergify merge automation, and Dependabot configuration tweaks.
- Expand project documentation around contribution standards, security reporting, and OSSF-aligned best practices.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
mise.lock |
Adds a pinned tool lockfile for local/dev automation tooling. |
cliff.toml |
Introduces git-cliff configuration for structured changelog generation. |
SECURITY.md |
Adds a security policy and reporting process (incl. PGP key). |
CONTRIBUTING.md |
Expands contributor guidance (MSRV, safety/CI rules, DCO, governance). |
AGENTS.md |
Adds OSSF-oriented PR/release quality standards. |
.serena/project.yml |
Makes Serena project configuration more explicit (project name/tools/modes). |
.mergify.yml |
Adds merge queue automation and status-check enforcement rules. |
.gitignore |
Ignores a local Claude-related markdown file. |
.github/workflows/scorecard.yml |
Adds OSSF Scorecard workflow for supply-chain security analysis. |
.github/workflows/ci.yml |
Removes path filtering and the aggregate gate job so all CI jobs run consistently. |
.github/dependabot.yml |
Disables automatic rebasing and removes the rust-toolchain ecosystem entry. |
| @@ -177,48 +133,3 @@ jobs: | |||
| with: | |||
| token: ${{ secrets.QLTY_COVERAGE_TOKEN }} | |||
| files: target/lcov.info | |||
There was a problem hiding this comment.
The qlty coverage upload is configured to read "target/lcov.info", but earlier in this job the LCOV report is written to "lcov.info" (repo root). This mismatch will prevent qlty from finding the report. Align the output path and the upload path (either write into target/lcov.info or upload lcov.info).
| files: target/lcov.info | |
| files: lcov.info |
| queue_rules: | ||
| - name: default | ||
| merge_method: squash | ||
| merge_conditions: |
There was a problem hiding this comment.
In Mergify queue_rules, the required-check list should be under the correct schema key (commonly conditions:). With merge_conditions: here, Mergify may ignore the rule and not enforce the required CI checks. Rename to the correct key so the queue rule is applied.
| merge_conditions: | |
| conditions: |
| - **Style**: Follows project conventions, passes `cargo fmt` and `cargo clippy -- -D warnings` | ||
| - **Documentation**: Public APIs have rustdoc, AGENTS.md updated if architecture changes | ||
|
|
||
| CI must pass before merge. This includes formatting, linting, tests, security audit, and CodeQL analysis. Branch protection enforces these checks on the `main` branch. |
There was a problem hiding this comment.
CONTRIBUTING.md says branch protection enforces formatting/lint/tests plus "security audit" and "CodeQL analysis" before merge, but the repo’s merge enforcement in .mergify.yml only requires the CI workflow jobs (quality/msrv/test/test-cross-platform/coverage). Either add the security/audit and CodeQL check names to the required checks enforcement, or clarify in the docs which checks are actually required for merges.
| CI must pass before merge. This includes formatting, linting, tests, security audit, and CodeQL analysis. Branch protection enforces these checks on the `main` branch. | |
| CI must pass before merge. Branch protection on the `main` branch currently enforces the required formatting, linting, test, MSRV, cross-platform, and coverage checks. Additional jobs such as security audit and CodeQL analysis may run as part of CI but are not guaranteed to be merge-blocking. |
| analysis: | ||
| name: Scorecard analysis | ||
| runs-on: ubuntu-latest | ||
| if: github.event.repository.default_branch == github.ref_name |
There was a problem hiding this comment.
The workflow has an if: condition that references github.event_name == 'pull_request', but this workflow is not configured to run on pull_request events. This makes the condition misleading/dead code. Either add a pull_request trigger (if PR runs are desired) or simplify/remove the condition to match the actual triggers.
| if: github.event.repository.default_branch == github.ref_name | |
| if: github.event.repository.default_branch == github.ref_name |
| base_modes: | ||
|
|
||
| # list of mode names that are to be activated by default. | ||
| # The full set of modes to be activated is base_modes + default_modes. | ||
| # If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. | ||
| # Otherwise, this overrides the setting from the global configuration (serena_config.yml). | ||
| # This setting can, in turn, be overridden by CLI parameters (--mode). | ||
| default_modes: |
There was a problem hiding this comment.
base_modes: is declared but has no value, which YAML parses as null. If the intent is an empty list, set it to []; if the intent is to inherit global base modes, omit this key entirely.
| base_modes: | |
| # list of mode names that are to be activated by default. | |
| # The full set of modes to be activated is base_modes + default_modes. | |
| # If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. | |
| # Otherwise, this overrides the setting from the global configuration (serena_config.yml). | |
| # This setting can, in turn, be overridden by CLI parameters (--mode). | |
| default_modes: | |
| # list of mode names that are to be activated by default. | |
| # The full set of modes to be activated is base_modes + default_modes. | |
| # If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. | |
| # Otherwise, this overrides the setting from the global configuration (serena_config.yml). | |
| # This setting can, in turn, be overridden by CLI parameters (--mode). |
| base_modes: | ||
|
|
||
| # list of mode names that are to be activated by default. | ||
| # The full set of modes to be activated is base_modes + default_modes. | ||
| # If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. | ||
| # Otherwise, this overrides the setting from the global configuration (serena_config.yml). | ||
| # This setting can, in turn, be overridden by CLI parameters (--mode). | ||
| default_modes: |
There was a problem hiding this comment.
default_modes: is declared but has no value, which YAML parses as null. If the intent is an empty list, set it to []; if the intent is to inherit global defaults, omit this key entirely.
| base_modes: | |
| # list of mode names that are to be activated by default. | |
| # The full set of modes to be activated is base_modes + default_modes. | |
| # If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. | |
| # Otherwise, this overrides the setting from the global configuration (serena_config.yml). | |
| # This setting can, in turn, be overridden by CLI parameters (--mode). | |
| default_modes: | |
| # list of mode names that are to be activated by default. | |
| # The full set of modes to be activated is base_modes + default_modes. | |
| # If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. | |
| # Otherwise, this overrides the setting from the global configuration (serena_config.yml). | |
| # This setting can, in turn, be overridden by CLI parameters (--mode). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/scorecard.yml:
- Line 20: The conditional "if: github.event.repository.default_branch ==
github.ref_name || github.event_name == 'pull_request'" contains a dead branch
because this workflow's triggers don't include "pull_request"; either add
"pull_request" to the workflow's on: triggers so github.event_name ==
'pull_request' can be true, or remove the "|| github.event_name ==
'pull_request'" clause (simplify the if to only compare default branch) so the
condition matches the actual triggers; update the scorecard.yml triggers or the
if condition accordingly to keep them consistent.
In `@CONTRIBUTING.md`:
- Line 85: CONTRIBUTING.md currently claims "security audit" and "CodeQL
analysis" are required CI checks, but .mergify.yml does not mark those workflows
as required; update the documentation (line ~85) so it only lists the actual
blocking checks: quality, msrv, test, test-cross-platform, and coverage, or
alternatively add the audit and codeql workflows to the required list in
.mergify.yml; reference the audit.yml and codeql.yml workflow names if you
choose to make them blocking so the doc and .mergify.yml remain consistent.
In `@SECURITY.md`:
- Around line 5-8: The table rows containing the emoji shortcodes
":white_check_mark:" and ":x:" should be replaced with ASCII-only text; update
the entries for versions "0.1.x" and "< 0.1" to use plain text like
"supported"/"unsupported" or "yes"/"no" (e.g., replace "| 0.1.x |
:white_check_mark: |" with "| 0.1.x | supported |" and "| < 0.1 | :x:
|" with "| < 0.1 | unsupported |") so the SECURITY.md table contains no emoji
shortcodes or Unicode characters.
| - **Style**: Follows project conventions, passes `cargo fmt` and `cargo clippy -- -D warnings` | ||
| - **Documentation**: Public APIs have rustdoc, AGENTS.md updated if architecture changes | ||
|
|
||
| CI must pass before merge. This includes formatting, linting, tests, security audit, and CodeQL analysis. Branch protection enforces these checks on the `main` branch. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the CONTRIBUTING.md file around line 85
cat -n CONTRIBUTING.md | sed -n '80,90p'Repository: EvilBit-Labs/Stringy
Length of output: 917
🏁 Script executed:
# Check for .mergify.yml and CI workflows
echo "=== Checking for .mergify.yml ==="
ls -la .mergify.yml 2>/dev/null || echo "No .mergify.yml found"
echo ""
echo "=== Checking for workflow files ==="
ls -la .github/workflows/ 2>/dev/null || echo "No .github/workflows/ found"
echo ""
echo "=== Looking for security audit references ==="
rg 'security.*audit|audit.*security|cargo-audit' .github/workflows/ || echo "No security audit found"
echo ""
echo "=== Looking for CodeQL workflow ==="
ls -la .github/workflows/ | grep -i codeql || echo "No explicit CodeQL file"Repository: EvilBit-Labs/Stringy
Length of output: 1252
🏁 Script executed:
# Check the content of .mergify.yml if it exists
if [ -f .mergify.yml ]; then
echo "=== Content of .mergify.yml ==="
cat .mergify.yml
fiRepository: EvilBit-Labs/Stringy
Length of output: 2809
🏁 Script executed:
echo "=== Content of audit.yml ==="
cat .github/workflows/audit.yml
echo ""
echo "=== Content of codeql.yml ==="
cat .github/workflows/codeql.ymlRepository: EvilBit-Labs/Stringy
Length of output: 1324
Update CONTRIBUTING.md line 85 to accurately reflect required CI checks.
The documented checks (security audit and CodeQL analysis) do run via audit.yml and codeql.yml workflows, but neither is configured as a required merge check in .mergify.yml. Line 85 should list only the actual blocking checks: quality, msrv, test, test-cross-platform, and coverage. Either update the documentation to remove aspirational checks, or add security audit and CodeQL to the merge requirements if they should be blocking.
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` at line 85, CONTRIBUTING.md currently claims "security
audit" and "CodeQL analysis" are required CI checks, but .mergify.yml does not
mark those workflows as required; update the documentation (line ~85) so it only
lists the actual blocking checks: quality, msrv, test, test-cross-platform, and
coverage, or alternatively add the audit and codeql workflows to the required
list in .mergify.yml; reference the audit.yml and codeql.yml workflow names if
you choose to make them blocking so the doc and .mergify.yml remain consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@SECURITY.md`:
- Around line 64-74: The SECURITY.md claims two enforcement mechanisms that are
not present; either add the crate-level pragma #![forbid(unsafe_code)] to the
crate root (e.g., lib.rs or main.rs) to match the "No unsafe code" claim or
remove/update that bullet to accurately reflect current code, and update the CI
claim: either remove "cargo-deny" from the dependency-auditing bullet and
reference only cargo-audit, or add a cargo-deny configuration to your CI (and
reference the new workflow) so the document matches the actual workflows; ensure
the Security Features list text is edited accordingly so both statements are
accurate.
| ## Security Features | ||
|
|
||
| Stringy includes several security-focused features: | ||
|
|
||
| - **No unsafe code**: `#![forbid(unsafe_code)]` is enforced project-wide | ||
| - **Memory-safe parsing**: All binary format parsing uses bounds-checked Rust libraries (goblin, pelite) | ||
| - **Graceful error handling**: Malformed inputs produce errors, not crashes | ||
| - **Dependency auditing**: `cargo-audit` and `cargo-deny` run in CI | ||
| - **Automated dependency updates**: Via Dependabot | ||
| - **Supply chain transparency**: CycloneDX SBOMs and Sigstore attestations per release | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get an overview of the repository structure
git ls-files | head -20Repository: EvilBit-Labs/Stringy
Length of output: 737
🏁 Script executed:
# Check for forbid(unsafe_code) in Rust sources
rg -n '#!\[forbid\(unsafe_code\)\]' --type=rsRepository: EvilBit-Labs/Stringy
Length of output: 90
🏁 Script executed:
# Look for Cargo.toml files to check dependencies
fd -n 'Cargo.toml$' -x head -30 {}Repository: EvilBit-Labs/Stringy
Length of output: 294
🏁 Script executed:
# Check .github directory for CI and release workflows
find .github -type f -name '*.yml' -o -name '*.yaml' | head -20Repository: EvilBit-Labs/Stringy
Length of output: 350
🏁 Script executed:
# Find and read Cargo.toml files
find . -name 'Cargo.toml' -type f | head -5Repository: EvilBit-Labs/Stringy
Length of output: 76
🏁 Script executed:
# Check for forbid(unsafe_code) using correct syntax
find . -name '*.rs' -type f -exec grep -l 'forbid(unsafe_code)' {} \;Repository: EvilBit-Labs/Stringy
Length of output: 46
🏁 Script executed:
# Read the main Cargo.toml
head -80 ./Cargo.tomlRepository: EvilBit-Labs/Stringy
Length of output: 1452
🏁 Script executed:
# Check the audit workflow
cat .github/workflows/audit.ymlRepository: EvilBit-Labs/Stringy
Length of output: 685
🏁 Script executed:
# Check the release workflow
cat .github/workflows/release.ymlRepository: EvilBit-Labs/Stringy
Length of output: 15852
🏁 Script executed:
# Directly read lib.rs and main.rs to check for forbid pragma
head -20 src/lib.rsRepository: EvilBit-Labs/Stringy
Length of output: 932
🏁 Script executed:
# Check main.rs
head -20 src/main.rsRepository: EvilBit-Labs/Stringy
Length of output: 671
🏁 Script executed:
# Search explicitly for forbid anywhere in the repo
rg -i 'forbid' --type=rust --type=toml --type=yamlRepository: EvilBit-Labs/Stringy
Length of output: 232
🏁 Script executed:
# Check dependabot.yml for cargo-deny or any deny config
cat .github/dependabot.ymlRepository: EvilBit-Labs/Stringy
Length of output: 501
🏁 Script executed:
# Check full lib.rs to see if forbid appears anywhere
cat src/lib.rs | head -50Repository: EvilBit-Labs/Stringy
Length of output: 2266
🏁 Script executed:
# Check full main.rs
cat src/main.rsRepository: EvilBit-Labs/Stringy
Length of output: 685
🏁 Script executed:
# Comprehensive search for any forbid, deny, or unsafe keywords
rg 'forbid|#!\[deny' src/Repository: EvilBit-Labs/Stringy
Length of output: 46
Correct SECURITY.md to reflect actual implementation.
The document claims two security enforcement mechanisms not currently implemented:
-
#![forbid(unsafe_code)]— Not present insrc/lib.rsorsrc/main.rs. Either add this pragma to the crate root or remove this claim. -
cargo-denyin CI — Not configured in.github/workflows/or.github/dependabot.yml. The audit workflow usescargo-auditvia GitHub Actions instead. Update line 70 to reference onlycargo-audit, or addcargo-denyconfiguration.
The other claims are accurate: goblin and pelite are dependencies, Dependabot is configured, and CycloneDX SBOMs + Sigstore attestations are generated in the release workflow.
🤖 Prompt for AI Agents
In `@SECURITY.md` around lines 64 - 74, The SECURITY.md claims two enforcement
mechanisms that are not present; either add the crate-level pragma
#![forbid(unsafe_code)] to the crate root (e.g., lib.rs or main.rs) to match the
"No unsafe code" claim or remove/update that bullet to accurately reflect
current code, and update the CI claim: either remove "cargo-deny" from the
dependency-auditing bullet and reference only cargo-audit, or add a cargo-deny
configuration to your CI (and reference the new workflow) so the document
matches the actual workflows; ensure the Security Features list text is edited
accordingly so both statements are accurate.
- Add #![forbid(unsafe_code)] to src/lib.rs (was claimed in AGENTS.md and SECURITY.md but not actually enforced) - Fix qlty coverage upload path: target/lcov.info -> lcov.info to match where cargo-llvm-cov writes the report - Clarify CONTRIBUTING.md: list actual merge-blocking checks (quality, MSRV, test, cross-platform, coverage) and note that security audit and CodeQL are informational, not merge-blocking
- Add #![forbid(unsafe_code)] to src/lib.rs (was claimed in AGENTS.md and SECURITY.md but not actually enforced) - Fix qlty coverage upload path: target/lcov.info -> lcov.info to match where cargo-llvm-cov writes the report - Clarify CONTRIBUTING.md: list actual merge-blocking checks (quality, MSRV, test, cross-platform, coverage) and note that security audit and CodeQL are informational, not merge-blocking Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
1 participant
This pull request introduces several improvements to project automation, documentation, and governance. The most significant changes are the addition of stricter CI/CD and merge requirements, enhanced documentation of contribution and security policies, and the introduction of automated security and merge management workflows.
CI/CD and Automation Improvements:
.github/workflows/ci.yml, ensuring all jobs (quality, MSRV, test, cross-platform test, coverage) always run regardless of which files are changed. The aggregateci-gatejob for branch protection was also removed. [1] [2] [3] [4] [5] [6].github/dependabot.ymlto disable automatic rebasing for all ecosystems and removed the unusedrust-toolchainupdate entry..github/workflows/scorecard.ymlto enable OSSF Scorecard supply-chain security analysis on the repository, including scheduled and branch protection-triggered runs..mergify.ymlto automate and enforce merge queueing, review, and branch protection policies, including conventional commit enforcement and required status checks.Documentation and Policy Updates:
CONTRIBUTING.mdwith clearer MSRV policy, explicit ban on unsafe code (#![forbid(unsafe_code)]), improved module layout guidance, detailed code review requirements, DCO (Developer Certificate of Origin) instructions, and a new section on project governance and decision-making. [1] [2] [3] [4]AGENTS.mdoutlining open-source quality standards for OSSF Scorecard compliance, including commit sign-off, CI requirements, test coverage, security practices, and documentation standards.Project Configuration:
.serena/project.ymlwith explicit fields for project name, included/excluded tools, and mode configuration, improving clarity for Serena-based automation.These changes collectively raise the project's quality bar, automate best practices, and clarify contributor expectations, aligning with industry standards for open-source software.
References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]