From b1d3c1b80e44317b9868bc212ea24c78a323ea5d Mon Sep 17 00:00:00 2001 From: Paradox <159635503+i-am-paradox@users.noreply.github.com> Date: Mon, 26 Oct 2026 04:40:27 +0530 Subject: [PATCH] fix(sieve): resolve case-exact file probes and deduplicate contents (fixes #645) On case-insensitive filesystems (such as macOS APFS and Windows NTFS), os.path.exists() and open() match lowercase file probes against uppercase files in the repository. As reported in #645, this causes sieve handlers to: 1. Match candidate probes that don't match the repository's actual case. 2. Read duplicate file contents in llm_eval_handler and llm_extract_handler when files_to_include or files specify both case variants. 3. Cite paths in audit evidence that do not exist under exact case in the repo. Changes: - Added resolve_case_exact_path(path, root=None) in darnit.core.utils. - Cached directory entries via public dir_entries(dir_path) keyed by st_mtime_ns. - Cleared directory entries cache on remediation apply and in orchestrator _recheck. - Added get_file_identity(full_path) returning (st_dev, st_ino) for physical file deduplication. - file_exists_handler: verifies exact case for globs, max_depth walks (including slash-containing entries like dist/app.exe), and direct path lookups while filtering broken symlinks, normalizing relative_path to POSIX forward slashes, and recording escaped paths in skipped_outside_repository per spec 3.0.4. - _resolve_regex_files & _regex_exclude_evidence: ensures exact case matches, supports slash-containing entries in max_depth walks, filters broken symlinks, records outside paths, and deduplicates by physical file identity. - llm_eval_handler & llm_extract_handler: enforces case-exact paths, records outside paths/dangling symlinks in skipped_outside_repository, and deduplicates opened files before adding to file_contents. - utils.py: recognized filenames prefixed with two dots (e.g. ..b.md) inside repository without false outside-of-repo classification. - discover_files: verifies case-exact paths before returning mapped references. - openssf-baseline.toml: added docs/ARCHITECTURE.md and docs/API.md variants to discover and files, and updated files_to_include in OSPS-SA-01.01 and OSPS-SA-02.01 to ["README.md", "$RESOLVED_FILES"]. - docs/architecture/framework-design.md: documented case-exact resolution and physical file deduplication in sections 3.2, 3.3, 3.5, and 11.1. - Unit and regression tests covering case-exact path validation, deduplication, broken symlink rejection, slash-containing max_depth entries, outside path recording, and ..b.md filenames. Fixes #645 Assisted-by: Claude:claude-3-5-sonnet Assisted-by: Gemini:gemini-2.5-pro Signed-off-by: Paradox <159635503+i-am-paradox@users.noreply.github.com> --- CHANGELOG.md | 6 + docs/architecture/framework-design.md | 8 +- .../src/darnit_baseline/openssf-baseline.toml | 16 +- .../remediation/orchestrator.py | 3 + .../darnit/src/darnit/config/discovery.py | 24 +- packages/darnit/src/darnit/core/utils.py | 101 ++++++++ .../darnit/src/darnit/remediation/executor.py | 10 + .../src/darnit/sieve/builtin_handlers.py | 140 ++++++++--- tests/darnit/config/test_discovery.py | 19 ++ tests/darnit/core/test_utils.py | 78 ++++++ tests/darnit/sieve/test_builtin_handlers.py | 222 ++++++++++++++++++ 11 files changed, 575 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80a1be814..adf93d6d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -692,6 +692,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 outcome now also lists the files kept because they had uncommitted user changes (`user_changes_present`). An apply that wrote some files and kept others reported only the written ones (#673). +- Case-exact file resolution and deduplication on case-insensitive filesystems + (macOS APFS, Windows NTFS): `file_exists_handler`, `regex_handler`, + `llm_eval_handler`, `llm_extract_handler`, and config file discovery verify + path components against case-exact directory entries, preventing spurious + matches on lowercase aliases, deduplicating file contents read into + consultation evidence, and citing exact repository paths (fixes #645). - Control `when` and step `when` now use one evaluator (framework-design 5.1.1). A list-valued control `when` such as `{ ci_provider = ["github", "gitlab"] }` made the control N/A even when diff --git a/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index 80f938854..3bc088143 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -425,6 +425,8 @@ files = ["SECURITY.md", ".github/SECURITY.md"] 1. If any file in `files` matches → PASS 2. If no file matches → FAIL +File paths in `files` are resolved case-exactly on every filesystem; candidate paths that do not match the on-disk directory entry case verbatim are not matched. + **Ceiling**: `{fail}`; `{pass, fail}` with `existence = true` (section 3.0.1). A file being present says nothing about its content, so a presence step concludes PASS only for a control whose requirement is literally that the file exists. #### Scenario: File found @@ -543,6 +545,8 @@ These are the step type's declared `settings` and common step fields (section 3. 2. No match -> INCONCLUSIVE, unless `fail_on_miss = true` -> FAIL 3. No files resolved -> INCONCLUSIVE +File resolution in `files` and `exclude_files` is case-exact across all filesystems, and repeated paths resolving to the same underlying physical file are deduplicated. + Exclude mode (`exclude_files`) is unchanged: no files found -> PASS, files found -> FAIL, usually refined by `expr`. #### Scenario: Pattern match found @@ -589,7 +593,7 @@ confidence_threshold = 0.8 | `analysis_hints` | `list[str]` | Hints to guide analysis | | `confidence_threshold` | `float` | Passed to the model with the consultation. Since feature 041 a judgment's confidence is recorded with it but is never a decision input: a judgment never concludes a control. | -**`files_to_include` resolution**: The handler MUST resolve `$FOUND_FILE` entries by looking up `found_file` in `context.gathered_evidence`. Each resolved file path MUST be read (up to 10KB per file, max 5 files) and included as `file_contents` in the `consultation_request`. File paths that are not absolute MUST be resolved relative to `context.local_path`. Files that cannot be read MUST be silently skipped. A file that resolves outside the repository MUST NOT be read and is recorded in the result's `skipped_outside_repository` (section 3.0.4). +**`files_to_include` resolution**: The handler MUST resolve `$FOUND_FILE` entries by looking up `found_file` in `context.gathered_evidence`. Each resolved file path MUST be read (up to 10KB per file, max 5 files) and included as `file_contents` in the `consultation_request`. File paths that are not absolute MUST be resolved relative to `context.local_path`. Files that cannot be read MUST be silently skipped. A file that resolves outside the repository MUST NOT be read and is recorded in the result's `skipped_outside_repository` (section 3.0.4). Files to include are resolved case-exactly on every filesystem, and duplicate paths resolving to the same physical file are deduplicated before reading. Similarly, `llm_extract` resolves candidate files case-exactly on every filesystem and deduplicates physical files before gathering content. **Ceiling**: `{}`. The handler returns INCONCLUSIVE with a `consultation_request`; with `stop_on_llm` the control becomes `PENDING` with `pending.kind = "llm_judgment"` (section 5.4). A model judgment never concludes PASS. No promotion to conclude PASS exists for a model judgment. @@ -2017,7 +2021,7 @@ The darnit framework package SHALL NOT contain code, modules, or string literals ### 11.1 use_locator for File Existence Checks -A deterministic handler invocation MAY use `use_locator = true` instead of specifying `files` directly. When `use_locator = true` is set, the framework SHALL use the control's `locator.discover` list as the handler's `files` parameter. +A deterministic handler invocation MAY use `use_locator = true` instead of specifying `files` directly. When `use_locator = true` is set, the framework SHALL use the control's `locator.discover` list as the handler's `files` parameter. Locator file discovery (`discover_files`) resolves candidate files case-exactly on every filesystem. - If the control has no `locator` configuration, the framework SHALL log a warning and return INCONCLUSIVE - If both `use_locator = true` and explicit `files` are specified, `files` takes precedence diff --git a/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml b/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml index 4686e4302..9f8d11407 100644 --- a/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml +++ b/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml @@ -4208,7 +4208,7 @@ help_md = """Create architecture/design documentation. [controls."OSPS-SA-01.01".locator] project_path = "documentation.design" -discover = ["ARCHITECTURE.md", "docs/architecture.md", "docs/design.md", "DESIGN.md"] +discover = ["ARCHITECTURE.md", "docs/ARCHITECTURE.md", "docs/architecture.md", "docs/design.md", "DESIGN.md"] kind = "file" [controls."OSPS-SA-01.01".locator.llm_hints] @@ -4220,7 +4220,7 @@ look_for_urls = true handler = "pattern" # rst variants added; ARCHITECTURE/DESIGN commonly live as .rst # in Sphinx-based Python projects. -files = ["ARCHITECTURE.md", "ARCHITECTURE.rst", "docs/architecture.md", "docs/architecture.rst", "docs/design.md", "docs/design.rst", "DESIGN.md", "DESIGN.rst", "README.md", "README.rst"] +files = ["ARCHITECTURE.md", "ARCHITECTURE.rst", "docs/ARCHITECTURE.md", "docs/ARCHITECTURE.rst", "docs/architecture.md", "docs/architecture.rst", "docs/design.md", "docs/design.rst", "DESIGN.md", "DESIGN.rst", "README.md", "README.rst"] pass_if_any = true [controls."OSPS-SA-01.01".passes.pattern.patterns] @@ -4236,8 +4236,8 @@ prompt = """Evaluate whether this design/architecture document describes real sy 2. Does it describe actors (users, systems) that interact with the software? 3. Does it explain data flows or interactions between components? A document with only TBD/TODO placeholders or generic headings without content MUST fail.""" -# Issue #402: mirror the sibling pattern's file list. -files_to_include = ["ARCHITECTURE.md", "docs/architecture.md", "docs/design.md", "DESIGN.md", "README.md"] +# Issue #402: include README.md and resolved files from sibling pattern pass. +files_to_include = ["README.md", "$RESOLVED_FILES"] confidence_threshold = 0.7 analysis_hints = [ "Look for named components with descriptions of what they do", @@ -4283,7 +4283,7 @@ help_md = """Document external APIs and interfaces. [controls."OSPS-SA-02.01".locator] project_path = "documentation.api" -discover = ["API.md", "docs/api.md", "docs/api/", "openapi.yaml", "openapi.json", "swagger.yaml", "swagger.json"] +discover = ["API.md", "docs/API.md", "docs/api.md", "docs/api/", "openapi.yaml", "openapi.json", "swagger.yaml", "swagger.json"] kind = "file" [controls."OSPS-SA-02.01".locator.llm_hints] @@ -4296,7 +4296,7 @@ look_for_urls = true handler = "pattern" # README.rst added for Sphinx-based projects; API.rst / USAGE.rst # variants common in the same ecosystem. -files = ["API.md", "API.rst", "docs/api.md", "docs/api.rst", "README.md", "README.rst", "docs/README.md", "USAGE.md", "USAGE.rst", "docs/getting-started.md", "openapi.yaml", "openapi.json", "swagger.yaml", "swagger.json"] +files = ["API.md", "API.rst", "docs/API.md", "docs/API.rst", "docs/api.md", "docs/api.rst", "README.md", "README.rst", "docs/README.md", "USAGE.md", "USAGE.rst", "docs/getting-started.md", "openapi.yaml", "openapi.json", "swagger.yaml", "swagger.json"] pass_if_any = true [controls."OSPS-SA-02.01".passes.pattern.patterns] @@ -4312,8 +4312,8 @@ prompt = """Evaluate whether this API/interface documentation describes real API 2. Does it include parameter descriptions, request/response formats, or usage examples? 3. Is this substantive documentation, not just a heading with no content? A document with only headings or TODO placeholders MUST fail.""" -# Issue #402: mirror the sibling pattern's file list (pick top 5 of 10). -files_to_include = ["API.md", "docs/api.md", "README.md", "openapi.yaml", "swagger.json"] +# Issue #402: include README.md and resolved files from sibling pattern pass. +files_to_include = ["README.md", "$RESOLVED_FILES"] confidence_threshold = 0.7 analysis_hints = [ "Look for documented endpoints, methods, or CLI commands", diff --git a/packages/darnit-baseline/src/darnit_baseline/remediation/orchestrator.py b/packages/darnit-baseline/src/darnit_baseline/remediation/orchestrator.py index 4136c2608..e248549ad 100644 --- a/packages/darnit-baseline/src/darnit_baseline/remediation/orchestrator.py +++ b/packages/darnit-baseline/src/darnit_baseline/remediation/orchestrator.py @@ -1509,8 +1509,11 @@ def _recheck( from darnit.config.control_loader import load_controls_from_effective from darnit.config.merger import merge_configs from darnit.config.operator.loader import resolve_operator_config + from darnit.core.utils import clear_dir_entries_cache from darnit.tools import audit as audit_tools + clear_dir_entries_cache() + framework = _get_framework_config() if framework is None: raise RuntimeError("the framework configuration could not be loaded") diff --git a/packages/darnit/src/darnit/config/discovery.py b/packages/darnit/src/darnit/config/discovery.py index 37554f7e6..e2f3cb38d 100644 --- a/packages/darnit/src/darnit/config/discovery.py +++ b/packages/darnit/src/darnit/config/discovery.py @@ -40,17 +40,27 @@ def discover_files( if file_locations is None: return {} + from darnit.core.utils import resolve_case_exact_path + discovered = {} for ref_path, patterns in file_locations.items(): for pattern in patterns: - full_pattern = os.path.join(local_path, pattern) - matches = glob_module.glob(full_pattern) - if matches: - # Use first match, relative path - rel_path = os.path.relpath(matches[0], local_path) - discovered[ref_path] = rel_path - break + if "*" in pattern or "?" in pattern: + full_pattern = os.path.join(local_path, pattern) + matches = glob_module.glob(full_pattern) + exact_matches = [m for m in matches if resolve_case_exact_path(m, local_path)] + if exact_matches: + # Use first match, relative path + rel_path = os.path.relpath(exact_matches[0], local_path) + discovered[ref_path] = rel_path + break + else: + exact = resolve_case_exact_path(pattern, local_path) + if exact and os.path.exists(exact): + rel_path = os.path.relpath(exact, local_path) + discovered[ref_path] = rel_path + break return discovered diff --git a/packages/darnit/src/darnit/core/utils.py b/packages/darnit/src/darnit/core/utils.py index 0fdc4bad2..1254aa21d 100644 --- a/packages/darnit/src/darnit/core/utils.py +++ b/packages/darnit/src/darnit/core/utils.py @@ -1,5 +1,6 @@ """Shared utility functions for the baseline MCP server.""" +import functools import glob as glob_module import json import os @@ -683,3 +684,103 @@ def get_git_ref(local_path: str) -> str | None: except (FileNotFoundError, OSError, subprocess.SubprocessError) as e: logger.debug(f"git ref command failed: {type(e).__name__}") return None + + +@functools.lru_cache(maxsize=2048) +def _dir_entries_for_mtime(dir_path: str, mtime_ns: int) -> frozenset[str]: + try: + return frozenset(os.listdir(dir_path)) + except OSError: + return frozenset() + + +def dir_entries(dir_path: str) -> frozenset[str]: + """Return directory entries as a frozenset, or empty frozenset on OSError. + + Keys by directory path and st_mtime_ns so modifications (e.g. remediation file creation) + automatically invalidate the cache without requiring manual clearing. + """ + try: + mtime_ns = os.stat(dir_path).st_mtime_ns + except OSError: + return frozenset() + return _dir_entries_for_mtime(dir_path, mtime_ns) + + +def clear_dir_entries_cache() -> None: + """Clear cached directory listings (primarily for test environments).""" + _dir_entries_for_mtime.cache_clear() + + +def resolve_case_exact_path(path: str, root: str | None = None) -> str | None: + """Verify that a path exists with case-exact directory entries. + + On case-insensitive filesystems (such as macOS APFS or Windows NTFS), + `os.path.exists` or `open()` can succeed when probing a lowercase + path (e.g. `readme.md`) even if the repository only contains `README.md`. + This function traverses each path component and verifies against the + actual directory entries (`os.listdir`), ensuring exact-case matching + identical to case-sensitive filesystems (Linux). + + Args: + path: File path (relative or absolute). + root: Optional repository root to anchor relative paths or bounds. + + Returns: + The normalized absolute path if all components exist with exact case, + or None if any component is missing or mismatched in case. + """ + if not path: + return None + + if root is not None and not os.path.isabs(path): + full_path = os.path.normpath(os.path.join(os.path.abspath(root), path)) + base = os.path.abspath(root) + else: + full_path = os.path.normpath(os.path.abspath(path)) + if root is not None: + base_cand = os.path.abspath(root) + try: + if os.path.commonpath([base_cand, full_path]) == base_cand: + base = base_cand + else: + base = os.path.abspath(os.sep) + except ValueError: + base = os.path.abspath(os.sep) + else: + base = os.path.abspath(os.sep) + + try: + rel = os.path.relpath(full_path, base) + except ValueError: + return None + + if rel == ".": + return base + if rel == ".." or rel.startswith(".." + os.sep): + return None + + curr = base + for part in rel.split(os.sep): + if not part or part == ".": + continue + if part not in dir_entries(curr): + return None + curr = os.path.join(curr, part) + + return curr + + +def get_file_identity(full_path: str) -> tuple[int, int] | str: + """Return a unique file identity (st_dev, st_ino) or realpath fallback. + + Used to deduplicate candidate files (e.g. across case-aliases, symlinks, + or multiple probes pointing to the same underlying file) before reading + or reporting evidence. + """ + try: + st = os.stat(full_path) + return (st.st_dev, st.st_ino) + except OSError: + return os.path.realpath(full_path) + diff --git a/packages/darnit/src/darnit/remediation/executor.py b/packages/darnit/src/darnit/remediation/executor.py index d6132bb9a..421370a1a 100644 --- a/packages/darnit/src/darnit/remediation/executor.py +++ b/packages/darnit/src/darnit/remediation/executor.py @@ -511,11 +511,18 @@ def execute( confirmation_required=needed.key, ) if not dry_run: + from darnit.core.utils import clear_dir_entries_cache + + clear_dir_entries_cache() result.plan = planned.plan result.approvals = self._used_approvals(planned.plan) if result.success and config.project_update and config.project_update.set: self._project_update(control_id, config.project_update, result) + if not dry_run: + from darnit.core.utils import clear_dir_entries_cache + + clear_dir_entries_cache() return result @@ -732,6 +739,9 @@ def _write(self, change: FileChange) -> FileChange: os.makedirs(os.path.dirname(full_path), exist_ok=True) _write_bytes_atomic(full_path, change.content.encode("utf-8")) manifest.record_file(repository, run_id, change.path, change.after_digest or "", checkout=self.local_path) + from darnit.core.utils import clear_dir_entries_cache + + clear_dir_entries_cache() return change.model_copy(update={"ignored": True}) if ignored else change def amend_created(self, created: FileChange, content: str) -> FileChange: diff --git a/packages/darnit/src/darnit/sieve/builtin_handlers.py b/packages/darnit/src/darnit/sieve/builtin_handlers.py index a189d6ed2..6844ed586 100644 --- a/packages/darnit/src/darnit/sieve/builtin_handlers.py +++ b/packages/darnit/src/darnit/sieve/builtin_handlers.py @@ -29,6 +29,7 @@ from darnit.core.error_class import ErrorClass from darnit.core.repo_files import escaping_entry, open_in_repository, within_repository +from darnit.core.utils import get_file_identity, resolve_case_exact_path from .handler_registry import ( HandlerContext, @@ -280,9 +281,10 @@ def file_exists_handler(config: dict[str, Any], context: HandlerContext) -> Hand # sorted() so "first match wins" is stable across filesystems. # Determinism Tier 1 (#418). matches = _skip_outside(sorted(glob.glob(os.path.join(context.local_path, pattern))), context, skipped) - if matches: - found = matches[0] - rel_path = os.path.relpath(found, context.local_path) + exact_matches = [m for m in matches if resolve_case_exact_path(m, context.local_path)] + if exact_matches: + found = exact_matches[0] + rel_path = os.path.relpath(found, context.local_path).replace(os.sep, "/") return _record_skipped( HandlerResult( status=HandlerResultStatus.PASS, @@ -299,16 +301,21 @@ def file_exists_handler(config: dict[str, Any], context: HandlerContext) -> Hand # downstream consumers (and audit reviewers) can see where it # actually lives. for dirpath, _depth in _walk_depth_limited(context.local_path, max_depth): - candidate = os.path.join(dirpath, pattern) - if os.path.exists(candidate) and _skip_outside([candidate], context, skipped): - rel_path = os.path.relpath(candidate, context.local_path) + rel_dir = os.path.relpath(dirpath, context.local_path) + candidate_rel = pattern if rel_dir == "." else os.path.join(rel_dir, pattern) + candidate = os.path.join(context.local_path, candidate_rel) + if not os.path.exists(candidate) or not _skip_outside([candidate], context, skipped): + continue + exact = resolve_case_exact_path(candidate_rel, context.local_path) + if exact and os.path.exists(exact): + rel_path = os.path.relpath(exact, context.local_path).replace(os.sep, "/") return _record_skipped( HandlerResult( status=HandlerResultStatus.PASS, message=f"Required file found: {rel_path}", confidence=1.0, evidence={ - "found_file": candidate, + "found_file": exact, "relative_path": rel_path, "files_checked": files, "max_depth": max_depth, @@ -317,14 +324,18 @@ def file_exists_handler(config: dict[str, Any], context: HandlerContext) -> Hand skipped, ) else: - path = os.path.join(context.local_path, pattern) - if os.path.exists(path) and _skip_outside([path], context, skipped): + cand = os.path.join(context.local_path, pattern) if not os.path.isabs(pattern) else pattern + if os.path.exists(cand) and not _skip_outside([cand], context, skipped): + continue + exact = resolve_case_exact_path(pattern, context.local_path) + if exact and os.path.exists(exact) and _skip_outside([exact], context, skipped): + rel_path = os.path.relpath(exact, context.local_path).replace(os.sep, "/") return _record_skipped( HandlerResult( status=HandlerResultStatus.PASS, - message=f"Required file found: {pattern}", + message=f"Required file found: {rel_path}", confidence=1.0, - evidence={"found_file": path, "relative_path": pattern, "files_checked": files}, + evidence={"found_file": exact, "relative_path": rel_path, "files_checked": files}, ), skipped, ) @@ -795,24 +806,42 @@ def _regex_exclude_evidence( os.path.join(context.local_path, pattern), recursive=True, ) - found.extend(matches) + found.extend(m for m in matches if resolve_case_exact_path(m, context.local_path)) elif max_depth > 0: # Depth-limited walk for plain filenames (no wildcards). # dirs.clear() at the boundary depth means directory entries AT # that depth are no longer descended into; only files at each # visited dir are matched here. for dirpath, _d in _walk_depth_limited(context.local_path, max_depth): - candidate = os.path.join(dirpath, pattern) - if os.path.exists(candidate): - found.append(candidate) + rel_dir = os.path.relpath(dirpath, context.local_path) + candidate_rel = pattern if rel_dir == "." else os.path.join(rel_dir, pattern) + cand = os.path.join(context.local_path, candidate_rel) + if not os.path.exists(cand): + continue + if not within_repository( + context.local_path, cand + ) or resolve_case_exact_path(candidate_rel, context.local_path): + found.append(cand) else: - candidate = os.path.join(context.local_path, pattern) - if os.path.exists(candidate): - found.append(candidate) + cand = os.path.join(context.local_path, pattern) if not os.path.isabs(pattern) else pattern + if os.path.exists(cand) and ( + not within_repository(context.local_path, cand) + or resolve_case_exact_path(pattern, context.local_path) + ): + found.append(cand) + + deduped_found: list[str] = [] + seen_ids: set[tuple[int, int] | str] = set() + for f in found: + fid = get_file_identity(f) + if fid not in seen_ids: + seen_ids.add(fid) + deduped_found.append(f) + found = deduped_found skipped: list[str] = [] found = _skip_outside(found, context, skipped) - rel_paths = [os.path.relpath(f, context.local_path) for f in found[:10]] + rel_paths = [os.path.relpath(f, context.local_path).replace(os.sep, "/") for f in found[:10]] evidence: dict[str, Any] = { "exclude_globs": exclude_globs, "files_found": len(found), @@ -859,6 +888,7 @@ def _resolve_regex_files( files_list = config.get("files", []) if files_list: resolved: list[str] = [] + seen_file_ids: set[tuple[int, int] | str] = set() for file_pattern in files_list: if "*" in file_pattern or "?" in file_pattern: # Glob patterns: always use glob.glob; max_depth does not apply. @@ -870,19 +900,43 @@ def _resolve_regex_files( recursive=True, ) ) - resolved.extend(m for m in matches if os.path.isfile(m)) + for m in matches: + if not os.path.isfile(m): + continue + if not within_repository(context.local_path, m): + resolved.append(m) + elif resolve_case_exact_path(m, context.local_path): + fid = get_file_identity(m) + if fid not in seen_file_ids: + seen_file_ids.add(fid) + resolved.append(m) elif max_depth > 0: # Depth-limited walk for plain filenames (no wildcards). # Only files are collected; dirs.clear() at the depth boundary # prevents descending further without skipping files at that depth. for dirpath, _d in _walk_depth_limited(context.local_path, max_depth): - candidate = os.path.join(dirpath, file_pattern) - if os.path.isfile(candidate): - resolved.append(candidate) + rel_dir = os.path.relpath(dirpath, context.local_path) + candidate_rel = file_pattern if rel_dir == "." else os.path.join(rel_dir, file_pattern) + cand = os.path.join(context.local_path, candidate_rel) + if not os.path.isfile(cand): + continue + if not within_repository(context.local_path, cand): + resolved.append(cand) + elif resolve_case_exact_path(candidate_rel, context.local_path): + fid = get_file_identity(cand) + if fid not in seen_file_ids: + seen_file_ids.add(fid) + resolved.append(cand) else: - full = os.path.join(context.local_path, file_pattern) - if os.path.isfile(full): - resolved.append(full) + cand = os.path.join(context.local_path, file_pattern) if not os.path.isabs(file_pattern) else file_pattern + if os.path.isfile(cand): + if not within_repository(context.local_path, cand): + resolved.append(cand) + elif resolve_case_exact_path(file_pattern, context.local_path): + fid = get_file_identity(cand) + if fid not in seen_file_ids: + seen_file_ids.add(fid) + resolved.append(cand) return resolved if resolved else None # Legacy singular format: file = "README.md" or file = "$FOUND_FILE" @@ -895,11 +949,12 @@ def _resolve_regex_files( if not file_path: return None - if not os.path.isabs(file_path): - file_path = os.path.join(context.local_path, file_path) - - if os.path.isfile(file_path): - return [file_path] + cand = os.path.join(context.local_path, file_path) if not os.path.isabs(file_path) else file_path + if os.path.isfile(cand): + if not within_repository(context.local_path, cand): + return [cand] + if resolve_case_exact_path(file_path, context.local_path): + return [cand] return None @@ -1082,16 +1137,24 @@ def llm_eval_handler(config: dict[str, Any], context: HandlerContext) -> Handler files_to_include = config.get("files_to_include", []) file_contents: dict[str, str] = {} skipped: list[str] = [] + seen_file_ids: set[tuple[int, int] | str] = set() def _read(resolved: str) -> None: if not resolved or len(file_contents) >= 5: return - full = os.path.join(context.local_path, resolved) if not os.path.isabs(resolved) else resolved - if not os.path.lexists(full) or not _skip_outside([full], context, skipped): + cand = os.path.join(context.local_path, resolved) if not os.path.isabs(resolved) else resolved + if not os.path.lexists(cand) or not _skip_outside([cand], context, skipped): + return + full = resolve_case_exact_path(resolved, context.local_path) + if not full or not os.path.isfile(full): return + file_id = get_file_identity(full) + if file_id in seen_file_ids: + return + seen_file_ids.add(file_id) try: with open_in_repository(context.local_path, full, encoding="utf-8", errors="ignore") as fh: - rel = os.path.relpath(full, context.local_path) + rel = os.path.relpath(full, context.local_path).replace(os.sep, "/") file_contents[rel] = fh.read()[:10000] except OSError: pass @@ -1167,15 +1230,22 @@ def llm_extract_handler(config: dict[str, Any], context: HandlerContext) -> Hand globs = config.get("files", []) file_contents: dict[str, str] = {} skipped: list[str] = [] + seen_file_ids: set[tuple[int, int] | str] = set() for pattern in globs[:5]: # cap breadth matches = globmod.glob( os.path.join(context.local_path, pattern), recursive=True, ) for m in _skip_outside(matches, context, skipped)[:5]: # cap depth per glob + if not resolve_case_exact_path(m, context.local_path): + continue + file_id = get_file_identity(m) + if file_id in seen_file_ids: + continue + seen_file_ids.add(file_id) try: with open_in_repository(context.local_path, m, encoding="utf-8", errors="ignore") as fh: - rel = os.path.relpath(m, context.local_path) + rel = os.path.relpath(m, context.local_path).replace(os.sep, "/") file_contents[rel] = fh.read()[:10000] except OSError: continue diff --git a/tests/darnit/config/test_discovery.py b/tests/darnit/config/test_discovery.py index d7b7a1455..f3b196380 100644 --- a/tests/darnit/config/test_discovery.py +++ b/tests/darnit/config/test_discovery.py @@ -89,6 +89,25 @@ def test_glob_pattern_matches_nested_file(self, temp_dir): assert "security.policy" in result + def test_case_mismatch_not_discovered(self, temp_dir): + from darnit.core.utils import clear_dir_entries_cache + + clear_dir_entries_cache() + (temp_dir / "docs").mkdir() + (temp_dir / "docs" / "ARCHITECTURE.md").write_text("# Arch") + + result = discover_files( + str(temp_dir), + {"doc": ["docs/architecture.md"]}, + ) + assert "doc" not in result + + result_correct = discover_files( + str(temp_dir), + {"doc": ["docs/ARCHITECTURE.md"]}, + ) + assert result_correct == {"doc": os.path.normpath("docs/ARCHITECTURE.md")} + # --------------------------------------------------------------------------- # discover_ci_config diff --git a/tests/darnit/core/test_utils.py b/tests/darnit/core/test_utils.py index 29f46b357..714eae4d2 100644 --- a/tests/darnit/core/test_utils.py +++ b/tests/darnit/core/test_utils.py @@ -7,12 +7,15 @@ import pytest from darnit.core.utils import ( + clear_dir_entries_cache, file_exists, + get_file_identity, get_git_commit, get_git_ref, gh_api, gh_api_safe, read_file, + resolve_case_exact_path, set_gh_api_responder, validate_local_path, ) @@ -183,3 +186,78 @@ def test_gh_api_safe_missing_cli_returns_none(self): """Safe helper should swallow missing gh CLI errors.""" with patch("darnit.core.utils.subprocess.run", side_effect=FileNotFoundError): assert gh_api_safe("repos/kusari-oss/darnit") is None + + +class TestResolveCaseExactPath: + """Tests for resolve_case_exact_path and case-exact resolution.""" + + @pytest.fixture(autouse=True) + def _clear_cache(self): + clear_dir_entries_cache() + yield + clear_dir_entries_cache() + + @pytest.mark.unit + def test_case_exact_file_in_root(self, temp_dir: Path): + readme = temp_dir / "README.md" + readme.write_text("content") + + assert resolve_case_exact_path("README.md", str(temp_dir)) == str(readme) + # Lowercase probe must return None even on case-insensitive filesystems + assert resolve_case_exact_path("readme.md", str(temp_dir)) is None + assert resolve_case_exact_path("Readme.md", str(temp_dir)) is None + + @pytest.mark.unit + def test_case_exact_nested_path(self, temp_dir: Path): + docs_dir = temp_dir / "docs" + docs_dir.mkdir() + arch = docs_dir / "ARCHITECTURE.md" + arch.write_text("arch") + + assert resolve_case_exact_path("docs/ARCHITECTURE.md", str(temp_dir)) == str(arch) + # Mismatched file case + assert resolve_case_exact_path("docs/architecture.md", str(temp_dir)) is None + # Mismatched directory case + assert resolve_case_exact_path("DOCS/ARCHITECTURE.md", str(temp_dir)) is None + + @pytest.mark.unit + def test_nonexistent_file_returns_none(self, temp_dir: Path): + assert resolve_case_exact_path("MISSING.md", str(temp_dir)) is None + assert resolve_case_exact_path("docs/MISSING.md", str(temp_dir)) is None + + @pytest.mark.unit + def test_empty_or_root_path(self, temp_dir: Path): + assert resolve_case_exact_path("", str(temp_dir)) is None + assert resolve_case_exact_path(".", str(temp_dir)) == str(temp_dir) + + @pytest.mark.unit + def test_traversal_outside_root_returns_none(self, temp_dir: Path): + assert resolve_case_exact_path("../outside.txt", str(temp_dir)) is None + + @pytest.mark.unit + def test_double_dot_prefixed_filename(self, temp_dir: Path): + f = temp_dir / "..b.md" + f.write_text("two dots") + assert resolve_case_exact_path("..b.md", str(temp_dir)) == str(f) + + +class TestGetFileIdentity: + """Tests for get_file_identity deduplication helper.""" + + @pytest.mark.unit + def test_same_file_returns_same_identity(self, temp_dir: Path): + f = temp_dir / "sample.txt" + f.write_text("hello") + + id1 = get_file_identity(str(f)) + id2 = get_file_identity(str(f)) + assert id1 == id2 + + @pytest.mark.unit + def test_different_files_return_different_identities(self, temp_dir: Path): + f1 = temp_dir / "a.txt" + f2 = temp_dir / "b.txt" + f1.write_text("a") + f2.write_text("b") + + assert get_file_identity(str(f1)) != get_file_identity(str(f2)) diff --git a/tests/darnit/sieve/test_builtin_handlers.py b/tests/darnit/sieve/test_builtin_handlers.py index 92c41055c..1a2831dad 100644 --- a/tests/darnit/sieve/test_builtin_handlers.py +++ b/tests/darnit/sieve/test_builtin_handlers.py @@ -6,10 +6,15 @@ unit test coverage. """ +import os +from pathlib import Path + import pytest +from darnit.core.utils import clear_dir_entries_cache from darnit.remediation.plan import FileChange from darnit.sieve.builtin_handlers import ( + _resolve_regex_files, exec_handler, file_create_handler, file_exists_handler, @@ -96,6 +101,41 @@ def test_fail_when_directory_missing(self, tmp_path, ctx): result = file_exists_handler({"files": [".github/workflows"]}, ctx) assert result.status == HandlerResultStatus.FAIL + def test_case_exact_rejection_on_case_insensitive_fs(self, tmp_path, ctx): + """Case mismatch must fail even on case-insensitive filesystems (issue #645).""" + clear_dir_entries_cache() + (tmp_path / "README.md").write_text("# Readme") + result_lower = file_exists_handler({"files": ["readme.md"]}, ctx) + assert result_lower.status == HandlerResultStatus.FAIL + + result_upper = file_exists_handler({"files": ["README.md"]}, ctx) + assert result_upper.status == HandlerResultStatus.PASS + assert result_upper.evidence["relative_path"] == "README.md" + + def test_nested_case_exact_rejection(self, tmp_path, ctx): + """Nested paths must match exact case component-by-component (issue #645).""" + clear_dir_entries_cache() + docs = tmp_path / "docs" + docs.mkdir() + (docs / "ARCHITECTURE.md").write_text("# Architecture") + + assert file_exists_handler({"files": ["docs/architecture.md"]}, ctx).status == HandlerResultStatus.FAIL + assert file_exists_handler({"files": ["DOCS/ARCHITECTURE.md"]}, ctx).status == HandlerResultStatus.FAIL + pass_res = file_exists_handler({"files": ["docs/ARCHITECTURE.md"]}, ctx) + assert pass_res.status == HandlerResultStatus.PASS + assert Path(pass_res.evidence["relative_path"]) == Path("docs/ARCHITECTURE.md") + + def test_glob_case_exact_match(self, tmp_path, ctx): + """Glob patterns verify case against real directory entries (issue #645).""" + clear_dir_entries_cache() + docs = tmp_path / "docs" + docs.mkdir() + (docs / "ARCHITECTURE.md").write_text("# Architecture") + + res = file_exists_handler({"files": ["docs/*.md"]}, ctx) + assert res.status == HandlerResultStatus.PASS + assert Path(res.evidence["relative_path"]) == Path("docs/ARCHITECTURE.md") + class TestFileExistsHandlerDepthLimited: """Regression tests for issue #221 — depth-limited file discovery. @@ -215,6 +255,76 @@ def test_max_depth_with_glob_pattern_unchanged(self, tmp_path, ctx): # No top-level *.md files → FAIL (glob doesn't recurse without **) assert result.status == HandlerResultStatus.FAIL + def test_max_depth_case_exact_rejection(self, tmp_path, ctx): + """Depth-limited walk checks case-exact entry in directory (issue #645).""" + clear_dir_entries_cache() + nested = tmp_path / "deep" / "dir" + nested.mkdir(parents=True) + (nested / "MANIFEST.toml").write_text("pkg") + + fail_res = file_exists_handler({"files": ["manifest.toml"], "max_depth": 2}, ctx) + assert fail_res.status == HandlerResultStatus.FAIL + + pass_res = file_exists_handler({"files": ["MANIFEST.toml"], "max_depth": 2}, ctx) + assert pass_res.status == HandlerResultStatus.PASS + assert pass_res.evidence["relative_path"] == os.path.join("deep", "dir", "MANIFEST.toml") + + def test_max_depth_broken_symlink_not_found(self, tmp_path, ctx): + """Broken symlinks matching pattern are not counted as found (issue #645).""" + clear_dir_entries_cache() + nested = tmp_path / "deep" / "dir" + nested.mkdir(parents=True) + broken = nested / "go.mod" + try: + broken.symlink_to(tmp_path / "nonexistent") + except OSError: + pytest.skip("Symlinks not supported on this filesystem") + + result = file_exists_handler( + {"files": ["go.mod"], "max_depth": 2}, ctx + ) + assert result.status == HandlerResultStatus.FAIL + + def test_max_depth_slash_containing_entry(self, tmp_path, ctx): + """Slash-containing entries in max_depth walks are matched case-exactly (fixes #645).""" + clear_dir_entries_cache() + dist = tmp_path / "dist" + dist.mkdir() + (dist / "app.exe").write_text("binary") + + pass_res = file_exists_handler( + {"files": ["dist/app.exe"], "max_depth": 1, "existence": True}, ctx + ) + assert pass_res.status == HandlerResultStatus.PASS + assert pass_res.evidence["relative_path"] == "dist/app.exe" + + fail_case = file_exists_handler( + {"files": ["dist/APP.EXE"], "max_depth": 1, "existence": True}, ctx + ) + assert fail_case.status == HandlerResultStatus.FAIL + + def test_file_exists_double_dot_prefixed_filename(self, tmp_path, ctx): + """Files starting with two dots (..b.md) are recognized in the repository.""" + clear_dir_entries_cache() + (tmp_path / "..b.md").write_text("content") + + res = file_exists_handler({"files": ["..b.md"], "existence": True}, ctx) + assert res.status == HandlerResultStatus.PASS + + def test_file_exists_records_outside_path(self, tmp_path, ctx): + """Paths escaping the repository are recorded in skipped_outside_repository.""" + clear_dir_entries_cache() + outside_file = tmp_path.parent / "outside_probe.md" + outside_file.write_text("outside") + try: + res = file_exists_handler({"files": ["../outside_probe.md"]}, ctx) + assert res.status == HandlerResultStatus.FAIL + assert "skipped_outside_repository" in res.evidence + assert any("outside_probe.md" in p for p in res.evidence["skipped_outside_repository"]) + finally: + if outside_file.exists(): + outside_file.unlink() + # ============================================================================= # exec_handler @@ -595,6 +705,64 @@ def test_evidence_includes_resolved_files_on_match_path(self, tmp_path, ctx): assert str(security) in resolved assert not any("MISSING.md" in p for p in resolved) + def test_regex_handler_case_exact_rejection(self, tmp_path, ctx): + """Regex handler ignores files with mismatched case (issue #645).""" + clear_dir_entries_cache() + (tmp_path / "README.md").write_text("target_token") + + res_wrong_case = regex_handler( + {"files": ["readme.md"], "pattern": "target_token"}, + ctx, + ) + assert res_wrong_case.status == HandlerResultStatus.INCONCLUSIVE + + res_correct = regex_handler( + {"files": ["README.md"], "pattern": "target_token"}, + ctx, + ) + assert res_correct.status == HandlerResultStatus.PASS + + def test_resolve_regex_files_deduplicates_same_file(self, tmp_path): + """Duplicate paths or aliases resolving to same physical file are deduplicated.""" + clear_dir_entries_cache() + f = tmp_path / "SECURITY.md" + f.write_text("security content") + + resolved = _resolve_regex_files( + {"files": ["SECURITY.md", "SECURITY.md"]}, + HandlerContext(local_path=str(tmp_path)), + ) + assert resolved == [str(f)] + + def test_regex_exclude_max_depth_slash_containing_entry(self, tmp_path, ctx): + """Exclude mode with slash-containing entry and max_depth finds candidate (fixes #645).""" + clear_dir_entries_cache() + dist = tmp_path / "dist" + dist.mkdir() + (dist / "app.exe").write_text("binary") + + res = regex_handler( + {"exclude_files": ["dist/app.exe"], "max_depth": 1, "expr": "output.files_found == 0", "existence": True}, + ctx, + ) + assert res.status == HandlerResultStatus.FAIL + assert res.evidence["files_found"] == 1 + assert "dist/app.exe" in res.evidence["found_files"] + + def test_regex_outside_path_recorded(self, tmp_path, ctx): + """Regex handler records files resolving outside the repository.""" + clear_dir_entries_cache() + outside_file = tmp_path.parent / "outside_regex.md" + outside_file.write_text("secret") + try: + res = regex_handler({"files": ["../outside_regex.md"], "pattern": "secret"}, ctx) + assert res.status == HandlerResultStatus.INCONCLUSIVE + assert "skipped_outside_repository" in res.evidence + assert any("outside_regex.md" in p for p in res.evidence["skipped_outside_repository"]) + finally: + if outside_file.exists(): + outside_file.unlink() + # ============================================================================= # regex_handler — depth-limited file discovery @@ -951,6 +1119,60 @@ def test_no_fallback_when_gathered_evidence_has_no_resolved_files(self, ctx): ) assert result.details["consultation_request"]["file_contents"] == {} + def test_files_to_include_skips_case_mismatched_paths(self, tmp_path, ctx): + """Case mismatched paths in files_to_include are not loaded (issue #645).""" + clear_dir_entries_cache() + docs = tmp_path / "docs" + docs.mkdir() + (docs / "ARCHITECTURE.md").write_text("# Real Architecture") + + result = llm_eval_handler( + {"prompt": "Evaluate", "files_to_include": ["docs/architecture.md"]}, + ctx, + ) + fc = result.details["consultation_request"]["file_contents"] + assert "docs/architecture.md" not in fc + assert "docs/ARCHITECTURE.md" not in fc + assert fc == {} + + def test_files_to_include_deduplicates_duplicate_or_alias_files(self, tmp_path, ctx): + """Duplicate references to same file are only loaded once (issue #645).""" + clear_dir_entries_cache() + (tmp_path / "README.md").write_text("# Readme Content") + + result = llm_eval_handler( + {"prompt": "Evaluate", "files_to_include": ["README.md", "README.md"]}, + ctx, + ) + fc = result.details["consultation_request"]["file_contents"] + assert list(fc.keys()) == ["README.md"] + assert fc["README.md"] == "# Readme Content" + + def test_llm_eval_records_outside_and_dangling_symlink(self, tmp_path, ctx): + """llm_eval records outside paths and dangling symlinks in skipped_outside_repository.""" + clear_dir_entries_cache() + outside_file = tmp_path.parent / "outside_eval.md" + outside_file.write_text("outside doc") + dangling = tmp_path / "dangling.md" + try: + dangling.symlink_to(tmp_path.parent / "nonexistent.md") + except OSError: + pass + + try: + res = llm_eval_handler( + {"prompt": "Evaluate", "files_to_include": ["../outside_eval.md", "dangling.md"]}, + ctx, + ) + assert res.status == HandlerResultStatus.INCONCLUSIVE + skipped = res.evidence.get("skipped_outside_repository", []) + assert any("outside_eval.md" in p for p in skipped) + if dangling.is_symlink(): + assert "dangling.md" in skipped + finally: + if outside_file.exists(): + outside_file.unlink() + # ============================================================================= # manual_steps_handler