microsoft / microsoft/amplifier
[amplifier-foundation] Failed pinned-SHA resolve leaves poisoned cache that silently serves the wrong commit
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 3.1k
- Forks
- 261
- Avg merge
- 3h 28m
- Merged PRs (30d)
- 13
Description
Component: amplifier-foundation (Issues disabled on foundation repo; filed here per ecosystem convention)
Summary
When a pinned-SHA git resolve fails on the full-clone fallback path (e.g. a typo'd but valid-looking 40-hex SHA), the cache directory is left behind containing a full clone checked out at the default-branch tip. The next resolve of the same pinned URI hits the cache, passes the integrity check, and silently serves that wrong commit — no error, ever again, until the cache is cleared. A mistyped pin silently becomes "latest", which is the exact failure mode pinning exists to prevent.
Reproduction
- Resolve a git URI pinned to a 40-hex SHA that doesn't exist in the repo (e.g. a typo of a real SHA):
git+https://…/repo@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - First resolve: the fast single-commit fetch fails (expected — "reference is not a tree"), the fallback full clone succeeds,
git checkout <sha>fails, andBundleNotFoundErrorpropagates with a clear error. So far so good. - But the exception propagates with no cleanup:
_clone_at_commit's fallback (amplifier_foundation/sources/git.py:274) does rmtree → full clone → checkout, and a checkout failure leavescache_pathcontaining a full clone at the default-branch tip. - Second resolve of the same URI: the cache-hit path (
git.py:303-315) checks only existence +_verify_clone_integrity. The orphan has.git+ the expected structure → integrity passes → the resolve succeeds and serves default-branch-tip content for a URI pinned to a SHA that never existed.
Observed Empirically
Bonus: the orphan has no .amplifier_cache_meta.json, and get_status on the URI still reports "Pinned to (no updates possible)" (git.py:419-423) — so the poisoned cache is also invisible to update checking.
What the Code Already Does (Partial-Fix Neighborhood)
- The fallback path DOES clean up before its own clone:
shutil.rmtree(cache_path, ignore_errors=True)atgit.py:274— but only before the clone, not on checkout failure after it. - The cache-hit path DOES self-heal one failure class:
_verify_clone_integrityfailure triggers rmtree + re-clone (git.py:305-307). Integrity verifies clone structure, not that the checked-out commit matches the requested pin — so a wrong-commit orphan sails through. - #284's retry wrapper (
_run_git_network_op,git.py:279) is unrelated: the clone succeeds; it's the checkout that fails, and checkout is not retried (correctly — retrying can't fix a nonexistent SHA).
Impact
Anyone using commit-SHA pinning for reproducibility — eval runs, CI, "same URI, same code" workflows — which is precisely the audience pinning targets. One mistyped pin turns a hard-fail into a silent tracking-latest source on every subsequent resolve from that cache. The failure is silent and persistent: no error, no update-status signal, wrong code served indefinitely.
Fix Shape
Two complementary changes:
- Cleanup on checkout failure (the writer): in
_clone_at_commit's fallback (git.py:274), wrap thegit checkout <sha>so a failure rmtree'scache_pathbefore re-raising. Two lines. - Resolved-commit check on cache hits for SHA refs (the reader): in
resolve()'s cache-hit path (git.py:303-315), whenparsed.refis a full commit SHA, verify the cached clone'sHEADequals the pinned SHA; on mismatch, treat as invalid cache (rmtree + re-resolve). This also heals any already-poisoned caches in the wild, which fix (1) alone cannot.
Test shape (composed-state, not branch coverage): one test whose input state is "cache dir left by a failed pin attempt" (full clone at wrong commit, no meta file) → resolve of the pinned URI must NOT serve it.
Environment
Linux aarch64 (Ubuntu, kernel 6.17), Python 3.13.11. Reproduced on amplifier-foundation main @ 49b97b1 (post-#283/#284); bug is pre-existing (present in #282 as merged; #284 did not change it).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read amplifier_foundation/sources/git.py, starting at _clone_at_commit around line 274 and the resolve() cache-hit path around lines 303-315. Reproduce the composed state with a wrong-commit full clone and no metadata, then add the regression test described in the issue. Done means failed checkouts leave no cache and a pinned resolve does not serve a cached clone whose HEAD differs from the requested SHA.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, python
- Domain
- devtools
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100