Harden Git provenance capture against repository config execution - #83
Open
tachyon-beep wants to merge 1 commit into
Open
Harden Git provenance capture against repository config execution#83tachyon-beep wants to merge 1 commit into
tachyon-beep wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR hardens annotation provenance capture by preventing repository-local Git configuration from triggering command execution during read-only Git invocations used by annotate_file.
Changes:
- Harden
_run_gitby passing per-invocationgitflags/config to disable repository-controlled helpers (e.g.,core.fsmonitor,diff.external). - Further harden
git diffcalls by adding--no-ext-diffand--no-textconv. - Add a regression test that configures hostile repo-local Git settings and asserts provenance capture completes without executing the helper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/filigree/db_annotations.py | Adds hardened git invocation flags and disables ext-diff/textconv for diff provenance. |
| tests/core/test_annotations.py | Adds a regression test to ensure repository-configured Git helpers are not executed during provenance capture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+199
to
+207
| "git", | ||
| "--no-optional-locks", | ||
| "-c", | ||
| "core.fsmonitor=false", | ||
| "-c", | ||
| "core.hooksPath=/dev/null", | ||
| "-c", | ||
| "diff.external=", | ||
| "-C", |
Comment on lines
+71
to
+79
| def test_provenance_does_not_execute_repository_git_config(self, tmp_path: Path) -> None: | ||
| db = _project_db(tmp_path) | ||
| try: | ||
| source = tmp_path / "tracked.py" | ||
| marker = tmp_path / "git-config-executed" | ||
| helper = tmp_path / "malicious-helper.sh" | ||
| helper.write_text(f"#!/bin/sh\ntouch '{marker}'\n") | ||
| helper.chmod(0o755) | ||
| source.write_text("original\n") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
core.fsmonitorordiff.external) from executing arbitrary commands during annotation provenance capture.Description
--no-optional-locksand per-invocation-coptions includingcore.fsmonitor=false,core.hooksPath=/dev/null, anddiff.external=in_run_git.--no-ext-diffand--no-textconvfordiffinvocations while preserving built-in Git diff output.test_provenance_does_not_execute_repository_git_configthat configures hostilecore.fsmonitoranddiff.externalhelpers and asserts that annotation provenance is captured without executing the repository helper.fix: harden annotation git provenance captureto harden annotation-related code paths used by the CLI and MCP tools.Testing
git diff --checksucceeded locally on the working tree.ruff check src/filigree/db_annotations.py tests/core/test_annotations.pypassed in the environment where checks were run.git --no-optional-locks -c core.fsmonitor=false -c core.hooksPath=/dev/null -c diff.external= -C . diff --no-ext-diff --no-textconv -- <path>) produced the expected diff output without invoking repo-local helpers.pytestandmypyruns could not be completed in the execution environment due to unavailable project dependencies and blocked PyPI access, so automated test execution of the new regression test was not fully verifiable in this environment.Codex Task