fix: prevent cache poisoning by force-resetting worktree on failed PR remediation

[AftAb-25]

Fixes #6367

Description

This fixes a cache poisoning bug in the PR remediator where failed remediations would leak uncommitted changes into the shared ingest cache, causing silent evaluation failures for subsequent rules.

When pull_request.go aborts a remediation and runs checkoutToOriginallyFetchedBranch, it previously used the default go-git checkout which preserves modifications to the worktree/index. Because the executor uses a single shared ingestCache.Fs per run, those left-behind dirty files would then be incorrectly evaluated by subsequent rules.

This PR aggressively resets the worktree during the checkout cleanup phase:

• Adds Force: true to the checkout options to discard modified tracked files.
• Calls wt.Clean(&git.CleanOptions{Dir: true}) to remove any untracked leftover files.

I've also added a unit test (checkout_cleanup_test.go) that simulates a dirty worktree and ensures both tracked modifications and untracked files are correctly wiped out.

Checklist

• Code compiles correctly
• Added tests that fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary

[coveralls]

coverage: 59.534% (-0.005%) from 59.539% — AftAb-25:fix/6367-remediation-cache-poisoning into mindersec:main

[evankanderson]

Nice find! A few minor cleanups, but I'd love to merge this soon.

[evankanderson]

Can you add this to pull_request_test.go, rather than creating a new test file (which doesn't correspond to any of the regular .go files in this directory)?

[evankanderson]

util.WriteFile from go-billy/v5/util might be simpler:

Suggested change

	f, err = mfs.Create("README.md")
	require.NoError(t, err)
	_, err = f.Write([]byte("dirty content from failed remediation"))
	require.NoError(t, err)
	require.NoError(t, f.Close())
	require.NoError(util.WriteFile(mfs, "README.md", []byte("dirty content from failed remediation"), 0644))

[evankanderson]

It looks like the default behavior is MixedReset, which will reset the index but not the working tree. This should prevent importing already-changed files into the branch in most cases, but the worktree corruption could affect future evaluations, since the order is currently "evaluate one rule, then evaluate its actions".

[AftAb-25]

Hi @evankanderson, thank you for the review and the thoughtful suggestion!

I looked into relying solely on go-git's native Checkout logic to evaluate tracked vs untracked files, and I ran unit tests dropping the .Clean() call to verify. Unfortunately, it turns out that go-git's Checkout does not natively handle explicitly untracked files.

According to the official go-git [CheckoutOptions documentation](https://pkg.go.dev/github.com/go-git/go-git/v5/worktree#CheckoutOptions):

"Force, if true, any local modification to existing files will be dropped, and any untracked files will remain."

My local test assertions confirmed this limitation:

1. If we remove Force: true: Checkout cleanly falls back to MergeReset. However, because the remediator strictly operates by modifying files, the checkout immediately throws an ErrUnstagedChanges exception. This leaves the modifications intact and leaves the cache fully poisoned.
2. If we use Force: true but remove .Clean(): The tracked files are successfully reverted, but any completely new, untracked files generated during the aborted remediation run (e.g. .lock files or build artifacts) persistently remain in the directory.

Because we are checking out over a natively shared ingestcache where subsequent, unrelated rule evaluations heavily depend on a pure state, both Checkout(Force: true) (to wipe modified tracked files) and wt.Clean() (to strictly purge newly created untracked files) are structurally required to guarantee a fully hermetic state after an abort.

Let me know if this context makes sense, or if you'd like me to look into implementing this via a different go-git mechanism (like a manual HardReset combination) instead!

[evankanderson]

Hi @evankanderson, thank you for the review and the thoughtful suggestion!

I looked into relying solely on go-git's native Checkout logic to evaluate tracked vs untracked files, and I ran unit tests dropping the .Clean() call to verify. Unfortunately, it turns out that go-git's Checkout does not natively handle explicitly untracked files.

Yes, I agreed with you in this comment: #6368 (comment)

(We shouldn't have any untracked files except in very unusual circumstances, but it looks like even tracked file changes may persist in the worktree for future evaluations.)

However, I had two other requests as well:

• Put the additional tests alongside the existing tests in pull_request_test.go
• Use util.WriteFile from Billy's util module to simplify the code

[github-actions]

This PR needs additional information before we can continue. It is now marked as stale because it has been open for 30 days with no activity. Please provide the necessary details to continue or it will be closed in 30 days.