feat(security): add DE0708 no-non-FIPS-hasher lint; replace all direct sha2 usage with FNV-1a#4080
Merged
Merged
Conversation
📝 WalkthroughWalkthroughReplaces direct SHA-2 usage with deterministic 64-bit FNV-1a in OIDC token caching and OData pagination, marks sha2 as a Phase B candidate in deny-fips.toml, adds SECURITY.md notes, and introduces DE0708 dylint with UI tests plus allow-list plumbing. ChangesSHA-256 Elimination and DE0708 Lint Enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
mattgarmon
force-pushed
the
fips
branch
2 times, most recently
from
June 11, 2026 23:10
de313ff to
a47f32d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md (1)
1-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a Table of Contents to this artifact README.
Line 1 starts a multi-section doc, but there is no TOC section.
📚 Proposed update
# DE0708: No Non-FIPS Hasher Imports +## Table of Contents + +- [Why](`#why`) +- [Allow-list](`#allow-list`) +- [Example](`#example`) +- [References](`#references`) + Prohibits imports of non-FIPS-validated hash crates (`sha2`, `sha1`, `md5`) outside an explicit allow-list.As per coding guidelines,
**/*.{md,markdown,adoc,asciidoc}: Generate Table of Contents (TOC) for artifact documentation.🤖 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 `@tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md` around lines 1 - 27, Add a Table of Contents section to the DE0708 README by inserting a "## Table of Contents" block after the title and listing the existing top-level sections (Why, Allow-list, Example, References) as links; update the README.md near the top so readers can jump to sections and ensure the TOC reflects the file’s headings and stays in sync with future edits (no code changes to is_in_hasher_allow_list() are required, but mention of that function can remain in the Allow-list section).Source: Coding guidelines
🧹 Nitpick comments (2)
gears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client.rs (1)
227-233: ⚡ Quick winConsider extracting
fnv1a_64to a shared utility module.The identical
fnv1a_64implementation is duplicated here and inlibs/toolkit-odata/src/pagination.rs. Extracting it to a common crate (e.g.,lint_utilsor a newhash_utilsmodule) would eliminate duplication and ensure consistent behavior if the algorithm ever needs adjustment.🤖 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 `@gears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client.rs` around lines 227 - 233, The fnv1a_64 hash implementation is duplicated; extract it to a shared utility (e.g., new crate or common module like hash_utils) as a single pub fn fnv1a_64(data: &[u8]) -> u64, move the implementation out of token_client.rs and pagination.rs, update both call sites to import the new module (use crate::hash_utils::fnv1a_64 or use hash_utils::fnv1a_64 depending on crate layout), remove the duplicated versions, run cargo build and tests, and ensure build_cache_key (and any callers) use the centralized fnv1a_64 so behavior remains identical across the codebase.tools/dylint_lints/README.md (1)
44-47: ⚡ Quick winAdd a TOC for this README.
The markdown docs guideline calls for a table of contents, and this file is now long enough that navigation would benefit from one.
As per coding guidelines,
**/*.{md,markdown,adoc,asciidoc}: Generate Table of Contents (TOC) for artifact documentation.🤖 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 `@tools/dylint_lints/README.md` around lines 44 - 47, Add a Table of Contents near the top of tools/dylint_lints/README.md (directly after the main title/header) that lists and links to the major sections (e.g., Security (DE07xx) and other primary headings present in the file) so readers can jump to subsections; ensure the TOC follows the repo markdown-docs guideline for generating TOCs for **/*.{md,markdown,adoc,asciidoc} by including internal markdown anchor links for each header in the README so navigation and automated TOC tooling will work.Source: Coding guidelines
🤖 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/security/SECURITY.md`:
- Around line 448-449: The AuthN Resolver documentation still mentions using
"sha256(token)" for the OIDC cache key, which conflicts with the new FNV-1a
usage; update that earlier sentence to match the change in
oidc-authn-plugin/src/infra/token_client.rs and
libs/toolkit-odata/src/pagination.rs by replacing the "sha256(token)" wording
with a description that the cache key uses an inline FNV-1a non-cryptographic
fingerprint (or equivalent phrasing referencing the FNV-1a implementation used)
so both sections consistently describe the same cache-key fingerprint.
In `@libs/toolkit-odata/src/pagination.rs`:
- Around line 82-101: Cursor hash format is not versioned so old cursors can
fail FilterMismatch when short_filter_hash
(toolkit_odata::pagination::short_filter_hash) or normalize_filter_for_hash
changes; update cursor encoding and validation to include a hash-algorithm
identifier and accept legacy formats during a transition. Specifically: change
CursorV1.f to store a prefixed string like "fnv1a:<hex>" (or add a separate
enum/field for algorithm) and update validate_cursor_against to parse and
compare algorithm-prefixed hashes, falling back to the legacy raw hex comparison
for backward compatibility; ensure effective_filter_hash generation in
short_filter_hash emits the same prefix and add acceptance logic for both
prefixed and unprefixed legacy values to avoid immediate FilterMismatch failures
during rollout.
---
Outside diff comments:
In `@tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md`:
- Around line 1-27: Add a Table of Contents section to the DE0708 README by
inserting a "## Table of Contents" block after the title and listing the
existing top-level sections (Why, Allow-list, Example, References) as links;
update the README.md near the top so readers can jump to sections and ensure the
TOC reflects the file’s headings and stays in sync with future edits (no code
changes to is_in_hasher_allow_list() are required, but mention of that function
can remain in the Allow-list section).
---
Nitpick comments:
In
`@gears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client.rs`:
- Around line 227-233: The fnv1a_64 hash implementation is duplicated; extract
it to a shared utility (e.g., new crate or common module like hash_utils) as a
single pub fn fnv1a_64(data: &[u8]) -> u64, move the implementation out of
token_client.rs and pagination.rs, update both call sites to import the new
module (use crate::hash_utils::fnv1a_64 or use hash_utils::fnv1a_64 depending on
crate layout), remove the duplicated versions, run cargo build and tests, and
ensure build_cache_key (and any callers) use the centralized fnv1a_64 so
behavior remains identical across the codebase.
In `@tools/dylint_lints/README.md`:
- Around line 44-47: Add a Table of Contents near the top of
tools/dylint_lints/README.md (directly after the main title/header) that lists
and links to the major sections (e.g., Security (DE07xx) and other primary
headings present in the file) so readers can jump to subsections; ensure the TOC
follows the repo markdown-docs guideline for generating TOCs for
**/*.{md,markdown,adoc,asciidoc} by including internal markdown anchor links for
each header in the README so navigation and automated TOC tooling will work.
🪄 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: 93184dd7-1caf-40a1-9626-e49db250d448
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktools/dylint_lints/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
deny-fips.tomldocs/security/SECURITY.mdgears/system/authn-resolver/plugins/oidc-authn-plugin/Cargo.tomlgears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client.rslibs/toolkit-odata/Cargo.tomllibs/toolkit-odata/src/pagination.rstools/dylint_lints/Cargo.tomltools/dylint_lints/README.mdtools/dylint_lints/de07_security/de0708_no_non_fips_hasher/Cargo.tomltools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.mdtools/dylint_lints/de07_security/de0708_no_non_fips_hasher/src/lib.rstools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/bad_sha2_import.rstools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/bad_sha2_import.stderrtools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/good_allowed_path.rstools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/good_allowed_path.stderrtools/dylint_lints/lint_utils/src/lib.rs
💤 Files with no reviewable changes (2)
- libs/toolkit-odata/Cargo.toml
- gears/system/authn-resolver/plugins/oidc-authn-plugin/Cargo.toml
…t sha2 usage with FNV-1a Signed-off-by: mattgarmon <mjg2790@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md (1)
1-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a TOC to this README.
This artifact doc is missing the required Table of Contents section, so navigation is less consistent with the repo’s documentation standard. As per coding guidelines,
**/*.{md,markdown,adoc,asciidoc}: Generate Table of Contents (TOC) for artifact documentation.🤖 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 `@tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md` around lines 1 - 27, The README for DE0708 lacks a Table of Contents; add a TOC near the top of tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md listing the major headings (Why, Allow-list, Example, References) with anchor links to each section so the document matches repo artifact-doc standards; ensure the TOC appears before the first section and update any section headings if needed so anchors are correct, and mention in the TOC the note about is_in_hasher_allow_list() in lint_utils/src/lib.rs as the locus for allow-list changes.Source: Coding guidelines
🧹 Nitpick comments (1)
libs/toolkit-odata/src/pagination.rs (1)
83-100: ⚡ Quick winAdd a golden hash regression test.
Line 83 changes the serialized
filter_hashalgorithm/format, but the current tests only prove self-consistency and output length. A future tweak to the FNV constants, hex casing, or normalized byte stream would still pass while silently changing the cursor contract consumed byvalidate_cursor_against.Please pin one representative expression to an exact 16-character hash value in the test suite.
🤖 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 `@libs/toolkit-odata/src/pagination.rs` around lines 83 - 100, Tests only check self-consistency of the filter hash but not a pinned output; add a golden regression test that computes the hash for one representative expression using normalize_filter_for_hash and fnv1a_64 (formatted with "{:016x}") and asserts it equals a fixed 16-character hex string; place the test alongside existing pagination tests (or in the same test module that covers validate_cursor_against) so any future accidental changes to fnv1a_64, hex formatting, or normalization will fail until intentionally updated.
🤖 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.
Outside diff comments:
In `@tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md`:
- Around line 1-27: The README for DE0708 lacks a Table of Contents; add a TOC
near the top of
tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.md listing the
major headings (Why, Allow-list, Example, References) with anchor links to each
section so the document matches repo artifact-doc standards; ensure the TOC
appears before the first section and update any section headings if needed so
anchors are correct, and mention in the TOC the note about
is_in_hasher_allow_list() in lint_utils/src/lib.rs as the locus for allow-list
changes.
---
Nitpick comments:
In `@libs/toolkit-odata/src/pagination.rs`:
- Around line 83-100: Tests only check self-consistency of the filter hash but
not a pinned output; add a golden regression test that computes the hash for one
representative expression using normalize_filter_for_hash and fnv1a_64
(formatted with "{:016x}") and asserts it equals a fixed 16-character hex
string; place the test alongside existing pagination tests (or in the same test
module that covers validate_cursor_against) so any future accidental changes to
fnv1a_64, hex formatting, or normalization will fail until intentionally
updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c15fcdf5-5df8-4575-a90c-405cb2df39e7
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktools/dylint_lints/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
deny-fips.tomldocs/security/SECURITY.mdgears/system/authn-resolver/plugins/oidc-authn-plugin/Cargo.tomlgears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client.rslibs/toolkit-odata/Cargo.tomllibs/toolkit-odata/src/pagination.rstools/dylint_lints/Cargo.tomltools/dylint_lints/README.mdtools/dylint_lints/de07_security/de0708_no_non_fips_hasher/Cargo.tomltools/dylint_lints/de07_security/de0708_no_non_fips_hasher/README.mdtools/dylint_lints/de07_security/de0708_no_non_fips_hasher/src/lib.rstools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/bad_sha2_import.rstools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/bad_sha2_import.stderrtools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/good_allowed_path.rstools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/good_allowed_path.stderrtools/dylint_lints/lint_utils/src/lib.rs
💤 Files with no reviewable changes (2)
- gears/system/authn-resolver/plugins/oidc-authn-plugin/Cargo.toml
- libs/toolkit-odata/Cargo.toml
✅ Files skipped from review due to trivial changes (2)
- tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/Cargo.toml
- tools/dylint_lints/README.md
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/bad_sha2_import.stderr
- tools/dylint_lints/lint_utils/src/lib.rs
- deny-fips.toml
- tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/bad_sha2_import.rs
- tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/ui/good_allowed_path.rs
- tools/dylint_lints/de07_security/de0708_no_non_fips_hasher/src/lib.rs
- docs/security/SECURITY.md
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client.rs
striped-zebra-dev
approved these changes
Jun 12, 2026
Artifizer
approved these changes
Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3415
Summary by CodeRabbit
New Features
Documentation
Chores