diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9dd268b..bf9ed11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,10 @@ on: permissions: contents: read +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + concurrency: group: ci-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true @@ -20,9 +24,11 @@ jobs: timeout-minutes: 10 steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Install Rust - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@1.85 - name: Cache cargo uses: actions/cache@v4 @@ -42,9 +48,11 @@ jobs: timeout-minutes: 10 steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Install Rust - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@1.85 with: components: clippy @@ -66,6 +74,8 @@ jobs: timeout-minutes: 5 steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: actions/setup-python@v5 with: @@ -74,6 +84,9 @@ jobs: - name: Validate catalog script syntax run: python -m compileall -q scripts + - name: Test catalog validation helpers + run: python -m unittest scripts/test_add_models.py + # data/models.json is generated from scripts/add_models.py; a hand edit # to the JSON would be lost on the script's next run. - name: Check the catalog matches its generator @@ -87,7 +100,7 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@1.85 with: components: rustfmt diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 842712f..94b7fc1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,6 +9,7 @@ jobs: build: name: Build ${{ matrix.target }} runs-on: ${{ matrix.os }} + timeout-minutes: 20 permissions: contents: write strategy: @@ -33,9 +34,11 @@ jobs: steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Install Rust - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@1.85 with: targets: ${{ matrix.target }} @@ -76,13 +79,16 @@ jobs: name: Publish to crates.io runs-on: ubuntu-latest needs: build + timeout-minutes: 10 permissions: contents: read steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Install Rust - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@1.85 - name: Publish run: cargo publish --locked --token ${{ secrets.CARGO_TOKEN }} diff --git a/README.md b/README.md index daa0a17..bf2e1f0 100644 --- a/README.md +++ b/README.md @@ -59,15 +59,22 @@ there is nothing else to install and nothing to fetch at runtime. Pull requests run formatting, locked dependency tests, Clippy with warnings denied, and a generated-catalog consistency check. +The Rust validation jobs use the repository’s declared Rust 1.85 toolchain and +enable backtraces for actionable CI failures. The catalog check also validates the generator’s Python syntax and rejects records with empty ids, non-positive parameter counts, or invalid contexts. +Small regression tests exercise those invariants directly, including rejection +of non-finite parameter counts. CI cancels superseded runs for the same ref and applies explicit timeouts to each validation job, preventing stale or wedged checks from consuming runners. +Checkout steps do not persist repository credentials into the runner workspace. Release builds and crates.io publishing also use the committed lockfile, so published artifacts are built from the reviewed dependency graph. +Release jobs use the same Rust toolchain as CI and have explicit timeouts for +cross-platform builds and publishing. --- @@ -82,6 +89,8 @@ llmspec bench # measure real tokens/sec Numeric hardware overrides are validated before probing the machine. Values such as `--cpu-cores 0` or `--gpu-count 0` fail with an explanatory error. +Size overrides also reject non-finite values instead of allowing them into +memory calculations. Four questions llmspec exists to answer: @@ -332,7 +341,8 @@ The CLI applies the same fail-closed rule to `--max-context` and the with an explanatory error instead of being silently ignored. The HTTP server also bounds request parsing: it accepts at most 64 query -parameters and rejects an individual query component larger than 4 KiB. +parameters, rejects duplicate keys, and rejects an individual query component +larger than 4 KiB. Built on `std::net` — serving adds no dependency. @@ -380,6 +390,11 @@ Numeric MCP arguments must also be finite; string values such as `"NaN"` and Runtime-reported parameter sizes follow the same rule: malformed, negative, zero and non-finite sizes are ignored rather than used to select a model. +Provider benchmark samples also require a finite, positive elapsed duration +before throughput is calculated. +Hand-edited persisted speed factors and cached bandwidth values are sanitized +on load; invalid values fall back to safe defaults rather than influencing +model ranking. --- @@ -391,6 +406,8 @@ without touching the weights. Verification caps untrusted header and metadata lengths at 64 MiB and rejects claims that cannot fit in the remaining file before allocating or iterating. +Duplicate GGUF metadata keys are treated as structural corruption rather than +silently allowing the last value to win. ```sh llmspec verify ~/.ollama/models/blobs/sha256-2bada8a745... diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 898a35b..85f2eef 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -411,3 +411,29 @@ and a model that does not shard cleanly will do worse than predicted. **The catalog is a snapshot.** It ships embedded in the binary. Models newer than the build are added through `models.json` in the config directory. +### HTTP input boundary + +The read-only API treats query parameters as a bounded, unique map. Oversized, +duplicated, or malformed input is rejected before it reaches fit analysis so +request parsing cannot change the meaning of an existing filter. +The request line must also terminate within the fixed header budget; a client +that leaves a partial line open is rejected instead of waiting indefinitely. + +Model verification applies the same rule to GGUF metadata: duplicate keys are +an error because accepting the last value would make the result depend on +record ordering. + +Runtime adapters validate timing and size provenance before attaching a +measurement to a catalog model, so invalid telemetry cannot affect ranking. + +Configuration is treated as untrusted input too: persisted factors and cache +values are checked during migration before the analysis session consumes them. + +CI uses the same Rust version declared by the package, keeping local and +reviewed builds on one compiler contract. + +Release automation inherits that contract and limits build/publish duration so +an unavailable target cannot hold the release indefinitely. + +Automation checkouts keep credentials ephemeral; validation jobs do not leave +tokens available to later build steps. diff --git a/scripts/add_models.py b/scripts/add_models.py index c84c86a..e478433 100644 --- a/scripts/add_models.py +++ b/scripts/add_models.py @@ -23,6 +23,7 @@ import argparse import json +import math import sys from pathlib import Path @@ -994,7 +995,11 @@ def validate_records(records): for entry in records: if not entry["id"].strip(): raise SystemExit("model records must have a non-empty id") - if not isinstance(entry["params_b"], (int, float)) or entry["params_b"] <= 0: + if ( + not isinstance(entry["params_b"], (int, float)) + or not math.isfinite(entry["params_b"]) + or entry["params_b"] <= 0 + ): raise SystemExit(f"invalid parameter count for {entry['id']}") if not isinstance(entry["context_length"], int) or entry["context_length"] <= 0: raise SystemExit(f"invalid context length for {entry['id']}") diff --git a/scripts/test_add_models.py b/scripts/test_add_models.py new file mode 100644 index 0000000..5b83aca --- /dev/null +++ b/scripts/test_add_models.py @@ -0,0 +1,27 @@ +import importlib.util +import math +import unittest +from pathlib import Path + + +MODULE_PATH = Path(__file__).with_name("add_models.py") +SPEC = importlib.util.spec_from_file_location("add_models", MODULE_PATH) +MODULE = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(MODULE) + + +class CatalogValidationTests(unittest.TestCase): + def test_valid_record_is_accepted(self): + MODULE.validate_records( + [MODULE.m("test/model", "Test", "Test", 1.0, 4096, "general")] + ) + + def test_non_finite_parameter_count_is_rejected(self): + record = MODULE.m("test/model", "Test", "Test", 1.0, 4096, "general") + record["params_b"] = math.inf + with self.assertRaises(SystemExit): + MODULE.validate_records([record]) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/config.rs b/src/config.rs index 2de9e5c..2fb0003 100644 --- a/src/config.rs +++ b/src/config.rs @@ -289,6 +289,35 @@ impl Config { self.ram_bandwidth_gb_s = None; self.ram_probe = RAM_PROBE_VERSION; } + let defaults = PersistedSpeed::default(); + for (value, fallback) in [ + (&mut self.speed.efficiency, defaults.efficiency), + (&mut self.speed.cpu_efficiency, defaults.cpu_efficiency), + (&mut self.speed.gpu_factor, defaults.gpu_factor), + ( + &mut self.speed.cpu_offload_factor, + defaults.cpu_offload_factor, + ), + ( + &mut self.speed.moe_offload_factor, + defaults.moe_offload_factor, + ), + ( + &mut self.speed.tensor_parallel_factor, + defaults.tensor_parallel_factor, + ), + (&mut self.speed.cpu_only_factor, defaults.cpu_only_factor), + ] { + if !value.is_finite() || *value <= 0.0 { + *value = fallback; + } + } + if self + .ram_bandwidth_gb_s + .is_some_and(|value| !value.is_finite() || value <= 0.0) + { + self.ram_bandwidth_gb_s = None; + } self } @@ -544,6 +573,25 @@ mod tests { assert!((loaded.speed.efficiency - 0.66).abs() < 1e-9); } + #[test] + fn invalid_persisted_factors_fall_back_to_safe_defaults() { + let config: Config = serde_json::from_str( + r#"{"speed_model":2,"ram_probe":2,"speed":{"efficiency":-1.0,"gpu_factor":0.0,"cpu_only_factor":2.0},"ram_bandwidth_gb_s":-4.0}"#, + ) + .unwrap(); + let loaded = config.migrated(); + assert_eq!( + loaded.speed.efficiency, + PersistedSpeed::default().efficiency + ); + assert_eq!( + loaded.speed.gpu_factor, + PersistedSpeed::default().gpu_factor + ); + assert_eq!(loaded.speed.cpu_only_factor, 2.0); + assert_eq!(loaded.ram_bandwidth_gb_s, None); + } + #[test] fn a_bandwidth_from_an_older_probe_is_measured_again() { // The single-threaded probe read about 15% below the threaded one on diff --git a/src/hardware.rs b/src/hardware.rs index 79fd3a3..2f25a78 100644 --- a/src/hardware.rs +++ b/src/hardware.rs @@ -1135,6 +1135,9 @@ pub fn parse_size_gb(input: &str) -> Result { "k" | "kb" | "kib" => value / (1024.0 * 1024.0), other => return Err(format!("unknown unit '{other}': use G, M or T")), }; + if !gb.is_finite() { + return Err(format!("invalid size '{raw}': value is not finite")); + } Ok(gb) } @@ -1150,8 +1153,12 @@ mod tests { assert_eq!(parse_size_gb("512M").unwrap(), 0.5); assert_eq!(parse_size_gb("1T").unwrap(), 1024.0); assert_eq!(parse_size_gb("24").unwrap(), 24.0); + // Zero is intentional: it requests CPU-only simulation. + assert_eq!(parse_size_gb("0").unwrap(), 0.0); assert!(parse_size_gb("abc").is_err()); assert!(parse_size_gb("12X").is_err()); + assert!(parse_size_gb("NaNG").is_err()); + assert!(parse_size_gb("infG").is_err()); } #[test] diff --git a/src/providers.rs b/src/providers.rs index b54cca1..1b46702 100644 --- a/src/providers.rs +++ b/src/providers.rs @@ -760,6 +760,9 @@ fn parse_ollama_sample(text: &str) -> Result { } fn parse_openai_sample(text: &str, elapsed: f64) -> Result { + if !elapsed.is_finite() || elapsed <= 0.0 { + return Err("response elapsed time must be finite and positive".to_string()); + } let parsed: ChatResponse = serde_json::from_str(text).map_err(|e| format!("parsing chat completion: {e}"))?; if let Some(err) = parsed.error { @@ -1017,6 +1020,14 @@ mod tests { assert!(parse_openai_sample(r#"{"choices":[]}"#, 1.0).is_err()); } + #[test] + fn openai_samples_require_positive_elapsed_time() { + let json = r#"{"usage":{"prompt_tokens":1,"completion_tokens":1}}"#; + assert!(parse_openai_sample(json, 0.0).is_err()); + assert!(parse_openai_sample(json, -1.0).is_err()); + assert!(parse_openai_sample(json, f64::NAN).is_err()); + } + #[test] fn normalization_unifies_the_runtimes_spellings_of_one_model() { // Upstream repo id, LM Studio's listing, and a llama.cpp GGUF file diff --git a/src/serve.rs b/src/serve.rs index eea2535..bf66034 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -371,6 +371,7 @@ fn read_request(stream: &TcpStream) -> Result { if consumed == 0 { return Err("empty request".to_string()); } + ensure_request_line_complete(consumed, &line)?; let request = parse_request_line(&line)?; @@ -392,6 +393,14 @@ fn read_request(stream: &TcpStream) -> Result { Ok(request) } +fn ensure_request_line_complete(consumed: usize, line: &str) -> Result<(), String> { + if consumed == MAX_HEADER_BYTES && !line.ends_with('\n') { + Err("request headers exceed the 8 KiB limit".to_string()) + } else { + Ok(()) + } +} + fn parse_request_line(line: &str) -> Result { let mut parts = line.split_whitespace(); let method = parts @@ -434,7 +443,11 @@ fn parse_request_line(line: &str) -> Result { Some((k, v)) => (k, v), None => (pair, ""), }; - query.insert(percent_decode(key), percent_decode(value)); + let key = percent_decode(key); + let value = percent_decode(value); + if query.insert(key.clone(), value).is_some() { + return Err(format!("duplicate query parameter '{key}'")); + } } } @@ -580,6 +593,18 @@ mod tests { assert!(parse_request_line(&format!("GET /?q={huge} HTTP/1.1")).is_err()); } + #[test] + fn duplicate_query_parameters_are_rejected() { + assert!(parse_request_line("GET /models?limit=1&limit=2 HTTP/1.1").is_err()); + } + + #[test] + fn unterminated_header_at_the_limit_is_rejected() { + assert!(ensure_request_line_complete(MAX_HEADER_BYTES, "partial").is_err()); + assert!(ensure_request_line_complete(MAX_HEADER_BYTES, "complete\n").is_ok()); + assert!(ensure_request_line_complete(MAX_HEADER_BYTES - 1, "partial").is_ok()); + } + #[test] fn method_is_normalised_and_missing_target_rejected() { assert_eq!(parse("get /health HTTP/1.1\r\n").method, "GET"); diff --git a/src/verify.rs b/src/verify.rs index 5c7804d..bba634f 100644 --- a/src/verify.rs +++ b/src/verify.rs @@ -369,7 +369,9 @@ fn read_metadata(reader: &mut Cursor, count: u64) -> Result