In file crates/tirith-core/src/rules/configfile.rs around line 1053:

IPv6 host handling is incorrect: `extract_host_from_url` treats `:` as a delimiter and `Ipv6Addr::parse` doesn’t accept bracketed hosts. Suggest: if the host starts with `[`, read to the matching `]`, strip brackets before parsing, then search for `:`, `/` or `?` after the `]`.

Evidence trail:
crates/tirith-core/src/rules/configfile.rs lines 1072-1080 (REVIEWED_COMMIT): `extract_host_from_url` function uses `.find(['/', ':', '?'])` as delimiter, which breaks IPv6 addresses that contain colons. Lines 1053-1054 show the parsed host is passed to `host.parse::<std::net::Ipv6Addr>()`. Rust std library documentation confirms `Ipv6Addr::from_str` does not accept bracketed format.

In file crates/tirith-core/src/rules/command.rs around line 453:

`strip_port` misparses IPv6. Unbracketed addresses and unmatched `[` are handled via `rfind(':')`, causing truncation or bogus port stripping. Suggest: if `[` is present without `]`, return unchanged; if there are multiple `:` and no brackets, treat as IPv6 and skip port stripping (or document this limitation).

Evidence trail:
crates/tirith-core/src/rules/command.rs lines 468-482 at REVIEWED_COMMIT: `strip_port` function. Line 469-472: IPv6 bracket handling only covers the case where `]` is found. Line 474-477: falls back to `rfind(':')` for all other cases. Unbracketed IPv6 `2001:db8::1` → `rfind(':')` finds last `:`, `"1".parse::<u16>().is_ok()` → true, returns `2001:db8::` (truncated). Unmatched bracket `[::1` → `starts_with('[')` true, `find(']')` returns None, falls through to `rfind(':')`, returns `[:` (garbage).

In file crates/tirith/src/cli/scan.rs around line 69:

Exit code 2 is used for both incomplete scans and non-Critical/High findings, contradicting the comment which documents `findings (exit 1) > clean (exit 0)`. Consider returning 1 for any findings, or updating the comment to reflect the intended behavior.

Evidence trail:
crates/tirith/src/cli/scan.rs lines 54-72 at REVIEWED_COMMIT:
- Line 54: Comment states 'Exit code precedence: incomplete scan (exit 2) > findings (exit 1) > clean (exit 0).'
- Line 59: Returns 2 for incomplete scan
- Line 67: Returns 1 only for Critical/High findings
- Line 69: Returns 2 for non-Critical/High findings (total_findings() > 0)
- Line 71: Returns 0 for clean

The comment claims 'findings (exit 1)' but non-Critical/High findings actually exit 2, confirming the discrepancy.

In file crates/tirith-core/src/rules/command.rs around line 417:

The `return` statements exit the entire function after the first finding, so if a command accesses multiple suspicious endpoints, only the first is reported. Consider removing these `return` statements to continue checking all arguments and segments.

Evidence trail:
crates/tirith-core/src/rules/command.rs lines 385-430 at REVIEWED_COMMIT. Function `check_network_destination` signature at line 385 shows `findings: &mut Vec<Finding>`. Nested loops at lines 386 and 395. `return;` statements at approximately lines 414 and 429 exit the entire function after the first finding is pushed, preventing detection of subsequent suspicious endpoints in remaining args or segments.

In file crates/tirith-core/src/scan.rs around line 833:

`.svg` files are XML text, not binary, and can contain embedded JavaScript. Consider removing `.svg` from this list so the scanner can detect scripts in SVG images.

Evidence trail:
crates/tirith-core/src/scan.rs lines 830-840 at REVIEWED_COMMIT - The `is_binary_extension` function includes `.svg` in the `binary_exts` array on line 833. The function comment on line 830 states 'File extensions that indicate binary content (skip scanning)'. SVG files are XML text format that can contain embedded JavaScript, making this classification incorrect for a security scanner.

In file crates/tirith-core/src/rules/configfile.rs around line 932:

Suggestion: Avoid ad‑hoc substring parsing of structured data in this module. Substring scans can misparse JSON keys and URLs (e.g., keys inside strings; userinfo before host). Prefer structured parsers (e.g., a JSON token stream for the servers object and a URL parser that strips userinfo), or add strict post‑checks if a parser isn’t feasible.

Evidence trail:
crates/tirith-core/src/rules/configfile.rs lines 927-1020 (check_mcp_duplicate_names function using raw find for JSON key detection), lines 1074-1087 (extract_host_from_url function), lines 1054-1060 (how extract_host_from_url is used for IP detection)

In file crates/tirith-core/src/scan.rs around line 140:

When `config.path` is a file, using it as `repo_root` for `ConfigPathMatcher` breaks classification. Consider using the file's parent directory instead: `config.path.parent().unwrap_or(&config.path)`.

Evidence trail:
crates/tirith-core/src/scan.rs:140 - `ConfigPathMatcher::new(&config.path, vec![])` uses config.path as repo_root
crates/tirith-core/src/scan.rs:482-484 - when path is_file(), it's pushed directly to files list
crates/tirith-core/src/rules/configfile.rs:239-244 - `path.strip_prefix(&self.repo_root)` normalizes absolute paths
crates/tirith-core/src/rules/configfile.rs:262 - `if components.is_empty() { return ConfigMatch::NotConfig; }` - empty components after stripping causes incorrect NotConfig result

In file crates/tirith-core/src/scan.rs around line 718:

`is_valid_config_extension_for_dir` doesn't handle `.roo/rules-{slug}/*.md` paths (pattern 7 in `is_known`). Files like `vendor/pkg/.roo/rules-myslug/evil.md` won't be detected by the excluded-tree probe. Consider adding the `rules-{slug}` pattern check to that function.

Evidence trail:
crates/tirith-core/src/rules/configfile.rs:168-228 (is_valid_config_extension_for_dir implementation - no rules-{slug} pattern), crates/tirith-core/src/rules/configfile.rs:340-363 (is_known pattern 7 handling .roo/rules-{slug}/*.md), crates/tirith-core/src/rules/configfile.rs:56-79 (KNOWN_CONFIG_DEEP_DIRS constant - only has '.roo/rules' not '.roo/rules-{slug}'), crates/tirith-core/src/scan.rs:790-794 (excluded-tree probe uses is_valid_config_extension_for_dir)

In file crates/tirith-core/src/scan.rs around line 170:

Ignore filtering is inconsistent: `ignore_patterns` are documented as globs but implemented as substring matches, and applied to basenames here while full paths elsewhere. Suggest implementing glob matching on a single target (e.g., repo‑relative full paths) and updating the docs.

Evidence trail:
crates/tirith-core/src/scan.rs line 18: `/// Glob patterns to ignore.` (documentation claims glob patterns)
crates/tirith-core/src/scan.rs line 521: `let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("");` (name is basename in collect_recursive_with_probing)
crates/tirith-core/src/scan.rs lines 593-595: `if ignore_patterns.iter().any(|pat| name.contains(pat.as_str()))` (substring match on basename)
crates/tirith-core/src/scan.rs line 167: `let name = f.to_string_lossy();` (name is full path in main scan)
crates/tirith-core/src/scan.rs lines 168-171: Same substring match but on full path

In file crates/tirith-core/src/rules/command.rs around line 505:

Private network detection only checks IPv4 literals, so hostnames (e.g., `localhost`) and IPv6 (`::1`, `fe80::/10`, `fc00::/7`) bypass it. Suggest expanding `is_private_ip` (and, if needed, `extract_host_from_arg`) to handle these cases, or document the limitation.

Evidence trail:
crates/tirith-core/src/rules/command.rs lines 486-499 (is_private_ip function definition showing IPv4-only parsing), line 418 (usage in PrivateNetworkAccess detection). The function uses `host.parse::<std::net::Ipv4Addr>()` which fails for hostnames and IPv6, causing the function to return `false` and bypassing the security check.

In file crates/tirith-core/src/scan.rs around lines 754-759:

The `depth > max_depth` check on line 755 appears unreachable—entries are only pushed when `depth < max_depth` (line 785-786), so popped entries never exceed `max_depth`. Consider removing this dead code or clarifying if the bounds logic was intended differently.

Evidence trail:
crates/tirith-core/src/scan.rs lines 745-800 at REVIEWED_COMMIT. Key lines: 750 (initial stack with depth 0), 754 (pop loop), 755 (the unreachable check `depth > max_depth`), 785-786 (push condition `if depth < max_depth { stack.push((path, depth + 1)); }`). The push condition ensures max pushed depth is `max_depth`, so the `> max_depth` check is unreachable.

In file crates/tirith-core/src/scan.rs around lines 520-521:

If `file_name()` returns `None` or contains non-UTF8, `name` becomes `""`, bypassing `is_excluded_dir`, `starts_with('.')`, and `is_binary_extension` checks. Consider using `continue` for these cases instead of falling back to empty string.

Evidence trail:
crates/tirith-core/src/scan.rs:520 - `let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("");`
crates/tirith-core/src/scan.rs:538 - `if is_excluded_dir(name)` check uses the potentially empty name
crates/tirith-core/src/scan.rs:562 - `if name.starts_with('.')` check uses the potentially empty name
crates/tirith-core/src/scan.rs:586 - `if is_binary_extension(name)` check uses the potentially empty name
crates/tirith-core/src/scan.rs:819-820 - `is_excluded_dir` checks `DEFAULT_EXCLUDE_DIRS.contains(&name)` - empty string not in list
crates/tirith-core/src/scan.rs:831-838 - `is_binary_extension` checks `ends_with(ext)` - empty string won't match any extension

In file crates/tirith-core/src/rules/command.rs around line 460:

URLs like `http://` or `http:///path` return `Some("")` instead of `None`. Consider checking if the host is empty before returning.

Evidence trail:
File: crates/tirith-core/src/rules/command.rs lines 447-474 (REVIEWED_COMMIT) - `extract_host_from_arg` function shows line 460: `return Some(strip_port(host_port));` with no check for empty host. Lines 477-490 - `strip_port` function returns the input unchanged if empty string. For `http://`: after_scheme="", split('/').next()="", returns Some(""). For `http:///path`: split '/path' by '/' → ["", "path"], first element is "", returns Some("").

In file crates/tirith/src/cli/scan.rs around line 143:

Unrecognized severity values silently default to `Critical`, which can mask typos (e.g., `--fail-on hig`). Consider returning a `Result` or panicking with a helpful error message listing valid options.

Evidence trail:
crates/tirith/src/cli/scan.rs lines 137-146 at REVIEWED_COMMIT: `parse_severity` function uses a wildcard `_` match arm that returns `Severity::Critical` for any unrecognized input string, confirming silent defaulting behavior.

In file crates/tirith-core/src/scan.rs around line 257:

Doc comment mentions `skipped_config_paths` parameter that doesn't exist in this function's signature. Consider removing that sentence since it describes internal behavior not exposed to callers.

Evidence trail:
crates/tirith-core/src/scan.rs lines 256-274 at REVIEWED_COMMIT. Doc comment at lines 257-259 mentions `skipped_config_paths` parameter. Function signature at line 260 shows only `file_path: &Path` and `matcher: &ConfigPathMatcher` parameters.