Report a dirty worktree only when git status does - #161
Conversation
A client's scan reported a dirty worktree on a tree their own git status called clean. Two sources of that: the CLI also treated assume-unchanged and skip-worktree index bits as dirtiness (a sparse checkout sets the latter on every file it leaves out), and libgit2's own status disagrees with git wherever it cannot reproduce git's work - it runs no clean filters (git-lfs and friends), cannot read a sparse index, and ignores a status.showUntrackedFiles preference. Dirtiness is now what git status reports, asked of git itself, with libgit2 standing in only when the git binary cannot answer. That collapses the upload flag and the user notice into one signal, so the scan, the --skip-if-commit-scanned-recently reuse gate, and the notice all describe the state a user can check for themselves. Co-authored-by: ibrahim <ibrahim@corgea.com>
There was a problem hiding this comment.
No actionable findings; I found no reason this change should block merging.
I verified the merge-base diff and all dirty-state call sites: upload reconciliation still fails closed on missing samples or SHA drift, --target/--exclude still force the upload dirty bit without producing a worktree notice, and skip-scan reuse now consumes the same Git-status signal as upload and the notice. The tests cover tracked, staged, untracked, ignored, assume-unchanged, skip-worktree, clean-filter, submodule, upload, and reuse behavior. I also checked the subprocess failure fallback, hook-exported Git environment scrubbing, and build portability across the release targets.
Sent by Cursor Automation: pr-flow
|
@cursor[bot] Regarding #161 (review): I agree with this finding and think it should be addressed. high: Hidden worktree changes can reuse an unrelated clean scan
Proof or reproduction: |
There was a problem hiding this comment.
Automated review risk: 4/5.
The new Git-status-only dirtiness check can reuse a scan for source different from the current worktree, creating a security-scan bypass.
Critical or high-priority changes must be addressed.
Automatic approval was not submitted: checks failed: rust-tests.
…-matches-git-status-ff66 Co-authored-by: ibrahim <ibrahim@corgea.com>
Resolves Cargo.lock in main's favour: #162 upgraded h2 and git2 together, superseding the h2-only bump on this branch. git2 0.21 changed Reference::symbolic_target to Result<Option<&str>, Error>, so default_branch unwraps both layers. #161 removed RepoInfo::status_dirty and made .dirty mean what git status reports; the incremental gate already read .dirty only, so it now refuses exactly the trees a user can see are dirty. Co-authored-by: ibrahim <ibrahim@corgea.com>


Summary
A client saw
corgea scanreport a dirty worktree on a checkout their owngit statuscalled clean. Dirtiness is now exactly whatgit statusreports, asked of git itself.Two things made the old answer diverge from the user's:
--skip-if-commit-scanned-recentlyreuse gate also counted index hide-bits —assume-unchangedandskip-worktree— as dirtiness. A sparse checkout setsskip-worktreeon every file it leaves out, so an entirely clean repository read as dirty.status.showUntrackedFilespreference. Each disagreement surfaced as an uncommitted changegit statusdoes not show.git --no-optional-locks status --porcelainnow answers, run at the worktree root with git's repository-pinning environment variables scrubbed (so a scan invoked from a hook still reads the tree it was pointed at), and--no-optional-locksso a read never writes the user's index. libgit2 stands in only when the git binary cannot answer, and a status nobody can produce still fails closed to dirty.That collapses
RepoInfo's two flags into one, so the scan upload'sdirty, the reuse gate, and the "Working tree has uncommitted changes" notice all describe the same state — the one a user can check for themselves. The upload flag still marks a--target/--excludearchive and SHA drift during packaging as not-an-exact-commit-snapshot; only worktree sampling changed.Behaviour change
--skip-if-commit-scanned-recentlynow reuses a scan on a tree whose only differences are invisible togit status(assume-unchanged, skip-worktree, filtered content).--ignore-dirty-worktreeis no longer needed for those cases; it keeps working for a genuinely dirty tree and for a prior scan that recordedworktree_dirty.Test plan
origin/main(d7ff670, the git2 0.21 upgrade) into the branch — no conflicts, and none of the 0.21 accessor changes touch this code../harness checkand./harness ciare both green after the merge: clippy (fix + strict), format, dep audit, 750 tests, coverage gate at min 13%.scan_clean_by_git_status_sends_dirty_false_without_worktree_notice: a filtered checkout uploadsdirty=falseand prints no notice. Against the pre-fix source it fails withmissing multipart field dirty="false".a_file_hidden_from_git_status_*: an assume-unchanged edit now reuses the commit's scan instead of scanning, and the--ignore-dirty-worktreevariant of it is gone with the behaviour it covered.origin/mainand from this branch, run against a stub API in a repository with a clean filter, an assume-unchanged file and stale timestamps — full output in the logs below.dirty_worktree_after_merge.log
dirty_worktree_tests.log
To show artifacts inline, enable in settings.