Skip to content

Commit dd9270f

Browse files
committed
Close public API gaps (closes #22)
- pub use FileFormat, Os, FileArchitecture, SecurityCheckStatus from the crate root — they appear in public fields but lived in private modules, so downstream could not name them. - FunctionCapabilities: address()/features()/capabilities() getters (fields stay private; additive). - BinarySecurityCheckOptions: builder-style no_libc(bool) — the documented option was unreachable (pub(crate) field, new() hard-coded false). - from_file/from_buffer: spawn the rules-load thread BEFORE format detection + disassembly so it actually overlaps (smda disassembles eagerly; previously join blocked immediately), propagate the real RuleSet::new error instead of misreporting DescriptionEvaluationError, and resume_unwind a loader panic instead of mislabeling it. - LibCSpec: strict FromStr (unknown versions error instead of silently degrading to LSB5); lenient From<String> kept for compat; capa_cli uses the strict parse. New Error::InvalidLibCSpec variant. - from_file takes impl AsRef<Path> (was AsRef<str>); existing &str/String callers keep compiling. Adds tests/public_api.rs integration tests pinning the public surface.
1 parent e760998 commit dd9270f

5 files changed

Lines changed: 189 additions & 33 deletions

File tree

CHANGELOG.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,33 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
2626
`u16::from_be_bytes` received the chunk bytes in reverse order, turning
2727
every UTF-16BE string into non-ASCII garbage that was then dropped.
2828

29+
### Fixed — public API gaps (closes [#22](https://github.com/marirs/capa-rs/issues/22))
30+
31+
- **Types in public fields are now nameable downstream** `pub use`
32+
from the crate root for `FileFormat`, `Os`, `FileArchitecture`
33+
(`Properties::format` / `os` / `arch`) and `SecurityCheckStatus`
34+
(`FileCapabilities::security_checks`) — previously their modules
35+
were private, so the fields were Debug-print-only.
36+
- **`FunctionCapabilities` gained getters** `address()`, `features()`,
37+
`capabilities()` — fields stay private (additive, non-breaking).
38+
- **`BinarySecurityCheckOptions.no_libc` is reachable** via a new
39+
builder-style `no_libc(bool)` method; the field is crate-private and
40+
`new()` hard-coded `false`, so the documented option could not be
41+
enabled before.
42+
- **Rule loading reports its real error and actually runs in
43+
parallel** the loader thread was spawned *after* the extractor was
44+
built (smda disassembles eagerly, so `join` blocked immediately and
45+
~1000 YAML files loaded serially), and every failure was misreported
46+
as `DescriptionEvaluationError`. The thread now spawns before format
47+
detection/disassembly, the real `RuleSet::new` error propagates, and
48+
a loader panic is re-thrown instead of mislabeled.
49+
- **Strict `LibCSpec::from_str`** unknown LSB versions now error
50+
(previously any typo silently became `LSB5`, changing fortify-check
51+
semantics); the lenient `From<String>` remains for compatibility,
52+
and `capa_cli` uses the strict parse.
53+
- **`from_file` accepts `impl AsRef<Path>`** (was `AsRef<str>`);
54+
existing `&str`/`String` callers keep compiling.
55+
2956
## [0.5.2] — xor-zero number(0), regex /i fast path, rule pre-pruning
3057

3158
### Fixed — feature extraction parity

examples/capa_cli.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use clap::Parser;
1010
use prettytable::{Attr, Cell, Row, Table, color, format::Alignment};
1111
use serde_json::{Map, Value, to_value};
1212

13-
use capa::{BinarySecurityCheckOptions, FileCapabilities};
13+
use capa::{BinarySecurityCheckOptions, FileCapabilities, LibCSpec};
1414

1515
#[derive(Parser)]
1616
#[clap(
@@ -77,7 +77,16 @@ fn main() {
7777
let json_path = cli.output;
7878
let libc = cli.libc.map(|s| s.into());
7979
let sysroot = cli.sysroot.map(|s| s.into());
80-
let libc_spec = cli.libc_spec.map(|s| s.into());
80+
// Strict parse (0.5.3, #22): an unknown LSB version used to fall
81+
// back to LSB5 silently, changing fortify-check semantics.
82+
let libc_spec = match cli.libc_spec.map(|s| s.parse::<LibCSpec>()) {
83+
Some(Ok(spec)) => Some(spec),
84+
Some(Err(e)) => {
85+
eprintln!("error: {e}");
86+
std::process::exit(1);
87+
}
88+
None => None,
89+
};
8190
let security_check_opts = BinarySecurityCheckOptions::new(libc, sysroot, libc_spec);
8291

8392
let start = Instant::now();

src/error.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,9 @@ pub enum Error {
109109
// nothing to match capabilities against.
110110
#[error("AnalyzeBuilder: .rules(path) must be called before .from_file() / .from_buffer()")]
111111
BuilderMissingRules,
112+
113+
// 0.5.3 (#22): strict `LibCSpec::from_str` rejects unknown LSB
114+
// version strings (the lenient `From<String>` fallback stays).
115+
#[error("invalid libc spec version: {0}")]
116+
InvalidLibCSpec(String),
112117
}

src/lib.rs

Lines changed: 86 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ use std::{
2626
use once_cell::sync::Lazy;
2727
use serde::{Deserialize, Serialize};
2828
use serde_json::{Value, json};
29-
use smda::FileArchitecture;
3029
use yaml_rust::Yaml;
3130

3231
// 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(|| {
4342
.expect("compile-time regex literal — pattern is valid")
4443
});
4544

46-
use consts::FileFormat;
47-
// 0.4.0: `Os` is referenced only by the properties-gated
48-
// `FileCapabilities::get_os` — gating the import avoids the
49-
// `--no-default-features` unused-import warning.
50-
#[cfg(feature = "properties")]
51-
use consts::Os;
52-
use sede::{from_hex, to_hex};
53-
45+
// 0.5.3 (#22): types that appear in public fields (`Properties::format`,
46+
// `Properties::os`, `Properties::arch`, `FileCapabilities::security_checks`)
47+
// are re-exported from the crate root — previously downstream could not
48+
// name them (their modules are private), making the fields unusable for
49+
// anything but Debug-printing.
50+
pub use crate::consts::{FileFormat, Os};
5451
pub use crate::error::Error;
55-
use crate::security::options::status::SecurityCheckStatus;
52+
pub use crate::security::options::status::SecurityCheckStatus;
53+
use sede::{from_hex, to_hex};
54+
pub use smda::FileArchitecture;
5655

5756
pub(crate) mod consts;
5857
mod error;
@@ -148,22 +147,37 @@ impl LibCSpec {
148147

149148
// Used for options for binary security checks.
150149
impl From<String> for LibCSpec {
150+
/// Lenient conversion kept for API compatibility: unknown versions
151+
/// fall back to the newest spec. Prefer `LibCSpec::from_str`
152+
/// (0.5.3, #22), which rejects unknown versions.
151153
fn from(value: String) -> Self {
152-
match value.as_str() {
153-
"1.0.0" => LibCSpec::LSB1,
154-
"1.1.0" => LibCSpec::LSB1dot1,
155-
"1.2.0" => LibCSpec::LSB1dot2,
156-
"1.3.0" => LibCSpec::LSB1dot3,
157-
"2.0.0" => LibCSpec::LSB2,
158-
"2.0.1" => LibCSpec::LSB2dot0dot1,
159-
"2.1.0" => LibCSpec::LSB2dot1,
160-
"3.0.0" => LibCSpec::LSB3,
161-
"3.1.0" => LibCSpec::LSB3dot1,
162-
"3.2.0" => LibCSpec::LSB3dot2,
163-
"4.0.0" => LibCSpec::LSB4,
164-
"4.1.0" => LibCSpec::LSB4dot1,
165-
"5.0.0" => LibCSpec::LSB5,
166-
_ => LibCSpec::LSB5,
154+
value.parse().unwrap_or(LibCSpec::LSB5)
155+
}
156+
}
157+
158+
impl std::str::FromStr for LibCSpec {
159+
type Err = Error;
160+
161+
/// Strict version parse — unknown versions are an error.
162+
/// 0.5.3 (#22): previously the only way in was `From<String>`,
163+
/// which silently mapped any typo (e.g. `4.0.1`) to `LSB5` and
164+
/// changed fortify-check semantics without a word.
165+
fn from_str(s: &str) -> Result<Self> {
166+
match s {
167+
"1.0.0" => Ok(LibCSpec::LSB1),
168+
"1.1.0" => Ok(LibCSpec::LSB1dot1),
169+
"1.2.0" => Ok(LibCSpec::LSB1dot2),
170+
"1.3.0" => Ok(LibCSpec::LSB1dot3),
171+
"2.0.0" => Ok(LibCSpec::LSB2),
172+
"2.0.1" => Ok(LibCSpec::LSB2dot0dot1),
173+
"2.1.0" => Ok(LibCSpec::LSB2dot1),
174+
"3.0.0" => Ok(LibCSpec::LSB3),
175+
"3.1.0" => Ok(LibCSpec::LSB3dot1),
176+
"3.2.0" => Ok(LibCSpec::LSB3dot2),
177+
"4.0.0" => Ok(LibCSpec::LSB4),
178+
"4.1.0" => Ok(LibCSpec::LSB4dot1),
179+
"5.0.0" => Ok(LibCSpec::LSB5),
180+
_ => Err(Error::InvalidLibCSpec(s.to_string())),
167181
}
168182
}
169183
}
@@ -206,6 +220,15 @@ impl BinarySecurityCheckOptions {
206220
input_file: PathBuf::new(),
207221
}
208222
}
223+
224+
/// Assume that input files do not use any C runtime libraries
225+
/// (disables the libc-dependent ELF checks such as FORTIFY-SOURCE).
226+
/// 0.5.3 (#22): the option existed but was unreachable — the field
227+
/// is crate-private and `new()` hard-coded it to `false`.
228+
pub fn no_libc(mut self, no_libc: bool) -> Self {
229+
self.no_libc = no_libc;
230+
self
231+
}
209232
}
210233

211234
impl Default for BinarySecurityCheckOptions {
@@ -450,9 +473,17 @@ impl<'a> AnalyzeBuilder<'a> {
450473
/// Terminal — analyse a binary on disk. Routes through capa-rs's
451474
/// magic-byte format detection (PE → dnfile-then-smda, ELF →
452475
/// smda, Mach-O → smda) and runs the binary security checklist.
453-
pub fn from_file(self, file_name: impl AsRef<str>) -> Result<FileCapabilities> {
476+
///
477+
/// Accepts any `AsRef<Path>` (0.5.3 — was `AsRef<str>`); non-UTF-8
478+
/// paths are converted with `to_string_lossy`.
479+
pub fn from_file(self, file_name: impl AsRef<std::path::Path>) -> Result<FileCapabilities> {
454480
let rule_path = self.rules.ok_or(Error::BuilderMissingRules)?;
455-
let f = file_name.as_ref().to_string();
481+
let f = file_name.as_ref().to_string_lossy().into_owned();
482+
// Spawn the rules load FIRST so it overlaps with format
483+
// detection and (eager) disassembly below — pre-#22 the thread
484+
// was spawned after the extractor was built, so `join` blocked
485+
// immediately and ~1000 rule files loaded strictly serially.
486+
let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
456487
let (format, buffer) = get_format(&f)?;
457488
let extractor = get_file_extractors(
458489
&f,
@@ -461,10 +492,14 @@ impl<'a> AnalyzeBuilder<'a> {
461492
self.high_accuracy,
462493
self.resolve_tailcalls,
463494
)?;
464-
let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
465495
let rules = match rules_thread_handle.join() {
466496
Ok(Ok(rules)) => rules,
467-
Ok(Err(_)) | Err(_) => return Err(Error::DescriptionEvaluationError),
497+
// Propagate the real RuleSet error (bad YAML, missing
498+
// dependency, …) — pre-#22 every failure was misreported
499+
// as DescriptionEvaluationError. A loader panic is a bug;
500+
// keep it a panic rather than mislabeling it.
501+
Ok(Err(e)) => return Err(e),
502+
Err(panic) => std::panic::resume_unwind(panic),
468503
};
469504

470505
// Security checks — defaults if caller didn't override.
@@ -553,6 +588,7 @@ impl<'a> AnalyzeBuilder<'a> {
553588
/// (pass `0` if the caller has no preference).
554589
pub fn from_buffer(self, raw: &[u8], base_addr: u64, bitness: u32) -> Result<FileCapabilities> {
555590
let rule_path = self.rules.ok_or(Error::BuilderMissingRules)?;
591+
let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
556592
// Construct the extractor directly via smda's parse_buffer —
557593
// get_file_extractors routes on PE/ELF/Mach-O magic, which
558594
// a raw buffer doesn't have.
@@ -565,10 +601,12 @@ impl<'a> AnalyzeBuilder<'a> {
565601
self.resolve_tailcalls,
566602
)?);
567603

568-
let rules_thread_handle = spawn(move || rules::RuleSet::new(&rule_path));
569604
let rules = match rules_thread_handle.join() {
570605
Ok(Ok(rules)) => rules,
571-
Ok(Err(_)) | Err(_) => return Err(Error::DescriptionEvaluationError),
606+
// See `from_file`: real error propagates, a loader panic
607+
// stays a panic (#22).
608+
Ok(Err(e)) => return Err(e),
609+
Err(panic) => std::panic::resume_unwind(panic),
572610
};
573611

574612
// 0.4.3: FLIRT setup — see `from_file` for rationale.
@@ -1391,6 +1429,23 @@ pub struct FunctionCapabilities {
13911429
capabilities: Vec<String>,
13921430
}
13931431

1432+
impl FunctionCapabilities {
1433+
/// Address of the analysed function.
1434+
pub fn address(&self) -> usize {
1435+
self.address
1436+
}
1437+
1438+
/// Number of features extracted from the function.
1439+
pub fn features(&self) -> usize {
1440+
self.features
1441+
}
1442+
1443+
/// Names of the rules that matched inside the function.
1444+
pub fn capabilities(&self) -> &[String] {
1445+
&self.capabilities
1446+
}
1447+
}
1448+
13941449
fn parse_parts_id(s: &str) -> Result<(Vec<String>, String)> {
13951450
// 0.4.2: cached at module scope (PARTS_ID_RE); was compiled per call.
13961451
let re = &*PARTS_ID_RE;

tests/public_api.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//! #22: public API surface — downstream must be able to *name* the
2+
//! types that appear in public fields, and the documented options must
3+
//! be reachable. Most assertions here only need to type-check: pre-#22
4+
//! these types were unreachable because their modules are private.
5+
6+
use capa::{BinarySecurityCheckOptions, LibCSpec};
7+
use std::str::FromStr;
8+
9+
#[test]
10+
fn exported_field_types_are_nameable() {
11+
// Pre-#22 none of these paths resolved from outside the crate.
12+
let _f: Option<capa::FileFormat> = None;
13+
let _o: Option<capa::Os> = None;
14+
let _a: Option<capa::FileArchitecture> = None;
15+
let _s: Option<capa::SecurityCheckStatus> = None;
16+
}
17+
18+
#[test]
19+
fn function_capabilities_getters_exist() {
20+
// Fields stay private; getters are the read API (verbose feature).
21+
let _: fn(&capa::FunctionCapabilities) -> usize = capa::FunctionCapabilities::address;
22+
let _: fn(&capa::FunctionCapabilities) -> usize = capa::FunctionCapabilities::features;
23+
let _: fn(&capa::FunctionCapabilities) -> &[String] = capa::FunctionCapabilities::capabilities;
24+
}
25+
26+
#[test]
27+
fn no_libc_option_is_reachable() {
28+
// Pre-#22 `no_libc` was pub(crate) and `new()` hard-coded false.
29+
let _opts = BinarySecurityCheckOptions::default().no_libc(true);
30+
}
31+
32+
#[test]
33+
fn libc_spec_from_str_is_strict() {
34+
assert!(matches!(
35+
LibCSpec::from_str("4.1.0"),
36+
Ok(LibCSpec::LSB4dot1)
37+
));
38+
assert!(matches!(LibCSpec::from_str("5.0.0"), Ok(LibCSpec::LSB5)));
39+
// Unknown versions error (pre-#22 they silently became LSB5).
40+
assert!(LibCSpec::from_str("4.0.1").is_err());
41+
assert!(LibCSpec::from_str("").is_err());
42+
// The lenient From<String> stays for compatibility.
43+
assert!(matches!(
44+
LibCSpec::from("4.0.1".to_string()),
45+
LibCSpec::LSB5
46+
));
47+
}
48+
49+
#[test]
50+
fn from_file_accepts_path_like_arguments() {
51+
// Only checks the signature: &str, String, &Path and PathBuf must
52+
// all compile (pre-#22 only AsRef<str> was accepted). The calls
53+
// fail at runtime for a missing rules dir — that's fine, we never
54+
// execute them.
55+
let _ = |p: &std::path::Path| {
56+
capa::FileCapabilities::analyze()
57+
.rules("definitely-missing-rules-dir")
58+
.from_file(p)
59+
};
60+
}

0 commit comments

Comments
 (0)