microsoft / microsoft/amplifier

[amplifier-foundation] Failed pinned-SHA resolve leaves poisoned cache that silently serves the wrong commit

Open
#347 0 comments 0 reactions 0 assignees View on GitHub

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

  1. 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
  2. First resolve: the fast single-commit fetch fails (expected — "reference is not a tree"), the fallback full clone succeeds, git checkout <sha> fails, and BundleNotFoundError propagates with a clear error. So far so good.
  3. 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 leaves cache_path containing a full clone at the default-branch tip.
  4. 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) at git.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_integrity failure 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:

  1. Cleanup on checkout failure (the writer): in _clone_at_commit's fallback (git.py:274), wrap the git checkout <sha> so a failure rmtree's cache_path before re-raising. Two lines.
  2. Resolved-commit check on cache hits for SHA refs (the reader): in resolve()'s cache-hit path (git.py:303-315), when parsed.ref is a full commit SHA, verify the cached clone's HEAD equals 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.