Skip to content

Fix: Panic on overly long filenames instead of silently dropping files#1287

Merged
j178 merged 6 commits intomasterfrom
copilot/fix-command-line-length-bug
Dec 30, 2025
Merged

Fix: Panic on overly long filenames instead of silently dropping files#1287
j178 merged 6 commits intomasterfrom
copilot/fix-command-line-length-bug

Conversation

Copy link
Contributor

Copilot AI commented Dec 30, 2025

The Partitions iterator would silently return None when encountering a filename too long to fit in a command line batch, causing all subsequent files to be skipped without warning.

Changes

  • crates/prek/src/run.rs:157-174: Replace silent None return with explicit panic including:

    • Problematic filename and byte length
    • Defensive bounds check to prevent unhelpful index out of bounds errors
  • crates/prek/src/run.rs:220-358: Add unit tests covering:

    • Normal partitioning behavior
    • Panic on single overly long filename
    • Panic when long filename appears mid-list (critical test ensuring we don't silently skip remaining files)
    • Batch size and CLI length limit enforcement

Error message

thread 'main' panicked at crates/prek/src/run.rs:169:13:
Filename `aaa...aaa` (5001 bytes) is too long to fit in command line

The minimum max_cli_length is 4096 bytes (non-Unix/Windows platforms), making this scenario rare but critical when it occurs.

Original prompt

This section details on the original issue you should resolve

<issue_title>Potential bug: long command lines might drop files</issue_title>
<issue_description>### Summary

This is theoretical issue I ran into while working on a PR for #1279.

The implementation of Iterator for Partitions in crates/prek/src/run.rs will end the iterator early if command_length + filename_length + 2 > max_length (roughly). Especially if the overly long filename is later in the list, it could result in some files not being checked.

prek/crates/prek/src/run.rs

Lines 121 to 139 in 1856285

let start_index = self.current_index;
let mut current_length = self.command_length + 1;
while self.current_index < self.filenames.len() {
let filename = self.filenames[self.current_index];
let length = filename.as_os_str().len() + 1;
if current_length + length > self.max_cli_length
|| self.current_index - start_index >= self.max_per_batch
{
break;
}
current_length += length;
self.current_index += 1;
}
if self.current_index == start_index {
None

It’s probably not a problem because the shortest maximum CLI length is 4096 bytes (on non-UNIX, non-Windows platforms):

#[cfg(not(any(unix, windows)))]
{
1 << 12
}

Still, maybe it’s worth a panic!? Figured I'd point it out and see if you thought it was worth changing.

Platform

non-UNIX, non-Windows

Version

prek 0.2.25

.pre-commit-config.yaml

No response

Log file

No response</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Replace silent None return with explicit panic when a filename is too long
- Add detailed error message showing filename, sizes, and limits
- Add comprehensive unit tests to validate the panic behavior
- Prevents silent file skipping that could lead to incomplete checking

Co-authored-by: j178 <10510431+j178@users.noreply.github.com>
Add defensive bounds check to prevent potential index out of bounds
panic with unhelpful error message.

Co-authored-by: j178 <10510431+j178@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix potential bug with long command lines dropping files Fix: Panic on overly long filenames instead of silently dropping files Dec 30, 2025
Copilot AI requested a review from j178 December 30, 2025 09:35
Changed from verbose multi-line message to simple single-line format:
"Filename `{}` ({} bytes) is too long to fit in command line"

Co-authored-by: j178 <10510431+j178@users.noreply.github.com>
@j178 j178 marked this pull request as ready for review December 30, 2025 09:58
@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.00%. Comparing base (0efd637) to head (07ca875).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1287      +/-   ##
==========================================
+ Coverage   89.94%   90.00%   +0.05%     
==========================================
  Files          80       80              
  Lines       15808    15881      +73     
==========================================
+ Hits        14219    14293      +74     
+ Misses       1589     1588       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

📦 Cargo Bloat Comparison

Binary size change: +0.00% (16.7 MiB → 16.7 MiB)

Expand for cargo-bloat output

Head Branch Results

 File  .text     Size          Crate Name
 0.7%   1.5% 115.5KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.6%   1.3% 107.3KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.3%   0.7%  54.7KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.7%  52.8KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.5%  39.8KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  39.2KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.5%  39.2KiB           prek prek::run::{{closure}}
 0.2%   0.5%  37.3KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
 0.2%   0.5%  36.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  36.5KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  30.8KiB           std? <core::marker::PhantomData<T> as serde_core::de::DeserializeSeed>::deserialize
 0.2%   0.4%  30.7KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  29.8KiB           prek prek::languages::rust::installer::RustInstaller::install::{{closure}}
 0.2%   0.4%  28.4KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  27.9KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.3%  27.6KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.3%  27.3KiB           prek prek::cli::run::run::run_hooks::{{closure}}
 0.2%   0.3%  26.2KiB           prek prek::cli::auto_update::update_repo::{{closure}}
 0.1%   0.3%  25.3KiB           prek prek::main
 0.1%   0.3%  24.7KiB           prek prek::hook::HookBuilder::build::{{closure}}
41.2%  88.5%   6.9MiB                And 10639 smaller methods. Use -n N to show more.
46.5% 100.0%   7.8MiB                .text section size, the file size is 16.7MiB

Base Branch Results

 File  .text     Size          Crate Name
 0.7%   1.4% 114.7KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.6%   1.3% 107.3KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.3%   0.7%  54.7KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.7%  52.8KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.5%  39.8KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  39.2KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.5%  39.2KiB           prek prek::run::{{closure}}
 0.2%   0.5%  37.3KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
 0.2%   0.5%  36.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  36.5KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  30.8KiB           std? <core::marker::PhantomData<T> as serde_core::de::DeserializeSeed>::deserialize
 0.2%   0.4%  30.7KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  29.8KiB           prek prek::languages::rust::installer::RustInstaller::install::{{closure}}
 0.2%   0.4%  28.4KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  27.9KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.3%  27.6KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.3%  27.3KiB           prek prek::cli::run::run::run_hooks::{{closure}}
 0.2%   0.3%  26.2KiB           prek prek::cli::auto_update::update_repo::{{closure}}
 0.1%   0.3%  25.3KiB           prek prek::main
 0.1%   0.3%  24.7KiB           prek prek::hook::HookBuilder::build::{{closure}}
41.2%  88.5%   6.9MiB                And 10638 smaller methods. Use -n N to show more.
46.5% 100.0%   7.8MiB                .text section size, the file size is 16.7MiB

@j178 j178 merged commit c5f1270 into master Dec 30, 2025
29 of 30 checks passed
@j178 j178 deleted the copilot/fix-command-line-length-bug branch December 30, 2025 10:09
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.

Potential bug: long command lines might drop files

2 participants