Skip to content

Commit ae39475

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(release): prove a stable tag is absent before copying over it
Review notes from #3435, and a correction to my first attempt at them. manifest_digest gated on skopeo`s exit status alone, so a zero exit with no bytes hashed the empty string to sha256:e3b0c442... — a digest that looks real and belongs to no manifest. The first fix simply required non-empty bytes, which was worse than it looked: the caller turns an empty result into "not published, safe to copy", so a registry answering 200 with an empty body went from an accidental refusal (the bogus digest mismatched, so the write-once check refused) to an actual copy over whatever the tag really held. Verified against real skopeo 1.23: a 200 with Content-Length 0 does exit 0 with no stdout. The helper is now explicitly three-state. A digest, or empty output for a tag the registry PROVED absent, or non-zero for anything indeterminate — which under set -e aborts the promotion before it writes. Absence has to be reported as "manifest unknown"; 429, 5xx, auth and network failures all exit non-zero with no bytes too, and reading those as "unpublished" is the same fail-open one level down. finalize retags ~42 refs with no retry wrapper, so a single rate-limit was enough to reach that path. skopeo`s stderr is captured instead of discarded, so the diagnostics now name the real failure. Tests: the write-once refusal branch the review found untested (an existing stable tag at a different digest must be refused before any write), the empty- body case (refuses, writes nothing, and the empty-input hash never appears in the diagnostics), and a 429 that must not read as an absent tag. The mock now emits realistic registry errors, since a bare non-zero exit is no longer proof of absence. Both new guards are mutation-checked: accepting any non-zero exit as absence fails the 429 test, dropping the empty-bytes check fails the empty-body test. 11/11 tests green, hack/image-pin-consistency.bats still 2/2, sh -n clean. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 5d8dd51 commit ae39475

2 files changed

Lines changed: 128 additions & 5 deletions

File tree

‎hack/promote-retag.sh‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,45 @@ ref_repo() {
8282
}
8383
ref_digest() { printf '%s' "${1##*@}"; } # sha256:...
8484

85+
# manifest_digest <ref> — the digest of <ref>'s raw manifest, or empty output if
86+
# the tag is PROVEN not to exist. Anything else is indeterminate and returns
87+
# non-zero, which under `set -e` aborts the promotion before it writes.
88+
#
89+
# The three states matter because the caller turns "empty" into "not published,
90+
# safe to copy over". Inferring that from a failure — or from a successful
91+
# response with no bytes — is how a rate-limit or a proxy hiccup gets read as
92+
# permission to overwrite released bytes. Absence has to be proven by the
93+
# registry saying so.
8594
manifest_digest() {
8695
_ref="$1"
8796
_manifest="$(mktemp)"
88-
# Hash the raw bytes so this works for images and OCI artifacts alike. A
89-
# file preserves inspect's status without relying on non-POSIX pipefail.
90-
if skopeo inspect --raw "docker://$_ref" >"$_manifest" 2>/dev/null; then
91-
printf 'sha256:%s' "$(sha256sum "$_manifest" | cut -d' ' -f1)"
97+
_mderr="$(mktemp)"
98+
# Hash the raw bytes so this works for images and OCI artifacts alike. Files
99+
# preserve inspect's status without relying on non-POSIX pipefail.
100+
if skopeo inspect --raw "docker://$_ref" >"$_manifest" 2>"$_mderr"; then
101+
if [ -s "$_manifest" ]; then
102+
printf 'sha256:%s' "$(sha256sum "$_manifest" | cut -d' ' -f1)"
103+
rm -f "$_manifest" "$_mderr"
104+
return 0
105+
fi
106+
# Exit 0 with no bytes — a registry answering 200 with an empty body. Not a
107+
# digest (hashing nothing yields sha256:e3b0c442…, which looks real and
108+
# belongs to no manifest) and not proof of absence either, so refuse to
109+
# decide rather than let the caller copy over whatever is really there.
110+
echo "::error::skopeo inspect of ${_ref} succeeded but returned no manifest bytes; refusing to decide whether that tag exists" >&2
111+
rm -f "$_manifest" "$_mderr"
112+
return 1
113+
fi
114+
# Non-zero. Only a registry that says the manifest is unknown proves absence;
115+
# 429/5xx/auth/network failures all also exit non-zero with no bytes, and
116+
# reading those as "not published" is exactly the fail-open this guards.
117+
if grep -qiE 'manifest unknown|manifest not known|manifest for [^ ]* not found|not found|404' "$_mderr"; then
118+
rm -f "$_manifest" "$_mderr"
119+
return 0
92120
fi
93-
rm -f "$_manifest"
121+
echo "::error::skopeo inspect of ${_ref} failed and did not report the manifest as unknown, so it cannot be treated as unpublished: $(tr '\n' ' ' <"$_mderr")" >&2
122+
rm -f "$_manifest" "$_mderr"
123+
return 1
94124
}
95125

96126
copy() {

‎hack/promote-retag_test.bats‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,22 @@ printf '%s\n' "$*" >>"$MOCK_SKOPEO_LOG"
4040
case "$1" in
4141
inspect)
4242
[ "$2" = "--raw" ]
43+
# An indeterminate failure: non-zero, no bytes, and the registry never says
44+
# the manifest is unknown. Must NOT read as "tag absent".
45+
if [ "${MOCK_TRANSIENT_ERROR:-0}" = "1" ]; then
46+
echo "Error: reading manifest from docker://${3:-?}: received unexpected HTTP status: 429 Too Many Requests" >&2
47+
exit 1
48+
fi
4349
if [ "$MOCK_MISSING_ONCE" = "1" ] && [ ! -f "$MOCK_STATE" ]; then
50+
# Absence as a registry actually reports it — the script requires proof,
51+
# not merely a non-zero exit.
52+
echo "Error: reading manifest from docker://${3:-?}: manifest unknown" >&2
4453
exit 1
4554
fi
55+
# A registry answering 200 with an empty body: exit 0, no bytes.
56+
if [ "${MOCK_EMPTY_MANIFEST:-0}" = "1" ]; then
57+
exit 0
58+
fi
4659
printf '%s' "$MOCK_MANIFEST"
4760
;;
4861
copy)
@@ -268,3 +281,83 @@ EOF
268281
[ "$(grep -c '^inspect --raw ' "$tmp/skopeo.log")" -eq 2 ]
269282
grep -q 'Retagged image refs to v1.6.0' "$tmp/out"
270283
}
284+
285+
@test "existing stable tag at a different digest is refused, not moved" {
286+
tmp=$(mktemp -d)
287+
trap 'rm -rf "$tmp"' EXIT
288+
_make_registry_mocks "$tmp"
289+
290+
# The stable tag already exists (MOCK_MISSING_ONCE=0) but resolves to a
291+
# manifest other than the rc's: released bytes must never be overwritten.
292+
published='{"schemaVersion":2,"config":{"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:1111111111111111111111111111111111111111111111111111111111111111"},"layers":[]}'
293+
rc_manifest='{"schemaVersion":2,"config":{"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:2222222222222222222222222222222222222222222222222222222222222222"},"layers":[]}'
294+
published_digest="sha256:$(printf '%s' "$published" | sha256sum | cut -d' ' -f1)"
295+
rc_digest="sha256:$(printf '%s' "$rc_manifest" | sha256sum | cut -d' ' -f1)"
296+
ref="example.com/cozystack/cozystack-packages:v1.6.0-rc.1@${rc_digest}"
297+
298+
rc=0
299+
REGISTRY="example.com/cozystack" MOCK_REF="$ref" MOCK_MANIFEST="$published" \
300+
MOCK_MISSING_ONCE=0 MOCK_STATE="$tmp/state" MOCK_SKOPEO_LOG="$tmp/skopeo.log" \
301+
PATH="$tmp/bin:/usr/bin:/bin" hack/promote-retag.sh v1.6.0 \
302+
>"$tmp/out" 2>"$tmp/err" || rc=$?
303+
304+
[ "$rc" -ne 0 ]
305+
grep -q "already exists at '${published_digest}'; refusing to move it to '${rc_digest}'" "$tmp/err"
306+
# The refusal must happen BEFORE any write.
307+
! grep -q '^copy ' "$tmp/skopeo.log"
308+
[ "$(grep -c '^inspect --raw ' "$tmp/skopeo.log")" -eq 1 ]
309+
}
310+
311+
@test "a zero-exit empty manifest refuses to decide and never writes" {
312+
tmp=$(mktemp -d)
313+
trap 'rm -rf "$tmp"' EXIT
314+
_make_registry_mocks "$tmp"
315+
316+
manifest='{"schemaVersion":2,"config":{"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:3333333333333333333333333333333333333333333333333333333333333333"},"layers":[]}'
317+
digest="sha256:$(printf '%s' "$manifest" | sha256sum | cut -d' ' -f1)"
318+
ref="example.com/cozystack/cozystack-packages:v1.6.0-rc.1@${digest}"
319+
320+
rc=0
321+
REGISTRY="example.com/cozystack" MOCK_REF="$ref" MOCK_MANIFEST="$manifest" \
322+
MOCK_EMPTY_MANIFEST=1 MOCK_MISSING_ONCE=0 MOCK_STATE="$tmp/state" \
323+
MOCK_SKOPEO_LOG="$tmp/skopeo.log" \
324+
PATH="$tmp/bin:/usr/bin:/bin" hack/promote-retag.sh v1.6.0 \
325+
>"$tmp/out" 2>"$tmp/err" || rc=$?
326+
327+
# A 200 with an empty body proves nothing: the tag may hold released bytes this
328+
# promotion must not overwrite. So the script must abort BEFORE any copy —
329+
# reading "no bytes" as "not published" is what would turn a proxy hiccup into
330+
# an overwrite of a published stable tag.
331+
[ "$rc" -ne 0 ]
332+
grep -q 'returned no manifest bytes' "$tmp/err"
333+
! grep -q '^copy ' "$tmp/skopeo.log"
334+
[ "$(grep -c '^inspect --raw ' "$tmp/skopeo.log")" -eq 1 ]
335+
# The empty-input hash must never appear: that is the guard being absent.
336+
! grep -q 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' "$tmp/err"
337+
}
338+
339+
@test "a transient registry failure is not read as an unpublished tag" {
340+
tmp=$(mktemp -d)
341+
trap 'rm -rf "$tmp"' EXIT
342+
_make_registry_mocks "$tmp"
343+
344+
manifest='{"schemaVersion":2,"config":{"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:4444444444444444444444444444444444444444444444444444444444444444"},"layers":[]}'
345+
digest="sha256:$(printf '%s' "$manifest" | sha256sum | cut -d' ' -f1)"
346+
ref="example.com/cozystack/cozystack-packages:v1.6.0-rc.1@${digest}"
347+
348+
# 429 during finalize: non-zero exit, no bytes, no "manifest unknown". The old
349+
# shape of this helper made that indistinguishable from an absent tag and
350+
# proceeded to copy over it; finalize retags ~42 refs with no retry wrapper, so
351+
# a single rate-limit was enough to reach that path.
352+
rc=0
353+
REGISTRY="example.com/cozystack" MOCK_REF="$ref" MOCK_MANIFEST="$manifest" \
354+
MOCK_TRANSIENT_ERROR=1 MOCK_MISSING_ONCE=0 MOCK_STATE="$tmp/state" \
355+
MOCK_SKOPEO_LOG="$tmp/skopeo.log" \
356+
PATH="$tmp/bin:/usr/bin:/bin" hack/promote-retag.sh v1.6.0 \
357+
>"$tmp/out" 2>"$tmp/err" || rc=$?
358+
359+
[ "$rc" -ne 0 ]
360+
grep -q 'cannot be treated as unpublished' "$tmp/err"
361+
grep -q '429' "$tmp/err"
362+
! grep -q '^copy ' "$tmp/skopeo.log"
363+
}

0 commit comments

Comments
 (0)