fix: preserve source lines for repeated configuration keys

.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",

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"

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 <file> — <reason>`.
 
+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`,

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).

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.<name>.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<ConfigBlock> {
     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<ConfigBlock> {
                 .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<ConfigBlock> {
 /// 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<Value, String> {
 /// Project a parsed tree into canonical `path = value` lines.
 pub fn project(value: &Value, block: &ConfigBlock) -> Vec<ProjectedLine> {
     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<ProjectedLine>,
 ) {
     if depth > MAX_DEPTH || out.len() >= MAX_NODES {
@@ -430,15 +440,29 @@ 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);
             }
         }
         Value::Array(items) => {
             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