Skip to content

Commit 2c66c17

Browse files
committed
fix(audit): ground evidence paths and reject registry paths that escape the repo
Two holes the audit kernel's own departments found in it, same root cause: a path string was trusted because it was well-formed. - validate() is filesystem-free, so a confirmed finding's `path` was only checked for being non-empty. A department is an agent; a fabricated or drifted citation validated green. validate_evidence_grounding(repo_root) now resolves every file evidence entry under the repository, requires it to exist, and requires any line range to be ordered and within the file. `audit --operation validate --repo <root>` runs it; `render` has no repo and does not claim to. - The department registry is read from --repo — a scanned repository's own file — but its rubric and prompt path strings were joined onto repo_root and only `.is_file()`-checked, so an absolute path replaced the base and `..` escaped it. A target repo could satisfy the "rubric files exist" invariant with host files and redirect a department's prompt, which is the instruction source an audit agent reads. Both are now parsed under the portable repo-relative contract artifact_ref.rs already enforces. Found by the ai-safety (001) and security (002) departments auditing this repository with the T1-T4 kernel.
1 parent d5b3394 commit 2c66c17

6 files changed

Lines changed: 226 additions & 9 deletions

File tree

crates/code-intel-cli/src/audit_report/cli.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,15 @@ fn set_once(slot: &mut Option<String>, value: &str, flag: &str) -> Result<(), St
8686

8787
/// The full validate pipeline the CLI runs against a real repository: read
8888
/// the report file, parse it structurally, load and self-validate the
89-
/// on-disk department registry, then check the report against it.
89+
/// on-disk department registry, check the report against it, and — because
90+
/// this path has the repository the report cites — ground every file evidence
91+
/// entry in that tree.
9092
fn validate(repo: &Path, report_path: &Path) -> Result<Value, String> {
9193
let bytes = read_report(report_path)?;
9294
let report = AuditReport::parse(&bytes)?;
9395
let registry = DepartmentRegistry::load(repo)?;
9496
registry.validate(repo)?;
97+
report.validate_evidence_grounding(repo)?;
9598
validate_report(&report, &registry)
9699
}
97100

crates/code-intel-cli/src/audit_report/registry.rs

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,34 @@ use super::json_helpers::{closed_object, required_bool, required_str, required_s
99
// orchestration/audit/departments.v1.json registry
1010
// ---------------------------------------------------------------------
1111

12+
/// Registry path strings name files *inside* the repository under audit, and
13+
/// the registry itself is read from `--repo` — a target repo's own file. A
14+
/// path that escapes the checkout would let a scanned repository satisfy the
15+
/// kernel's "these files exist" invariant with files anywhere on the host, and
16+
/// point a department's `prompt` — the instruction source an audit agent reads
17+
/// — outside the tree the operator pointed at. Same portable-relative contract
18+
/// `artifact_ref.rs` already enforces for artifact paths.
19+
pub(crate) fn repo_relative_path(value: &str, label: &str) -> Result<String, String> {
20+
if value.is_empty()
21+
|| value.contains('\0')
22+
|| value.contains('\\')
23+
|| value.starts_with('/')
24+
|| value.contains(':')
25+
{
26+
return Err(format!(
27+
"{label} path is not portable repo-relative syntax: {value}"
28+
));
29+
}
30+
for component in value.split('/') {
31+
if component.is_empty() || component == "." || component == ".." {
32+
return Err(format!(
33+
"{label} path is not portable repo-relative syntax: {value}"
34+
));
35+
}
36+
}
37+
Ok(value.to_string())
38+
}
39+
1240
pub(crate) struct RubricPaths {
1341
pub(crate) severity: String,
1442
pub(crate) confidence: String,
@@ -26,11 +54,26 @@ impl RubricPaths {
2654
"rubrics",
2755
)?;
2856
Ok(Self {
29-
severity: required_str(object, "severity", "rubrics")?,
30-
confidence: required_str(object, "confidence", "rubrics")?,
31-
evidence: required_str(object, "evidence", "rubrics")?,
32-
coverage: required_str(object, "coverage", "rubrics")?,
33-
scoring: required_str(object, "scoring", "rubrics")?,
57+
severity: repo_relative_path(
58+
&required_str(object, "severity", "rubrics")?,
59+
"rubrics.severity",
60+
)?,
61+
confidence: repo_relative_path(
62+
&required_str(object, "confidence", "rubrics")?,
63+
"rubrics.confidence",
64+
)?,
65+
evidence: repo_relative_path(
66+
&required_str(object, "evidence", "rubrics")?,
67+
"rubrics.evidence",
68+
)?,
69+
coverage: repo_relative_path(
70+
&required_str(object, "coverage", "rubrics")?,
71+
"rubrics.coverage",
72+
)?,
73+
scoring: repo_relative_path(
74+
&required_str(object, "scoring", "rubrics")?,
75+
"rubrics.scoring",
76+
)?,
3477
})
3578
}
3679

@@ -84,7 +127,10 @@ impl DepartmentEntry {
84127
id,
85128
title: required_str(object, "title", "department entry")?,
86129
enabled: required_bool(object, "enabled", "department entry")?,
87-
prompt: required_str(object, "prompt", "department entry")?,
130+
prompt: repo_relative_path(
131+
&required_str(object, "prompt", "department entry")?,
132+
"department prompt",
133+
)?,
88134
consumes,
89135
applicability_check: required_str(object, "applicabilityCheck", "department entry")?,
90136
tracking_issue: required_str(object, "trackingIssue", "department entry")?,

crates/code-intel-cli/src/audit_report/registry_tests.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,60 @@ fn registry_rejects_duplicate_department_ids() {
3737
assert!(error.contains("duplicate department id"), "{error}");
3838
}
3939

40+
/// The registry is read from `--repo` — a scanned repository's own file — so
41+
/// a path that leaves the checkout must not load at all.
42+
#[test]
43+
fn registry_rejects_rubric_path_escaping_the_repository() {
44+
let value = json!({
45+
"schema": "code-intel-audit-departments.v1",
46+
"catalogVersion": "1.0.0",
47+
"rubrics": {
48+
"severity": "../../../etc/passwd",
49+
"confidence": "orchestration/audit/rubrics/confidence.md",
50+
"evidence": "orchestration/audit/rubrics/evidence.md",
51+
"coverage": "orchestration/audit/rubrics/coverage.md",
52+
"scoring": "orchestration/audit/rubrics/scoring.md"
53+
},
54+
"findingContract": "docs/audit-report.md",
55+
"departments": []
56+
});
57+
let error = match DepartmentRegistry::from_value(&value) {
58+
Ok(_) => panic!("registry loaded a path that escapes the repository root"),
59+
Err(error) => error,
60+
};
61+
assert!(
62+
error.contains("not portable repo-relative syntax"),
63+
"{error}"
64+
);
65+
}
66+
67+
#[test]
68+
fn registry_rejects_absolute_department_prompt_path() {
69+
let value = json!({
70+
"schema": "code-intel-audit-departments.v1",
71+
"catalogVersion": "1.0.0",
72+
"rubrics": {
73+
"severity": "orchestration/audit/rubrics/severity.md",
74+
"confidence": "orchestration/audit/rubrics/confidence.md",
75+
"evidence": "orchestration/audit/rubrics/evidence.md",
76+
"coverage": "orchestration/audit/rubrics/coverage.md",
77+
"scoring": "orchestration/audit/rubrics/scoring.md"
78+
},
79+
"findingContract": "docs/audit-report.md",
80+
"departments": [
81+
{"id":"security","title":"Security","enabled":true,"prompt":"/etc/attacker-prompt.md","consumes":[],"applicabilityCheck":"always","trackingIssue":"t"}
82+
]
83+
});
84+
let error = match DepartmentRegistry::from_value(&value) {
85+
Ok(_) => panic!("registry loaded a path that escapes the repository root"),
86+
Err(error) => error,
87+
};
88+
assert!(
89+
error.contains("not portable repo-relative syntax"),
90+
"{error}"
91+
);
92+
}
93+
4094
#[test]
4195
fn registry_rejects_missing_rubric_file() {
4296
let value = json!({

crates/code-intel-cli/src/audit_report/validate.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
use std::{fs, path::Path};
2+
13
use super::{
24
enums::{Coverage, DepartmentRunStatus, EvidenceKind, FindingStatus},
35
model::AuditReport,
4-
registry::DepartmentRegistry,
6+
registry::{repo_relative_path, DepartmentRegistry},
57
};
68

79
fn round1(value: f64) -> f64 {
@@ -310,6 +312,53 @@ impl AuditReport {
310312

311313
Ok(())
312314
}
315+
316+
/// Grounds file evidence in the tree it claims to cite. `validate()` above
317+
/// is filesystem-free — it can only check that a confirmed finding *has* a
318+
/// path, not that the path names a real file. A department is an agent, so
319+
/// an unresolvable or drifted citation is the expected failure mode, and
320+
/// without this pass a fabricated `path` validates green. Every `file`
321+
/// evidence entry must therefore be repo-relative, exist under `repo_root`,
322+
/// and carry a line range that fits the file it points at.
323+
pub(crate) fn validate_evidence_grounding(&self, repo_root: &Path) -> Result<(), String> {
324+
for finding in &self.findings {
325+
for entry in &finding.evidence {
326+
if entry.kind != EvidenceKind::File {
327+
continue;
328+
}
329+
let Some(path) = entry.path.as_deref() else {
330+
continue;
331+
};
332+
let relative =
333+
repo_relative_path(path, &format!("finding \"{}\" evidence", finding.id))?;
334+
let resolved = repo_root.join(&relative);
335+
let contents = fs::read(&resolved).map_err(|error| {
336+
format!(
337+
"finding \"{}\" cites evidence that does not resolve under the repository: {relative} ({error})",
338+
finding.id
339+
)
340+
})?;
341+
let Some(line_start) = entry.line_start else {
342+
continue;
343+
};
344+
let lines = contents.iter().filter(|byte| **byte == b'\n').count() as u64 + 1;
345+
let line_end = entry.line_end.unwrap_or(line_start);
346+
if line_end < line_start {
347+
return Err(format!(
348+
"finding \"{}\" cites {relative} with line_end {line_end} before line_start {line_start}",
349+
finding.id
350+
));
351+
}
352+
if line_end > lines {
353+
return Err(format!(
354+
"finding \"{}\" cites {relative} lines {line_start}-{line_end} but the file has {lines} lines",
355+
finding.id
356+
));
357+
}
358+
}
359+
}
360+
Ok(())
361+
}
313362
}
314363

315364
#[cfg(test)]

crates/code-intel-cli/src/audit_report/validate_tests.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,3 +339,66 @@ fn rejects_assessed_department_with_a_not_assessed_coverage_row() {
339339
"{error}"
340340
);
341341
}
342+
343+
/// Evidence grounding: a department is an agent, so the failure mode the
344+
/// filesystem-free rules cannot see is a citation that resolves nowhere.
345+
#[test]
346+
fn grounds_the_example_fixture_evidence_in_the_real_tree() {
347+
let report = AuditReport::parse(&fixture_bytes()).unwrap();
348+
report.validate_evidence_grounding(&repo_root()).unwrap();
349+
}
350+
351+
#[test]
352+
fn rejects_file_evidence_that_does_not_exist_in_the_repository() {
353+
let mut value = fixture_value();
354+
value["findings"][0]["evidence"][0] = json!({
355+
"kind": "file",
356+
"source": "targeted read",
357+
"path": "crates/code-intel-cli/src/does-not-exist.rs",
358+
"line_start": 1,
359+
"line_end": 2
360+
});
361+
let report = AuditReport::parse(&serde_json::to_vec(&value).unwrap()).unwrap();
362+
let error = report
363+
.validate_evidence_grounding(&repo_root())
364+
.unwrap_err();
365+
assert!(
366+
error.contains("does not resolve under the repository"),
367+
"{error}"
368+
);
369+
}
370+
371+
#[test]
372+
fn rejects_file_evidence_escaping_the_repository_root() {
373+
let mut value = fixture_value();
374+
value["findings"][0]["evidence"][0] = json!({
375+
"kind": "file",
376+
"source": "targeted read",
377+
"path": "../../../etc/passwd"
378+
});
379+
let report = AuditReport::parse(&serde_json::to_vec(&value).unwrap()).unwrap();
380+
let error = report
381+
.validate_evidence_grounding(&repo_root())
382+
.unwrap_err();
383+
assert!(
384+
error.contains("not portable repo-relative syntax"),
385+
"{error}"
386+
);
387+
}
388+
389+
#[test]
390+
fn rejects_file_evidence_citing_lines_past_the_end_of_the_file() {
391+
let mut value = fixture_value();
392+
value["findings"][0]["evidence"][0] = json!({
393+
"kind": "file",
394+
"source": "targeted read",
395+
"path": "Cargo.toml",
396+
"line_start": 900_000,
397+
"line_end": 900_001
398+
});
399+
let report = AuditReport::parse(&serde_json::to_vec(&value).unwrap()).unwrap();
400+
let error = report
401+
.validate_evidence_grounding(&repo_root())
402+
.unwrap_err();
403+
assert!(error.contains("but the file has"), "{error}");
404+
}

docs/audit-report.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ Findings must never write secret material in plaintext. A finding about a leaked
5050
8. Every department listed in the report has exactly one `coverage_matrix` row and at most one `score_dashboard` entry.
5151
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`.
5252

53-
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.
53+
`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 <root>` runs it — that operation holds the repository the report cites. `--operation render` does not, and does not claim to.
54+
55+
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.
5456

5557
## Registering a Department
5658

0 commit comments

Comments
 (0)