Skip to content

Block only repository moves, not tag moves, on upgrade - #6225

Merged
samuv merged 2 commits into
mainfrom
fix/6213-ref-change-guard
Aug 6, 2026
Merged

samuv merged 2 commits into
mainfrom
fix/6213-ref-change-guard

Conversation

@samuv

@samuv samuv commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

A skill installed by catalog name records the resolved tag in its lock entry. When the catalog publishes a new version the tag moves with it, so planUpgrade's ref-change guard fired on every ordinary upgrade:

tdd: reference change blocked
  (would move to ghcr.io/stacklok/dockyard/skills/tdd:0.2.0; use --allow-ref-change)

That is the only way a catalog-sourced skill ever advances, so keeping one current meant passing --allow-ref-change unconditionally — and passing it disabled the check the guard exists for: a registry entry being repointed at a different repository. A guard that everyone has to switch off protects nothing, so the flag had quietly become a no-op in practice.

The fix compares the registry and repository path rather than the whole tagged reference:

  • a tag moving within one repository proceeds (ghcr.io/org/skill:0.1.0 → :0.2.0)
  • a move to a different repository, org, or registry still blocks
  • git references and unparseable input fall back to exact comparison, so they fail closed

A permitted move is still reported — new_resolved_reference is populated on the outcome either way — so the tag change stays visible in output and in the lock diff.

Neither adjacent protection is weakened: the digest is still pinned from the plan (so a permitted tag move installs exactly what was resolved, not whatever the tag points at later), and the signer-change guard still runs independently, so identity substitution is caught regardless of where the artifact lives.

Closes #6213

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)
  • Manual testing (describe below)

TestRepositoryMoved is a table over the comparison itself: identical refs, version bump, tag→digest in the same repository, implicit vs explicit latest, different repository path, different org, different registry, git-to-git, OCI-to-git, and unparseable input. Plus TestPlanUpgrade_TagMoveWithinRepositoryIsNotBlocked and TestPlanUpgrade_RegistryChangeStaysBlocked at the plan level. The pre-existing TestPlanUpgrade_OCIRefChangeGuard passes unchanged — it exercises a genuine repository repoint, which still blocks.

Manually, against the real catalog with a lock entry pinned at tdd:0.1.0 and source: tdd:

// no --allow-ref-change
{"name": "tdd", "status": "upgraded",
 "new_resolved_reference": "ghcr.io/stacklok/dockyard/skills/tdd:0.2.0"}

then, with the lock's resolvedReference hand-edited to ghcr.io/someone-else/skills/tdd:0.1.0 to simulate a repoint:

{"name": "tdd", "status": "ref-change-blocked",
 "new_resolved_reference": "ghcr.io/stacklok/dockyard/skills/tdd:0.2.0"}

Does this introduce a user-facing change?

Yes. thv skill upgrade no longer requires --allow-ref-change for a version bump within the same repository. The flag now means "permit the artifact to move to a different repository", and the blocked message says "repository change blocked". Anyone currently passing the flag keeps working — it is strictly more permissive than before.

task docs regenerated docs/cli/thv_skill_upgrade.md for the reworded flag help.

Special notes for reviewers

This loosens a security control, so it deserves a deliberate decision rather than a quiet merge. The issue laid out three options; this implements option 1 (compare repository, not full reference), which the issue argues is closest to the guard's stated intent. Option 2 — keep blocking both but give callers a narrower flag — is also defensible and I'm happy to switch if you'd prefer the more conservative shape.

The one behaviour worth naming explicitly: a downgrade within the same repository (catalog moving 0.2.0 → 0.1.0) is now permitted without the flag, where before it was blocked. It was already permitted with the flag, and the digest and signer checks still apply, but it is a real difference.

Generated with Claude Code

A skill installed by catalog name records the resolved tag in its lock
entry. When the catalog publishes a new version the tag moves with it, so
the ref-change guard fired on every ordinary upgrade and refused to
proceed without --allow-ref-change.

That made the flag mandatory for routine automation, and passing it
disabled the check the guard actually exists for: an entry being
repointed at a different repository. A guard everyone must switch off
protects nothing.

Compare the registry and repository path instead of the whole tagged
reference. A tag moving within one repository proceeds; a move to another
repository, org, or registry still blocks. Git references and
unparseable input fall back to exact comparison, so they fail closed.

The digest pin and the signer-change guard are unaffected: a permitted
tag move still installs exactly the planned digest and still verifies
against the recorded signer identity.

Closes #6213
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.46%. Comparing base (8863eb0) to head (582d25c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6225      +/-   ##
==========================================
- Coverage   72.48%   72.46%   -0.02%     
==========================================
  Files         739      739              
  Lines       76728    76744      +16     
==========================================
  Hits        55613    55613              
- Misses      17151    17167      +16     
  Partials     3964     3964              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JAORMX
JAORMX previously approved these changes Aug 6, 2026
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 6, 2026
@samuv samuv self-assigned this Aug 6, 2026
@samuv
samuv merged commit d29e0d5 into main Aug 6, 2026
48 checks passed
@samuv
samuv deleted the fix/6213-ref-change-guard branch August 6, 2026 09:11
@github-actions github-actions Bot mentioned this pull request Aug 10, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Catalog version bumps are classified as reference changes, so routine upgrades need --allow-ref-change

3 participants