From 49e0e7c9008040a5c10985dfefa8113d9ed844b2 Mon Sep 17 00:00:00 2001 From: Linxiushen <142557582+Linxiushen@users.noreply.github.com> Date: Thu, 24 Sep 2026 16:28:24 +0800 Subject: [PATCH 1/2] fix(plugin): exclude nested Git metadata from scan inventories The shared file inventory is built with `rg --files --hidden --glob '!.git/**'`. A ripgrep glob that contains a slash is anchored at the search root, so `!.git/**` only prunes the repository's own top-level `.git` directory. Two common layouts slip through: - a vendored or nested checkout keeps its own `.git` directory, and `--hidden` makes ripgrep descend into it, so `vendor/lib/.git/HEAD`, `vendor/lib/.git/config`, and loose object files land in the inventory; - a linked `git worktree` stores its gitdir pointer in a `.git` *file*, which `.git/**` never matches, so `./.git` is listed as a source file. The inventory is the authorized list that scan coverage is reconciled against, so Git internals become paths the scan is expected to review or defer, even though ranking already drops any path with a `.git` component and can never score them. Use `!**/.git` instead. The leading `**/` makes the pattern match a `.git` entry at any depth, ripgrep stops descending once the directory is excluded, and a plain `.git` file is excluded as well. Apply the same glob to the scoped ripgrep call in `generate_rank_input.py` and to the two mirrored oracle commands in the tests so they keep matching the command the scripts actually run. Co-Authored-By: Claude Opus 5 (1M context) --- .../mcp-app/tests/test_artifact_inventory.mjs | 2 +- .../scripts/generate_in_scope_files.py | 2 +- .../scripts/generate_rank_input.py | 2 +- .../tests/test_generate_in_scope_files.py | 42 ++++++++++++++++++- 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs index 9ff683970a..4cb3fa2bb9 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs @@ -456,7 +456,7 @@ async function standardInventory(repository, scope) { "--files", "--hidden", "--glob", - "!.git/**", + "!**/.git", "--path-separator=/", "--", scope, diff --git a/plugins/codex-security/scripts/generate_in_scope_files.py b/plugins/codex-security/scripts/generate_in_scope_files.py index 6b5807933b..5f9f8b56c1 100644 --- a/plugins/codex-security/scripts/generate_in_scope_files.py +++ b/plugins/codex-security/scripts/generate_in_scope_files.py @@ -95,7 +95,7 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: "--path-separator", "/", "--glob", - "!.git/**", + "!**/.git", "--", scope, ] diff --git a/plugins/codex-security/scripts/generate_rank_input.py b/plugins/codex-security/scripts/generate_rank_input.py index 0d35f3d62d..60111ead9b 100644 --- a/plugins/codex-security/scripts/generate_rank_input.py +++ b/plugins/codex-security/scripts/generate_rank_input.py @@ -543,7 +543,7 @@ def make_repo_scope_input(args: argparse.Namespace) -> None: "--no-require-git", "--null", "--glob", - "!.git/**", + "!**/.git", "--", str(scope_path.relative_to(repo)), ] diff --git a/plugins/codex-security/tests/test_generate_in_scope_files.py b/plugins/codex-security/tests/test_generate_in_scope_files.py index 4dbf9d8de9..16494a0cff 100644 --- a/plugins/codex-security/tests/test_generate_in_scope_files.py +++ b/plugins/codex-security/tests/test_generate_in_scope_files.py @@ -3,7 +3,7 @@ import os import subprocess import sys -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest @@ -73,7 +73,7 @@ def run_inventory( def standard_inventory(repository: Path, scope: str) -> bytes: files = subprocess.run( - ["rg", "--files", "--hidden", "--glob", "!.git/**", "--path-separator=/", "--", scope], + ["rg", "--files", "--hidden", "--glob", "!**/.git", "--path-separator=/", "--", scope], cwd=repository, capture_output=True, check=False, @@ -148,6 +148,44 @@ def test_inventory_keeps_ignored_tracked_files_without_ignored_untracked_files( assert "./app/ignored.skip" not in paths +@pytest.mark.parametrize( + ("scope", "expected"), + [(".", "./vendor/lib/handler.py"), ("vendor", "vendor/lib/handler.py")], +) +def test_inventory_excludes_git_metadata_from_a_nested_repository( + tmp_path: Path, scope: str, expected: str +) -> None: + repository = make_repository(tmp_path) + write_file(repository, "vendor/lib/handler.py") + write_file(repository, "vendor/lib/.git/HEAD", b"ref: refs/heads/main\n") + write_file(repository, "vendor/lib/.git/config", b"[core]\n\tbare = false\n") + write_file(repository, "vendor/lib/.git/objects/ab/cdef", b"blob") + output = tmp_path / "in_scope_files.txt" + + result = run_inventory(repository, scope, output) + + assert result.returncode == 0, result.stderr + paths = output.read_text(encoding="utf-8").splitlines() + assert expected in paths + assert all(".git" not in PurePosixPath(path).parts for path in paths) + + +def test_inventory_excludes_the_git_file_of_a_linked_worktree(tmp_path: Path) -> None: + repository = make_repository(tmp_path) + git(repository, "add", ".") + git(repository, "commit", "-qm", "base") + worktree = tmp_path / "worktree" + git(repository, "worktree", "add", "-q", str(worktree), "HEAD") + output = tmp_path / "in_scope_files.txt" + + result = run_inventory(worktree, ".", output) + + assert result.returncode == 0, result.stderr + paths = output.read_text(encoding="utf-8").splitlines() + assert "./app/routes.py" in paths + assert "./.git" not in paths + + @pytest.mark.skipif(os.name == "nt", reason="Windows does not allow CR/LF in filenames") @pytest.mark.parametrize("separator", ["\n", "\r", "\r\n"]) @pytest.mark.parametrize("ignored_tracked", [False, True]) From 77a5906886845d0ceb27e85951cfdd1b87faca84 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 30 Sep 2026 13:52:50 +0000 Subject: [PATCH 2/2] fix(plugin): preserve Git metadata exclusions for directory scopes --- .../mcp-app/tests/test_artifact_inventory.mjs | 46 +++++++++++++++++ .../scripts/generate_in_scope_files.py | 3 ++ .../scripts/generate_rank_input.py | 3 ++ .../tests/test_generate_in_scope_files.py | 50 ++++++++++++++++--- 4 files changed, 96 insertions(+), 6 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs index 4cb3fa2bb9..755ecee483 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_inventory.mjs @@ -33,6 +33,7 @@ const inventory = await import( try { await testSchemasAreBoundAndExact(); await testPrepareUsesTheExistingStandardGenerator(); + await testPrepareExcludesGitMetadata(); await testPrepareUsesOnlyAuthoritativeDiffChanges(); await testPrepareIncludesStagedAndUnstagedChanges(); await testWorkerReadsItsOwnBoundInventory(); @@ -147,6 +148,49 @@ async function testPrepareUsesTheExistingStandardGenerator() { ); } +async function testPrepareExcludesGitMetadata() { + const fixture = await createFixture("nested repositories"); + for (const [name, source] of [ + [".git/config", "[core]\n"], + ["vendor/lib/.git/HEAD", "ref: refs/heads/main\n"], + ["vendor/lib/.git/hooks/example.py", "pass\n"], + ["vendor/lib/handler.py", "pass\n"], + ["vendor/worktree/.git", "gitdir: ../../.git/worktrees/example\n"], + ["vendor/worktree/handler.py", "pass\n"], + [".gitignore", "*.skip\n"], + [".github/workflows/check.yml", "name: example\n"], + ["src/widget.git", "example\n"], + ]) { + await writeRepositoryFile(fixture.repoRoot, name, source); + } + + assert.deepEqual( + await inventory.prepareCodexSecurityReviewItems(fixture.scan), + { reviewItemsTotal: 5 }, + ); + assert.deepEqual(await inventory.listCodexSecurityReviewItems(fixture.scan), { + items: [ + { path: "./.github/workflows/check.yml" }, + { path: "./.gitignore" }, + { path: "./src/widget.git" }, + { path: "./vendor/lib/handler.py" }, + { path: "./vendor/worktree/handler.py" }, + ], + }); + + const metadataScope = { ...fixture.scan, scope: "vendor/lib/.git/hooks" }; + assert.deepEqual( + await inventory.prepareCodexSecurityReviewItems(metadataScope), + { reviewItemsTotal: 0 }, + ); + assert.deepEqual( + await inventory.listCodexSecurityReviewItems(metadataScope), + { + items: [], + }, + ); +} + async function testPrepareUsesOnlyAuthoritativeDiffChanges() { const fixture = await createFixture("selected committed changes"); await initializeRepository(fixture.repoRoot); @@ -457,6 +501,8 @@ async function standardInventory(repository, scope) { "--hidden", "--glob", "!**/.git", + "--glob", + "!**/.git/**", "--path-separator=/", "--", scope, diff --git a/plugins/codex-security/scripts/generate_in_scope_files.py b/plugins/codex-security/scripts/generate_in_scope_files.py index 5f9f8b56c1..ad3d843ae1 100644 --- a/plugins/codex-security/scripts/generate_in_scope_files.py +++ b/plugins/codex-security/scripts/generate_in_scope_files.py @@ -94,8 +94,11 @@ def generate_in_scope_files(repository: Path, scope: str, output: Path) -> int: "--hidden", "--path-separator", "/", + # Prune Git metadata and its contents even when the scope starts inside .git. "--glob", "!**/.git", + "--glob", + "!**/.git/**", "--", scope, ] diff --git a/plugins/codex-security/scripts/generate_rank_input.py b/plugins/codex-security/scripts/generate_rank_input.py index 60111ead9b..02b1f59245 100644 --- a/plugins/codex-security/scripts/generate_rank_input.py +++ b/plugins/codex-security/scripts/generate_rank_input.py @@ -542,8 +542,11 @@ def make_repo_scope_input(args: argparse.Namespace) -> None: "--hidden", "--no-require-git", "--null", + # Also exclude descendants when the scope starts inside .git. "--glob", "!**/.git", + "--glob", + "!**/.git/**", "--", str(scope_path.relative_to(repo)), ] diff --git a/plugins/codex-security/tests/test_generate_in_scope_files.py b/plugins/codex-security/tests/test_generate_in_scope_files.py index 16494a0cff..e54032d576 100644 --- a/plugins/codex-security/tests/test_generate_in_scope_files.py +++ b/plugins/codex-security/tests/test_generate_in_scope_files.py @@ -73,7 +73,18 @@ def run_inventory( def standard_inventory(repository: Path, scope: str) -> bytes: files = subprocess.run( - ["rg", "--files", "--hidden", "--glob", "!**/.git", "--path-separator=/", "--", scope], + [ + "rg", + "--files", + "--hidden", + "--glob", + "!**/.git", + "--glob", + "!**/.git/**", + "--path-separator=/", + "--", + scope, + ], cwd=repository, capture_output=True, check=False, @@ -170,20 +181,47 @@ def test_inventory_excludes_git_metadata_from_a_nested_repository( assert all(".git" not in PurePosixPath(path).parts for path in paths) -def test_inventory_excludes_the_git_file_of_a_linked_worktree(tmp_path: Path) -> None: +@pytest.mark.parametrize( + "scope", + [ + ".git", + "./.git", + ".git/hooks", + "vendor/lib/.git", + "./vendor/lib/.git", + "vendor/lib/.git/hooks", + ], +) +def test_inventory_excludes_metadata_when_scoped_inside_a_git_directory( + tmp_path: Path, scope: str +) -> None: + repository = make_repository(tmp_path) + for prefix in (".git", "vendor/lib/.git"): + write_file(repository, f"{prefix}/hooks/example.py") + output = tmp_path / "in_scope_files.txt" + + result = run_inventory(repository, scope, output) + + assert result.returncode == 0, result.stderr + assert output.read_bytes() == b"" + + +@pytest.mark.parametrize("nested", [False, True]) +def test_inventory_excludes_the_git_file_of_a_linked_worktree(tmp_path: Path, nested: bool) -> None: repository = make_repository(tmp_path) git(repository, "add", ".") git(repository, "commit", "-qm", "base") - worktree = tmp_path / "worktree" + worktree = repository / "vendor" / "lib" if nested else tmp_path / "worktree" git(repository, "worktree", "add", "-q", str(worktree), "HEAD") output = tmp_path / "in_scope_files.txt" - result = run_inventory(worktree, ".", output) + result = run_inventory(repository if nested else worktree, ".", output) assert result.returncode == 0, result.stderr paths = output.read_text(encoding="utf-8").splitlines() - assert "./app/routes.py" in paths - assert "./.git" not in paths + prefix = "./vendor/lib" if nested else "." + assert f"{prefix}/app/routes.py" in paths + assert all(".git" not in PurePosixPath(path).parts for path in paths) @pytest.mark.skipif(os.name == "nt", reason="Windows does not allow CR/LF in filenames")