diff --git a/.github/code-scanning-baseline.json b/.github/code-scanning-baseline.json index 0ac2579..30c05dc 100644 --- a/.github/code-scanning-baseline.json +++ b/.github/code-scanning-baseline.json @@ -315,7 +315,7 @@ "pattern_id": "PI060", "digest": "sha256:f50a2555fa562bf13fb99143666c9852bd2939baaff1a3747e5904ef70edb028", "count": 1, - "first_seen_line": 5 + "first_seen_line": 7 }, { "file": "examples/mcp-tool-poisoning-attack.md", @@ -329,7 +329,7 @@ "pattern_id": "PI062", "digest": "sha256:c4a1149fe99bd7fe6f006329a57fea0842b0a97542da130e944a4a555be6e094", "count": 1, - "first_seen_line": 5 + "first_seen_line": 15 }, { "file": "examples/mixed-attack.md", @@ -602,7 +602,7 @@ "pattern_id": "PI051", "digest": "sha256:7de3d557b90e82691c253f55e7d1a14dd7c5842f028bc93a55d828523244f9fc", "count": 1, - "first_seen_line": 6 + "first_seen_line": 7 }, { "file": "examples/tool-permission-abuse-attack.md", @@ -2366,7 +2366,7 @@ "pattern_id": "PI050", "digest": "sha256:50667d8e792a1b186182a00df8a299fbfb4ee697ed42208f25f454f27c9ca753", "count": 1, - "first_seen_line": 4 + "first_seen_line": 7 }, { "file": "tests/corpus/attack/structural/tool-permission-abuse/02-scalar-wildcard-tools-grant.md", diff --git a/Cargo.lock b/Cargo.lock index 668c166..0ab7758 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,12 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "arraydeque" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" + [[package]] name = "autocfg" version = "1.5.1" @@ -321,6 +327,12 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "futures-core" version = "0.3.34" @@ -379,12 +391,30 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "foldhash", +] + [[package]] name = "hashbrown" version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" +[[package]] +name = "hashlink" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7382cf6263419f2d8df38c55d7da83da5c18aef87fc7a7fc1fb1e344edfe14c1" +dependencies = [ + "hashbrown 0.15.5", +] + [[package]] name = "heck" version = "0.5.0" @@ -414,7 +444,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.16.1", ] [[package]] @@ -434,8 +464,10 @@ dependencies = [ "sha2", "thiserror", "toml", + "toml_edit", "unicode-normalization", "unicode-security", + "yaml-rust2", ] [[package]] @@ -1013,6 +1045,16 @@ dependencies = [ "memchr", ] +[[package]] +name = "yaml-rust2" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2462ea039c445496d8793d052e13787f2b90e750b833afee748e601c17621ed9" +dependencies = [ + "arraydeque", + "hashlink", +] + [[package]] name = "zerocopy" version = "0.8.56" diff --git a/Cargo.toml b/Cargo.toml index cdb8081..128df48 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,9 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" serde_yaml = { package = "serde_yaml_ng", version = "0.10" } toml = "0.8" +# Source marks for structural findings (#165); semantic parsing stays unchanged. +toml_edit = { version = "0.22", default-features = false, features = ["parse"] } +yaml-rust2 = { version = "0.10", default-features = false } regex = "1" anyhow = "1" thiserror = "2" diff --git a/README.md b/README.md index ce6eff2..21e7176 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,13 @@ scanned exactly as its uncommented form would be. A config block that still cannot be parsed after that is reported on stderr rather than skipped silently: `warning: structural config pass skipped for — `. +Structural findings track the source key or sequence item through its full +configuration path, so repeated hook commands and URLs report their own lines. +Line-specific suppression applies to that occurrence; an ignore on an earlier +key no longer suppresses later keys with the same name (#165). Baseline identity +still uses matched text and occurrence counts, not line numbers. YAML aliases +are attributed to the alias reference. See [ADR-007](docs/adr/ADR-007-config-source-positions.md). + ### Controlling what gets walked `check .` honours `.gitignore` and never descends into build output — `.git`, diff --git a/docs/adr/ADR-007-config-source-positions.md b/docs/adr/ADR-007-config-source-positions.md new file mode 100644 index 0000000..1cd5a7a --- /dev/null +++ b/docs/adr/ADR-007-config-source-positions.md @@ -0,0 +1,78 @@ +# ADR-007: Path-aware source positions for structural findings + +Date: 2026-10-25 +Status: Proposed + +## Context + +Issue #165: after parsing configuration into `serde_json::Value`, projection +searched the original text for the leaf key and selected its first occurrence. +Repeated command and URL keys therefore shared a source line. Traversal can be +sorted rather than source ordered, so moving a search cursor forward is not a +correct fix. Matching by scalar value also fails for repeated identical values. + +The structural scanner already consumes `ProjectedLine.line` correctly. +Suppression is keyed by this line. The baseline implementation excludes lines +from identity: it hashes matched text and budgets an explicit +occurrence count. SARIF adds occurrence ordinals to the same text digest. +Those contracts must remain stable. + +## Decision + +Build an auxiliary source-position tree and look up each projected leaf by its +actual key/index path. Semantic parsing, canonical projection text, and pattern +matching remain authoritative in their existing implementations. + +- `yaml-rust2` 0.10.4 supplies marked events for YAML and JSON source positions. + Default encoding features are disabled because input is already UTF-8. + JSONC is comment/comma-relaxed first; JSON string literals are decoded and + re-encoded for this position pass to support surrogate pairs and prevent + Unicode line separators inside strings from shifting physical line numbers. + Neither transformation changes the semantic tree or physical newline count. +- `toml_edit` 0.22 supplies immutable parsed spans. This crate was already in + the locked dependency graph through `toml`; it is now also a direct dependency. + Line offsets are indexed once and searched rather than repeatedly scanning + the source prefix. Tables, arrays of tables, arrays, and inline tables are + traversed without cloning TOML subtrees. +- Map values use their key's line, sequence members use their own line, and + alias-expanded YAML leaves use the alias reference. JSON duplicate members + follow the semantic parser's surviving last member. +- Position collection is bounded by the projection depth and an auxiliary + node budget of `MAX_NODES * (MAX_DEPTH + 1)` to accommodate containers as + well as leaves. Alias expansion also consumes this budget. An unavailable + position falls back to the block's first line; projection is never dropped + merely because the optional source-position pass cannot map it. + +Also correct the JSON block start-line calculation for leading blank lines. + +## Consequences + +Repeated keys now have independent source lines in reports, suppression, and +SARIF locations. Consumers may see later findings previously hidden by an +ignore on the first key. Regression tests cover the three real clean hook +fixtures named by the issue, repeated identical commands, reordered objects, +JSONC comments, escaped and unusual keys, Unicode, YAML blocks and aliases, +TOML containers, sequence items, and independent suppression and baselining. + +The JSON/SARIF field sets, baseline schema and fingerprint algorithm are +unchanged. Existing baseline fingerprints remain valid; first-seen line +metadata is regenerated where corrected locations move it. No patterns, +severity levels, or recall expectations change. + +Costs: structured documents need a second parser pass and a bounded position +tree. The lockfile adds `yaml-rust2` and four transitive packages. Tests, release +benchmarks, and the existing performance guard check this path. The source-map +fallback remains best effort for unsupported or budget-exceeding structures. + +## Alternatives considered + +- Per-key occurrence counters depend on source traversal order and cannot + resolve repeated keys in reordered objects, comments, or string values. +- Matching key/value text cannot distinguish identical repeated commands. +- A handwritten source lexer for three configuration languages would duplicate + string escapes, flow syntax, aliases, and multiline-string handling. +- Replacing the semantic parsers would broaden compatibility risk beyond the + source-attribution bug. The additional parsers supply positions only. + +References: [yaml-rust2 marked events](https://docs.rs/yaml-rust2/0.10.4/yaml_rust2/parser/index.html), +[toml_edit immutable documents](https://docs.rs/toml_edit/0.22.27/toml_edit/struct.ImDocument.html). diff --git a/src/frontmatter.rs b/src/frontmatter.rs index 490e2c3..1b6cb5a 100644 --- a/src/frontmatter.rs +++ b/src/frontmatter.rs @@ -54,6 +54,8 @@ use serde_json::Value; +mod source; + /// Maximum nesting depth walked when projecting. Deeper nodes are dropped. /// /// Real agent configuration is shallow: `mcpServers..args[0]` is depth 4. @@ -100,12 +102,9 @@ pub struct ConfigBlock { pub struct ProjectedLine { pub path: String, pub value: String, - /// Best-effort 1-based line in the ORIGINAL document. - /// - /// Parsers do not preserve source spans through `serde_json::Value`, so - /// this is resolved by searching the block for the leaf key. When the key - /// cannot be found the block's own start line is used — a slightly wrong - /// line is far better than a finding that cannot be located at all. + /// 1-based source key (or sequence item) line in the ORIGINAL document. + /// Source positions follow the full tree path, independently of map order. + /// If the auxiliary source map is unavailable, use the block's start line. pub line: usize, } @@ -136,7 +135,11 @@ pub fn extract(content: &str) -> Option { let trimmed = content.trim_start(); if trimmed.starts_with('{') { let leading_blank = content.len() - trimmed.len(); - let start_line = content[..leading_blank].lines().count().max(1); + let start_line = content[..leading_blank] + .bytes() + .filter(|&b| b == b'\n') + .count() + + 1; return Some(ConfigBlock { syntax: ConfigSyntax::Json, body: trimmed.to_string(), @@ -167,10 +170,8 @@ pub fn extract(content: &str) -> Option { .count(); return Some(ConfigBlock { syntax: ConfigSyntax::Json, - // The ORIGINAL text from the `{` onward, comments intact — - // `locate` searches this raw body for a leaf key, and a - // relaxed (space-blanked) body would just make every key - // search fail. + // Keep the original body so source positions and diagnostics + // refer to the document the user actually supplied. body: content[brace_offset..].to_string(), start_line, }); @@ -185,8 +186,8 @@ pub fn extract(content: &str) -> Option { /// Returns a string of the SAME byte length and the SAME number of line /// breaks as `body`, with comment bytes and trailing-comma bytes overwritten /// by ASCII spaces, and nothing inside a string literal touched. That -/// invariant is what lets [`locate`]'s `block.body.lines()` arithmetic, -/// `block.start_line`, and every reported line number stay valid against the +/// invariant is what lets source marks, `block.start_line`, and every reported +/// line number stay valid against the /// ORIGINAL document rather than the relaxed one. /// /// A single forward pass over `body.as_bytes()`, tracking whether the cursor @@ -408,7 +409,15 @@ pub fn parse(block: &ConfigBlock) -> Result { /// Project a parsed tree into canonical `path = value` lines. pub fn project(value: &Value, block: &ConfigBlock) -> Vec { let mut out = Vec::new(); - walk(value, &mut String::new(), 0, block, &mut out); + let positions = source::positions(block); + walk( + value, + &mut String::new(), + 0, + block, + positions.as_ref(), + &mut out, + ); out } @@ -417,6 +426,7 @@ fn walk( path: &mut String, depth: usize, block: &ConfigBlock, + source: Option<&source::Source>, out: &mut Vec, ) { if depth > MAX_DEPTH || out.len() >= MAX_NODES { @@ -430,7 +440,14 @@ fn walk( path.push('.'); } path.push_str(key); - walk(child, path, depth + 1, block, out); + walk( + child, + path, + depth + 1, + block, + source.and_then(|s| s.key(key)), + out, + ); path.truncate(saved); } } @@ -438,7 +455,14 @@ fn walk( for (index, child) in items.iter().enumerate() { let saved = path.len(); path.push_str(&format!("[{index}]")); - walk(child, path, depth + 1, block, out); + walk( + child, + path, + depth + 1, + block, + source.and_then(|s| s.index(index)), + out, + ); path.truncate(saved); } } @@ -461,7 +485,9 @@ fn walk( rendered.truncate(end); } out.push(ProjectedLine { - line: locate(path, block), + line: source.map_or(block.start_line, |s| { + block.start_line + s.line.saturating_sub(1) + }), path: path.clone(), value: rendered, }); @@ -481,35 +507,6 @@ fn render_scalar(value: &Value) -> String { } } -/// Best-effort mapping from a projected path back to an original line. -/// -/// Searches the block body for the leaf key. Exact source spans would need a -/// span-preserving parser for all three syntaxes; a near-right line is worth far -/// more than the complexity, because the finding still points a reader at the -/// correct region. -fn locate(path: &str, block: &ConfigBlock) -> usize { - let leaf = path - .rsplit('.') - .next() - .unwrap_or(path) - .trim_end_matches(|c: char| c == ']' || c.is_ascii_digit()) - .trim_end_matches('['); - if leaf.is_empty() { - return block.start_line; - } - for (offset, line) in block.body.lines().enumerate() { - let trimmed = line.trim_start().trim_start_matches("- "); - let key_end = trimmed.find([':', '=']).unwrap_or(0); - if key_end > 0 { - let key = trimmed[..key_end].trim().trim_matches('"'); - if key == leaf { - return block.start_line + offset; - } - } - } - block.start_line -} - /// Extract, parse and project in one step. /// /// `Ok(None)` means "no configuration here", which is the common case and not a diff --git a/src/frontmatter/source.rs b/src/frontmatter/source.rs new file mode 100644 index 0000000..51469a9 --- /dev/null +++ b/src/frontmatter/source.rs @@ -0,0 +1,350 @@ +//! Source positions only. The existing parsers remain authoritative for values. +use std::collections::HashMap; + +use yaml_rust2::parser::{Event, MarkedEventReceiver, Parser}; +use yaml_rust2::scanner::Marker; + +use super::{relax_jsonc, ConfigBlock, ConfigSyntax, MAX_DEPTH, MAX_NODES}; + +// Projection counts leaves; the position tree also needs their container nodes. +const SOURCE_NODE_LIMIT: usize = MAX_NODES * (MAX_DEPTH + 1); + +#[derive(Clone, Default)] +pub(super) struct Source { + pub line: usize, + key_text: String, + keys: HashMap, + items: Vec, +} + +impl Source { + pub fn key(&self, key: &str) -> Option<&Self> { + self.keys.get(key) + } + + pub fn index(&self, index: usize) -> Option<&Self> { + self.items.get(index) + } +} + +pub(super) fn positions(block: &ConfigBlock) -> Option { + match block.syntax { + ConfigSyntax::Json | ConfigSyntax::Yaml => { + let body = if block.syntax == ConfigSyntax::Json { + json_source(&relax_jsonc(&block.body))? + } else { + block.body.clone() + }; + let mut receiver = Locations::default(); + Parser::new_from_str(&body) + .load(&mut receiver, false) + .ok()?; + receiver.root + } + ConfigSyntax::Toml => { + let document = toml_edit::ImDocument::parse(&block.body).ok()?; + let newlines: Vec<_> = block + .body + .match_indices('\n') + .map(|(index, _)| index) + .collect(); + let mut remaining = SOURCE_NODE_LIMIT; + Some(toml_item(document.as_item(), &newlines, 0, &mut remaining)) + } + } +} + +// YAML's quoted-string grammar does not accept JSON surrogate-pair escapes. +// Decode/re-encode just strings for the position parser. This preserves physical +// newlines, including escaped newlines, while the semantic JSON tree is untouched. +fn json_source(body: &str) -> Option { + let bytes = body.as_bytes(); + let mut out = String::with_capacity(body.len()); + let mut copied = 0; + let mut cursor = 0; + while cursor < bytes.len() { + if bytes[cursor] != b'"' { + cursor += 1; + continue; + } + let start = cursor; + cursor += 1; + while cursor < bytes.len() { + match bytes[cursor] { + b'\\' => cursor += 2, + b'"' => break, + _ => cursor += 1, + } + } + cursor += 1; + let text: String = serde_json::from_str(body.get(start..cursor)?).ok()?; + let encoded = serde_json::to_string(&text).ok()?; + out.push_str(&body[copied..start]); + out.push_str( + &encoded + .replace('\u{85}', "\\u0085") + .replace('\u{2028}', "\\u2028") + .replace('\u{2029}', "\\u2029"), + ); + copied = cursor; + } + out.push_str(&body[copied..]); + Some(out) +} + +struct Frame { + node: Source, + mapping: bool, + key: Option<(String, usize)>, + anchor: usize, +} + +#[derive(Default)] +struct Locations { + root: Option, + stack: Vec, + anchors: HashMap, + nodes: usize, + skipped: usize, +} + +impl Locations { + fn attach(&mut self, mut node: Source) { + if let Some(parent) = self.stack.last_mut() { + if parent.mapping { + if let Some((key, line)) = parent.key.take() { + node.line = line; + parent.node.keys.insert(key, node); + } else { + parent.key = Some((node.key_text, node.line)); + } + } else { + parent.node.items.push(node); + } + } else { + self.root = Some(node); + } + } +} + +impl MarkedEventReceiver for Locations { + fn on_event(&mut self, event: Event, mark: Marker) { + // Keep the auxiliary position tree within the projection's own bounds. + // Once a container is skipped, consume its balanced events, not siblings. + if self.skipped > 0 { + match event { + Event::MappingStart(..) | Event::SequenceStart(..) => self.skipped += 1, + Event::MappingEnd | Event::SequenceEnd => self.skipped -= 1, + Event::Nothing + | Event::StreamStart + | Event::StreamEnd + | Event::DocumentStart + | Event::DocumentEnd + | Event::Scalar(..) + | Event::Alias(_) => {} + } + return; + } + match event { + Event::MappingStart(anchor, _) | Event::SequenceStart(anchor, _) => { + if self.stack.len() > MAX_DEPTH || self.nodes >= SOURCE_NODE_LIMIT { + self.skipped = 1; + // Consume the map's pending key even when its value is skipped. + self.attach(Source { + line: mark.line(), + ..Source::default() + }); + return; + } + self.nodes += 1; + self.stack.push(Frame { + node: Source { + line: mark.line(), + ..Source::default() + }, + mapping: matches!(event, Event::MappingStart(..)), + key: None, + anchor, + }); + } + Event::MappingEnd | Event::SequenceEnd => { + if let Some(frame) = self.stack.pop() { + if frame.anchor != 0 { + self.anchors.insert(frame.anchor, frame.node.clone()); + } + self.attach(frame.node); + } + } + Event::Scalar(text, _, anchor, _) => { + let is_key = self + .stack + .last() + .is_some_and(|frame| frame.mapping && frame.key.is_none()); + if self.nodes >= SOURCE_NODE_LIMIT { + return; + } + if !is_key { + self.nodes += 1; + } + let node = Source { + line: mark.line(), + key_text: if is_key || anchor != 0 { + text + } else { + String::new() + }, + ..Source::default() + }; + if anchor != 0 { + self.anchors.insert(anchor, node.clone()); + } + self.attach(node); + } + Event::Alias(anchor) => { + if let Some(node) = self.anchors.get(&anchor) { + let mut remaining = SOURCE_NODE_LIMIT.saturating_sub(self.nodes); + if let Some(copy) = alias_at(node, mark.line(), &mut remaining) { + self.nodes = SOURCE_NODE_LIMIT - remaining; + self.attach(copy); + } + } + } + Event::Nothing + | Event::StreamStart + | Event::StreamEnd + | Event::DocumentStart + | Event::DocumentEnd => {} + } + } +} + +// An alias is attributed to its reference, not the anchor's earlier declaration. +fn alias_at(node: &Source, line: usize, remaining: &mut usize) -> Option { + *remaining = remaining.checked_sub(1)?; + Some(Source { + line, + key_text: node.key_text.clone(), + keys: node + .keys + .iter() + .filter_map(|(key, child)| { + alias_at(child, line, remaining).map(|copy| (key.clone(), copy)) + }) + .collect(), + items: node + .items + .iter() + .filter_map(|child| alias_at(child, line, remaining)) + .collect(), + }) +} + +fn source_line(newlines: &[usize], offset: usize) -> usize { + newlines.partition_point(|&newline| newline < offset) + 1 +} + +fn toml_item( + item: &toml_edit::Item, + newlines: &[usize], + depth: usize, + remaining: &mut usize, +) -> Source { + if depth > MAX_DEPTH || *remaining == 0 { + return Source::default(); + } + *remaining -= 1; + let mut node = Source { + line: item.span().map_or(1, |s| source_line(newlines, s.start)), + ..Source::default() + }; + match item { + toml_edit::Item::None => {} + toml_edit::Item::Value(value) => return toml_value(value, newlines, depth, remaining), + toml_edit::Item::Table(table) => { + node.keys = toml_table(table, newlines, depth, remaining); + } + toml_edit::Item::ArrayOfTables(tables) => { + for table in tables.iter().take(MAX_NODES) { + if *remaining == 0 { + break; + } + *remaining -= 1; + node.items.push(Source { + line: table.span().map_or(1, |s| source_line(newlines, s.start)), + keys: toml_table(table, newlines, depth + 1, remaining), + ..Source::default() + }); + } + } + } + node +} + +fn toml_table( + table: &toml_edit::Table, + newlines: &[usize], + depth: usize, + remaining: &mut usize, +) -> HashMap { + let mut keys = HashMap::new(); + if depth > MAX_DEPTH { + return keys; + } + for (key, child) in table.iter().take(MAX_NODES) { + if *remaining == 0 { + break; + } + let mut source = toml_item(child, newlines, depth + 1, remaining); + if let Some(span) = table.key(key).and_then(|key| key.span()) { + source.line = source_line(newlines, span.start); + } + keys.insert(key.to_owned(), source); + } + keys +} + +fn toml_value( + value: &toml_edit::Value, + newlines: &[usize], + depth: usize, + remaining: &mut usize, +) -> Source { + let mut node = Source { + line: value.span().map_or(1, |s| source_line(newlines, s.start)), + ..Source::default() + }; + if depth > MAX_DEPTH || *remaining == 0 { + return node; + } + match value { + toml_edit::Value::Array(array) => { + for child in array.iter().take(MAX_NODES) { + if *remaining == 0 { + break; + } + *remaining = remaining.saturating_sub(1); + node.items + .push(toml_value(child, newlines, depth + 1, remaining)); + } + } + toml_edit::Value::InlineTable(table) => { + for (key, child) in table.iter().take(MAX_NODES) { + if *remaining == 0 { + break; + } + *remaining = remaining.saturating_sub(1); + let mut source = toml_value(child, newlines, depth + 1, remaining); + if let Some(span) = table.key(key).and_then(|key| key.span()) { + source.line = source_line(newlines, span.start); + } + node.keys.insert(key.to_owned(), source); + } + } + toml_edit::Value::String(_) + | toml_edit::Value::Integer(_) + | toml_edit::Value::Float(_) + | toml_edit::Value::Boolean(_) + | toml_edit::Value::Datetime(_) => {} + } + node +} diff --git a/tests/config_source_lines_test.rs b/tests/config_source_lines_test.rs new file mode 100644 index 0000000..485c00d --- /dev/null +++ b/tests/config_source_lines_test.rs @@ -0,0 +1,230 @@ +use injection_scanner::allowlist::{parse_suppressions, Suppressions}; +use injection_scanner::baseline::Baseline; +use injection_scanner::frontmatter::analyze; + +mod common; + +fn leaf_lines(document: &str, leaf: &str) -> Vec<(String, usize)> { + let (_, projected) = analyze(document).unwrap().unwrap(); + projected + .into_iter() + .filter(|p| p.path.ends_with(leaf)) + .map(|p| (p.path, p.line)) + .collect() +} + +#[test] +fn repeated_json_keys_follow_the_full_path_not_sorted_map_order() { + let document = "\n\n{\n \"z\": {\n \"command\": \"same\"\n },\n \"a\": {\n \"command\": \"same\"\n }\n}"; + assert_eq!( + leaf_lines(document, "command"), + [("a.command".into(), 8), ("z.command".into(), 5)] + ); +} + +#[test] +fn jsonc_ignores_decoy_keys_in_comments_and_maps_escaped_keys() { + let document = "{\n // \"command\": \"decoy\"\n \"hooks\": [\n {\"comm\\u0061nd\": \"same\"},\n {\"command\":\n \"same\"},\n ],\n}"; + assert_eq!( + leaf_lines(document, "command"), + [ + ("hooks[0].command".into(), 4), + ("hooks[1].command".into(), 5) + ] + ); +} + +#[test] +fn duplicate_json_members_use_the_surviving_last_members_location() { + assert_eq!( + leaf_lines( + "{\n \"command\": \"first\",\n \"command\": \"last\"\n}", + "command" + ), + [("command".into(), 3)] + ); +} + +#[test] +fn json_surrogate_pairs_and_escaped_slashes_do_not_disable_source_mapping() { + let document = "{\r\n \"note\": \"\\uD83D\\uDE00 https:\\/\\/example.org\",\r\n \"hooks\": [\r\n {\"command\": \"same\"},\r\n {\"command\": \"same\"}\r\n ]\r\n}"; + assert_eq!( + leaf_lines(document, "command"), + [ + ("hooks[0].command".into(), 4), + ("hooks[1].command".into(), 5) + ] + ); +} + +#[test] +fn literal_dots_brackets_and_digits_in_keys_keep_the_actual_tree_location() { + let document = "{\n \"a.b\": {\"command2\": \"same\"},\n \"a\": {\"b\": {\"command2\": \"same\"}},\n \"x[0]\": {\"command2\": \"same\"}\n}"; + let lines = leaf_lines(document, "command2"); + assert_eq!( + lines.iter().map(|(_, line)| *line).collect::>(), + [3, 2, 4] + ); +} + +#[test] +fn yaml_sequences_block_scalars_and_aliases_keep_their_source_lines() { + let document = "---\nhooks:\n z:\n - command: &cmd |\n same\n - command: *cmd\n a:\n - command: same\n---\n"; + assert_eq!( + leaf_lines(document, "command"), + [ + ("hooks.a[0].command".into(), 8), + ("hooks.z[0].command".into(), 4), + ("hooks.z[1].command".into(), 6), + ] + ); +} + +#[test] +fn a_yaml_mapping_alias_is_attributed_to_its_reference() { + let document = "---\nz: &hook\n command: same\na: *hook\n---\n"; + assert_eq!( + leaf_lines(document, "command"), + [("a.command".into(), 4), ("z.command".into(), 3)] + ); +} + +#[test] +fn toml_tables_arrays_and_inline_tables_keep_distinct_key_lines() { + let document = "+++\n[[hooks.z]]\ncommand = '''\nsame\n'''\n[[hooks.z]]\ncommand = 'same'\n[hooks.a]\ncommand = 'same'\ninline = { command = 'same' }\n+++\n"; + assert_eq!( + leaf_lines(document, "command"), + [ + ("hooks.a.command".into(), 9), + ("hooks.a.inline.command".into(), 10), + ("hooks.z[0].command".into(), 3), + ("hooks.z[1].command".into(), 7), + ] + ); +} + +#[test] +fn sequence_scalars_are_attributed_to_the_item_not_the_array_key() { + for document in [ + "{\n \"args\": [\n \"one\",\n \"two\"\n ]\n}", + "---\nargs:\n - one\n - two\n---", + "+++\nargs = [\n 'one',\n 'two'\n]\n+++", + ] { + let (_, projected) = analyze(document).unwrap().unwrap(); + assert_eq!(projected.iter().map(|p| p.line).collect::>(), [3, 4]); + } +} + +#[test] +fn positions_cover_container_heavy_documents_within_the_projection_budget() { + let entries = (0..3_000) + .map(|_| "{\"command\": \"same\"}") + .collect::>() + .join(",\n"); + let document = format!("{{\"hooks\":[\n{entries}\n]}}"); + let lines = leaf_lines(&document, "command"); + assert_eq!(lines.len(), 3_000); + assert_eq!(lines.last().unwrap().1, 3_001); +} + +#[test] +fn unicode_json_line_separator_characters_do_not_shift_physical_lines() { + let document = "{\n \"note\": \"a\u{85}b\u{2028}c\u{2029}d\",\n \"command\": \"same\"\n}"; + assert_eq!(leaf_lines(document, "command"), [("command".into(), 3)]); +} + +#[test] +fn real_repeated_hook_keys_point_to_their_own_lines() { + for document in [ + include_str!("corpus/clean/persistence-legitimate-hooks-config.json"), + include_str!("corpus/clean/persistence-root-form-hooks-config.json"), + include_str!("corpus/clean/persistence-corporate-audit-endpoint.json"), + ] { + for leaf in ["command", "bash", "powershell", "url"] { + let actual: Vec<_> = leaf_lines(document, leaf) + .into_iter() + .map(|(_, line)| line) + .collect(); + let mut expected: Vec<_> = document + .lines() + .enumerate() + .filter_map(|(index, line)| { + line.trim_start() + .starts_with(&format!("\"{leaf}\":")) + .then_some(index + 1) + }) + .collect(); + let mut actual = actual; + actual.sort_unstable(); + expected.sort_unstable(); + assert_eq!(actual, expected, "{leaf}"); + } + } +} + +const HOOKS: &str = "{\n \"hooks\": {\n \"SessionStart\": [\n {\"command\": \"cat ~/.aws/credentials\"},\n {\"command\": \"cat ~/.aws/credentials\"}\n ]\n }\n}"; + +#[test] +fn each_repeated_command_can_be_suppressed_independently() { + for suppressed_line in [4, 5] { + let document = HOOKS + .lines() + .enumerate() + .map(|(index, line)| { + if index + 1 == suppressed_line { + format!("{line} // injection-scanner:ignore PI077") + } else { + line.into() + } + }) + .collect::>() + .join("\n"); + let report = + common::scanner().scan("hooks.jsonc", &document, &parse_suppressions(&document)); + let live: Vec<_> = report + .matches + .iter() + .filter(|m| m.pattern_id == "PI077") + .map(|m| m.line) + .collect(); + let suppressed: Vec<_> = report + .suppressed + .iter() + .filter(|m| m.pattern_id == "PI077") + .map(|m| m.line) + .collect(); + assert_eq!(suppressed, [suppressed_line]); + assert_eq!(live, [9 - suppressed_line]); + } +} + +#[test] +fn accepting_one_structural_finding_does_not_accept_a_second_occurrence() { + let scanner = common::scanner(); + let first = scanner.scan("hooks.json", HOOKS, &Suppressions::default()); + assert_eq!( + first + .matches + .iter() + .filter(|m| m.pattern_id == "PI077") + .map(|m| m.line) + .collect::>(), + [4, 5] + ); + let mut accepted = first.clone(); + accepted + .matches + .retain(|m| m.pattern_id == "PI077" && m.line == 4); + let baseline = Baseline::from_reports(&[accepted]); + let mut reports = vec![first]; + baseline.apply(&mut reports); + assert_eq!( + reports[0] + .matches + .iter() + .filter(|m| m.pattern_id == "PI077") + .map(|m| m.line) + .collect::>(), + [5] + ); +}