diff --git a/crates/code-intel-cli/src/audit_report/cli.rs b/crates/code-intel-cli/src/audit_report/cli.rs index 8c45dbc..f9a346b 100644 --- a/crates/code-intel-cli/src/audit_report/cli.rs +++ b/crates/code-intel-cli/src/audit_report/cli.rs @@ -86,12 +86,15 @@ fn set_once(slot: &mut Option, value: &str, flag: &str) -> Result<(), St /// The full validate pipeline the CLI runs against a real repository: read /// the report file, parse it structurally, load and self-validate the -/// on-disk department registry, then check the report against it. +/// on-disk department registry, check the report against it, and — because +/// this path has the repository the report cites — ground every file evidence +/// entry in that tree. fn validate(repo: &Path, report_path: &Path) -> Result { let bytes = read_report(report_path)?; let report = AuditReport::parse(&bytes)?; let registry = DepartmentRegistry::load(repo)?; registry.validate(repo)?; + report.validate_evidence_grounding(repo)?; validate_report(&report, ®istry) } diff --git a/crates/code-intel-cli/src/audit_report/registry.rs b/crates/code-intel-cli/src/audit_report/registry.rs index 3434952..e589c6e 100644 --- a/crates/code-intel-cli/src/audit_report/registry.rs +++ b/crates/code-intel-cli/src/audit_report/registry.rs @@ -9,6 +9,34 @@ use super::json_helpers::{closed_object, required_bool, required_str, required_s // orchestration/audit/departments.v1.json registry // --------------------------------------------------------------------- +/// Registry path strings name files *inside* the repository under audit, and +/// the registry itself is read from `--repo` — a target repo's own file. A +/// path that escapes the checkout would let a scanned repository satisfy the +/// kernel's "these files exist" invariant with files anywhere on the host, and +/// point a department's `prompt` — the instruction source an audit agent reads +/// — outside the tree the operator pointed at. Same portable-relative contract +/// `artifact_ref.rs` already enforces for artifact paths. +pub(crate) fn repo_relative_path(value: &str, label: &str) -> Result { + if value.is_empty() + || value.contains('\0') + || value.contains('\\') + || value.starts_with('/') + || value.contains(':') + { + return Err(format!( + "{label} path is not portable repo-relative syntax: {value}" + )); + } + for component in value.split('/') { + if component.is_empty() || component == "." || component == ".." { + return Err(format!( + "{label} path is not portable repo-relative syntax: {value}" + )); + } + } + Ok(value.to_string()) +} + pub(crate) struct RubricPaths { pub(crate) severity: String, pub(crate) confidence: String, @@ -26,11 +54,26 @@ impl RubricPaths { "rubrics", )?; Ok(Self { - severity: required_str(object, "severity", "rubrics")?, - confidence: required_str(object, "confidence", "rubrics")?, - evidence: required_str(object, "evidence", "rubrics")?, - coverage: required_str(object, "coverage", "rubrics")?, - scoring: required_str(object, "scoring", "rubrics")?, + severity: repo_relative_path( + &required_str(object, "severity", "rubrics")?, + "rubrics.severity", + )?, + confidence: repo_relative_path( + &required_str(object, "confidence", "rubrics")?, + "rubrics.confidence", + )?, + evidence: repo_relative_path( + &required_str(object, "evidence", "rubrics")?, + "rubrics.evidence", + )?, + coverage: repo_relative_path( + &required_str(object, "coverage", "rubrics")?, + "rubrics.coverage", + )?, + scoring: repo_relative_path( + &required_str(object, "scoring", "rubrics")?, + "rubrics.scoring", + )?, }) } @@ -84,7 +127,10 @@ impl DepartmentEntry { id, title: required_str(object, "title", "department entry")?, enabled: required_bool(object, "enabled", "department entry")?, - prompt: required_str(object, "prompt", "department entry")?, + prompt: repo_relative_path( + &required_str(object, "prompt", "department entry")?, + "department prompt", + )?, consumes, applicability_check: required_str(object, "applicabilityCheck", "department entry")?, tracking_issue: required_str(object, "trackingIssue", "department entry")?, diff --git a/crates/code-intel-cli/src/audit_report/registry_tests.rs b/crates/code-intel-cli/src/audit_report/registry_tests.rs index 07e9f56..a7d0a68 100644 --- a/crates/code-intel-cli/src/audit_report/registry_tests.rs +++ b/crates/code-intel-cli/src/audit_report/registry_tests.rs @@ -37,6 +37,60 @@ fn registry_rejects_duplicate_department_ids() { assert!(error.contains("duplicate department id"), "{error}"); } +/// The registry is read from `--repo` — a scanned repository's own file — so +/// a path that leaves the checkout must not load at all. +#[test] +fn registry_rejects_rubric_path_escaping_the_repository() { + let value = json!({ + "schema": "code-intel-audit-departments.v1", + "catalogVersion": "1.0.0", + "rubrics": { + "severity": "../../../etc/passwd", + "confidence": "orchestration/audit/rubrics/confidence.md", + "evidence": "orchestration/audit/rubrics/evidence.md", + "coverage": "orchestration/audit/rubrics/coverage.md", + "scoring": "orchestration/audit/rubrics/scoring.md" + }, + "findingContract": "docs/audit-report.md", + "departments": [] + }); + let error = match DepartmentRegistry::from_value(&value) { + Ok(_) => panic!("registry loaded a path that escapes the repository root"), + Err(error) => error, + }; + assert!( + error.contains("not portable repo-relative syntax"), + "{error}" + ); +} + +#[test] +fn registry_rejects_absolute_department_prompt_path() { + let value = json!({ + "schema": "code-intel-audit-departments.v1", + "catalogVersion": "1.0.0", + "rubrics": { + "severity": "orchestration/audit/rubrics/severity.md", + "confidence": "orchestration/audit/rubrics/confidence.md", + "evidence": "orchestration/audit/rubrics/evidence.md", + "coverage": "orchestration/audit/rubrics/coverage.md", + "scoring": "orchestration/audit/rubrics/scoring.md" + }, + "findingContract": "docs/audit-report.md", + "departments": [ + {"id":"security","title":"Security","enabled":true,"prompt":"/etc/attacker-prompt.md","consumes":[],"applicabilityCheck":"always","trackingIssue":"t"} + ] + }); + let error = match DepartmentRegistry::from_value(&value) { + Ok(_) => panic!("registry loaded a path that escapes the repository root"), + Err(error) => error, + }; + assert!( + error.contains("not portable repo-relative syntax"), + "{error}" + ); +} + #[test] fn registry_rejects_missing_rubric_file() { let value = json!({ diff --git a/crates/code-intel-cli/src/audit_report/validate.rs b/crates/code-intel-cli/src/audit_report/validate.rs index c526ae5..e80d542 100644 --- a/crates/code-intel-cli/src/audit_report/validate.rs +++ b/crates/code-intel-cli/src/audit_report/validate.rs @@ -1,7 +1,9 @@ +use std::{fs, path::Path}; + use super::{ enums::{Coverage, DepartmentRunStatus, EvidenceKind, FindingStatus}, model::AuditReport, - registry::DepartmentRegistry, + registry::{repo_relative_path, DepartmentRegistry}, }; fn round1(value: f64) -> f64 { @@ -310,6 +312,53 @@ impl AuditReport { Ok(()) } + + /// Grounds file evidence in the tree it claims to cite. `validate()` above + /// is filesystem-free — it can only check that a confirmed finding *has* a + /// path, not that the path names a real file. A department is an agent, so + /// an unresolvable or drifted citation is the expected failure mode, and + /// without this pass a fabricated `path` validates green. Every `file` + /// evidence entry must therefore be repo-relative, exist under `repo_root`, + /// and carry a line range that fits the file it points at. + pub(crate) fn validate_evidence_grounding(&self, repo_root: &Path) -> Result<(), String> { + for finding in &self.findings { + for entry in &finding.evidence { + if entry.kind != EvidenceKind::File { + continue; + } + let Some(path) = entry.path.as_deref() else { + continue; + }; + let relative = + repo_relative_path(path, &format!("finding \"{}\" evidence", finding.id))?; + let resolved = repo_root.join(&relative); + let contents = fs::read(&resolved).map_err(|error| { + format!( + "finding \"{}\" cites evidence that does not resolve under the repository: {relative} ({error})", + finding.id + ) + })?; + let Some(line_start) = entry.line_start else { + continue; + }; + let lines = contents.iter().filter(|byte| **byte == b'\n').count() as u64 + 1; + let line_end = entry.line_end.unwrap_or(line_start); + if line_end < line_start { + return Err(format!( + "finding \"{}\" cites {relative} with line_end {line_end} before line_start {line_start}", + finding.id + )); + } + if line_end > lines { + return Err(format!( + "finding \"{}\" cites {relative} lines {line_start}-{line_end} but the file has {lines} lines", + finding.id + )); + } + } + } + Ok(()) + } } #[cfg(test)] diff --git a/crates/code-intel-cli/src/audit_report/validate_tests.rs b/crates/code-intel-cli/src/audit_report/validate_tests.rs index 6b27b98..dc63d6a 100644 --- a/crates/code-intel-cli/src/audit_report/validate_tests.rs +++ b/crates/code-intel-cli/src/audit_report/validate_tests.rs @@ -339,3 +339,66 @@ fn rejects_assessed_department_with_a_not_assessed_coverage_row() { "{error}" ); } + +/// Evidence grounding: a department is an agent, so the failure mode the +/// filesystem-free rules cannot see is a citation that resolves nowhere. +#[test] +fn grounds_the_example_fixture_evidence_in_the_real_tree() { + let report = AuditReport::parse(&fixture_bytes()).unwrap(); + report.validate_evidence_grounding(&repo_root()).unwrap(); +} + +#[test] +fn rejects_file_evidence_that_does_not_exist_in_the_repository() { + let mut value = fixture_value(); + value["findings"][0]["evidence"][0] = json!({ + "kind": "file", + "source": "targeted read", + "path": "crates/code-intel-cli/src/does-not-exist.rs", + "line_start": 1, + "line_end": 2 + }); + let report = AuditReport::parse(&serde_json::to_vec(&value).unwrap()).unwrap(); + let error = report + .validate_evidence_grounding(&repo_root()) + .unwrap_err(); + assert!( + error.contains("does not resolve under the repository"), + "{error}" + ); +} + +#[test] +fn rejects_file_evidence_escaping_the_repository_root() { + let mut value = fixture_value(); + value["findings"][0]["evidence"][0] = json!({ + "kind": "file", + "source": "targeted read", + "path": "../../../etc/passwd" + }); + let report = AuditReport::parse(&serde_json::to_vec(&value).unwrap()).unwrap(); + let error = report + .validate_evidence_grounding(&repo_root()) + .unwrap_err(); + assert!( + error.contains("not portable repo-relative syntax"), + "{error}" + ); +} + +#[test] +fn rejects_file_evidence_citing_lines_past_the_end_of_the_file() { + let mut value = fixture_value(); + value["findings"][0]["evidence"][0] = json!({ + "kind": "file", + "source": "targeted read", + "path": "Cargo.toml", + "line_start": 900_000, + "line_end": 900_001 + }); + let report = AuditReport::parse(&serde_json::to_vec(&value).unwrap()).unwrap(); + let error = report + .validate_evidence_grounding(&repo_root()) + .unwrap_err(); + assert!(error.contains("but the file has"), "{error}"); +} diff --git a/docs/audit-report.md b/docs/audit-report.md index 70a04c5..679c3c1 100644 --- a/docs/audit-report.md +++ b/docs/audit-report.md @@ -50,7 +50,9 @@ Findings must never write secret material in plaintext. A finding about a leaked 8. Every department listed in the report has exactly one `coverage_matrix` row and at most one `score_dashboard` entry. 9. A department whose `status` is `assessed` actually moves the health score: it has a non-null `score_dashboard` entry, and its `coverage_matrix` row is not `not_assessed`. -The registry itself (`orchestration/audit/departments.v1.json`) has its own invariants, checked by `DepartmentRegistry::validate()`: department ids are unique, every rubric file it points at exists on disk, and every `enabled: true` department's prompt file exists on disk. A disabled department may point at a prompt file that does not exist yet — that file is the department ticket's job, not the kernel's. +`validate()` is filesystem-free: it can see that a confirmed finding *has* a `path`, not that the path names a real file. Because a department is an agent, an unresolvable or drifted citation is the expected failure mode, so `validate_evidence_grounding(repo_root)` is a second pass that grounds every `file` evidence entry in the tree it claims to cite: the `path` must be portable repo-relative syntax, must resolve to a file under the repository root, and any `line_start`/`line_end` must be ordered and within that file. `code-intel audit --operation validate --repo ` runs it — that operation holds the repository the report cites. `--operation render` does not, and does not claim to. + +The registry itself (`orchestration/audit/departments.v1.json`) has its own invariants. Its path strings are parsed under the same portable repo-relative contract — the registry is read from `--repo`, so a scanned repository must not be able to name rubric files outside the checkout or point a department's `prompt` (the instruction source an audit agent reads) at an arbitrary host file. `DepartmentRegistry::validate()` then checks: department ids are unique, every rubric file it points at exists on disk, and every `enabled: true` department's prompt file exists on disk. A disabled department may point at a prompt file that does not exist yet — that file is the department ticket's job, not the kernel's. ## Registering a Department