Skip to content

fix: honor git global core.excludesFile as an ignore source#542

Open
mvanhorn wants to merge 2 commits into
DeusData:mainfrom
mvanhorn:fix/499-global-core-excludesfile-ignore-source
Open

fix: honor git global core.excludesFile as an ignore source#542
mvanhorn wants to merge 2 commits into
DeusData:mainfrom
mvanhorn:fix/499-global-core-excludesfile-ignore-source

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

What does this PR do?

Honors git's global core.excludesFile (including the XDG default ~/.config/git/ignore) as a repo-root ignore source during discovery, matching git's own ignore precedence. Previously only in-repo .gitignore / .git/info/exclude were consulted, so files a user globally ignores were still indexed.

This resolves the half of #499 the maintainer separated from the in-flight .git/info/exclude work in #493: the global excludes chain (explicit core.excludesFile, then $XDG_CONFIG_HOME/git/ignore, then ~/.config/git/ignore) is now read and applied at the repo root.

Closes #499

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

AI was used for assistance.

Implements 2026-06-20-013-fix-global-core-excludesfile-ignore-source.

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
@DeusData

Copy link
Copy Markdown
Owner

Hi @mvanhorn — thank you for this. Honoring git's global excludes (#499) is a real gap, and your layering (~/.gitconfig, XDG, plus .cbmignore negation) is the right shape. The tests are thorough, too.

Before we can land it, there's one security issue in the resolution order we need to sort out. resolve_global_excludes_path() reads core.excludesFile from the repo-local .git/config first, and expand_git_path() accepts any absolute path, which then gets opened by cbm_gitignore_load()fopen() with no containment check.

Because cbm indexes arbitrary directories on request (an MCP client/agent can point it at any path, including an untrusted repo someone just cloned), a repository can ship a .git/config like:

[core]
    excludesFile = ~/.ssh/id_rsa

and simply indexing that repo will open and read that file. git itself is safe here because it only reads a repo's .git/config after you've deliberately cloned/init'd it — that trust step doesn't exist for an indexer pointed at an arbitrary tree.

The direction we'd suggest:

  • Drop the repo-local .git/config pass and resolve excludesFile only from the user-owned global configs (~/.gitconfig and $XDG_CONFIG_HOME/git/config). Those are the files the user controls, and they're what index_repository silently ignores git global core.excludesFile — globally-excluded files are indexed #499 is actually asking for. A repo's own ignores are already handled by .gitignore / .cbmignore.
  • If you do want to keep any repo-sourced path, resolve it and verify it stays within the repository root before opening it, and reject anything that escapes the tree.
  • Add a guard test: a repo-local .git/config whose excludesFile points outside the repo (e.g. into $HOME) must be ignored, not read.

One note on timing: we think this is worth doing, but we're capacity-constrained right now, so even once the security fix is in it may take a while to land in a release. If others want it, a 👍 on #499 helps us prioritize. And if you'd rather not carry the rework, we're happy to build on top of your branch and credit you — just say the word.

Really appreciate the PR and the careful tests. 🙏

Drop the repo-local .git/config pass in resolve_global_excludes_path() so
an untrusted repo cannot point core.excludesFile at an arbitrary file (e.g.
~/.ssh/id_rsa) and have cbm open and read it merely by being indexed. git is
safe here because it only trusts a repo's .git/config after a deliberate
clone/init; the indexer has no such trust step. Global excludes now resolve
only from the user-owned ~/.gitconfig and $XDG_CONFIG_HOME/git/config. A
repo's own ignores remain handled by .gitignore / .cbmignore.

Add a guard test asserting a repo-local .git/config whose excludesFile points
outside the repo is ignored, not read.

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
@DeusData

Copy link
Copy Markdown
Owner

Follow-up after a closer security review — I want to reframe my earlier comment.

On reflection this isn't a blocking vulnerability: reading core.excludesFile from the repo-local .git/config mirrors git's own --exclude-standard behavior, the file is opened read-only, its contents aren't returned to the caller, and planting a malicious .git/config already implies local file access.

So please treat the earlier note as recommended hardening / defense-in-depth, not a blocker: restricting a repo-local excludesFile to paths within the repo (or only honoring the user-owned global configs) is still a nice-to-have given cbm can be pointed at untrusted trees — but it's optional, and the feature itself is fine. Thanks again, and apologies for the over-strong initial framing. 🙏

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Done in 9f66e42.

  • Dropped the repo-local .git/config pass entirely in resolve_global_excludes_path(). core.excludesFile now resolves only from the user-owned configs (~/.gitconfig, then $XDG_CONFIG_HOME/git/config), so an untrusted repo can no longer point it at something like ~/.ssh/id_rsa and have cbm read it during indexing. A repo's own ignores stay handled by .gitignore / .cbmignore, as you said.
  • Added a guard test (discover_repo_local_excludesfile_is_ignored): a repo-local .git/config whose excludesFile points into $HOME is ignored, and the file it tried to hide is still discovered.

Verified locally with the discover suite under ASan/UBSan (319 passing, including the new test). No rush on landing given your capacity note, and happy to leave it here as-is. Thanks for the careful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

index_repository silently ignores git global core.excludesFile — globally-excluded files are indexed

2 participants