Conversation
Summary of ChangesHello @kalbasit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness of narinfo hash validation by implementing strict adherence to the Nix32 encoding specification. The changes prevent potential data corruption or compatibility issues by ensuring that only correctly formatted hashes are processed, thereby improving the overall reliability of the system's data handling. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request significantly improves the narinfo hash validation by enforcing strict Nix32 encoding rules for character set and length, which is a great improvement over the previous, more permissive validation. The addition of a comprehensive test suite with numerous cases is excellent, covering a wide range of valid and invalid inputs to ensure the new logic is robust. The changes are well-implemented. I have one minor suggestion to improve comment clarity.
6cde063 to
6ffbcd0
Compare
8520594 to
d14dbe2
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
6ffbcd0 to
b226ff0
Compare
d14dbe2 to
af8d25f
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
af8d25f to
abf888a
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
abf888a to
72c518e
Compare
b226ff0 to
ebe277b
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
72c518e to
0cc36f7
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
926ba30 to
15b5c18
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
15b5c18 to
714c33b
Compare
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
714c33b to
d7e4ebc
Compare
This commit hardens the narinfo hash validation to strictly enforce the Nix32 encoding specification: - Enforce exactly 32-character hash length (previously allowed any length) - Restrict to valid Nix32 alphabet: 0-9, a-d, f-n, p-s, v-z (previously allowed any lowercase letter and digit) - Explicitly reject forbidden characters: e, o, u, t - Reject uppercase letters and special characters Added 18 comprehensive test cases covering: - Valid hashes with different character combinations - Invalid hashes with forbidden characters - Invalid hashes with wrong lengths - Invalid hashes with special characters and uppercase letters This ensures that only valid Nix32 hashes are accepted, preventing potential data corruption or compatibility issues in the narinfo processing pipeline.
d7e4ebc to
05cd73e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #840 +/- ##
=====================================
Coverage 3.96% 3.96%
=====================================
Files 6 6
Lines 429 429
=====================================
Hits 17 17
Misses 409 409
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-0.8
git worktree add -d .worktree/backport-840-to-release-0.8 origin/release-0.8
cd .worktree/backport-840-to-release-0.8
git switch --create backport-840-to-release-0.8
git cherry-pick -x 91945f3df2fadd881c7dc0cc883b327458ee88a0 |
1 similar comment
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-0.8
git worktree add -d .worktree/backport-840-to-release-0.8 origin/release-0.8
cd .worktree/backport-840-to-release-0.8
git switch --create backport-840-to-release-0.8
git cherry-pick -x 91945f3df2fadd881c7dc0cc883b327458ee88a0 |
This commit hardens the narinfo hash validation to strictly enforce the Nix32 encoding specification: - Enforce exactly 32-character hash length (previously allowed any length) - Restrict to valid Nix32 alphabet: 0-9, a-d, f-n, p-s, v-z (previously allowed any lowercase letter and digit) - Explicitly reject forbidden characters: e, o, u, t - Reject uppercase letters and special characters Added 18 comprehensive test cases covering: - Valid hashes with different character combinations - Invalid hashes with forbidden characters - Invalid hashes with wrong lengths - Invalid hashes with special characters and uppercase letters This ensures that only valid Nix32 hashes are accepted, preventing potential data corruption or compatibility issues in the narinfo processing pipeline. (cherry picked from commit 91945f3)
This commit hardens the narinfo hash validation to strictly enforce the Nix32 encoding specification: - Enforce exactly 32-character hash length (previously allowed any length) - Restrict to valid Nix32 alphabet: 0-9, a-d, f-n, p-s, v-z (previously allowed any lowercase letter and digit) - Explicitly reject forbidden characters: e, o, u, t - Reject uppercase letters and special characters Added 18 comprehensive test cases covering: - Valid hashes with different character combinations - Invalid hashes with forbidden characters - Invalid hashes with wrong lengths - Invalid hashes with special characters and uppercase letters This ensures that only valid Nix32 hashes are accepted, preventing potential data corruption or compatibility issues in the narinfo processing pipeline. (cherry picked from commit 91945f3)
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.
The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern. (cherry picked from commit e91ec45)
Bot-based backport to `release-0.8`, triggered by a label in #841. The narinfo hash was updated in #840 to reflect the upstream definition in NixOS/nix#15004 and so the server should only allow narinfo requests that match this pattern.

This commit hardens the narinfo hash validation to strictly enforce the Nix32
encoding specification:
allowed any lowercase letter and digit)
Added 18 comprehensive test cases covering:
This ensures that only valid Nix32 hashes are accepted, preventing potential
data corruption or compatibility issues in the narinfo processing pipeline.