Skip to content

Commit f42f2fb

Browse files
Harden parsers and share pack validation
1 parent 0bae45f commit f42f2fb

8 files changed

Lines changed: 265 additions & 72 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66

77
- Added a real `ai` feature implementation for `summarize --input report.json --provider openai` using an OpenAI-compatible `/chat/completions` endpoint configured by `BTC_RISK_LAB_AI_BASE_URL`, `BTC_RISK_LAB_AI_API_KEY`, and optional `BTC_RISK_LAB_AI_MODEL`.
88
- Added a global `--offline` flag that turns network-backed commands into explanatory errors before any HTTP client is used.
9+
- Added property tests covering transaction, PSBT, and script analysis against arbitrary bytes and mutated valid fixtures.
910

1011
### Changed
1112

1213
- AI summary output is explicitly marked `AI-assisted draft` and the command sends only the already-produced btc-risk-lab JSON report to the configured provider.
1314
- Builds compiled without `--features ai` now fail the summary command with `compiled without ai feature`.
15+
- Extracted shared pack input validation, file presence checks, file reading, and metadata helpers into `pack_common`.
1416

1517
### Security
1618

Cargo.lock

Lines changed: 74 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,5 @@ tokio = { version = "1", default-features = false, features = ["rt-multi-thread"
3131
[dev-dependencies]
3232
assert_cmd = "2"
3333
predicates = "3"
34+
proptest = "1"
3435
tempfile = "3"

src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod analyzer;
2+
mod pack_common;
23
pub mod policy_pack;
34
pub mod report;
45
pub mod review_pack;

src/pack_common.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use std::{
2+
fs,
3+
path::{Path, PathBuf},
4+
};
5+
6+
use anyhow::{bail, Context, Result};
7+
8+
pub fn validate_input_dir(input_dir: &Path, pack_label: &str) -> Result<()> {
9+
if !input_dir.is_dir() {
10+
bail!(
11+
"{pack_label} input must be a directory: {}",
12+
input_dir.display()
13+
);
14+
}
15+
16+
Ok(())
17+
}
18+
19+
pub fn optional_file(input_dir: &Path, file: &str) -> Option<PathBuf> {
20+
let path = input_dir.join(file);
21+
path.exists().then_some(path)
22+
}
23+
24+
pub fn has_file(input_dir: &Path, file: &str) -> bool {
25+
input_dir.join(file).exists()
26+
}
27+
28+
pub fn read_to_string(path: &Path, context: impl FnOnce() -> String) -> Result<String> {
29+
fs::read_to_string(path).with_context(context)
30+
}
31+
32+
pub fn file_name(path: &Path) -> String {
33+
path.file_name()
34+
.map(|name| PathBuf::from(name).display().to_string())
35+
.unwrap_or_else(|| path.display().to_string())
36+
}
37+
38+
pub fn is_metadata_artifact(artifact: &str) -> bool {
39+
artifact == "metadata"
40+
}

src/policy_pack.rs

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
use std::{
2-
fs,
3-
path::{Path, PathBuf},
4-
};
1+
use std::path::Path;
52

6-
use anyhow::{bail, Context, Result};
3+
use anyhow::Result;
74
use serde::{Deserialize, Serialize};
85
use serde_json::Value;
96

107
use crate::{
118
analyzer::{RiskLevel, RiskWarning, SummaryItem},
9+
pack_common,
1210
review_pack::{
1311
analyze_review_pack, ArtifactSummary, CrossArtifactFinding, DetectedArtifact,
1412
ReviewPackReport,
@@ -49,12 +47,7 @@ pub struct PolicyFinding {
4947
}
5048

5149
pub fn analyze_policy_pack(input_dir: &Path) -> Result<PolicyPackReport> {
52-
if !input_dir.is_dir() {
53-
bail!(
54-
"policy pack input must be a directory: {}",
55-
input_dir.display()
56-
);
57-
}
50+
pack_common::validate_input_dir(input_dir, "policy pack")?;
5851

5952
let review_pack = analyze_review_pack(input_dir)?;
6053
let evidence_documents = read_evidence_documents(input_dir)?;
@@ -119,16 +112,16 @@ fn read_evidence_documents(input_dir: &Path) -> Result<Vec<EvidenceDocument>> {
119112
let mut documents = Vec::new();
120113

121114
for spec in evidence_specs() {
122-
let path = input_dir.join(spec.file);
123-
if !path.exists() {
115+
let Some(path) = pack_common::optional_file(input_dir, spec.file) else {
124116
continue;
125-
}
117+
};
126118

127-
let input = fs::read_to_string(&path)
128-
.with_context(|| format!("failed to read evidence document {}", path.display()))?;
119+
let input = pack_common::read_to_string(&path, || {
120+
format!("failed to read evidence document {}", path.display())
121+
})?;
129122
documents.push(EvidenceDocument {
130123
artifact: spec.artifact.to_owned(),
131-
file: file_name(&path),
124+
file: pack_common::file_name(&path),
132125
format: spec.format.to_owned(),
133126
summary: summarize_evidence(spec.format, &input),
134127
});
@@ -424,7 +417,7 @@ fn has_policy_notes(documents: &[EvidenceDocument]) -> bool {
424417
fn has_metadata(documents: &[EvidenceDocument]) -> bool {
425418
documents
426419
.iter()
427-
.any(|document| document.artifact == "metadata")
420+
.any(|document| pack_common::is_metadata_artifact(&document.artifact))
428421
}
429422

430423
fn push_detected_once(artifacts: &mut Vec<DetectedArtifact>, artifact: DetectedArtifact) {
@@ -436,12 +429,6 @@ fn push_detected_once(artifacts: &mut Vec<DetectedArtifact>, artifact: DetectedA
436429
}
437430
}
438431

439-
fn file_name(path: &Path) -> String {
440-
path.file_name()
441-
.map(|name| PathBuf::from(name).display().to_string())
442-
.unwrap_or_else(|| path.display().to_string())
443-
}
444-
445432
fn artifact_label(input: &str) -> String {
446433
if input == "psbt" {
447434
return "PSBT".to_owned();

0 commit comments

Comments
 (0)