amadan

carlos / platform Public

settings-2 task 7: a held stage that cannot tell is retried, never dropped

HeldAuthorized now skips only a pointer that is gone, fails verification,
does not name the version, or that the ratchet refuses. A store read
error, an unpersisted ratchet mark, or a mark that cannot be read is
returned as is, so the root relay keeps the request. The naming check
runs before Admit, and a naming entry that signs a digest is preferred
over a digestless one whatever the list order.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 85c23a6771fff1ef91048035984ab2dd5012f1dc parent 9d4f8d0
3 files changed, +201 −10
  • internal/release/heldauth_test.go +154 −2
  • internal/release/materializeheld.go +33 −7
  • internal/release/ratchet.go +14 −1
diff --git a/internal/release/heldauth_test.go b/internal/release/heldauth_test.go
index 6f63d229b..6776ec750 100644
--- a/internal/release/heldauth_test.go
+++ b/internal/release/heldauth_test.go
@@ -2,6 +2,7 @@ package release
import (
"context"
+ "crypto/ed25519"
"errors"
"os"
"path/filepath"
@@ -118,8 +119,8 @@ func TestHeldStageBindsTheRecordDigest(t *testing.T) {
if _, err := s.Put(ctx, PromotionRecordKey("bab", "km", "v1"), saved, ""); err != nil {
t.Fatal(err)
}
- if _, err := StageHeld(ctx, s, pub, nr(t), opt, heldAuthRoute("a.example.com"), "v1"); !errors.Is(err, ErrManifestDigest) && !errors.Is(err, ErrHeldNotAuthorized) {
- t.Fatalf("a reused name under a replayed record = %v", err)
+ if _, err := StageHeld(ctx, s, pub, nr(t), opt, heldAuthRoute("a.example.com"), "v1"); !errors.Is(err, ErrManifestDigest) {
+ t.Fatalf("a reused name under a replayed record = %v; want ErrManifestDigest", err)
}
if _, err := os.Stat(VersionPath(opt, "bab", "v1", "km")); err == nil {
t.Fatal("the reused name's bytes were staged")
@@ -182,3 +183,154 @@ func TestServeHeldStageRequestsDropsUnauthorizedBuilds(t *testing.T) {
})
}
}
+
+// pointerOnly ships v1 of bab/km, promotes it to each channel in turn (a
+// channel listed in digested is promoted after the digest marker is written,
+// so its pointer signs v1's digest), and deletes v1's promotion record, so
+// the pointers are the only evidence.
+func pointerOnly(t *testing.T, s Store, priv ed25519.PrivateKey, legacy, digested []string) {
+ t.Helper()
+ ctx := context.Background()
+ shipRelease(t, s, "bab", "km", "v1", adoptPlatform, []byte("v1"))
+ for _, c := range legacy {
+ if err := Promote(ctx, s, priv, "bab", "km", c, "v1"); err != nil {
+ t.Fatal(err)
+ }
+ }
+ if len(digested) > 0 {
+ if err := WriteDigestMarker(ctx, s, RootSigner(priv), "bab", "test"); err != nil {
+ t.Fatal(err)
+ }
+ }
+ for _, c := range digested {
+ if err := Promote(ctx, s, priv, "bab", "km", c, "v1"); err != nil {
+ t.Fatal(err)
+ }
+ }
+ if err := s.Delete(ctx, PromotionRecordKey("bab", "km", "v1")); err != nil {
+ t.Fatal(err)
+ }
+}
+
+// Fix round 1, I1: a held stage that cannot read the pointer that may name
+// its build cannot tell, so it is not ErrHeldNotAuthorized (a final refusal)
+// but an error the root relay keeps the request for.
+func TestHeldStageRetriesAPointerItCannotRead(t *testing.T) {
+ base := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ opt := t.TempDir()
+ pointerOnly(t, base, priv, []string{"canary/x"}, nil)
+ s := failingGetStore{Store: base, key: ChannelKey("bab", "km", "canary/x")}
+ _, err := StageHeld(ctx, s, pub, nr(t), opt, heldAuthRoute("a.example.com"), "v1")
+ if err == nil || errors.Is(err, ErrHeldNotAuthorized) || !errors.Is(err, errTransient) {
+ t.Fatalf("StageHeld over an unreadable pointer = %v; want the read error, not ErrHeldNotAuthorized", err)
+ }
+ if _, err := os.Stat(VersionPath(opt, "bab", "v1", "km")); err == nil {
+ t.Fatal("a build nothing could vouch for was staged")
+ }
+}
+
+// The root relay keeps a request whose authority it could not read.
+func TestServeHeldStageRequestsKeepsARequestItCannotAuthorizeYet(t *testing.T) {
+ ctx, base, opt := heldFixture(t)
+ if err := base.Delete(ctx, PromotionRecordKey("bab", "console", "v1")); err != nil {
+ t.Fatal(err) // canary/x's pointer is v1's only evidence
+ }
+ s := failingGetStore{Store: base, key: ChannelKey("bab", "console", "canary/x")}
+ req := goodHeldRequest()
+ dd := t.TempDir()
+ path := writeHeldRequest(t, dd, req.Host+".json", req)
+ if err := ServeHeldStageRequests(ctx, s, heldPub, nr(t), opt, dd, offGate(t), passAuth(nil)); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := os.Stat(path); err != nil {
+ t.Fatalf("the request was dropped on a transient read error: %v", err)
+ }
+ gone(t, LiveLink(opt, heldTestHost))
+ gone(t, filepath.Join(HeldStagedDir(dd), heldTestHost+".json"))
+}
+
+// A ratchet that cannot tell (it could not persist the mark for a pointer
+// with digests, or could not read the mark for one without) is retried, not
+// taken for a refusal.
+func TestHeldStageRetriesARatchetThatCannotTell(t *testing.T) {
+ t.Run("the mark could not be persisted", func(t *testing.T) {
+ if os.Geteuid() == 0 {
+ t.Skip("root can write a 0555 directory")
+ }
+ s := NewDirStore(t.TempDir())
+ priv, pub := testKeys(t)
+ pointerOnly(t, s, priv, nil, []string{"canary/x"})
+ ro := filepath.Join(t.TempDir(), "ro")
+ if err := os.MkdirAll(ro, 0o555); err != nil {
+ t.Fatal(err)
+ }
+ _, err := HeldAuthorized(context.Background(), s, pub, NewDigestRatchet(ro, nil, false), "bab", "km", "v1")
+ if errors.Is(err, ErrHeldNotAuthorized) || !errors.Is(err, ErrRatchetUnpersisted) {
+ t.Fatalf("HeldAuthorized = %v; want ErrRatchetUnpersisted, not ErrHeldNotAuthorized", err)
+ }
+ })
+ t.Run("the mark could not be read", func(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ priv, pub := testKeys(t)
+ pointerOnly(t, s, priv, []string{"canary/x"}, nil)
+ dir := t.TempDir()
+ if err := os.MkdirAll(markPath(dir, "bab", "km", "canary/x"), 0o755); err != nil {
+ t.Fatal(err) // a directory where the mark belongs: Admit cannot tell
+ }
+ _, err := HeldAuthorized(context.Background(), s, pub, NewDigestRatchet(dir, nil, false), "bab", "km", "v1")
+ if err == nil || errors.Is(err, ErrHeldNotAuthorized) {
+ t.Fatalf("HeldAuthorized = %v; want a retryable error, not ErrHeldNotAuthorized", err)
+ }
+ })
+}
+
+// Among pointers that name the build, the one that signs its digest wins,
+// whichever channel lists first: binding to "" when a signed digest exists
+// would throw away the stronger evidence.
+func TestHeldAuthorizedPrefersASignedDigest(t *testing.T) {
+ for name, chans := range map[string][2][]string{
+ "the digestless channel lists first": {{"a"}, {"b"}},
+ "the digested channel lists first": {{"b"}, {"a"}},
+ } {
+ t.Run(name, func(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ pointerOnly(t, s, priv, chans[0], chans[1])
+ raw, _, err := s.Get(ctx, ManifestKey("bab", "km", "v1"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ want, err := HeldAuthorized(ctx, s, pub, nr(t), "bab", "km", "v1")
+ if err != nil || want != ManifestDigest(raw) {
+ t.Fatalf("HeldAuthorized = %q, %v; want the signed digest %q", want, err, ManifestDigest(raw))
+ }
+ })
+ }
+}
+
+// A pointer that does not name the build never reaches the ratchet: it is
+// not evidence, so it neither marks its channel nor, when the mark cannot be
+// written, holds up a build another pointer authorizes.
+func TestHeldAuthorizedAdmitsOnlyNamingPointers(t *testing.T) {
+ s := NewDirStore(t.TempDir())
+ ctx := context.Background()
+ priv, pub := testKeys(t)
+ shipRelease(t, s, "bab", "km", "v0", adoptPlatform, []byte("v0"))
+ pointerOnly(t, s, priv, []string{"b"}, nil)
+ if err := WriteDigestMarker(ctx, s, RootSigner(priv), "bab", "test"); err != nil {
+ t.Fatal(err)
+ }
+ if err := Promote(ctx, s, priv, "bab", "km", "a", "v0"); err != nil {
+ t.Fatal(err) // channel a carries digests and does not name v1
+ }
+ dir := t.TempDir()
+ if want, err := HeldAuthorized(ctx, s, pub, NewDigestRatchet(dir, nil, false), "bab", "km", "v1"); err != nil || want != "" {
+ t.Fatalf("HeldAuthorized = %q, %v; want channel b's pointer trust", want, err)
+ }
+ if _, err := os.Stat(markPath(dir, "bab", "km", "a")); err == nil {
+ t.Fatal("a pointer that does not name the build was admitted")
+ }
+}
diff --git a/internal/release/materializeheld.go b/internal/release/materializeheld.go
index 01c3c1f50..cac75e879 100644
--- a/internal/release/materializeheld.go
+++ b/internal/release/materializeheld.go
@@ -46,12 +46,20 @@ var ErrHeldNotProven = errors.New("release: the held release is not proven for t
// ErrHeldNotAuthorized is a held stage of a build §4.1 does not authorize:
// no promotion record, and no verified pointer of the app names it (settings
-// spec rev 11 §4.3a). The request is dropped, never retried.
+// spec rev 11 §4.3a). The request is dropped, never retried, so it is
+// returned only when every pointer of the app was read and judged.
var ErrHeldNotAuthorized = errors.New("release: the held release is not authorized: no promotion record and no signed pointer names it")
// HeldAuthorized is §4.1 for a move's held stage. want is the digest staging
-// must bind: the record's, else the naming pointer entry's ("" for an entry
-// signed before digests, today's pointer trust).
+// must bind: the record's, else a naming pointer entry's, preferring one that
+// signs a digest ("" only when every admitted naming entry was signed before
+// digests, today's pointer trust).
+//
+// A pointer that is gone, fails verification, does not name the version, or
+// that the box's ratchet refuses is not evidence and is skipped. A read that
+// fails any other way, or a ratchet that cannot tell (the mark could not be
+// persisted or read), is returned as is: the answer is unknown, not no, so
+// the root relay keeps the request for the next pass.
func HeldAuthorized(ctx context.Context, s Store, pub ed25519.PublicKey, r *DigestRatchet, acct, app, version string) (string, error) {
rec, err := ReadPromotionRecord(ctx, s, pub, acct, app, version)
if err == nil {
@@ -65,13 +73,24 @@ func HeldAuthorized(ctx context.Context, s Store, pub ed25519.PublicKey, r *Dige
if err != nil {
return "", err
}
+ named := false // some admitted pointer names the version, digest or not
for _, k := range keys {
name, ok := strings.CutSuffix(strings.TrimPrefix(k, prefix), ".json")
if !ok || name == "" {
continue
}
- p, err := Channel(ctx, s, pub, acct, app, name)
+ raw, _, err := GetQualified(ctx, s, ChannelKey(acct, app, name))
+ if errors.Is(err, os.ErrNotExist) {
+ continue // gone since the list
+ }
+ if err != nil {
+ return "", fmt.Errorf("release: held stage of %s/%s at %s: reading channel %s: %w", acct, app, version, name, err)
+ }
+ p, err := verifyPointer(pub, raw, acct, app, name)
if err != nil {
+ continue // a pointer that does not verify is not evidence
+ }
+ if p.Version != version && !slices.Contains(p.History, version) {
continue
}
// The box's ratchet and CARLOS_REQUIRE_POINTER_DIGEST apply here as
@@ -81,11 +100,18 @@ func HeldAuthorized(ctx context.Context, s Store, pub ed25519.PublicKey, r *Dige
return "", fmt.Errorf("%w: no pointer digest ratchet", ErrHeldNotAuthorized)
}
if rerr := r.Admit(acct, p); rerr != nil {
- continue
+ if errors.Is(rerr, ErrPointerWithoutDigests) && !errors.Is(rerr, errRatchetMarkUnreadable) {
+ continue // the ratchet's verdict: this pointer is refused
+ }
+ return "", fmt.Errorf("release: held stage of %s/%s at %s: channel %s: %w", acct, app, version, name, rerr)
}
- if p.Version == version || slices.Contains(p.History, version) {
- return p.DigestOf(version), nil
+ if d := p.DigestOf(version); d != "" {
+ return d, nil
}
+ named = true
+ }
+ if named {
+ return "", nil
}
return "", fmt.Errorf("%w: %s/%s at %s", ErrHeldNotAuthorized, acct, app, version)
}
diff --git a/internal/release/ratchet.go b/internal/release/ratchet.go
index 3fb553378..114fde1c1 100644
--- a/internal/release/ratchet.go
+++ b/internal/release/ratchet.go
@@ -28,6 +28,19 @@ var ErrPointerWithoutDigests = errors.New("release: pointer carries no digests")
// durable, because a restart would otherwise forget the channel (Astra I1 #4).
var ErrRatchetUnpersisted = errors.New("release: the pointer digest mark could not be persisted")
+// errRatchetMarkUnreadable marks Admit's refusal of a pointer without
+// digests because the channel's mark could not be read: unlike the flag or
+// a mark that is there, it is not a verdict, so a caller that can retry
+// (the held stage) does. markUnreadable carries it without changing the
+// error's text.
+var errRatchetMarkUnreadable = errors.New("release: the pointer digest mark could not be read")
+
+type markUnreadable struct{ err error }
+
+func (e markUnreadable) Error() string { return e.err.Error() }
+func (e markUnreadable) Unwrap() error { return e.err }
+func (markUnreadable) Is(target error) bool { return target == errRatchetMarkUnreadable }
+
// RequirePointerDigest reads the flag: 1, true, on or yes in any case.
func RequirePointerDigest(getenv func(string) string) bool {
switch strings.ToLower(strings.TrimSpace(getenv(RequirePointerDigestEnv))) {
@@ -100,7 +113,7 @@ func (r *DigestRatchet) Admit(acct string, p *ChannelPointer) error {
marked, err := r.marked(acct, p.App, p.Channel)
if err != nil {
return fmt.Errorf("%w: %s/%s/%s, and this box cannot tell whether it has already verified one with digests for that channel: %w",
- ErrPointerWithoutDigests, acct, p.App, p.Channel, err)
+ ErrPointerWithoutDigests, acct, p.App, p.Channel, markUnreadable{err})
}
if marked {
return fmt.Errorf("%w: %s/%s/%s, and this box has already verified one with digests for that channel",