diff --git a/.editorconfig b/.editorconfig deleted file mode 100644 index 30be905..0000000 --- a/.editorconfig +++ /dev/null @@ -1,24 +0,0 @@ -root = true - -[*] -charset = utf-8 -end_of_line = lf -indent_style = space -indent_size = 2 -insert_final_newline = true -tab_width = 2 -trim_trailing_whitespace = true - -[*.md] -indent_style = space -indent_size = 2 -trim_trailing_whitespace = false - -[*.{lock,diff,patch}] -indent_style = unset -indent_size = unset -insert_final_newline = unset -trim_trailing_whitespace = unset -end_of_line = unset - - diff --git a/.gitignore b/.gitignore index 15ab3a1..809cc8f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,2 @@ target/ bin/ -completions/ diff --git a/.rustfmt.toml b/.rustfmt.toml index ac283d5..8b13789 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -1,27 +1 @@ -condense_wildcard_suffixes = true -doc_comment_code_block_width = 80 -edition = "2024" # Keep in sync with Cargo.toml. -enum_discrim_align_threshold = 60 -force_explicit_abi = false -force_multiline_blocks = true -format_code_in_doc_comments = true -format_macro_matchers = true -format_strings = true -group_imports = "StdExternalCrate" -hex_literal_case = "Upper" -imports_granularity = "Crate" -imports_layout = "HorizontalVertical" -inline_attribute_width = 60 -match_block_trailing_comma = true -max_width = 80 -newline_style = "Unix" -normalize_comments = true -normalize_doc_attributes = true -overflow_delimited_expr = true -struct_field_align_threshold = 60 -tab_spaces = 2 -unstable_features = true -use_field_init_shorthand = true -use_try_shorthand = true -wrap_comments = true diff --git a/Cargo.lock b/Cargo.lock index bd32f1d..d295e00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "anstream" +version = "0.6.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "301af1932e46185686725e0fad2f8f2aa7da69dd70bf6ecc44d6b703844a3933" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is_terminal_polyfill", + "utf8parse", +] + [[package]] name = "anstyle" version = "1.0.11" @@ -18,10 +33,33 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "862ed96ca487e809f1c8e5a8447f6ee2cf102f846893800b20cebdf541fc6bbd" [[package]] -name = "bitflags" -version = "2.10.0" +name = "anstyle-parse" +version = "0.2.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" +checksum = "4e7644824f0aa2c7b9384579234ef10eb7efb6a0deb83f9630a49594dd9c15c2" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c8bdeb6047d8983be085bab0ba1472e6dc604e7041dbf6fcd5e71523014fae9" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "403f75924867bb1033c59fbf0797484329750cfbe3c4325cd33127941fabc882" +dependencies = [ + "anstyle", + "once_cell_polyfill", + "windows-sys", +] [[package]] name = "cfg-if" @@ -31,9 +69,9 @@ checksum = "9555578bc9e57714c812a1f84e4fc5b4d21fcb063490c624de019f7464c91268" [[package]] name = "clap" -version = "4.5.51" +version = "4.5.41" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c26d721170e0295f191a69bd9a1f93efcdb0aff38684b61ab5750468972e5f5" +checksum = "be92d32e80243a54711e5d7ce823c35c41c9d929dc4ab58e1276f625841aadf9" dependencies = [ "clap_builder", "clap_derive", @@ -41,28 +79,21 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.51" +version = "4.5.41" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75835f0c7bf681bfd05abe44e965760fea999a5286c6eb2d59883634fd02011a" +checksum = "707eab41e9622f9139419d573eca0900137718000c517d47da73045f54331c3d" dependencies = [ + "anstream", "anstyle", "clap_lex", -] - -[[package]] -name = "clap_complete" -version = "4.5.60" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e602857739c5a4291dfa33b5a298aeac9006185229a700e5810a3ef7272d971" -dependencies = [ - "clap", + "strsim", ] [[package]] name = "clap_derive" -version = "4.5.49" +version = "4.5.41" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a0b5487afeab2deb2ff4e03a807ad1a03ac532ff5a2cee5d86884440c7f7671" +checksum = "ef4f52386a59ca4c860f7393bcf8abd8dfd91ecccc0f774635ff68e92eeef491" dependencies = [ "heck", "proc-macro2", @@ -76,72 +107,41 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b94f61472cee1439c0b966b47e3aca9ae07e45d070759512cd390ea2bebc6675" +[[package]] +name = "colorchoice" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b05b61dc5112cbb17e4b6cd61790d9845d13888356391624cbe7e41efeac1e75" + [[package]] name = "eh" -version = "0.1.3" +version = "0.1.0" dependencies = [ "clap", "regex", - "tempfile", - "thiserror", "tracing", "tracing-subscriber", - "walkdir", "yansi", ] -[[package]] -name = "errno" -version = "0.3.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" -dependencies = [ - "libc", - "windows-sys", -] - -[[package]] -name = "fastrand" -version = "2.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" - -[[package]] -name = "getrandom" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" -dependencies = [ - "cfg-if", - "libc", - "r-efi", - "wasip2", -] - [[package]] name = "heck" version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" +[[package]] +name = "is_terminal_polyfill" +version = "1.70.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" + [[package]] name = "lazy_static" version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" -[[package]] -name = "libc" -version = "0.2.177" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2874a2af47a2325c2001a6e6fad9b16a53b802102b528163885171cf92b15976" - -[[package]] -name = "linux-raw-sys" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df1d3c3b53da64cf5760482273a98e575c651a67eec7f77df96b5b642de8f039" - [[package]] name = "log" version = "0.4.27" @@ -156,11 +156,12 @@ checksum = "32a282da65faaf38286cf3be983213fcf1d2e2a58700e808f83f4ea9a4804bc0" [[package]] name = "nu-ansi-term" -version = "0.50.3" +version = "0.46.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" +checksum = "77a8165726e8236064dbb45459242600304b42a5ea24ee2948e18e023bf7ba84" dependencies = [ - "windows-sys", + "overload", + "winapi", ] [[package]] @@ -169,6 +170,18 @@ version = "1.21.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" +[[package]] +name = "once_cell_polyfill" +version = "1.70.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4895175b425cb1f87721b59f0f286c2092bd4af812243672510e1ac53e2e0ad" + +[[package]] +name = "overload" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" + [[package]] name = "pin-project-lite" version = "0.2.16" @@ -193,17 +206,11 @@ dependencies = [ "proc-macro2", ] -[[package]] -name = "r-efi" -version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" - [[package]] name = "regex" -version = "1.12.2" +version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "843bc0191f75f3e22651ae5f1e72939ab2f72a4bc30fa80a066bd66edefc24d4" +checksum = "b544ef1b4eac5dc2db33ea63606ae9ffcfac26c1416a2806ae0bf5f56b201191" dependencies = [ "aho-corasick", "memchr", @@ -213,9 +220,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.13" +version = "0.4.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5276caf25ac86c8d810222b3dbb938e512c55c6831a10f3e6ed1c93b84041f1c" +checksum = "809e8dc61f6de73b46c85f4c96486310fe304c434cfa43669d7b40f711150908" dependencies = [ "aho-corasick", "memchr", @@ -228,28 +235,6 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" -[[package]] -name = "rustix" -version = "1.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd15f8a2c5551a84d56efdc1cd049089e409ac19a3072d5037a17fd70719ff3e" -dependencies = [ - "bitflags", - "errno", - "libc", - "linux-raw-sys", - "windows-sys", -] - -[[package]] -name = "same-file" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" -dependencies = [ - "winapi-util", -] - [[package]] name = "sharded-slab" version = "0.1.7" @@ -265,6 +250,12 @@ version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "syn" version = "2.0.104" @@ -276,39 +267,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "tempfile" -version = "3.23.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d31c77bdf42a745371d260a26ca7163f1e0924b64afa0b688e61b5a9fa02f16" -dependencies = [ - "fastrand", - "getrandom", - "once_cell", - "rustix", - "windows-sys", -] - -[[package]] -name = "thiserror" -version = "2.0.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f63587ca0f12b72a0600bcba1d40081f830876000bb46dd2337a3051618f4fc8" -dependencies = [ - "thiserror-impl", -] - -[[package]] -name = "thiserror-impl" -version = "2.0.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ff15c8ecd7de3849db632e14d18d2571fa09dfc5ed93479bc4485c7a517c913" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "thread_local" version = "1.1.9" @@ -363,9 +321,9 @@ dependencies = [ [[package]] name = "tracing-subscriber" -version = "0.3.20" +version = "0.3.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2054a14f5307d601f88daf0553e1cbf472acc4f2c51afab632431cdcd72124d5" +checksum = "e8189decb5ac0fa7bc8b96b7cb9b2701d60d48805aca84a238004d665fcc4008" dependencies = [ "nu-ansi-term", "sharded-slab", @@ -381,6 +339,12 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5a5f39404a5da50712a4c1eecf25e90dd62b613502b7e925fd4e4d19b5c96512" +[[package]] +name = "utf8parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" + [[package]] name = "valuable" version = "0.1.1" @@ -388,61 +352,105 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" [[package]] -name = "walkdir" -version = "2.5.0" +name = "winapi" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" dependencies = [ - "same-file", - "winapi-util", + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", ] [[package]] -name = "wasip2" -version = "1.0.1+wasi-0.2.4" +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0562428422c63773dad2c345a1882263bbf4d65cf3f42e90921f787ef5ad58e7" -dependencies = [ - "wit-bindgen", -] +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" [[package]] -name = "winapi-util" -version = "0.1.11" +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" -dependencies = [ - "windows-sys", -] - -[[package]] -name = "windows-link" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "windows-sys" -version = "0.61.2" +version = "0.59.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" +checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" dependencies = [ - "windows-link", + "windows-targets", ] [[package]] -name = "wit-bindgen" -version = "0.46.0" +name = "windows-targets" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "xtask" -version = "0.1.3" +version = "0.1.0" dependencies = [ "clap", - "clap_complete", - "eh", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 9cc10d5..116535e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,23 +8,12 @@ description = "Ergonomic Nix CLI helper" edition = "2024" license = "MPL-2.0" readme = true -rust-version = "1.89" -version = "0.1.3" +rust-version = "1.85" +version = "0.1.0" [workspace.dependencies] -clap = { default-features = false, features = [ "std", "help", "derive" ], version = "4.5.51" } -clap_complete = "4.5.60" -regex = "1.12.2" -tempfile = "3.23.0" -thiserror = "2.0.17" -tracing = "0.1.41" -tracing-subscriber = "0.3.20" -walkdir = "2.5.0" -yansi = "1.0.1" - -[profile.release] -codegen-units = 1 -lto = true -opt-level = "z" -panic = "abort" -strip = true +clap = { features = [ "derive" ], version = "4.5" } +regex = "1.0" +tracing = "0.1" +tracing-subscriber = "0.3" +yansi = "1.0" diff --git a/REPORT.md b/REPORT.md deleted file mode 100644 index 50d4355..0000000 --- a/REPORT.md +++ /dev/null @@ -1,932 +0,0 @@ -# Code Review Report - -**Review Date**: 2025-11-15\ -**Reviewed Files**: eh/src/main.rs, eh/src/lib.rs, eh/src/command.rs, -eh/src/error.rs, eh/src/run.rs, eh/src/shell.rs, eh/src/util.rs, -eh/src/build.rs, eh/Cargo.toml, Cargo.toml, xtask/src/main.rs\ -**Total Issues Found**: 12 - ---- - -## Executive Summary - -I conducted a comprehensive code review of the "eh" ergonomic Nix helper -project, examining all 11 Rust source files totaling 4,159 lines of code. The -codebase is well-structured with clear separation of concerns, implementing a -command-line tool that wraps Nix commands with intelligent retry logic for hash -mismatches and license issues. The architecture uses traits for extensibility -and follows Rust best practices for error handling. - -The review identified 12 issues across severity levels: 3 critical issues -related to potential security vulnerabilities and error handling, 5 major issues -concerning code robustness and maintainability, and 4 minor issues related to -code quality and documentation. The most significant concerns involve -insufficient input validation in multicall scenarios, potential resource leaks -in command execution, and inconsistent error handling patterns. - ---- - -## Critical Issues (Severity: High) - -### Issue #1: Insufficient Input Validation in Multicall Dispatch - -**File**: `eh/src/main.rs`\ -**Lines**: 34-70\ -**Severity**: High\ -**Category**: Security - -**Problem Description**: The `dispatch_multicall` function processes -command-line arguments without validation before passing them to Nix commands. -This could allow injection of dangerous shell commands through specially crafted -arguments when the tool is invoked as multicall binaries (nr, ns, nb). - -**Current Code**: - -```rust -fn dispatch_multicall( - app_name: &str, - args: std::env::Args, -) -> Option> { - let rest: Vec = args.collect(); - let hash_extractor = util::RegexHashExtractor; - let fixer = util::DefaultNixFileFixer; - let classifier = util::DefaultNixErrorClassifier; - - match app_name { - "nr" => { - Some(run::handle_nix_run( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - "ns" => { - Some(shell::handle_nix_shell( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - "nb" => { - Some(build::handle_nix_build( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - _ => None, - } -} -``` - -**Recommended Fix**: - -```rust -fn dispatch_multicall( - app_name: &str, - args: std::env::Args, -) -> Option> { - let rest: Vec = args.collect(); - - // Validate arguments before processing - if let Err(e) = util::validate_nix_args(&rest) { - return Some(Err(e)); - } - - let hash_extractor = util::RegexHashExtractor; - let fixer = util::DefaultNixFileFixer; - let classifier = util::DefaultNixErrorClassifier; - - match app_name { - "nr" => { - Some(run::handle_nix_run( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - "ns" => { - Some(shell::handle_nix_shell( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - "nb" => { - Some(build::handle_nix_build( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - _ => None, - } -} -``` - -**Rationale**: Adding input validation in the multicall dispatch ensures that -dangerous arguments are caught early, regardless of how the tool is invoked. -This prevents command injection attacks that could bypass the validation in -`handle_nix_with_retry`. - -**Impact if Unfixed**: Attackers could execute arbitrary commands by passing -malicious arguments to multicall binaries, potentially leading to system -compromise. - ---- - -### Issue #2: Resource Leak in Non-Interactive Command Execution - -**File**: `eh/src/command.rs`\ -**Lines**: 151-179\ -**Severity**: High\ -**Category**: Performance/Resource Management - -**Problem Description**: The `run_with_logs` method creates child processes with -piped stdout/stderr but does not properly handle the case where the child -process hangs or produces infinite output, potentially causing resource -exhaustion. - -**Current Code**: - -```rust -loop { - let mut did_something = false; - - match stdout.read(&mut out_buf) { - Ok(0) => {}, - Ok(n) => { - interceptor.on_stdout(&out_buf[..n]); - out_queue.push_back(Vec::from(&out_buf[..n])); - did_something = true; - }, - Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}, - Err(e) => return Err(EhError::Io(e)), - } - - match stderr.read(&mut err_buf) { - Ok(0) => {}, - Ok(n) => { - interceptor.on_stderr(&err_buf[..n]); - err_queue.push_back(Vec::from(&err_buf[..n])); - did_something = true; - }, - Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}, - Err(e) => return Err(EhError::Io(e)), - } - - if !did_something && child.try_wait()?.is_some() { - break; - } -} -``` - -**Recommended Fix**: - -```rust -use std::time::{Duration, Instant}; - -let start_time = Instant::now(); -const TIMEOUT: Duration = Duration::from_secs(300); // 5 minute timeout - -loop { - let mut did_something = false; - - // Check for timeout - if start_time.elapsed() > TIMEOUT { - let _ = child.kill(); - return Err(EhError::CommandFailed { - command: format!("nix {} timed out after 5 minutes", self.subcommand), - }); - } - - match stdout.read(&mut out_buf) { - Ok(0) => {}, - Ok(n) => { - interceptor.on_stdout(&out_buf[..n]); - out_queue.push_back(Vec::from(&out_buf[..n])); - did_something = true; - }, - Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}, - Err(e) => return Err(EhError::Io(e)), - } - - match stderr.read(&mut err_buf) { - Ok(0) => {}, - Ok(n) => { - interceptor.on_stderr(&err_buf[..n]); - err_queue.push_back(Vec::from(&err_buf[..n])); - did_something = true; - }, - Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}, - Err(e) => return Err(EhError::Io(e)), - } - - if !did_something && child.try_wait()?.is_some() { - break; - } - - // Prevent busy waiting - if !did_something { - std::thread::sleep(Duration::from_millis(10)); - } -} -``` - -**Rationale**: Adding a timeout prevents infinite loops from hanging the -application, and a small sleep prevents CPU spinning when no data is available. - -**Impact if Unfixed**: Malicious or buggy Nix commands could cause the -application to hang indefinitely, consuming system resources and potentially -causing denial of service. - ---- - -### Issue #3: Inconsistent Error Code Mapping - -**File**: `eh/src/error.rs`\ -**Lines**: 40-47\ -**Severity**: High\ -**Category**: Logic/Error Handling - -**Problem Description**: The `exit_code` method returns the same exit code (1) -for most error types, making it impossible to distinguish between different -failure modes in scripts that depend on the tool. - -**Current Code**: - -```rust -impl EhError { - #[must_use] - pub const fn exit_code(&self) -> i32 { - match self { - Self::ProcessExit { code } => *code, - Self::NixCommandFailed(_) => 1, - Self::CommandFailed { .. } => 1, - _ => 1, - } - } -} -``` - -**Recommended Fix**: - -```rust -impl EhError { - #[must_use] - pub const fn exit_code(&self) -> i32 { - match self { - Self::ProcessExit { code } => *code, - Self::NixCommandFailed(_) => 2, - Self::CommandFailed { .. } => 3, - Self::HashExtractionFailed => 4, - Self::NoNixFilesFound => 5, - Self::HashFixFailed { .. } => 6, - Self::InvalidInput { .. } => 7, - Self::Io(_) => 8, - Self::Regex(_) => 9, - Self::Utf8(_) => 10, - } - } -} -``` - -**Rationale**: Unique exit codes for each error type allow scripts and -automation to handle different failure scenarios appropriately. - -**Impact if Unfixed**: Scripts cannot distinguish between different types of -failures, making error handling and recovery impossible. - ---- - -## Major Issues (Severity: Medium) - -### Issue #4: Missing Error Context in Hash Extraction - -**File**: `eh/src/util.rs`\ -**Lines**: 24-40\ -**Severity**: Medium\ -**Category**: Error Handling - -**Problem Description**: The `extract_hash` method returns `None` without -providing information about why hash extraction failed, making debugging -difficult. - -**Current Code**: - -```rust -impl HashExtractor for RegexHashExtractor { - fn extract_hash(&self, stderr: &str) -> Option { - let patterns = [ - r"got:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"have:\s+(sha256-[a-zA-Z0-9+/=]+)", - ]; - for pattern in &patterns { - if let Ok(re) = Regex::new(pattern) - && let Some(captures) = re.captures(stderr) - && let Some(hash) = captures.get(1) - { - return Some(hash.as_str().to_string()); - } - } - None - } -} -``` - -**Recommended Fix**: - -```rust -impl HashExtractor for RegexHashExtractor { - fn extract_hash(&self, stderr: &str) -> Option { - let patterns = [ - r"got:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"have:\s+(sha256-[a-zA-Z0-9+/=]+)", - ]; - for pattern in &patterns { - if let Ok(re) = Regex::new(pattern) { - if let Some(captures) = re.captures(stderr) { - if let Some(hash) = captures.get(1) { - tracing::debug!("Extracted hash '{}' using pattern '{}'", hash.as_str(), pattern); - return Some(hash.as_str().to_string()); - } - } - } else { - tracing::warn!("Invalid regex pattern: {}", pattern); - } - } - tracing::debug!("No hash found in stderr output"); - None - } -} -``` - -**Rationale**: Adding debug logging helps diagnose why hash extraction failed, -which is crucial for troubleshooting Nix build issues. - -**Impact if Unfixed**: Users cannot determine why hash fixing failed, making it -difficult to resolve Nix build problems. - ---- - -### Issue #5: Unsafe File Path Handling - -**File**: `eh/src/util.rs`\ -**Lines**: 120-133\ -**Severity**: Medium\ -**Category**: Security - -**Problem Description**: The `fix_hash_in_file` method uses -`unwrap_or(Path::new("."))` for the parent directory, which could lead to -writing files to unintended locations. - -**Current Code**: - -```rust -let temp_file = - NamedTempFile::new_in(file_path.parent().unwrap_or(Path::new(".")))?; -``` - -**Recommended Fix**: - -```rust -let parent_dir = file_path.parent() - .ok_or_else(|| EhError::HashFixFailed { - path: file_path.to_string_lossy().to_string(), - })?; -let temp_file = NamedTempFile::new_in(parent_dir)?; -``` - -**Rationale**: Explicitly handling the case where a file has no parent directory -prevents unintended file creation in the current directory. - -**Impact if Unfixed**: Temporary files could be created in unexpected locations, -potentially causing security issues or file system pollution. - ---- - -### Issue #6: Inefficient Regex Compilation in Loop - -**File**: `eh/src/util.rs`\ -**Lines**: 85-102\ -**Severity**: Medium\ -**Category**: Performance - -**Problem Description**: Regex patterns are compiled for every file that needs -hash fixing, even though the patterns are constant. - -**Current Code**: - -```rust -fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result { - // Pre-compile regex patterns once to avoid repeated compilation - let patterns: Vec<(Regex, String)> = [ - (r#"hash\s*=\s*"[^"]*""#, format!(r#"hash = "{new_hash}""#)), - ( - r#"sha256\s*=\s*"[^"]*""#, - format!(r#"sha256 = "{new_hash}""#), - ), - ( - r#"outputHash\s*=\s*"[^"]*""#, - format!(r#"outputHash = "{new_hash}""#), - ), - ] - .into_iter() - .map(|(pattern, replacement)| { - Regex::new(pattern) - .map(|re| (re, replacement)) - .map_err(EhError::Regex) - }) - .collect::>>()?; -``` - -**Recommended Fix**: - -```rust -// Add lazy_static or once_cell at the top of the file -use once_cell::sync::Lazy; -use std::sync::Arc; - -static HASH_PATTERNS: Lazy> = Lazy::new(|| { - vec![ - (Regex::new(r#"hash\s*=\s*"[^"]*""#).unwrap(), r#"hash = "{hash}""#), - (Regex::new(r#"sha256\s*=\s*"[^"]*""#).unwrap(), r#"sha256 = "{hash}""#), - (Regex::new(r#"outputHash\s*=\s*"[^"]*""#).unwrap(), r#"outputHash = "{hash}""#), - ] -}); - -fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result { - // Use pre-compiled patterns - let patterns: Vec<(Regex, String)> = HASH_PATTERNS - .iter() - .map(|(re, template)| { - let replacement = template.replace("{hash}", new_hash); - (re.clone(), replacement) - }) - .collect(); -``` - -**Rationale**: Pre-compiling regex patterns once and reusing them significantly -improves performance when fixing multiple files. - -**Impact if Unfixed**: Performance degradation when processing many Nix files, -especially in large projects. - ---- - -### Issue #7: Missing Error Handling in Completion Generation - -**File**: `xtask/src/main.rs`\ -**Lines**: 164-168\ -**Severity**: Medium\ -**Category**: Error Handling - -**Problem Description**: The completion generation code creates symlinks without -proper error handling for the case where the target file doesn't exist. - -**Current Code**: - -```rust -#[cfg(unix)] -{ - let canonical_target = fs::canonicalize(&completion_file)?; - std::os::unix::fs::symlink(&canonical_target, &symlink_path)?; - println!("completion symlink created: {}", symlink_path.display()); -} -``` - -**Recommended Fix**: - -```rust -#[cfg(unix)] -{ - match fs::canonicalize(&completion_file) { - Ok(canonical_target) => { - if let Err(e) = std::os::unix::fs::symlink(&canonical_target, &symlink_path) { - eprintln!("warning: failed to create symlink {}: {}", symlink_path.display(), e); - // Fall back to copying - if let Err(e) = fs::copy(&completion_file, &symlink_path) { - eprintln!("error: failed to copy completion file: {}", e); - return Err(e.into()); - } - println!("completion copy created: {}", symlink_path.display()); - } else { - println!("completion symlink created: {}", symlink_path.display()); - } - }, - Err(e) => { - eprintln!("error: completion file not found: {}", e); - return Err(e.into()); - } - } -} -``` - -**Rationale**: Proper error handling ensures the completion generation process -is robust and provides meaningful error messages. - -**Impact if Unfixed**: Completion generation could fail silently or with unclear -error messages, making it difficult for users to troubleshoot. - ---- - -### Issue #8: Inconsistent Tracing Configuration - -**File**: `eh/src/main.rs`\ -**Lines**: 14-20\ -**Severity**: Medium\ -**Category**: Configuration - -**Problem Description**: The tracing configuration is hardcoded and doesn't -respect environment variables or provide flexibility for different output -formats. - -**Current Code**: - -```rust -let format = tracing_subscriber::fmt::format() - .with_level(true) // don't include levels in formatted output - .with_target(true) // don't include targets - .with_thread_ids(false) // include the thread ID of the current thread - .with_thread_names(false) // include the name of the current thread - .compact(); // use the `Compact` formatting style. -tracing_subscriber::fmt().event_format(format).init(); -``` - -**Recommended Fix**: - -```rust -use tracing_subscriber::{fmt, EnvFilter}; - -let filter = EnvFilter::try_from_default_env() - .unwrap_or_else(|_| EnvFilter::new("eh=info")); - -let format = fmt::format() - .with_level(true) - .with_target(true) - .with_thread_ids(false) - .with_thread_names(false) - .compact(); - -fmt() - .event_format(format) - .with_env_filter(filter) - .init(); -``` - -**Rationale**: Respecting environment variables allows users to control log -levels and output without code changes, making debugging easier. - -**Impact if Unfixed**: Users cannot customize logging behavior, making it -difficult to debug issues in different environments. - ---- - -## Minor Issues (Severity: Low) - -### Issue #9: Inconsistent Comment Style - -**File**: `eh/src/command.rs`\ -**Lines**: 57, 67, 82, 88, 94\ -**Severity**: Low\ -**Category**: Code Style - -**Problem Description**: Some methods have -`#[allow(dead_code, reason = "FIXME")]` attributes while others have -`#[must_use]` attributes, creating inconsistent documentation style. - -**Current Code**: - -```rust -#[allow(dead_code, reason = "FIXME")] -pub fn args(mut self, args: I) -> Self -where - I: IntoIterator, - S: Into, -{ - self.args.extend(args.into_iter().map(Into::into)); - self -} - -#[must_use] -pub fn args_ref(mut self, args: &[String]) -> Self { - self.args.extend(args.iter().cloned()); - self -} -``` - -**Recommended Fix**: - -```rust -#[must_use] -pub fn args(mut self, args: I) -> Self -where - I: IntoIterator, - S: Into, -{ - self.args.extend(args.into_iter().map(Into::into)); - self -} - -#[must_use] -pub fn args_ref(mut self, args: &[String]) -> Self { - self.args.extend(args.iter().cloned()); - self -} -``` - -**Rationale**: Consistent use of `#[must_use]` attributes improves API clarity -and prevents unused method calls. - -**Impact if Unfixed**: Minor inconsistency in code style, but no functional -impact. - ---- - -### Issue #10: Missing Documentation for Public Traits - -**File**: `eh/src/util.rs`\ -**Lines**: 17-19, 42-46, 139-141\ -**Severity**: Low\ -**Category**: Documentation - -**Problem Description**: Public traits lack documentation comments, making it -difficult for users to understand their purpose and usage. - -**Current Code**: - -```rust -pub trait HashExtractor { - fn extract_hash(&self, stderr: &str) -> Option; -} - -pub trait NixFileFixer { - fn fix_hash_in_files(&self, new_hash: &str) -> Result; - fn find_nix_files(&self) -> Result>; - fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result; -} - -pub trait NixErrorClassifier { - fn should_retry(&self, stderr: &str) -> bool; -} -``` - -**Recommended Fix**: - -```rust -/// Extracts Nix output hashes from error messages. -/// -/// This trait is used to parse Nix error output and extract -/// the correct hash values when hash mismatches occur. -pub trait HashExtractor { - /// Attempts to extract a hash from the given stderr output. - /// - /// # Arguments - /// * `stderr` - The stderr output from a failed Nix command - /// - /// # Returns - /// * `Some(String)` - The extracted hash if found - /// * `None` - No hash could be extracted - fn extract_hash(&self, stderr: &str) -> Option; -} - -/// Fixes hash values in Nix configuration files. -/// -/// This trait provides methods to locate Nix files and update -/// hash values when they change due to source modifications. -pub trait NixFileFixer { - /// Fixes hash values in all discovered Nix files. - /// - /// # Arguments - /// * `new_hash` - The new hash value to set - /// - /// # Returns - /// * `Ok(true)` - At least one file was modified - /// * `Ok(false)` - No files needed modification - /// * `Err(EhError)` - An error occurred during processing - fn fix_hash_in_files(&self, new_hash: &str) -> Result; - - /// Discovers all Nix files in the current directory tree. - /// - /// # Returns - /// * `Ok(Vec)` - List of found Nix files - /// * `Err(EhError::NoNixFilesFound)` - No Nix files found - fn find_nix_files(&self) -> Result>; - - /// Fixes hash values in a specific Nix file. - /// - /// # Arguments - /// * `file_path` - Path to the Nix file to modify - /// * `new_hash` - The new hash value to set - /// - /// # Returns - /// * `Ok(true)` - The file was modified - /// * `Ok(false)` - The file didn't need modification - /// * `Err(EhError)` - An error occurred during processing - fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result; -} - -/// Classifies Nix errors to determine if retry is appropriate. -/// -/// This trait analyzes Nix error output to determine whether -/// a command should be retried with different flags or parameters. -pub trait NixErrorClassifier { - /// Determines if a command should be retried based on error output. - /// - /// # Arguments - /// * `stderr` - The stderr output from a failed Nix command - /// - /// # Returns - /// * `true` - The command should be retried - /// * `false` - The command should not be retried - fn should_retry(&self, stderr: &str) -> bool; -} -``` - -**Rationale**: Comprehensive documentation makes the API easier to understand -and use correctly. - -**Impact if Unfixed**: Users may struggle to understand how to implement or use -these traits correctly. - ---- - -### Issue #11: Unused Variable in Test - -**File**: `eh/src/util.rs`\ -**Lines**: 481-482\ -**Severity**: Low\ -**Category**: Code Quality - -**Problem Description**: The test extracts file permissions but never uses them, -creating dead code. - -**Current Code**: - -```rust -// Get original permissions -let original_metadata = std::fs::metadata(file_path).unwrap(); -let _original_permissions = original_metadata.permissions(); -``` - -**Recommended Fix**: - -```rust -// Get original permissions -let original_metadata = std::fs::metadata(file_path).unwrap(); -let original_permissions = original_metadata.permissions(); - -// Test hash replacement -let fixer = DefaultNixFileFixer; -let result = fixer.fix_hash_in_file(file_path, "sha256-newhash").unwrap(); - -assert!(result, "Hash replacement should succeed"); - -// Verify file still exists and permissions are preserved -let new_metadata = std::fs::metadata(file_path).unwrap(); -assert!( - new_metadata.is_file(), - "File should still exist after replacement" -); -assert_eq!( - original_permissions, - new_metadata.permissions(), - "File permissions should be preserved" -); -``` - -**Rationale**: Actually using the permissions check makes the test meaningful -and verifies that file permissions are preserved. - -**Impact if Unfixed**: The test doesn't actually verify what it claims to test. - ---- - -### Issue #12: Inconsistent Error Message Formatting - -**File**: `eh/src/util.rs`\ -**Lines**: 265-269, 284-289, 305-309\ -**Severity**: Low\ -**Category**: Code Style - -**Problem Description**: Warning messages use inconsistent formatting and some -have unnecessary line breaks in the string literals. - -**Current Code**: - -```rust -warn!( - "{}", - Paint::yellow( - "⚠ Unfree package detected, retrying with NIXPKGS_ALLOW_UNFREE=1..." - ) -); -warn!( - "{}", - Paint::yellow( - "⚠ Insecure package detected, retrying with \ - NIXPKGS_ALLOW_INSECURE=1..." - ) -); -warn!( - "{}", - Paint::yellow( - "⚠ Broken package detected, retrying with NIXPKGS_ALLOW_BROKEN=1..." - ) -); -``` - -**Recommended Fix**: - -```rust -warn!( - "{}", - Paint::yellow("⚠ Unfree package detected, retrying with NIXPKGS_ALLOW_UNFREE=1...") -); -warn!( - "{}", - Paint::yellow("⚠ Insecure package detected, retrying with NIXPKGS_ALLOW_INSECURE=1...") -); -warn!( - "{}", - Paint::yellow("⚠ Broken package detected, retrying with NIXPKGS_ALLOW_BROKEN=1...") -); -``` - -**Rationale**: Consistent formatting makes the code more readable and -maintainable. - -**Impact if Unfixed**: Minor inconsistency in code style, but no functional -impact. - ---- - -## Code Quality Observations - -### Positive Patterns - -1. **Good Separation of Concerns**: The codebase properly separates command - execution, error handling, and utility functions into distinct modules. -2. **Trait-Based Design**: The use of traits for `HashExtractor`, - `NixFileFixer`, and `NixErrorClassifier` provides good extensibility. -3. **Comprehensive Testing**: The `util.rs` module includes thorough unit tests - covering edge cases and error conditions. -4. **Atomic File Operations**: The hash fixing uses temporary files and atomic - operations to prevent corruption. - -### Areas for Improvement - -1. **Configuration Management**: Consider using a configuration file for default - values and timeouts instead of hardcoding them. -2. **Logging Strategy**: Implement structured logging with consistent log levels - throughout the application. -3. **Error Recovery**: Add more sophisticated error recovery mechanisms for - transient failures. - ---- - -## Summary Statistics - -- Critical Issues: 3 -- Major Issues: 5 -- Minor Issues: 4 -- Files Affected: 7 -- Total Lines Reviewed: 4,159 - ---- - -## Prioritized Action Plan - -1. **Fix input validation in multicall dispatch** - `eh/src/main.rs:34-70` - (Critical - Security) -2. **Add timeout protection to command execution** - `eh/src/command.rs:151-179` - (Critical - Resource Management) -3. **Implement unique error exit codes** - `eh/src/error.rs:40-47` (Critical - - Error Handling) -4. **Add debug logging to hash extraction** - `eh/src/util.rs:24-40` (Major - - Debugging) -5. **Fix unsafe file path handling** - `eh/src/util.rs:120-133` (Major - - Security) -6. **Optimize regex compilation** - `eh/src/util.rs:85-102` (Major - - Performance) -7. **Improve completion generation error handling** - - `xtask/src/main.rs:164-168` (Major - Robustness) -8. **Make tracing configuration flexible** - `eh/src/main.rs:14-20` (Major - - Configuration) -9. **Standardize method attributes** - `eh/src/command.rs:57,67,82,88,94` - (Minor - Style) -10. **Add trait documentation** - `eh/src/util.rs:17-19,42-46,139-141` (Minor - - Documentation) -11. **Fix unused variable in test** - `eh/src/util.rs:481-482` (Minor - Test - Quality) -12. **Standardize warning message formatting** - - `eh/src/util.rs:265-269,284-289,305-309` (Minor - Style) - diff --git a/TODO.md b/TODO.md deleted file mode 100644 index e098166..0000000 --- a/TODO.md +++ /dev/null @@ -1,383 +0,0 @@ -# TODO List: Codebase Improvements - -## 1. Extract Binary Name Dispatch Logic (Medium Priority) - -**Context**: Code duplication in main.rs:42-65 where the same trait -instantiation pattern repeats 3 times for nr/ns/nb dispatch. - -**Pseudocode**: - -```rust -fn dispatch_multicall_command(app_name: &str, args: Vec) -> Result { - let hash_extractor = util::RegexHashExtractor; - let fixer = util::DefaultNixFileFixer; - let classifier = util::DefaultNixErrorClassifier; - - match app_name { - "nr" => run::handle_nix_run(&args, &hash_extractor, &fixer, &classifier), - "ns" => shell::handle_nix_shell(&args, &hash_extractor, &fixer, &classifier), - "nb" => build::handle_nix_build(&args, &hash_extractor, &fixer, &classifier), - _ => Err(EhError::UnknownCommand(app_name.to_string())) - } -} -``` - -## 2. Replace Magic Buffer Sizes (Low Priority) - -**Context**: Hardcoded `[0u8; 4096]` in command.rs:119-120 should be named -constants for maintainability. - -**Pseudocode**: - -```rust -const DEFAULT_BUFFER_SIZE: usize = 4096; - -// Replace: -let mut out_buf = [0u8; 4096]; -let mut err_buf = [0u8; 4096]; - -// With: -let mut out_buf = [0u8; DEFAULT_BUFFER_SIZE]; -let mut err_buf = [0u8; DEFAULT_BUFFER_SIZE]; -``` - -## 3. Standardize Error Handling (Medium Priority) - -**Context**: Inconsistent error handling patterns - some functions use `?` while -others manually handle errors. - -**Pseudocode**: - -```rust -// Instead of manual error handling: -match some_operation() { - Ok(result) => result, - Err(e) => return Err(EhError::from(e)) -} - -// Use consistent ? operator: -let result = some_operation()?; -``` - -## 4. Replace Manual Directory Traversal (High Priority) - -**Context**: Manual stack-based traversal in util.rs:55-75 is inefficient and -error-prone compared to walkdir crate. - -**Pseudocode**: - -```rust -// Add to Cargo.toml: -walkdir = "2.4" - -// Replace find_nix_files implementation: -use walkdir::WalkDir; - -fn find_nix_files(&self) -> Result> { - let files: Vec = WalkDir::new(".") - .into_iter() - .filter_map(|entry| entry.ok()) - .filter(|entry| { - entry.file_type().is_file() && - entry.path().extension() - .map(|ext| ext.eq_ignore_ascii_case("nix")) - .unwrap_or(false) - }) - .map(|entry| entry.path().to_path_buf()) - .collect(); - - if files.is_empty() { - Err(EhError::NoNixFilesFound) - } else { - Ok(files) - } -} -``` - -## 5. Eliminate Unnecessary String Allocations (Medium Priority) - -**Context**: Multiple string clones in retry logic, especially args cloning in -util.rs:152-200. - -**Pseudocode**: - -```rust -// Instead of cloning args repeatedly: -.handle_nix_with_retry("run", &args, ...) - -// Use references and avoid cloning: -.handle_nix_with_retry("run", args, ...) - -// In retry logic, reuse the same command builder: -let mut cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args(args); // Pass by reference instead of cloning -``` - -## 6. Convert to Async/Await (Low Priority) - -**Context**: Blocking I/O operations could benefit from async for better -concurrency. - -**Pseudocode**: - -```rust -// Add to Cargo.toml: -tokio = { version = "1.0", features = ["full"] } - -// Convert command execution: -pub async fn run_with_logs_async( - &self, - mut interceptor: I, -) -> Result { - let mut cmd = tokio::process::Command::new("nix"); - // ... rest of implementation using async I/O -} -``` - -## 7. Add Comprehensive Unit Tests (High Priority) - -**Context**: Current tests are basic integration tests. Need unit tests for core -logic components. - -**Pseudocode**: - -```rust -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_hash_extraction_patterns() { - let extractor = RegexHashExtractor; - let test_cases = vec![ - ("got: sha256-abc123", Some("sha256-abc123".to_string())), - ("actual: sha256-def456", Some("sha256-def456".to_string())), - ("no hash here", None), - ]; - - for (input, expected) in test_cases { - assert_eq!(extractor.extract_hash(input), expected); - } - } - - #[test] - fn test_nix_file_fixer_replacements() { - // Test each pattern replacement individually - let test_content = r#" - hash = "oldhash" - sha256 = "oldsha256" - outputHash = "oldoutput" - "#; - - let expected = r#" - hash = "newhash" - sha256 = "newhash" - outputHash = "newhash" - "#; - - // Test replacement logic - } -} -``` - -## 8. Create Mock Nix Files for Testing (High Priority) - -**Context**: Tests rely on external network dependencies. Should use mock files -instead. - -**Pseudocode**: - -```rust -#[cfg(test)] -mod mock_tests { - use tempfile::TempDir; - use std::fs::File; - use std::io::Write; - - fn create_mock_nix_file(dir: &TempDir, content: &str) -> PathBuf { - let file_path = dir.path().join("test.nix"); - let mut file = File::create(&file_path).unwrap(); - file.write_all(content.as_bytes()).unwrap(); - file_path - } - - #[test] - fn test_hash_fixing_with_mock_files() { - let temp_dir = TempDir::new().unwrap(); - let mock_content = r#" - stdenv.mkDerivation { - name = "test"; - src = fetchurl { - url = "https://example.com"; - hash = "sha256-oldhash"; - }; - } - "#; - - let file_path = create_mock_nix_file(&temp_dir, mock_content); - - // Test hash fixing logic with mock file - let fixer = DefaultNixFileFixer; - let fixed = fixer.fix_hash_in_file(&file_path, "sha256-newhash").unwrap(); - assert!(fixed); - - // Verify the content was updated correctly - let updated_content = std::fs::read_to_string(&file_path).unwrap(); - assert!(updated_content.contains("sha256-newhash")); - } -} -``` - -## 9. Add File Size Limits (Medium Priority) - -**Context**: File operations read entire files into memory without size limits. - -**Pseudocode**: - -```rust -const MAX_FILE_SIZE: usize = 10 * 1024 * 1024; // 10MB limit - -fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result { - let metadata = fs::metadata(file_path)?; - if metadata.len() as usize > MAX_FILE_SIZE { - return Err(EhError::FileTooLarge { - path: file_path.to_string_lossy().to_string(), - size: metadata.len(), - }); - } - - // Continue with existing logic... -} - -// Add to EhError enum: -#[error("File too large: {path} ({size} bytes)")] -FileTooLarge { path: String, size: u64 }, -``` - -## 10. Implement Input Validation (Medium Priority) - -**Context**: Need better input sanitization to prevent command injection. - -**Pseudocode**: - -```rust -fn validate_nix_args(args: &[String]) -> Result<()> { - const DANGEROUS_PATTERNS: &[&str] = &[ - ";", "&&", "||", "|", "`", "$(", "${", - ">", "<", ">>", "<<", "2>", "2>>" - ]; - - for arg in args { - for pattern in DANGEROUS_PATTERNS { - if arg.contains(pattern) { - return Err(EhError::InvalidInput { - input: arg.clone(), - reason: format!("contains potentially dangerous pattern: {}", pattern), - }); - } - } - } - Ok(()) -} - -// Add to EhError enum: -#[error("Invalid input: {input} - {reason}")] -InvalidInput { input: String, reason: String }, -``` - -## 11. Make Configuration External (Low Priority) - -**Context**: Hardcoded retry logic, hash patterns, and error classifications -should be configurable. - -**Pseudocode**: - -```rust -#[derive(Debug, Clone, Deserialize)] -pub struct EhConfig { - pub hash_patterns: Vec, - pub max_retries: u32, - pub file_size_limit: Option, - pub allowed_packages: Option>, -} - -impl Default for EhConfig { - fn default() -> Self { - Self { - hash_patterns: vec![ - r"got:\s+(sha256-[a-zA-Z0-9+/=]+)".to_string(), - r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)".to_string(), - r"have:\s+(sha256-[a-zA-Z0-9+/=]+)".to_string(), - ], - max_retries: 3, - file_size_limit: Some(10 * 1024 * 1024), - allowed_packages: None, - } - } -} - -// Load from config file or environment: -fn load_config() -> EhConfig { - if let Ok(config_path) = std::env::var("EH_CONFIG_FILE") { - // Load from file - } else { - EhConfig::default() - } -} -``` - -## 12. Implement Dependency Injection (Low Priority) - -**Context**: Tight coupling makes testing difficult. Use dependency injection -for better testability. - -**Pseudocode**: - -```rust -pub struct EhApp { - hash_extractor: H, - file_fixer: F, - error_classifier: C, -} - -impl EhApp -where - H: HashExtractor, - F: NixFileFixer, - C: NixErrorClassifier, -{ - pub fn new(hash_extractor: H, file_fixer: F, error_classifier: C) -> Self { - Self { - hash_extractor, - file_fixer, - error_classifier, - } - } - - pub fn handle_command(&self, subcommand: &str, args: &[String]) -> Result { - handle_nix_with_retry( - subcommand, - args, - &self.hash_extractor, - &self.file_fixer, - &self.error_classifier, - subcommand == "run" || subcommand == "shell", - ) - } -} - -// Usage in production: -let app = EhApp::new( - RegexHashExtractor, - DefaultNixFileFixer, - DefaultNixErrorClassifier, -); - -// Usage in tests: -let mock_extractor = MockHashExtractor::new(); -let mock_fixer = MockNixFileFixer::new(); -let mock_classifier = MockNixErrorClassifier::new(); -let app = EhApp::new(mock_extractor, mock_fixer, mock_classifier); -``` - diff --git a/eh/Cargo.toml b/eh/Cargo.toml index 1e62b9b..db2736d 100644 --- a/eh/Cargo.toml +++ b/eh/Cargo.toml @@ -6,16 +6,9 @@ edition.workspace = true authors.workspace = true rust-version.workspace = true -[lib] -crate-type = [ "lib" ] -name = "eh" - [dependencies] clap.workspace = true regex.workspace = true -tempfile.workspace = true -thiserror.workspace = true tracing.workspace = true tracing-subscriber.workspace = true -walkdir.workspace = true yansi.workspace = true diff --git a/eh/src/build.rs b/eh/src/build.rs index 4ec4540..137b111 100644 --- a/eh/src/build.rs +++ b/eh/src/build.rs @@ -1,18 +1,10 @@ -use crate::{ - error::Result, - util::{ - HashExtractor, - NixErrorClassifier, - NixFileFixer, - handle_nix_with_retry, - }, -}; +use crate::util::{HashExtractor, NixErrorClassifier, NixFileFixer, handle_nix_with_retry}; pub fn handle_nix_build( - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, -) -> Result { - handle_nix_with_retry("build", args, hash_extractor, fixer, classifier, false) + args: &[String], + hash_extractor: &dyn HashExtractor, + fixer: &dyn NixFileFixer, + classifier: &dyn NixErrorClassifier, +) { + handle_nix_with_retry("build", args, hash_extractor, fixer, classifier, false); } diff --git a/eh/src/command.rs b/eh/src/command.rs index 7eace98..f94623e 100644 --- a/eh/src/command.rs +++ b/eh/src/command.rs @@ -1,232 +1,177 @@ -use std::{ - collections::VecDeque, - io::{self, Read, Write}, - process::{Command, ExitStatus, Output, Stdio}, - time::{Duration, Instant}, -}; - -use crate::error::{EhError, Result}; +use std::collections::VecDeque; +use std::io::{self, Read, Write}; +use std::process::{Command, ExitStatus, Output, Stdio}; /// Trait for log interception and output handling. pub trait LogInterceptor: Send { - fn on_stderr(&mut self, chunk: &[u8]); - fn on_stdout(&mut self, chunk: &[u8]); + fn on_stderr(&mut self, chunk: &[u8]); + fn on_stdout(&mut self, chunk: &[u8]); } /// Default log interceptor that just writes to stdio. pub struct StdIoInterceptor; impl LogInterceptor for StdIoInterceptor { - fn on_stderr(&mut self, chunk: &[u8]) { - let _ = io::stderr().write_all(chunk); - } - fn on_stdout(&mut self, chunk: &[u8]) { - let _ = io::stdout().write_all(chunk); - } + fn on_stderr(&mut self, chunk: &[u8]) { + let _ = io::stderr().write_all(chunk); + } + fn on_stdout(&mut self, chunk: &[u8]) { + let _ = io::stdout().write_all(chunk); + } } -/// Default buffer size for reading command output -const DEFAULT_BUFFER_SIZE: usize = 4096; - -/// Default timeout for command execution -const DEFAULT_TIMEOUT: Duration = Duration::from_secs(300); // 5 minutes - /// Builder and executor for Nix commands. pub struct NixCommand { - subcommand: String, - args: Vec, - env: Vec<(String, String)>, - impure: bool, - print_build_logs: bool, - interactive: bool, + subcommand: String, + args: Vec, + env: Vec<(String, String)>, + impure: bool, + print_build_logs: bool, + interactive: bool, } impl NixCommand { - pub fn new>(subcommand: S) -> Self { - Self { - subcommand: subcommand.into(), - args: Vec::new(), - env: Vec::new(), - impure: false, - print_build_logs: true, - interactive: false, + pub fn new>(subcommand: S) -> Self { + Self { + subcommand: subcommand.into(), + args: Vec::new(), + env: Vec::new(), + impure: false, + print_build_logs: true, + interactive: false, + } } - } - pub fn arg>(mut self, arg: S) -> Self { - self.args.push(arg.into()); - self - } + pub fn arg>(mut self, arg: S) -> Self { + self.args.push(arg.into()); + self + } - #[allow(dead_code, reason = "FIXME")] - pub fn args(mut self, args: I) -> Self - where - I: IntoIterator, - S: Into, - { - self.args.extend(args.into_iter().map(Into::into)); - self - } - - #[must_use] - pub fn args_ref(mut self, args: &[String]) -> Self { - self.args.extend(args.iter().cloned()); - self - } - - pub fn env, V: Into>( - mut self, - key: K, - value: V, - ) -> Self { - self.env.push((key.into(), value.into())); - self - } - - #[must_use] - pub const fn impure(mut self, yes: bool) -> Self { - self.impure = yes; - self - } - - #[must_use] - pub const fn interactive(mut self, yes: bool) -> Self { - self.interactive = yes; - self - } - - #[must_use] - pub const fn print_build_logs(mut self, yes: bool) -> Self { - self.print_build_logs = yes; - self - } - - /// Run the command, streaming output to the provided interceptor. - pub fn run_with_logs( - &self, - mut interceptor: I, - ) -> Result { - let mut cmd = Command::new("nix"); - cmd.arg(&self.subcommand); - - if self.print_build_logs - && !self.args.iter().any(|a| a == "--no-build-output") + pub fn args(mut self, args: I) -> Self + where + I: IntoIterator, + S: Into, { - cmd.arg("--print-build-logs"); - } - if self.impure { - cmd.arg("--impure"); - } - for (k, v) in &self.env { - cmd.env(k, v); - } - cmd.args(&self.args); - - if self.interactive { - cmd.stdout(Stdio::inherit()); - cmd.stderr(Stdio::inherit()); - cmd.stdin(Stdio::inherit()); - return Ok(cmd.status()?); + self.args.extend(args.into_iter().map(Into::into)); + self } - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); - - let mut child = cmd.spawn()?; - let child_stdout = child.stdout.take().ok_or_else(|| { - EhError::CommandFailed { - command: format!("nix {}", self.subcommand), - } - })?; - let child_stderr = child.stderr.take().ok_or_else(|| { - EhError::CommandFailed { - command: format!("nix {}", self.subcommand), - } - })?; - let mut stdout = child_stdout; - let mut stderr = child_stderr; - - let mut out_buf = [0u8; DEFAULT_BUFFER_SIZE]; - let mut err_buf = [0u8; DEFAULT_BUFFER_SIZE]; - - let mut out_queue = VecDeque::new(); - let mut err_queue = VecDeque::new(); - let start_time = Instant::now(); - - loop { - let mut did_something = false; - - // Check for timeout - if start_time.elapsed() > DEFAULT_TIMEOUT { - let _ = child.kill(); - return Err(EhError::CommandFailed { - command: format!("nix {} timed out after 5 minutes", self.subcommand), - }); - } - - match stdout.read(&mut out_buf) { - Ok(0) => {}, - Ok(n) => { - interceptor.on_stdout(&out_buf[..n]); - out_queue.push_back(Vec::from(&out_buf[..n])); - did_something = true; - }, - Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}, - Err(e) => return Err(EhError::Io(e)), - } - - match stderr.read(&mut err_buf) { - Ok(0) => {}, - Ok(n) => { - interceptor.on_stderr(&err_buf[..n]); - err_queue.push_back(Vec::from(&err_buf[..n])); - did_something = true; - }, - Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {}, - Err(e) => return Err(EhError::Io(e)), - } - - if !did_something && child.try_wait()?.is_some() { - break; - } - - // Prevent busy waiting when no data is available - if !did_something { - std::thread::sleep(Duration::from_millis(10)); - } + pub fn env, V: Into>(mut self, key: K, value: V) -> Self { + self.env.push((key.into(), value.into())); + self } - let status = child.wait()?; - Ok(status) - } - - /// Run the command and capture all output. - pub fn output(&self) -> Result { - let mut cmd = Command::new("nix"); - cmd.arg(&self.subcommand); - - if self.print_build_logs - && !self.args.iter().any(|a| a == "--no-build-output") - { - cmd.arg("--print-build-logs"); - } - if self.impure { - cmd.arg("--impure"); - } - for (k, v) in &self.env { - cmd.env(k, v); - } - cmd.args(&self.args); - - if self.interactive { - cmd.stdout(Stdio::inherit()); - cmd.stderr(Stdio::inherit()); - cmd.stdin(Stdio::inherit()); - } else { - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); + pub fn impure(mut self, yes: bool) -> Self { + self.impure = yes; + self } - Ok(cmd.output()?) - } + pub fn interactive(mut self, yes: bool) -> Self { + self.interactive = yes; + self + } + + pub fn print_build_logs(mut self, yes: bool) -> Self { + self.print_build_logs = yes; + self + } + + /// Run the command, streaming output to the provided interceptor. + pub fn run_with_logs( + &self, + mut interceptor: I, + ) -> io::Result { + let mut cmd = Command::new("nix"); + cmd.arg(&self.subcommand); + + if self.print_build_logs && !self.args.iter().any(|a| a == "--no-build-output") { + cmd.arg("--print-build-logs"); + } + if self.impure { + cmd.arg("--impure"); + } + for (k, v) in &self.env { + cmd.env(k, v); + } + cmd.args(&self.args); + + if self.interactive { + cmd.stdout(Stdio::inherit()); + cmd.stderr(Stdio::inherit()); + return cmd.status(); + } + + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + + let mut child = cmd.spawn()?; + let mut stdout = child.stdout.take().unwrap(); + let mut stderr = child.stderr.take().unwrap(); + + let mut out_buf = [0u8; 4096]; + let mut err_buf = [0u8; 4096]; + + let mut out_queue = VecDeque::new(); + let mut err_queue = VecDeque::new(); + + loop { + let mut did_something = false; + + match stdout.read(&mut out_buf) { + Ok(0) => {} + Ok(n) => { + interceptor.on_stdout(&out_buf[..n]); + out_queue.push_back(Vec::from(&out_buf[..n])); + did_something = true; + } + Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {} + Err(e) => return Err(e), + } + + match stderr.read(&mut err_buf) { + Ok(0) => {} + Ok(n) => { + interceptor.on_stderr(&err_buf[..n]); + err_queue.push_back(Vec::from(&err_buf[..n])); + did_something = true; + } + Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {} + Err(e) => return Err(e), + } + + if !did_something && child.try_wait()?.is_some() { + break; + } + } + + let status = child.wait()?; + Ok(status) + } + + /// Run the command and capture all output. + pub fn output(&self) -> io::Result { + let mut cmd = Command::new("nix"); + cmd.arg(&self.subcommand); + + if self.print_build_logs && !self.args.iter().any(|a| a == "--no-build-output") { + cmd.arg("--print-build-logs"); + } + if self.impure { + cmd.arg("--impure"); + } + for (k, v) in &self.env { + cmd.env(k, v); + } + cmd.args(&self.args); + + if self.interactive { + cmd.stdout(Stdio::inherit()); + cmd.stderr(Stdio::inherit()); + } else { + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + } + + cmd.output() + } } diff --git a/eh/src/error.rs b/eh/src/error.rs deleted file mode 100644 index 173f047..0000000 --- a/eh/src/error.rs +++ /dev/null @@ -1,54 +0,0 @@ -use thiserror::Error; - -#[derive(Error, Debug)] -pub enum EhError { - #[error("Nix command failed: {0}")] - NixCommandFailed(String), - - #[error("IO error: {0}")] - Io(#[from] std::io::Error), - - #[error("Regex error: {0}")] - Regex(#[from] regex::Error), - - #[error("UTF-8 conversion error: {0}")] - Utf8(#[from] std::string::FromUtf8Error), - - #[error("Hash extraction failed")] - HashExtractionFailed, - - #[error("No Nix files found")] - NoNixFilesFound, - - #[error("Failed to fix hash in file: {path}")] - HashFixFailed { path: String }, - - #[error("Process exited with code: {code}")] - ProcessExit { code: i32 }, - - #[error("Command execution failed: {command}")] - CommandFailed { command: String }, - - #[error("Invalid input: {input} - {reason}")] - InvalidInput { input: String, reason: String }, -} - -pub type Result = std::result::Result; - -impl EhError { - #[must_use] - pub const fn exit_code(&self) -> i32 { - match self { - Self::ProcessExit { code } => *code, - Self::NixCommandFailed(_) => 2, - Self::CommandFailed { .. } => 3, - Self::HashExtractionFailed => 4, - Self::NoNixFilesFound => 5, - Self::HashFixFailed { .. } => 6, - Self::InvalidInput { .. } => 7, - Self::Io(_) => 8, - Self::Regex(_) => 9, - Self::Utf8(_) => 10, - } - } -} diff --git a/eh/src/lib.rs b/eh/src/lib.rs deleted file mode 100644 index 5dd2c7b..0000000 --- a/eh/src/lib.rs +++ /dev/null @@ -1,37 +0,0 @@ -pub mod build; -pub mod command; -pub mod error; -pub mod run; -pub mod shell; -pub mod util; - -pub use clap::{CommandFactory, Parser, Subcommand}; -pub use error::{EhError, Result}; - -#[derive(Parser)] -#[command(name = "eh")] -#[command(about = "Ergonomic Nix helper", long_about = None)] -#[command(version)] -pub struct Cli { - #[command(subcommand)] - pub command: Option, -} - -#[derive(Subcommand)] -pub enum Command { - /// Run a Nix derivation - Run { - #[arg(trailing_var_arg = true)] - args: Vec, - }, - /// Enter a Nix shell - Shell { - #[arg(trailing_var_arg = true)] - args: Vec, - }, - /// Build a Nix derivation - Build { - #[arg(trailing_var_arg = true)] - args: Vec, - }, -} diff --git a/eh/src/main.rs b/eh/src/main.rs index 370049f..c7aad53 100644 --- a/eh/src/main.rs +++ b/eh/src/main.rs @@ -1,116 +1,110 @@ -use std::{env, path::Path}; - -use eh::{Cli, Command, CommandFactory, Parser}; -use error::Result; +use clap::{CommandFactory, Parser, Subcommand}; +use std::env; +use std::path::Path; mod build; mod command; -mod error; mod run; mod shell; mod util; +#[derive(Parser)] +#[command(name = "eh")] +#[command(about = "Ergonomic Nix helper", long_about = None)] +struct Cli { + #[command(subcommand)] + command: Option, +} + +#[derive(Subcommand)] +enum Command { + /// Run a Nix derivation + Run { + #[arg(trailing_var_arg = true)] + args: Vec, + }, + /// Enter a Nix shell + Shell { + #[arg(trailing_var_arg = true)] + args: Vec, + }, + /// Build a Nix derivation + Build { + #[arg(trailing_var_arg = true)] + args: Vec, + }, +} + fn main() { - let format = tracing_subscriber::fmt::format() + let format = tracing_subscriber::fmt::format() .with_level(true) // don't include levels in formatted output .with_target(true) // don't include targets .with_thread_ids(false) // include the thread ID of the current thread .with_thread_names(false) // include the name of the current thread .compact(); // use the `Compact` formatting style. - tracing_subscriber::fmt().event_format(format).init(); + tracing_subscriber::fmt().event_format(format).init(); - let result = run_app(); + let mut args = env::args(); + let bin = args.next().unwrap_or_else(|| "eh".to_string()); + let app_name = Path::new(&bin) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("eh"); - match result { - Ok(code) => std::process::exit(code), - Err(e) => { - eprintln!("Error: {e}"); - std::process::exit(e.exit_code()); - }, - } -} - -// Design partially taken from Stash -fn dispatch_multicall( - app_name: &str, - args: std::env::Args, -) -> Option> { - let rest: Vec = args.collect(); - - // Validate arguments before processing - if let Err(e) = util::validate_nix_args(&rest) { - return Some(Err(e)); - } - - let hash_extractor = util::RegexHashExtractor; - let fixer = util::DefaultNixFileFixer; - let classifier = util::DefaultNixErrorClassifier; - - match app_name { - "nr" => { - Some(run::handle_nix_run( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - "ns" => { - Some(shell::handle_nix_shell( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - "nb" => { - Some(build::handle_nix_build( - &rest, - &hash_extractor, - &fixer, - &classifier, - )) - }, - _ => None, - } -} - -fn run_app() -> Result { - let mut args = env::args(); - let bin = args.next().unwrap_or_else(|| "eh".to_string()); - let app_name = Path::new(&bin) - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or("eh"); - - // If invoked as nr/ns/nb, dispatch directly and exit - if let Some(result) = dispatch_multicall(app_name, args) { - return result; - } - - let cli = Cli::parse(); - - let hash_extractor = util::RegexHashExtractor; - let fixer = util::DefaultNixFileFixer; - let classifier = util::DefaultNixErrorClassifier; - - match cli.command { - Some(Command::Run { args }) => { - run::handle_nix_run(&args, &hash_extractor, &fixer, &classifier) - }, - - Some(Command::Shell { args }) => { - shell::handle_nix_shell(&args, &hash_extractor, &fixer, &classifier) - }, - - Some(Command::Build { args }) => { - build::handle_nix_build(&args, &hash_extractor, &fixer, &classifier) - }, - - _ => { - Cli::command().print_help()?; - println!(); - Ok(0) - }, - } + // If invoked as nr/ns/nb, dispatch directly and exit + match app_name { + "nr" => { + let rest: Vec = args.collect(); + let hash_extractor = util::RegexHashExtractor; + let fixer = util::DefaultNixFileFixer; + let classifier = util::DefaultNixErrorClassifier; + run::handle_nix_run(&rest, &hash_extractor, &fixer, &classifier); + return; + } + + "ns" => { + let rest: Vec = args.collect(); + let hash_extractor = util::RegexHashExtractor; + let fixer = util::DefaultNixFileFixer; + let classifier = util::DefaultNixErrorClassifier; + shell::handle_nix_shell(&rest, &hash_extractor, &fixer, &classifier); + return; + } + + "nb" => { + let rest: Vec = args.collect(); + let hash_extractor = util::RegexHashExtractor; + let fixer = util::DefaultNixFileFixer; + let classifier = util::DefaultNixErrorClassifier; + build::handle_nix_build(&rest, &hash_extractor, &fixer, &classifier); + return; + } + _ => {} + } + + let cli = Cli::parse(); + + let hash_extractor = util::RegexHashExtractor; + let fixer = util::DefaultNixFileFixer; + let classifier = util::DefaultNixErrorClassifier; + + match cli.command { + Some(Command::Run { args }) => { + run::handle_nix_run(&args, &hash_extractor, &fixer, &classifier); + } + + Some(Command::Shell { args }) => { + shell::handle_nix_shell(&args, &hash_extractor, &fixer, &classifier); + } + + Some(Command::Build { args }) => { + build::handle_nix_build(&args, &hash_extractor, &fixer, &classifier); + } + + _ => { + Cli::command().print_help().unwrap(); + println!(); + std::process::exit(0); + } + } } diff --git a/eh/src/run.rs b/eh/src/run.rs index fff81a1..dc1b1e7 100644 --- a/eh/src/run.rs +++ b/eh/src/run.rs @@ -1,18 +1,10 @@ -use crate::{ - error::Result, - util::{ - HashExtractor, - NixErrorClassifier, - NixFileFixer, - handle_nix_with_retry, - }, -}; +use crate::util::{HashExtractor, NixErrorClassifier, NixFileFixer, handle_nix_with_retry}; pub fn handle_nix_run( - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, -) -> Result { - handle_nix_with_retry("run", args, hash_extractor, fixer, classifier, true) + args: &[String], + hash_extractor: &dyn HashExtractor, + fixer: &dyn NixFileFixer, + classifier: &dyn NixErrorClassifier, +) { + handle_nix_with_retry("run", args, hash_extractor, fixer, classifier, false); } diff --git a/eh/src/shell.rs b/eh/src/shell.rs index c0f4409..523049b 100644 --- a/eh/src/shell.rs +++ b/eh/src/shell.rs @@ -1,18 +1,10 @@ -use crate::{ - error::Result, - util::{ - HashExtractor, - NixErrorClassifier, - NixFileFixer, - handle_nix_with_retry, - }, -}; +use crate::util::{HashExtractor, NixErrorClassifier, NixFileFixer, handle_nix_with_retry}; pub fn handle_nix_shell( - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, -) -> Result { - handle_nix_with_retry("shell", args, hash_extractor, fixer, classifier, true) + args: &[String], + hash_extractor: &dyn HashExtractor, + fixer: &dyn NixFileFixer, + classifier: &dyn NixErrorClassifier, +) { + handle_nix_with_retry("shell", args, hash_extractor, fixer, classifier, true); } diff --git a/eh/src/util.rs b/eh/src/util.rs index f452296..fc1f9ca 100644 --- a/eh/src/util.rs +++ b/eh/src/util.rs @@ -1,541 +1,224 @@ -use std::{ - io::{BufWriter, Write}, - path::{Path, PathBuf}, -}; - +use crate::command::{NixCommand, StdIoInterceptor}; use regex::Regex; -use tempfile::NamedTempFile; +use std::fs; +use std::io::Write; +use std::path::{Path, PathBuf}; use tracing::{info, warn}; -use walkdir::WalkDir; use yansi::Paint; -use crate::{ - command::{NixCommand, StdIoInterceptor}, - error::{EhError, Result}, -}; - pub trait HashExtractor { - fn extract_hash(&self, stderr: &str) -> Option; + fn extract_hash(&self, stderr: &str) -> Option; } pub struct RegexHashExtractor; impl HashExtractor for RegexHashExtractor { - fn extract_hash(&self, stderr: &str) -> Option { - let patterns = [ - r"got:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"actual:\s+(sha256-[a-zA-Z0-9+/=]+)", - r"have:\s+(sha256-[a-zA-Z0-9+/=]+)", - ]; - for pattern in &patterns { - if let Ok(re) = Regex::new(pattern) - && let Some(captures) = re.captures(stderr) - && let Some(hash) = captures.get(1) - { - return Some(hash.as_str().to_string()); - } + fn extract_hash(&self, stderr: &str) -> Option { + let patterns = [ + r"got:\s+([a-zA-Z0-9+/=]+)", + r"actual:\s+([a-zA-Z0-9+/=]+)", + r"have:\s+([a-zA-Z0-9+/=]+)", + ]; + for pattern in &patterns { + if let Ok(re) = Regex::new(pattern) { + if let Some(captures) = re.captures(stderr) { + if let Some(hash) = captures.get(1) { + return Some(hash.as_str().to_string()); + } + } + } + } + None } - None - } } pub trait NixFileFixer { - fn fix_hash_in_files(&self, new_hash: &str) -> Result; - fn find_nix_files(&self) -> Result>; - fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result; + fn fix_hash_in_files(&self, new_hash: &str) -> bool; + fn find_nix_files(&self) -> Vec; + fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> bool; } pub struct DefaultNixFileFixer; impl NixFileFixer for DefaultNixFileFixer { - fn fix_hash_in_files(&self, new_hash: &str) -> Result { - let nix_files = self.find_nix_files()?; - let mut fixed = false; - for file_path in nix_files { - if self.fix_hash_in_file(&file_path, new_hash)? { - println!("Updated hash in {}", file_path.display()); - fixed = true; - } - } - Ok(fixed) - } - - fn find_nix_files(&self) -> Result> { - let files: Vec = WalkDir::new(".") - .into_iter() - .filter_map(std::result::Result::ok) - .filter(|entry| { - entry.file_type().is_file() - && entry - .path() - .extension() - .is_some_and(|ext| ext.eq_ignore_ascii_case("nix")) - }) - .map(|entry| entry.path().to_path_buf()) - .collect(); - - if files.is_empty() { - return Err(EhError::NoNixFilesFound); - } - Ok(files) - } - - fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> Result { - // Pre-compile regex patterns once to avoid repeated compilation - let patterns: Vec<(Regex, String)> = [ - (r#"hash\s*=\s*"[^"]*""#, format!(r#"hash = "{new_hash}""#)), - ( - r#"sha256\s*=\s*"[^"]*""#, - format!(r#"sha256 = "{new_hash}""#), - ), - ( - r#"outputHash\s*=\s*"[^"]*""#, - format!(r#"outputHash = "{new_hash}""#), - ), - ] - .into_iter() - .map(|(pattern, replacement)| { - Regex::new(pattern) - .map(|re| (re, replacement)) - .map_err(EhError::Regex) - }) - .collect::>>()?; - - // Read the entire file content - let content = std::fs::read_to_string(file_path)?; - let mut replaced = false; - let mut result_content = content; - - // Apply replacements - for (re, replacement) in &patterns { - if re.is_match(&result_content) { - result_content = re - .replace_all(&result_content, replacement.as_str()) - .into_owned(); - replaced = true; - } - } - - // Write back to file atomically - if replaced { - let temp_file = - NamedTempFile::new_in(file_path.parent().unwrap_or(Path::new(".")))?; - { - let mut writer = BufWriter::new(temp_file.as_file()); - writer.write_all(result_content.as_bytes())?; - writer.flush()?; - } - temp_file.persist(file_path).map_err(|_e| { - EhError::HashFixFailed { - path: file_path.to_string_lossy().to_string(), + fn fix_hash_in_files(&self, new_hash: &str) -> bool { + let nix_files = self.find_nix_files(); + let mut fixed = false; + for file_path in nix_files { + if self.fix_hash_in_file(&file_path, new_hash) { + println!("Updated hash in {}", file_path.display()); + fixed = true; + } } - })?; + fixed } - Ok(replaced) - } + fn find_nix_files(&self) -> Vec { + let mut files = Vec::new(); + let mut stack = vec![PathBuf::from(".")]; + while let Some(dir) = stack.pop() { + if let Ok(entries) = fs::read_dir(&dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + } else if let Some(ext) = path.extension() { + if ext.eq_ignore_ascii_case("nix") { + files.push(path); + } + } + } + } + } + files + } + + fn fix_hash_in_file(&self, file_path: &Path, new_hash: &str) -> bool { + if let Ok(content) = fs::read_to_string(file_path) { + let patterns = [ + (r#"hash\s*=\s*"[^"]*""#, format!(r#"hash = "{new_hash}""#)), + ( + r#"sha256\s*=\s*"[^"]*""#, + format!(r#"sha256 = "{new_hash}""#), + ), + ( + r#"outputHash\s*=\s*"[^"]*""#, + format!(r#"outputHash = "{new_hash}""#), + ), + ]; + let mut new_content = content.clone(); + let mut replaced = false; + for (pattern, replacement) in &patterns { + if let Ok(re) = Regex::new(pattern) { + if re.is_match(&new_content) { + new_content = re + .replace_all(&new_content, replacement.as_str()) + .into_owned(); + replaced = true; + } + } + } + if replaced && fs::write(file_path, new_content).is_ok() { + return true; + } + } + false + } } pub trait NixErrorClassifier { - fn should_retry(&self, stderr: &str) -> bool; -} - -/// Pre-evaluate expression to catch errors early -fn pre_evaluate(_subcommand: &str, args: &[String]) -> Result { - // Find flake references or expressions to evaluate - // Only take the first non-flag argument (the package/expression) - let eval_arg = args.iter().find(|arg| !arg.starts_with('-')); - - let Some(eval_arg) = eval_arg else { - return Ok(true); // No expression to evaluate - }; - - let eval_cmd = NixCommand::new("eval").arg(eval_arg).arg("--raw"); - - let output = eval_cmd.output()?; - - if output.status.success() { - return Ok(true); - } - - let stderr = String::from_utf8_lossy(&output.stderr); - - // If eval fails due to unfree/insecure/broken, don't fail pre-evaluation - // Let the main command handle it with retry logic - if stderr.contains("has an unfree license") - || stderr.contains("refusing to evaluate") - || stderr.contains("has been marked as insecure") - || stderr.contains("has been marked as broken") - { - return Ok(true); - } - - // For other eval failures, fail early - Ok(false) -} - -pub fn validate_nix_args(args: &[String]) -> Result<()> { - const DANGEROUS_PATTERNS: &[&str] = &[ - ";", "&&", "||", "|", "`", "$(", "${", ">", "<", ">>", "<<", "2>", "2>>", - ]; - - for arg in args { - for pattern in DANGEROUS_PATTERNS { - if arg.contains(pattern) { - return Err(EhError::InvalidInput { - input: arg.clone(), - reason: format!("contains potentially dangerous pattern: {pattern}"), - }); - } - } - } - Ok(()) + fn should_retry(&self, stderr: &str) -> bool; } /// Shared retry logic for nix commands (build/run/shell). pub fn handle_nix_with_retry( - subcommand: &str, - args: &[String], - hash_extractor: &dyn HashExtractor, - fixer: &dyn NixFileFixer, - classifier: &dyn NixErrorClassifier, - interactive: bool, -) -> Result { - validate_nix_args(args)?; - // Pre-evaluate for build commands to catch errors early - if !pre_evaluate(subcommand, args)? { - return Err(EhError::NixCommandFailed( - "Expression evaluation failed".to_string(), - )); - } - - // For run commands, try interactive first to avoid breaking terminal - if subcommand == "run" && interactive { - let status = NixCommand::new(subcommand) - .print_build_logs(true) - .interactive(true) - .args_ref(args) - .run_with_logs(StdIoInterceptor)?; + subcommand: &str, + args: &[String], + hash_extractor: &dyn HashExtractor, + fixer: &dyn NixFileFixer, + classifier: &dyn NixErrorClassifier, + interactive: bool, +) -> ! { + let mut cmd = NixCommand::new(subcommand).print_build_logs(true); + if interactive { + cmd = cmd.interactive(true); + } + for arg in args { + cmd = cmd.arg(arg); + } + let status = cmd + .run_with_logs(StdIoInterceptor) + .expect("failed to run nix command"); if status.success() { - return Ok(0); + std::process::exit(0); } - } - // First, always capture output to check for errors that need retry - let output_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args); - let output = output_cmd.output()?; - let stderr = String::from_utf8_lossy(&output.stderr); + let mut output_cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .args(args.iter().cloned()); + if interactive { + output_cmd = output_cmd.interactive(true); + } + let output = output_cmd.output().expect("failed to capture output"); + let stderr = String::from_utf8_lossy(&output.stderr); - // Check if we need to retry with special flags - if let Some(new_hash) = hash_extractor.extract_hash(&stderr) { - match fixer.fix_hash_in_files(&new_hash) { - Ok(true) => { - info!("{}", Paint::green("✔ Fixed hash mismatch, retrying...")); - let mut retry_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args); - if interactive { - retry_cmd = retry_cmd.interactive(true); + if let Some(new_hash) = hash_extractor.extract_hash(&stderr) { + if fixer.fix_hash_in_files(&new_hash) { + info!("{}", Paint::green("✔ Fixed hash mismatch, retrying...")); + let mut retry_cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .args(args.iter().cloned()); + if interactive { + retry_cmd = retry_cmd.interactive(true); + } + let retry_status = retry_cmd.run_with_logs(StdIoInterceptor).unwrap(); + std::process::exit(retry_status.code().unwrap_or(1)); } - let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; - return Ok(retry_status.code().unwrap_or(1)); - }, - Ok(false) => { - // No files were fixed, continue with normal error handling - }, - Err(EhError::NoNixFilesFound) => { - warn!("No .nix files found to fix hash in"); - // Continue with normal error handling - }, - Err(e) => { - return Err(e); - }, } - } else if stderr.contains("hash") || stderr.contains("sha256") { - // If there's a hash-related error but we couldn't extract it, that's a - // failure - return Err(EhError::HashExtractionFailed); - } - if classifier.should_retry(&stderr) { - if stderr.contains("has an unfree license") && stderr.contains("refusing") { - warn!( - "{}", - Paint::yellow( - "⚠ Unfree package detected, retrying with NIXPKGS_ALLOW_UNFREE=1..." - ) - ); - let mut retry_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args) - .env("NIXPKGS_ALLOW_UNFREE", "1") - .impure(true); - if interactive { - retry_cmd = retry_cmd.interactive(true); - } - let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; - return Ok(retry_status.code().unwrap_or(1)); + if classifier.should_retry(&stderr) { + if stderr.contains("unfree") { + warn!( + "{}", + Paint::yellow("⚠ Unfree package detected, retrying with NIXPKGS_ALLOW_UNFREE=1...") + ); + let mut retry_cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .args(args.iter().cloned()) + .env("NIXPKGS_ALLOW_UNFREE", "1") + .impure(true); + if interactive { + retry_cmd = retry_cmd.interactive(true); + } + let retry_status = retry_cmd.run_with_logs(StdIoInterceptor).unwrap(); + std::process::exit(retry_status.code().unwrap_or(1)); + } + if stderr.contains("insecure") { + warn!( + "{}", + Paint::yellow( + "⚠ Insecure package detected, retrying with NIXPKGS_ALLOW_INSECURE=1..." + ) + ); + let mut retry_cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .args(args.iter().cloned()) + .env("NIXPKGS_ALLOW_INSECURE", "1") + .impure(true); + if interactive { + retry_cmd = retry_cmd.interactive(true); + } + let retry_status = retry_cmd.run_with_logs(StdIoInterceptor).unwrap(); + std::process::exit(retry_status.code().unwrap_or(1)); + } + if stderr.contains("broken") { + warn!( + "{}", + Paint::yellow("⚠ Broken package detected, retrying with NIXPKGS_ALLOW_BROKEN=1...") + ); + let mut retry_cmd = NixCommand::new(subcommand) + .print_build_logs(true) + .args(args.iter().cloned()) + .env("NIXPKGS_ALLOW_BROKEN", "1") + .impure(true); + if interactive { + retry_cmd = retry_cmd.interactive(true); + } + let retry_status = retry_cmd.run_with_logs(StdIoInterceptor).unwrap(); + std::process::exit(retry_status.code().unwrap_or(1)); + } } - if stderr.contains("has been marked as insecure") - && stderr.contains("refusing") - { - warn!( - "{}", - Paint::yellow( - "⚠ Insecure package detected, retrying with \ - NIXPKGS_ALLOW_INSECURE=1..." - ) - ); - let mut retry_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args) - .env("NIXPKGS_ALLOW_INSECURE", "1") - .impure(true); - if interactive { - retry_cmd = retry_cmd.interactive(true); - } - let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; - return Ok(retry_status.code().unwrap_or(1)); - } - if stderr.contains("has been marked as broken") - && stderr.contains("refusing") - { - warn!( - "{}", - Paint::yellow( - "⚠ Broken package detected, retrying with NIXPKGS_ALLOW_BROKEN=1..." - ) - ); - let mut retry_cmd = NixCommand::new(subcommand) - .print_build_logs(true) - .args_ref(args) - .env("NIXPKGS_ALLOW_BROKEN", "1") - .impure(true); - if interactive { - retry_cmd = retry_cmd.interactive(true); - } - let retry_status = retry_cmd.run_with_logs(StdIoInterceptor)?; - return Ok(retry_status.code().unwrap_or(1)); - } - } - // If the first attempt succeeded, we're done - if output.status.success() { - return Ok(0); - } - - // Otherwise, show the error and return error - std::io::stderr() - .write_all(&output.stderr) - .map_err(EhError::Io)?; - Err(EhError::ProcessExit { - code: output.status.code().unwrap_or(1), - }) + std::io::stderr().write_all(output.stderr.as_ref()).unwrap(); + std::process::exit(status.code().unwrap_or(1)); } - pub struct DefaultNixErrorClassifier; impl NixErrorClassifier for DefaultNixErrorClassifier { - fn should_retry(&self, stderr: &str) -> bool { - RegexHashExtractor.extract_hash(stderr).is_some() - || (stderr.contains("has an unfree license") - && stderr.contains("refusing")) - || (stderr.contains("has been marked as insecure") - && stderr.contains("refusing")) - || (stderr.contains("has been marked as broken") - && stderr.contains("refusing")) - } -} - -#[cfg(test)] -mod tests { - use std::io::Write; - - use tempfile::NamedTempFile; - - use super::*; - - #[test] - fn test_streaming_hash_replacement() { - let temp_file = NamedTempFile::new().unwrap(); - let file_path = temp_file.path(); - - // Write test content with multiple hash patterns - let test_content = r#"stdenv.mkDerivation { - name = "test-package"; - src = fetchurl { - url = "https://example.com.tar.gz"; - hash = "sha256-oldhash123"; - sha256 = "sha256-oldhash456"; - outputHash = "sha256-oldhash789"; - }; -}"#; - - let mut file = std::fs::File::create(file_path).unwrap(); - file.write_all(test_content.as_bytes()).unwrap(); - file.flush().unwrap(); - - let fixer = DefaultNixFileFixer; - let result = fixer - .fix_hash_in_file(file_path, "sha256-newhash999") - .unwrap(); - - assert!(result, "Hash replacement should return true"); - - // Verify the content was updated - let updated_content = std::fs::read_to_string(file_path).unwrap(); - assert!(updated_content.contains("sha256-newhash999")); - assert!(!updated_content.contains("sha256-oldhash123")); - assert!(!updated_content.contains("sha256-oldhash456")); - assert!(!updated_content.contains("sha256-oldhash789")); - } - - #[test] - fn test_streaming_no_replacement_needed() { - let temp_file = NamedTempFile::new().unwrap(); - let file_path = temp_file.path().to_path_buf(); - - let test_content = r#"stdenv.mkDerivation { - name = "test-package"; - src = fetchurl { - url = "https://example.com.tar.gz"; - }; -}"#; - - { - let mut file = std::fs::File::create(&file_path).unwrap(); - file.write_all(test_content.as_bytes()).unwrap(); - file.flush().unwrap(); - } // File is closed here - - // Test hash replacement - let fixer = DefaultNixFileFixer; - let result = fixer - .fix_hash_in_file(&file_path, "sha256-newhash999") - .unwrap(); - - assert!( - !result, - "Hash replacement should return false when no patterns found" - ); - - // Verify the content was unchanged, ignoring trailing newline differences - let updated_content = std::fs::read_to_string(&file_path).unwrap(); - let normalized_original = test_content.trim_end(); - let normalized_updated = updated_content.trim_end(); - assert_eq!(normalized_updated, normalized_original); - } - - // FIXME: this is a little stupid, but it works - #[test] - fn test_streaming_large_file_handling() { - let temp_file = NamedTempFile::new().unwrap(); - let file_path = temp_file.path(); - let mut file = std::fs::File::create(file_path).unwrap(); - - // Write header with hash - file.write_all(b"stdenv.mkDerivation {\n name = \"large-package\";\n src = fetchurl {\n url = \"https://example.com/large.tar.gz\";\n hash = \"sha256-oldhash\";\n };\n").unwrap(); - - for i in 0..10000 { - writeln!(file, " # Large comment line {} to simulate file size", i) - .unwrap(); + fn should_retry(&self, stderr: &str) -> bool { + RegexHashExtractor.extract_hash(stderr).is_some() + || (stderr.contains("unfree") && stderr.contains("refusing")) + || (stderr.contains("insecure") && stderr.contains("refusing")) + || (stderr.contains("broken") && stderr.contains("refusing")) } - - file.flush().unwrap(); - - // Test that streaming can handle large files without memory issues - let fixer = DefaultNixFileFixer; - let result = fixer - .fix_hash_in_file(file_path, "sha256-newhash999") - .unwrap(); - - assert!(result, "Hash replacement should work for large files"); - - // Verify the hash was replaced - let updated_content = std::fs::read_to_string(file_path).unwrap(); - assert!(updated_content.contains("sha256-newhash999")); - assert!(!updated_content.contains("sha256-oldhash")); - } - - #[test] - fn test_streaming_file_permissions_preserved() { - let temp_file = NamedTempFile::new().unwrap(); - let file_path = temp_file.path(); - - // Write test content - let test_content = r#"stdenv.mkDerivation { - name = "test"; - src = fetchurl { - url = "https://example.com"; - hash = "sha256-oldhash"; - }; -}"#; - - let mut file = std::fs::File::create(file_path).unwrap(); - file.write_all(test_content.as_bytes()).unwrap(); - file.flush().unwrap(); - - // Get original permissions - let original_metadata = std::fs::metadata(file_path).unwrap(); - let _original_permissions = original_metadata.permissions(); - - // Test hash replacement - let fixer = DefaultNixFileFixer; - let result = fixer.fix_hash_in_file(file_path, "sha256-newhash").unwrap(); - - assert!(result, "Hash replacement should succeed"); - - // Verify file still exists and has reasonable permissions - let new_metadata = std::fs::metadata(file_path).unwrap(); - assert!( - new_metadata.is_file(), - "File should still exist after replacement" - ); - } - - #[test] - fn test_input_validation_blocks_dangerous_patterns() { - let dangerous_args = vec![ - "package; rm -rf /".to_string(), - "package && echo hacked".to_string(), - "package || echo hacked".to_string(), - "package | cat /etc/passwd".to_string(), - "package `whoami`".to_string(), - "package $(echo hacked lol!)".to_string(), - "package ${HOME}/file".to_string(), - ]; - - for arg in dangerous_args { - let result = validate_nix_args(std::slice::from_ref(&arg)); - assert!(result.is_err(), "Should reject dangerous argument: {}", arg); - - match result.unwrap_err() { - EhError::InvalidInput { input, reason } => { - assert_eq!(input, arg); - assert!(reason.contains("dangerous pattern")); - }, - _ => panic!("Expected InvalidInput error"), - } - } - } - - #[test] - fn test_input_validation_allows_safe_args() { - let safe_args = vec![ - "nixpkgs#hello".to_string(), - "--impure".to_string(), - "--print-build-logs".to_string(), - "/path/to/flake".to_string(), - ".#default".to_string(), - ]; - - let result = validate_nix_args(&safe_args); - assert!( - result.is_ok(), - "Should allow safe arguments: {:?}", - safe_args - ); - } } diff --git a/flake.lock b/flake.lock index 6012087..8530de9 100644 --- a/flake.lock +++ b/flake.lock @@ -2,11 +2,11 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1762844143, - "narHash": "sha256-SlybxLZ1/e4T2lb1czEtWVzDCVSTvk9WLwGhmxFmBxI=", + "lastModified": 1752950548, + "narHash": "sha256-NS6BLD0lxOrnCiEOcvQCDVPXafX1/ek1dfJHX1nUIzc=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "9da7f1cf7f8a6e2a7cb3001b048546c92a8258b4", + "rev": "c87b95e25065c028d31a94f06a62927d18763fdf", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index c747b2b..18bd6fa 100644 --- a/flake.nix +++ b/flake.nix @@ -1,5 +1,6 @@ { - inputs.nixpkgs.url = "github:NixOS/nixpkgs?ref=nixos-unstable"; + description = "Rust Project Template"; + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; outputs = { self, @@ -7,11 +8,11 @@ }: let systems = ["x86_64-linux" "aarch64-linux"]; forEachSystem = nixpkgs.lib.genAttrs systems; + pkgsForEach = nixpkgs.legacyPackages; in { packages = forEachSystem (system: { - eh = pkgsForEach.${system}.callPackage ./nix/package.nix {}; - default = self.packages.${system}.eh; + default = pkgsForEach.${system}.callPackage ./nix/package.nix {}; }); devShells = forEachSystem (system: { diff --git a/nix/package.nix b/nix/package.nix index 0a6afd6..7a91195 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -1,13 +1,10 @@ { lib, rustPlatform, - stdenv, - installShellFiles, - versionCheckHook, }: rustPlatform.buildRustPackage (finalAttrs: { pname = "eh"; - version = (lib.importTOML ../Cargo.toml).workspace.package.version; + version = (builtins.fromTOML (builtins.readFile ../Cargo.toml)).workspace.package.version; src = let fs = lib.fileset; @@ -24,47 +21,11 @@ rustPlatform.buildRustPackage (finalAttrs: { ]); }; - # xtask doesn't support passing --targe - # but nix hooks expect the folder structure from when it's set cargoLock.lockFile = "${finalAttrs.src}/Cargo.lock"; enableParallelBuilding = true; - # xtask doesn't support passing --target - # but nix hooks expect the folder structure from when it's set - env.CARGO_BUILD_TARGET = stdenv.hostPlatform.rust.cargoShortTarget; - - nativeInstallCheckInputs = [versionCheckHook]; - versionCheckProgram = "${placeholder "out"}/bin/${finalAttrs.meta.mainProgram}"; - versionCheckProgramArg = "--version"; - doInstallCheck = true; - - strictDeps = true; - - nativeBuildInputs = [installShellFiles]; - - postInstall = '' - # Install required files with the 'dist' task - $out/bin/xtask multicall \ - --bin-dir $out/bin \ - --main-binary $out/bin/eh - - # Generate shell completions and install them. - for shell in bash zsh fish; do - $out/bin/xtask completions $shell - done - - installShellCompletion completions/* - - # The xtask output has been built as a part of the build phase. If - # we don't remove it, it'll be linked in $out/bin alongside the actual - # binary and populate $PATH with a dedicated 'xtask' command. Remove. - rm -rf $out/bin/xtask - ''; - meta = { description = "Ergonomic Nix CLI helper"; - maintainers = with lib.maintainers; [NotAShelf]; - license = lib.licenses.mpl20; - mainProgram = "eh"; + maintainers = with lib.licenses; [NotAShelf]; }; }) diff --git a/nix/shell.nix b/nix/shell.nix index 4584b59..965a0eb 100644 --- a/nix/shell.nix +++ b/nix/shell.nix @@ -1,26 +1,19 @@ { mkShell, - rustc, - cargo, + rust-analyzer, rustfmt, clippy, - taplo, - rust-analyzer-unwrapped, + cargo, rustPlatform, }: mkShell { name = "rust"; - packages = [ - rustc - cargo - - (rustfmt.override {asNightly = true;}) + rust-analyzer + rustfmt clippy cargo - taplo - rust-analyzer-unwrapped ]; - env.RUST_SRC_PATH = "${rustPlatform.rustLibSrc}"; + RUST_SRC_PATH = "${rustPlatform.rustLibSrc}"; } diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index ece4292..a8d8f20 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -9,6 +9,4 @@ publish = false [dependencies] -clap.workspace = true -clap_complete.workspace = true -eh = { path = "../eh" } +clap.workspace = true diff --git a/xtask/src/main.rs b/xtask/src/main.rs index ad449c4..6a2d7b7 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -1,179 +1,119 @@ use std::{ - error, - fs, - path::{Path, PathBuf}, - process, + error, fs, + path::{Path, PathBuf}, + process, }; -use clap::{CommandFactory, Parser}; -use clap_complete::{Shell, generate}; +use clap::Parser; #[derive(clap::Parser)] struct Cli { - #[clap(subcommand)] - command: Command, + #[clap(subcommand)] + command: Command, } #[derive(clap::Subcommand)] enum Command { - /// Create multicall binaries (hardlinks or copies). - Multicall { - /// Directory to install multicall binaries. - #[arg(long, default_value = "bin")] - bin_dir: PathBuf, + /// Create multicall binaries (hardlinks or copies). + Multicall { + /// Directory to install multicall binaries. + #[arg(long, default_value = "bin")] + bin_dir: PathBuf, - /// Path to the main binary. - #[arg(long, default_value = "target/release/eh")] - main_binary: PathBuf, - }, - /// Generate shell completion scripts - Completions { - /// Shell to generate completions for - #[arg(value_enum)] - shell: Shell, - /// Directory to output completion files - #[arg(long, default_value = "completions")] - output_dir: PathBuf, - }, + /// Path to the main binary. + #[arg(long, default_value = "target/release/eh")] + main_binary: PathBuf, + }, } #[derive(Debug, Clone, Copy)] enum Binary { - Nr, - Ns, - Nb, + Nr, + Ns, + Nb, } impl Binary { - const fn name(self) -> &'static str { - match self { - Self::Nr => "nr", - Self::Ns => "ns", - Self::Nb => "nb", + fn name(self) -> &'static str { + match self { + Self::Nr => "nr", + Self::Ns => "ns", + Self::Nb => "nb", + } } - } } fn main() { - let cli = Cli::parse(); + let cli = Cli::parse(); - match cli.command { - Command::Multicall { - bin_dir, - main_binary, - } => { - if let Err(error) = create_multicall_binaries(&bin_dir, &main_binary) { - eprintln!("error creating multicall binaries: {error}"); - process::exit(1); - } - }, - Command::Completions { shell, output_dir } => { - if let Err(error) = generate_completions(shell, &output_dir) { - eprintln!("error generating completions: {error}"); - process::exit(1); - } - }, - } + match cli.command { + Command::Multicall { + bin_dir, + main_binary, + } => { + if let Err(error) = create_multicall_binaries(&bin_dir, &main_binary) { + eprintln!("error creating multicall binaries: {error}"); + process::exit(1); + } + } + } } fn create_multicall_binaries( - bin_dir: &Path, - main_binary: &Path, + bin_dir: &Path, + main_binary: &Path, ) -> Result<(), Box> { - println!("creating multicall binaries..."); + println!("creating multicall binaries..."); - fs::create_dir_all(bin_dir)?; + fs::create_dir_all(bin_dir)?; - if !main_binary.exists() { - return Err( - format!("main binary not found at: {}", main_binary.display()).into(), - ); - } - - let multicall_binaries = [Binary::Nr, Binary::Ns, Binary::Nb]; - let bin_path = Path::new(bin_dir); - - for binary in multicall_binaries { - let target_path = bin_path.join(binary.name()); - - if target_path.exists() { - fs::remove_file(&target_path)?; + if !main_binary.exists() { + return Err(format!("main binary not found at: {}", main_binary.display()).into()); } - if let Err(e) = fs::hard_link(main_binary, &target_path) { - eprintln!( - " warning: could not create hardlink for {}: {e}", - binary.name(), - ); - eprintln!(" warning: falling back to copying binary..."); + let multicall_binaries = [Binary::Nr, Binary::Ns, Binary::Nb]; + let bin_path = Path::new(bin_dir); - fs::copy(main_binary, &target_path)?; + for binary in multicall_binaries { + let target_path = bin_path.join(binary.name()); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&target_path)?.permissions(); - perms.set_mode(perms.mode() | 0o755); - fs::set_permissions(&target_path, perms)?; - } + if target_path.exists() { + fs::remove_file(&target_path)?; + } - println!(" created copy: {}", target_path.display()); - } else { - println!( - " created hardlink: {} points to {}", - target_path.display(), - main_binary.display(), - ); + match fs::hard_link(main_binary, &target_path) { + Ok(()) => { + println!( + " created hardlink: {} points to {}", + target_path.display(), + main_binary.display(), + ); + } + Err(e) => { + eprintln!( + " warning: could not create hardlink for {}: {e}", + binary.name(), + ); + eprintln!(" warning: falling back to copying binary..."); + + fs::copy(main_binary, &target_path)?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = fs::metadata(&target_path)?.permissions(); + perms.set_mode(perms.mode() | 0o755); + fs::set_permissions(&target_path, perms)?; + } + + println!(" created copy: {}", target_path.display()); + } + } } - } - println!("multicall binaries created successfully!"); - println!("multicall binaries are in: {}", bin_dir.display()); - println!(); + println!("multicall binaries created successfully!"); + println!("multicall binaries are in: {}", bin_dir.display()); + println!(); - Ok(()) -} - -fn generate_completions( - shell: Shell, - output_dir: &Path, -) -> Result<(), Box> { - println!("generating {shell} completions..."); - - fs::create_dir_all(output_dir)?; - - let mut cmd = eh::Cli::command(); - let bin_name = "eh"; - - let completion_file = output_dir.join(format!("{bin_name}.{shell}")); - let mut file = fs::File::create(&completion_file)?; - - generate(shell, &mut cmd, bin_name, &mut file); - - println!("completion file generated: {}", completion_file.display()); - - // Create symlinks for multicall binaries - let multicall_names = ["nb", "nr", "ns"]; - for name in &multicall_names { - let symlink_path = output_dir.join(format!("{name}.{shell}")); - if symlink_path.exists() { - fs::remove_file(&symlink_path)?; - } - - #[cfg(unix)] - { - let canonical_target = fs::canonicalize(&completion_file)?; - std::os::unix::fs::symlink(&canonical_target, &symlink_path)?; - println!("completion symlink created: {}", symlink_path.display()); - } - - #[cfg(not(unix))] - { - fs::copy(&completion_file, &symlink_path)?; - println!("completion copy created: {}", symlink_path.display()); - } - } - - println!("completions generated successfully!"); - Ok(()) + Ok(()) }