Skip to content

refactor: deduplicate shared utilities into utils module #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
darkmaster0345 merged 1 commit into from
Jun 19, 2026
Merged

refactor: deduplicate shared utilities into utils module #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 1 addition & 46 deletions neuron-encrypt/src/bin/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use clap_complete::{generate, Shell};
use indicatif::{ProgressBar, ProgressState, ProgressStyle};
use neuron_encrypt_core::crypto::{self, ProgressReporter, ThrottledReporter};
use neuron_encrypt_core::error::CryptoError;
use neuron_encrypt_core::utils::{constant_time_eq, is_vx2_file, HumanBytes};
use serde::Serialize;
use sha2::{Digest, Sha256};
use zeroize::Zeroizing;
Expand Down Expand Up @@ -147,29 +148,6 @@ fn read_password(password_file: &Option<PathBuf>) -> Zeroizing<String> {
}
}

fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
let len_a = a.len();
let len_b = b.len();
let max_len = len_a.max(len_b);

let mut byte_comparison_result = 0;

for i in 0..max_len {
let byte_a = a.get(i).unwrap_or(&0);
let byte_b = b.get(i).unwrap_or(&0);
byte_comparison_result |= byte_a ^ byte_b;
}

let len_diff = len_a ^ len_b;
// len_mismatch_flag will be 0 if lengths are equal, 1 if lengths are different
let len_mismatch_flag = (((len_diff | len_diff.wrapping_neg()) >> (usize::BITS - 1)) & 1) as u8;

// Combine byte comparison result with length mismatch flag
// If either bytes don't match OR lengths don't match, final_result will be non-zero
let final_result = byte_comparison_result | len_mismatch_flag;

final_result == 0
}

fn read_password_confirmed(password_file: &Option<PathBuf>) -> Result<Zeroizing<String>, String> {
let pass1 = read_password(password_file);
Expand Down Expand Up @@ -252,23 +230,6 @@ impl Drop for IndProgress {
}
}

struct HumanBytes(u64);

impl std::fmt::Display for HumanBytes {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let b = self.0;
if b < 1024 {
write!(f, "{b} B")
} else if b < 1024 * 1024 {
write!(f, "{:.1} KB", b as f64 / 1024.0)
} else if b < 1024 * 1024 * 1024 {
write!(f, "{:.1} MB", b as f64 / (1024.0 * 1024.0))
} else {
write!(f, "{:.2} GB", b as f64 / (1024.0 * 1024.0 * 1024.0))
}
}
}

struct QuietReporter;
impl ProgressReporter for QuietReporter {
fn report(&self, _progress: f32, _message: &str) {}
Expand Down Expand Up @@ -386,12 +347,6 @@ fn check_output_exists(path: &Path, force: bool, json: bool) -> Result<(), ExitC
Ok(())
}

fn is_vx2_file(path: &Path) -> bool {
path.extension()
.and_then(|e| e.to_str())
.map(|s| s.eq_ignore_ascii_case("vx2"))
.unwrap_or(false)
}

fn emit_json(result: &JsonResult) {
match serde_json::to_string(result) {
Expand Down
45 changes: 7 additions & 38 deletions neuron-encrypt/src/gui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use eframe::egui::{
};
use neuron_encrypt_core::crypto::{self, ProgressReporter, ThrottledReporter};
use neuron_encrypt_core::error::CryptoError;
use neuron_encrypt_core::utils::{constant_time_eq, format_size, is_vx2_file};
use rand_core::RngCore;
use sha2::{Digest, Sha256};
use zeroize::Zeroizing;
Expand Down Expand Up @@ -160,27 +161,6 @@ pub struct NeuronEncryptApp {
cancel_flag: Arc<AtomicBool>,
}

fn is_vx2_file(path: &Path) -> bool {
path.extension()
.and_then(|e| e.to_str())
.map(|s| s.eq_ignore_ascii_case("vx2"))
.unwrap_or(false)
}

fn constant_time_eq(a: &str, b: &str) -> bool {
let (ab, bb) = (a.as_bytes(), b.as_bytes());
let max_len = ab.len().max(bb.len());
let mut acc: u8 = 0;

for i in 0..max_len {
let av = ab.get(i).copied().unwrap_or(0);
let bv = bb.get(i).copied().unwrap_or(0);
acc |= av ^ bv;
}

acc |= (ab.len() ^ bb.len()).min(0xff) as u8;
acc == 0
}

fn truncate_chars(s: &str, n: usize) -> String {
let mut out = s.chars().take(n).collect::<String>();
Expand All @@ -194,17 +174,6 @@ fn sanitize_text(s: &str) -> String {
s.trim().to_owned()
}

fn format_size(bytes: u64) -> String {
if bytes < 1024 {
format!("{bytes} B")
} else if bytes < 1024 * 1024 {
format!("{:.1} KB", bytes as f64 / 1024.0)
} else if bytes < 1024 * 1024 * 1024 {
format!("{:.1} MB", bytes as f64 / 1024.0 / 1024.0)
} else {
format!("{:.1} GB", bytes as f64 / 1024.0 / 1024.0 / 1024.0)
}
}

fn strength_color(strength: Strength) -> Color32 {
match strength {
Expand Down Expand Up @@ -441,7 +410,7 @@ impl NeuronEncryptApp {
return;
}

if self.mode == Mode::Encrypt && !constant_time_eq(&self.password, &self.confirm_password) {
if self.mode == Mode::Encrypt && !constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes()) {
self.status = Some("Passphrases do not match.".to_owned());
Comment on lines +413 to 414

@sourcery-ai sourcery-ai Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: The repeated password/confirm_password comparisons could be centralized to reduce duplication and potential drift.

Since this comparison appears in multiple branches with similar conditions, consider extracting a helper like fn passwords_match(&self) -> bool that performs the mode check and constant-time comparison. This avoids duplicated logic and keeps any future change to the comparison behavior in a single place.

Suggested implementation:

        if !self.passwords_match() {
            self.status = Some("Passphrases do not match.".to_owned());
            return;
        }

To fully implement the suggestion and centralize the comparison logic, you should:

  1. Add a helper method on the relevant GUI/app struct (the one that owns mode, password, and confirm_password) inside its impl block, for example:

    impl App {  // or whatever the struct is named
    fn passwords_match(&self) -> bool {
        if self.mode != Mode::Encrypt {
            return true;
        }

        constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes())
    }
}

  2. Replace any other occurrences of repeated comparison logic that look like:

    if self.mode == Mode::Encrypt && !constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes()) { ... }

    or similar direct comparisons, with:

    if !self.passwords_match() { ... }

    so that all passphrase comparison behavior is centralized in passwords_match.

return;
}
Expand Down Expand Up @@ -535,7 +504,7 @@ impl NeuronEncryptApp {
));
return;
}
if self.mode == Mode::Encrypt && !constant_time_eq(&self.password, &self.confirm_password) {
if self.mode == Mode::Encrypt && !constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes()) {
self.status = Some("Passphrases do not match.".to_owned());
return;
}
Expand Down Expand Up @@ -1178,7 +1147,7 @@ impl NeuronEncryptApp {

if !self.password.is_empty()
&& !self.confirm_password.is_empty()
&& !constant_time_eq(&self.password, &self.confirm_password)
&& !constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes())
{
ui.add_space(8.0);
self.draw_notice(
Expand Down Expand Up @@ -1239,7 +1208,7 @@ impl NeuronEncryptApp {
let mismatch = self.mode == Mode::Encrypt
&& !self.password.is_empty()
&& !self.confirm_password.is_empty()
&& !constant_time_eq(&self.password, &self.confirm_password);
&& !constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes());
let disabled = self.password.chars().count() < crypto::MIN_PASSWORD_LEN
|| mismatch
|| (self.mode == Mode::Encrypt
Expand Down Expand Up @@ -1558,7 +1527,7 @@ impl NeuronEncryptApp {

if !self.password.is_empty()
&& !self.confirm_password.is_empty()
&& !constant_time_eq(&self.password, &self.confirm_password)
&& !constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes())
{
ui.add_space(8.0);
self.draw_notice(
Expand All @@ -1578,7 +1547,7 @@ impl NeuronEncryptApp {
ui.add_space(20.0);
let mut enabled = self.password.chars().count() >= crypto::MIN_PASSWORD_LEN;
if self.mode == Mode::Encrypt {
enabled &= constant_time_eq(&self.password, &self.confirm_password);
enabled &= constant_time_eq(self.password.as_bytes(), self.confirm_password.as_bytes());
}

if let Some(msg) = &self.status {
Expand Down
1 change: 1 addition & 0 deletions neuron-encrypt/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
pub mod crypto;
pub mod error;
pub mod utils;
56 changes: 56 additions & 0 deletions neuron-encrypt/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
pub fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
let len_a = a.len();
let len_b = b.len();
let max_len = len_a.max(len_b);

let mut byte_comparison_result = 0;

for i in 0..max_len {
let byte_a = a.get(i).unwrap_or(&0);
let byte_b = b.get(i).unwrap_or(&0);
byte_comparison_result |= byte_a ^ byte_b;
}

let len_diff = len_a ^ len_b;
let len_mismatch_flag = (((len_diff | len_diff.wrapping_neg()) >> (usize::BITS - 1)) & 1) as u8;

let final_result = byte_comparison_result | len_mismatch_flag;

final_result == 0
}
Comment on lines +1 to +20

@sourcery-ai sourcery-ai Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider a more flexible signature for constant_time_eq to avoid repeated as_bytes() at call sites.

Now that this utility is shared between CLI and GUI, consider making it generic over AsRef<[u8]>, e.g. pub fn constant_time_eq<A: AsRef<[u8]>, B: AsRef<[u8]>>(a: A, b: B) -> bool. That would allow direct use with String, &str, Vec<u8>, etc., and remove the need for repeated as_bytes() calls while keeping the implementation unchanged.

Suggested change
pub fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
let len_a = a.len();
let len_b = b.len();
let max_len = len_a.max(len_b);
let mut byte_comparison_result = 0;
for i in 0..max_len {
let byte_a = a.get(i).unwrap_or(&0);
let byte_b = b.get(i).unwrap_or(&0);
byte_comparison_result |= byte_a ^ byte_b;
}
let len_diff = len_a ^ len_b;
let len_mismatch_flag = (((len_diff | len_diff.wrapping_neg()) >> (usize::BITS - 1)) & 1) as u8;
let final_result = byte_comparison_result | len_mismatch_flag;
final_result == 0
}
pub fn constant_time_eq<A: AsRef<[u8]>, B: AsRef<[u8]>>(a: A, b: B) -> bool {
let a = a.as_ref();
let b = b.as_ref();
let len_a = a.len();
let len_b = b.len();
let max_len = len_a.max(len_b);
let mut byte_comparison_result = 0;
for i in 0..max_len {
let byte_a = a.get(i).unwrap_or(&0);
let byte_b = b.get(i).unwrap_or(&0);
byte_comparison_result |= byte_a ^ byte_b;
}
let len_diff = len_a ^ len_b;
let len_mismatch_flag = (((len_diff | len_diff.wrapping_neg()) >> (usize::BITS - 1)) & 1) as u8;
let final_result = byte_comparison_result | len_mismatch_flag;
final_result == 0
}


pub fn is_vx2_file(path: &std::path::Path) -> bool {
path.extension()
.and_then(|e| e.to_str())
.map(|s| s.eq_ignore_ascii_case("vx2"))
.unwrap_or(false)
}

pub fn format_size(bytes: u64) -> String {
if bytes < 1024 {
format!("{bytes} B")
} else if bytes < 1024 * 1024 {
format!("{:.1} KB", bytes as f64 / 1024.0)
} else if bytes < 1024 * 1024 * 1024 {
format!("{:.1} MB", bytes as f64 / 1024.0 / 1024.0)
} else {
format!("{:.1} GB", bytes as f64 / 1024.0 / 1024.0 / 1024.0)
}
}

pub struct HumanBytes(pub u64);

@kilo-code-bot kilo-code-bot Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔥 The Roast: You moved two byte-size formatters into the same module and let them keep different GB precision (format_size uses {:.1}, HumanBytes uses {:.2}). That's not deduplication so much as putting twins in matching outfits with different shoe sizes.

🩹 The Fix: Pick one source of truth: either have HumanBytes delegate to format_size(self.0) if one decimal is acceptable, or factor a shared formatter that takes precision and use it from both paths. Add a small boundary-size test/table so future formatting tweaks don't quietly diverge.

📏 Severity: nitpick

Reply with @kilocode-bot fix it to have Kilo Code address this issue.


impl std::fmt::Display for HumanBytes {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let b = self.0;
if b < 1024 {
write!(f, "{b} B")
} else if b < 1024 * 1024 {
write!(f, "{:.1} KB", b as f64 / 1024.0)
} else if b < 1024 * 1024 * 1024 {
write!(f, "{:.1} MB", b as f64 / (1024.0 * 1024.0))
} else {
write!(f, "{:.2} GB", b as f64 / (1024.0 * 1024.0 * 1024.0))
}
}
}
Loading