Close public API gaps: nameable field types, reachable no_libc, honest rule-load errors

CHANGELOG.md

@@ -43,6 +43,33 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   hottest operation of the rule engine. All sites now do one
   `HashMap::get`, matching the pattern `RangeStatement` already used.
 
+### Fixed — public API gaps (closes [#22](https://github.com/marirs/capa-rs/issues/22))
+
+- **Types in public fields are now nameable downstream** `pub use`
+  from the crate root for `FileFormat`, `Os`, `FileArchitecture`
+  (`Properties::format` / `os` / `arch`) and `SecurityCheckStatus`
+  (`FileCapabilities::security_checks`) — previously their modules
+  were private, so the fields were Debug-print-only.
+- **`FunctionCapabilities` gained getters** `address()`, `features()`,
+  `capabilities()` — fields stay private (additive, non-breaking).
+- **`BinarySecurityCheckOptions.no_libc` is reachable** via a new
+  builder-style `no_libc(bool)` method; the field is crate-private and
+  `new()` hard-coded `false`, so the documented option could not be
+  enabled before.
+- **Rule loading reports its real error and actually runs in
+  parallel** the loader thread was spawned *after* the extractor was
+  built (smda disassembles eagerly, so `join` blocked immediately and
+  ~1000 YAML files loaded serially), and every failure was misreported
+  as `DescriptionEvaluationError`. The thread now spawns before format
+  detection/disassembly, the real `RuleSet::new` error propagates, and
+  a loader panic is re-thrown instead of mislabeled.
+- **Strict `LibCSpec::from_str`** unknown LSB versions now error
+  (previously any typo silently became `LSB5`, changing fortify-check
+  semantics); the lenient `From<String>` remains for compatibility,
+  and `capa_cli` uses the strict parse.
+- **`from_file` accepts `impl AsRef<Path>`** (was `AsRef<str>`);
+  existing `&str`/`String` callers keep compiling.
+
 ## [0.5.2] — xor-zero number(0), regex /i fast path, rule pre-pruning
 
 ### Fixed — feature extraction parity

examples/capa_cli.rs

@@ -10,7 +10,7 @@ use clap::Parser;
 use prettytable::{Attr, Cell, Row, Table, color, format::Alignment};
 use serde_json::{Map, Value, to_value};
 
-use capa::{BinarySecurityCheckOptions, FileCapabilities};
+use capa::{BinarySecurityCheckOptions, FileCapabilities, LibCSpec};
 
 #[derive(Parser)]
 #[clap(
@@ -77,7 +77,16 @@ fn main() {
     let json_path = cli.output;
     let libc = cli.libc.map(|s| s.into());
     let sysroot = cli.sysroot.map(|s| s.into());
-    let libc_spec = cli.libc_spec.map(|s| s.into());
+    // Strict parse (0.5.3, #22): an unknown LSB version used to fall
+    // back to LSB5 silently, changing fortify-check semantics.
+    let libc_spec = match cli.libc_spec.map(|s| s.parse::<LibCSpec>()) {
+        Some(Ok(spec)) => Some(spec),
+        Some(Err(e)) => {
+            eprintln!("error: {e}");
+            std::process::exit(1);
+        }
+        None => None,
+    };
     let security_check_opts = BinarySecurityCheckOptions::new(libc, sysroot, libc_spec);
 
     let start = Instant::now();

src/error.rs

@@ -109,4 +109,9 @@ pub enum Error {
     // nothing to match capabilities against.
     #[error("AnalyzeBuilder: .rules(path) must be called before .from_file() / .from_buffer()")]
     BuilderMissingRules,
+
+    // 0.5.3 (#22): strict `LibCSpec::from_str` rejects unknown LSB
+    // version strings (the lenient `From<String>` fallback stays).
+    #[error("invalid libc spec version: {0}")]
+    InvalidLibCSpec(String),
 }

src/lib.rs

@@ -26,7 +26,6 @@ use std::{
 use once_cell::sync::Lazy;
 use serde::{Deserialize, Serialize};
 use serde_json::{Value, json};
-use smda::FileArchitecture;
 use yaml_rust::Yaml;
 
 // 0.4.2: regexes used in tag-string parsing — compiled once per
@@ -43,16 +42,16 @@ static PARTS_ID_RE: Lazy<regex::Regex> = Lazy::new(|| {
         .expect("compile-time regex literal — pattern is valid")
 });
 
-use consts::FileFormat;
-// 0.4.0: `Os` is referenced only by the properties-gated
-// `FileCapabilities::get_os` — gating the import avoids the
-// `--no-default-features` unused-import warning.
-#[cfg(feature = "properties")]
-use consts::Os;
-use sede::{from_hex, to_hex};
-
+// 0.5.3 (#22): types that appear in public fields (`Properties::format`,
+// `Properties::os`, `Properties::arch`, `FileCapabilities::security_checks`)
+// are re-exported from the crate root — previously downstream could not
+// name them (their modules are private), making the fields unusable for
+// anything but Debug-printing.
+pub use crate::consts::{FileFormat, Os};
 pub use crate::error::Error;
-use crate::security::options::status::SecurityCheckStatus;
+pub use crate::security::options::status::SecurityCheckStatus;
+use sede::{from_hex, to_hex};
+pub use smda::FileArchitecture;
 
 pub(crate) mod consts;
 mod error;
@@ -148,22 +147,37 @@ impl LibCSpec {
 
 // Used for options for binary security checks.
 impl From<String> for LibCSpec {
+    /// Lenient conversion kept for API compatibility: unknown versions
+    /// fall back to the newest spec. Prefer `LibCSpec::from_str`
+    /// (0.5.3, #22), which rejects unknown versions.
     fn from(value: String) -> Self {
-        match value.as_str() {
-            "1.0.0" => LibCSpec::LSB1,
-            "1.1.0" => LibCSpec::LSB1dot1,
-            "1.2.0" => LibCSpec::LSB1dot2,
-            "1.3.0" => LibCSpec::LSB1dot3,
-            "2.0.0" => LibCSpec::LSB2,
-            "2.0.1" => LibCSpec::LSB2dot0dot1,
-            "2.1.0" => LibCSpec::LSB2dot1,
-            "3.0.0" => LibCSpec::LSB3,
-            "3.1.0" => LibCSpec::LSB3dot1,
-            "3.2.0" => LibCSpec::LSB3dot2,
-            "4.0.0" => LibCSpec::LSB4,
-            "4.1.0" => LibCSpec::LSB4dot1,
-            "5.0.0" => LibCSpec::LSB5,
-            _ => LibCSpec::LSB5,
+        value.parse().unwrap_or(LibCSpec::LSB5)
+    }
+}
+
+impl std::str::FromStr for LibCSpec {
+    type Err = Error;
+
+    /// Strict version parse — unknown versions are an error.
+    /// 0.5.3 (#22): previously the only way in was `From<String>`,
+    /// which silently mapped any typo (e.g. `4.0.1`) to `LSB5` and
+    /// changed fortify-check semantics without a word.
+    fn from_str(s: &str) -> Result<Self> {
+        match s {
+            "1.0.0" => Ok(LibCSpec::LSB1),
+            "1.1.0" => Ok(LibCSpec::LSB1dot1),
+            "1.2.0" => Ok(LibCSpec::LSB1dot2),
+            "1.3.0" => Ok(LibCSpec::LSB1dot3),
+            "2.0.0" => Ok(LibCSpec::LSB2),
+            "2.0.1" => Ok(LibCSpec::LSB2dot0dot1),
+            "2.1.0" => Ok(LibCSpec::LSB2dot1),
+            "3.0.0" => Ok(LibCSpec::LSB3),
+            "3.1.0" => Ok(LibCSpec::LSB3dot1),
+            "3.2.0" => Ok(LibCSpec::LSB3dot2),
+            "4.0.0" => Ok(LibCSpec::LSB4),
+            "4.1.0" => Ok(LibCSpec::LSB4dot1),
+            "5.0.0" => Ok(LibCSpec::LSB5),
+            _ => Err(Error::InvalidLibCSpec(s.to_string())),
         }
     }
 }
@@ -206,6 +220,15 @@ impl BinarySecurityCheckOptions {
             input_file: PathBuf::new(),
         }
     }
+
+    /// Assume that input files do not use any C runtime libraries
+    /// (disables the libc-dependent ELF checks such as FORTIFY-SOURCE).
+    /// 0.5.3 (#22): the option existed but was unreachable — the field
+    /// is crate-private and `new()` hard-coded it to `false`.
+    pub fn no_libc(mut self, no_libc: bool) -> Self {
+        self.no_libc = no_libc;
+        self
+    }
 }
 
 impl Default for BinarySecurityCheckOptions {
@@ -450,9 +473,17 @@ impl<'a> AnalyzeBuilder<'a> {
     /// Terminal — analyse a binary on disk. Routes through capa-rs's
     /// magic-byte format detection (PE → dnfile-then-smda, ELF →
     /// smda, Mach-O → smda) and runs the binary security checklist.
-    pub fn from_file(self, file_name: impl AsRef<str>) -> Result<FileCapabilities> {
+    ///
+    /// Accepts any `AsRef<Path>` (0.5.3 — was `AsRef<str>`); non-UTF-8
+    /// paths are converted with `to_string_lossy`.
+    pub fn from_file(self, file_name: impl AsRef<std::path::Path>) -> Result<FileCapabilities> {
         let rule_path = self.rules.ok_or(Error::BuilderMissingRules)?;
-        let f = file_name.as_ref().to_string();
+        let f = file_name.as_ref().to_string_lossy().into_owned();
+        // Spawn the rules load FIRST so it overlaps with format
+        // detection and (eager) disassembly below — pre-#22 the thread
+        // was spawned after the extractor was built, so `join` blocked
+        // immediately and ~1000 rule files loaded strictly serially.
+        let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
         let (format, buffer) = get_format(&f)?;
         let extractor = get_file_extractors(
             &f,
@@ -461,10 +492,14 @@ impl<'a> AnalyzeBuilder<'a> {
             self.high_accuracy,
             self.resolve_tailcalls,
         )?;
-        let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
         let rules = match rules_thread_handle.join() {
             Ok(Ok(rules)) => rules,
-            Ok(Err(_)) | Err(_) => return Err(Error::DescriptionEvaluationError),
+            // Propagate the real RuleSet error (bad YAML, missing
+            // dependency, …) — pre-#22 every failure was misreported
+            // as DescriptionEvaluationError. A loader panic is a bug;
+            // keep it a panic rather than mislabeling it.
+            Ok(Err(e)) => return Err(e),
+            Err(panic) => std::panic::resume_unwind(panic),
         };
 
         // Security checks — defaults if caller didn't override.
@@ -553,6 +588,7 @@ impl<'a> AnalyzeBuilder<'a> {
     /// (pass `0` if the caller has no preference).
     pub fn from_buffer(self, raw: &[u8], base_addr: u64, bitness: u32) -> Result<FileCapabilities> {
         let rule_path = self.rules.ok_or(Error::BuilderMissingRules)?;
+        let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
         // Construct the extractor directly via smda's parse_buffer —
         // get_file_extractors routes on PE/ELF/Mach-O magic, which
         // a raw buffer doesn't have.
@@ -565,10 +601,12 @@ impl<'a> AnalyzeBuilder<'a> {
                 self.resolve_tailcalls,
             )?);
 
-        let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
         let rules = match rules_thread_handle.join() {
             Ok(Ok(rules)) => rules,
-            Ok(Err(_)) | Err(_) => return Err(Error::DescriptionEvaluationError),
+            // See `from_file`: real error propagates, a loader panic
+            // stays a panic (#22).
+            Ok(Err(e)) => return Err(e),
+            Err(panic) => std::panic::resume_unwind(panic),
         };
 
         // 0.4.3: FLIRT setup — see `from_file` for rationale.
@@ -1391,6 +1429,23 @@ pub struct FunctionCapabilities {
     capabilities: Vec<String>,
 }
 
+impl FunctionCapabilities {
+    /// Address of the analysed function.
+    pub fn address(&self) -> usize {
+        self.address
+    }
+
+    /// Number of features extracted from the function.
+    pub fn features(&self) -> usize {
+        self.features
+    }
+
+    /// Names of the rules that matched inside the function.
+    pub fn capabilities(&self) -> &[String] {
+        &self.capabilities
+    }
+}
+
 fn parse_parts_id(s: &str) -> Result<(Vec<String>, String)> {
     // 0.4.2: cached at module scope (PARTS_ID_RE); was compiled per call.
     let re = &*PARTS_ID_RE;

tests/public_api.rs

@@ -0,0 +1,60 @@
+//! #22: public API surface — downstream must be able to *name* the
+//! types that appear in public fields, and the documented options must
+//! be reachable. Most assertions here only need to type-check: pre-#22
+//! these types were unreachable because their modules are private.
+
+use capa::{BinarySecurityCheckOptions, LibCSpec};
+use std::str::FromStr;
+
+#[test]
+fn exported_field_types_are_nameable() {
+    // Pre-#22 none of these paths resolved from outside the crate.
+    let _f: Option<capa::FileFormat> = None;
+    let _o: Option<capa::Os> = None;
+    let _a: Option<capa::FileArchitecture> = None;
+    let _s: Option<capa::SecurityCheckStatus> = None;
+}
+
+#[test]
+fn function_capabilities_getters_exist() {
+    // Fields stay private; getters are the read API (verbose feature).
+    let _: fn(&capa::FunctionCapabilities) -> usize = capa::FunctionCapabilities::address;
+    let _: fn(&capa::FunctionCapabilities) -> usize = capa::FunctionCapabilities::features;
+    let _: fn(&capa::FunctionCapabilities) -> &[String] = capa::FunctionCapabilities::capabilities;
+}
+
+#[test]
+fn no_libc_option_is_reachable() {
+    // Pre-#22 `no_libc` was pub(crate) and `new()` hard-coded false.
+    let _opts = BinarySecurityCheckOptions::default().no_libc(true);
+}
+
+#[test]
+fn libc_spec_from_str_is_strict() {
+    assert!(matches!(
+        LibCSpec::from_str("4.1.0"),
+        Ok(LibCSpec::LSB4dot1)
+    ));
+    assert!(matches!(LibCSpec::from_str("5.0.0"), Ok(LibCSpec::LSB5)));
+    // Unknown versions error (pre-#22 they silently became LSB5).
+    assert!(LibCSpec::from_str("4.0.1").is_err());
+    assert!(LibCSpec::from_str("").is_err());
+    // The lenient From<String> stays for compatibility.
+    assert!(matches!(
+        LibCSpec::from("4.0.1".to_string()),
+        LibCSpec::LSB5
+    ));
+}
+
+#[test]
+fn from_file_accepts_path_like_arguments() {
+    // Only checks the signature: &str, String, &Path and PathBuf must
+    // all compile (pre-#22 only AsRef<str> was accepted). The calls
+    // fail at runtime for a missing rules dir — that's fine, we never
+    // execute them.
+    let _ = |p: &std::path::Path| {
+        capa::FileCapabilities::analyze()
+            .rules("definitely-missing-rules-dir")
+            .from_file(p)
+    };
+}