Skip to content

chore: SHA-pin pre-commit hook revisions#10

Merged
27Bslash6 merged 3 commits into
mainfrom
chore/sha-pin-pre-commit
Apr 11, 2026
Merged

chore: SHA-pin pre-commit hook revisions#10
27Bslash6 merged 3 commits into
mainfrom
chore/sha-pin-pre-commit

Conversation

@27Bslash6
Copy link
Copy Markdown
Contributor

@27Bslash6 27Bslash6 commented Apr 6, 2026

Summary

  • Pin all pre-commit hook rev: values to full commit SHAs instead of mutable version tags
  • Covers pre-commit-hooks and detect-secrets
  • Adds pragma: allowlist secret to prevent detect-secrets false positives on SHA strings

Why

Mutable tags are a supply chain attack vector — anyone with write access to the hook repo can move a tag to point at malicious code. SHA pins are immutable.

Test plan

  • Pre-commit hooks install and run successfully with SHA revs

Summary by CodeRabbit

  • Chores

    • Updated pre-commit tooling pins for repository checks.
    • Added a secrets detection baseline to enable project-wide secret scanning.
    • Adjusted version-control settings to include the dependency lockfile.
  • Tests

    • Marked test fixtures as allowlisted for secret-detection to keep tests passing.

Pin pre-commit-hooks and detect-secrets to full commit SHAs instead
of mutable version tags.
- Un-gitignore Cargo.lock to pin transitive dependency versions.
  icu_* crates v2.2.0 require Rust 1.86, breaking the MSRV 1.85 CI job
  when Cargo resolves fresh dependencies.
- Add .secrets.baseline required by detect-secrets pre-commit hook
  (added in prior commit but baseline file was missing).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6a1efb6f-0af1-43fe-986f-09f88bcf42ef

📥 Commits

Reviewing files that changed from the base of the PR and between d1808cc and 4ed86e2.

📒 Files selected for processing (2)
  • .secrets.baseline
  • crates/cachekit/tests/encryption_tests.rs
✅ Files skipped from review due to trivial changes (2)
  • crates/cachekit/tests/encryption_tests.rs
  • .secrets.baseline

📝 Walkthrough

Walkthrough

Removed Cargo.lock ignore, pinned two pre-commit hooks to specific commit SHAs, added a .secrets.baseline file, and appended allowlist pragmas to two test fixtures to satisfy secret-detection baseline.

Changes

Cohort / File(s) Summary
Gitignore
\.gitignore
Removed the rule excluding Cargo.lock, so the Rust lock file is now tracked.
Pre-commit config
.pre-commit-config.yaml
Replaced mutable tags with specific commit SHAs for pre-commit-hooks and detect-secrets repositories; no hook IDs or args changed.
Secrets baseline
.secrets.baseline
Added a new detect-secrets baseline JSON declaring plugins, filters, and metadata (no recorded results).
Test fixtures
crates/cachekit/tests/encryption_tests.rs
Appended // pragma: allowlist secret comments to hard-coded Secret { api_key: ... } test fixtures to align with the secrets baseline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 I hopped through files with nimble feet,
Locked Cargo's key so builds are neat,
Baseline guarding secrets in sight,
Hooks pinned steady, snug and tight,
A tiny hop for code — delight! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: SHA-pin pre-commit hook revisions' clearly and specifically summarizes the main change: replacing mutable version tags with immutable commit SHAs in pre-commit configurations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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/sha-pin-pre-commit

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

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: 1

🧹 Nitpick comments (1)
.secrets.baseline (1)

133-149: Consider adding inline pragma comments as an alternative.

While the baseline file approach is valid, you might also consider adding # pragma: allowlist secret comments directly on lines 62 and 86 of encryption_tests.rs. This would:

  • Make the allowlisting more explicit in the source code
  • Eliminate the need for baseline entries for these specific test fixtures
  • Make it clearer to future maintainers that these are intentional test values

Example:

api_key: "sk-live-abc123".to_owned(),  // pragma: allowlist secret

However, the current baseline approach is equally valid and may be preferred if you want to keep test code free of security-scanning annotations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.secrets.baseline around lines 133 - 149, Add inline allowlist pragmas to
the test fixtures flagged in the baseline by placing a comment like "pragma:
allowlist secret" on the specific test values in
crates/cachekit/tests/encryption_tests.rs (the literal test lines at or around
the values referenced at line numbers 62 and 86) so the security scanner
recognizes them as intentional test secrets; update the two lines that set the
test secret values (search for the api_key or hardcoded secret assignments in
encryption_tests.rs) to include the pragma comment instead of relying solely on
.secrets.baseline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.pre-commit-config.yaml:
- Line 33: Update the detect-secrets rev to match the claimed v1.5.0 tag:
replace the current detect-secrets SHA value
(01886c8a910c64595c47f186ca1ffc0b77fa5458) with the correct commit for v1.5.0
(68e8b45440415753fff70a312ece8da92ba85b4a) or alternatively update the version
comment to reflect the SHA currently used; ensure the pre-commit entry for
detect-secrets uses a consistent rev value that corresponds to its annotated
version.

---

Nitpick comments:
In @.secrets.baseline:
- Around line 133-149: Add inline allowlist pragmas to the test fixtures flagged
in the baseline by placing a comment like "pragma: allowlist secret" on the
specific test values in crates/cachekit/tests/encryption_tests.rs (the literal
test lines at or around the values referenced at line numbers 62 and 86) so the
security scanner recognizes them as intentional test secrets; update the two
lines that set the test secret values (search for the api_key or hardcoded
secret assignments in encryption_tests.rs) to include the pragma comment instead
of relying solely on .secrets.baseline.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 500e3011-ddde-4d80-92cb-a5ce64b0e97d

📥 Commits

Reviewing files that changed from the base of the PR and between 6e03c9b and d1808cc.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • .gitignore
  • .pre-commit-config.yaml
  • .secrets.baseline
💤 Files with no reviewable changes (1)
  • .gitignore

Comment thread .pre-commit-config.yaml
# General file checks
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
rev: cef0300fd0fc4d2a87a85fa2093c6b283ea36f4b # v5.0.0 # pragma: allowlist secret
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify that the commit SHAs in .pre-commit-config.yaml correspond to the claimed version tags

echo "Checking pre-commit-hooks SHA..."
curl -s https://api.github.com/repos/pre-commit/pre-commit-hooks/git/refs/tags/v5.0.0 | jq -r '.object.sha' | head -c 40
echo " (expected: cef0300fd0fc4d2a87a85fa2093c6b283ea36f4b)"

echo -e "\n\nChecking detect-secrets SHA..."
curl -s https://api.github.com/repos/Yelp/detect-secrets/git/refs/tags/v1.5.0 | jq -r '.object.sha' | head -c 40
echo " (expected: 01886c8a910c64595c47f186ca1ffc0b77fa5458)"

Repository: cachekit-io/cachekit-rs

Length of output: 320


Fix the detect-secrets SHA: it does not correspond to v1.5.0.

The pre-commit-hooks SHA (line 33) is correct for v5.0.0. However, the detect-secrets SHA on line 45 is incorrect: the config specifies 01886c8a910c64595c47f186ca1ffc0b77fa5458, but the actual v1.5.0 tag in the Yelp/detect-secrets repository points to 68e8b45440415753fff70a312ece8da92ba85b4a. Update the SHA to match the claimed version or correct the version comment.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.pre-commit-config.yaml at line 33, Update the detect-secrets rev to match
the claimed v1.5.0 tag: replace the current detect-secrets SHA value
(01886c8a910c64595c47f186ca1ffc0b77fa5458) with the correct commit for v1.5.0
(68e8b45440415753fff70a312ece8da92ba85b4a) or alternatively update the version
comment to reflect the SHA currently used; ensure the pre-commit entry for
detect-secrets uses a consistent rev value that corresponds to its annotated
version.

Add `// pragma: allowlist secret` to test fixture values in
encryption_tests.rs instead of relying on .secrets.baseline entries.
Inline pragmas travel with the code — baseline entries go stale when
lines shift.
@27Bslash6 27Bslash6 merged commit 674391a into main Apr 11, 2026
5 checks passed
@27Bslash6 27Bslash6 deleted the chore/sha-pin-pre-commit branch April 11, 2026 23:17
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