settings-2 task 4: report a manifest read failure as itself, not as a change
Split the manifest re-read check in claimRawAuthorize to distinguish transient store failures from actual manifest changes. If GetManifestRaw fails with os.ErrNotExist (version vanished), wrap as ErrAuthorizeChanged with the cause. Any other error (bucket unreachable, corrupt manifest) returns a separate error that does not match ErrAuthorizeChanged, preserving the original failure reason. Add TestAuthorizeReportsAStoreFailureAsItself with a wrapped store that fails on manifest Get, verifying the error is reported as itself, not as a change.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
2 files changed,
+54
−1
internal/release/authorize_record.go+11 −1internal/release/authorize_record_test.go+43 −0
diff --git a/internal/release/authorize_record.go b/internal/release/authorize_record.go| index dfbda69a2..2425f9067 100644 |
| --- a/internal/release/authorize_record.go |
| +++ b/internal/release/authorize_record.go |
| @@ -4,6 +4,7 @@ import ( |
| "context" |
| "errors" |
| "fmt" |
| + "os" |
| "github.com/carlosframework/platform/internal/updatewords" |
| ) |
| @@ -40,7 +41,16 @@ func (sg Signer) claimRawAuthorize(ctx context.Context, s Store, plan *Authorize |
| return false, fmt.Errorf("%w: %s", ErrAuthorizeRootOnly, updatewords.Text("settings.authorize.root-only")) |
| } |
| _, raw, err := GetManifestRaw(ctx, s, plan.Account, plan.App, plan.Version) |
| - if err != nil || ManifestDigest(raw) != plan.ManifestDigest { |
| + if err != nil { |
| + // If the version vanished (os.ErrNotExist), that's a change; keep the cause |
| + if errors.Is(err, os.ErrNotExist) { |
| + return false, fmt.Errorf("%w: %s: %w", ErrAuthorizeChanged, fmt.Sprintf(updatewords.Text("settings.authorize.changed"), plan.Version), err) |
| + } |
| + // Any other error is a real failure, not a change |
| + return false, fmt.Errorf("authorize: read the manifest of %s: %w", plan.Version, err) |
| + } |
| + // Read succeeded, check if digest changed |
| + if ManifestDigest(raw) != plan.ManifestDigest { |
| return false, fmt.Errorf("%w: %s", ErrAuthorizeChanged, fmt.Sprintf(updatewords.Text("settings.authorize.changed"), plan.Version)) |
| } |
| if cur, err := ReadPromotionRecord(ctx, s, sg.verifyPub(), plan.Account, plan.App, plan.Version); err == nil && cur.ManifestDigest == plan.ManifestDigest { |
diff --git a/internal/release/authorize_record_test.go b/internal/release/authorize_record_test.go| index 795d38136..5377638e7 100644 |
| --- a/internal/release/authorize_record_test.go |
| +++ b/internal/release/authorize_record_test.go |
| @@ -3,6 +3,7 @@ package release |
| import ( |
| "context" |
| "errors" |
| + "strings" |
| "testing" |
| "time" |
| ) |
| @@ -81,3 +82,45 @@ func TestAuthorizeIsRootOnly(t *testing.T) { |
| t.Fatalf("scoped Authorize = %v; want ErrAuthorizeRootOnly", err) |
| } |
| } |
| + |
| +func TestAuthorizeReportsAStoreFailureAsItself(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + shipRelease(t, s, "bab", "km", "v1", adoptPlatform, []byte("one")) |
| + plan, _ := PlanAuthorize(ctx, s, "bab", "km", "v1") |
| + |
| + // Wrap the store to fail on manifest Get only |
| + failStore := &failOnGetStore{Store: s, failKey: ManifestKey("bab", "km", "v1")} |
| + |
| + // Authorize should report the failure, not ErrAuthorizeChanged |
| + _, err := RootSigner(priv).Authorize(ctx, failStore, plan, "ops") |
| + if err == nil { |
| + t.Fatal("Authorize should fail when store Get fails") |
| + } |
| + if errors.Is(err, ErrAuthorizeChanged) { |
| + t.Fatal("Authorize should NOT wrap store failure as ErrAuthorizeChanged") |
| + } |
| + if !strings.Contains(err.Error(), "bucket unreachable") { |
| + t.Fatalf("error text should contain 'bucket unreachable', got: %v", err) |
| + } |
| + |
| + // Verify no record was written |
| + _, err = ReadPromotionRecord(ctx, s, pub, "bab", "km", "v1") |
| + if !errors.Is(err, ErrNoPromotionRecord) { |
| + t.Fatalf("record should not exist, got: %v", err) |
| + } |
| +} |
| + |
| +// failOnGetStore wraps a Store and returns an error for a specific key |
| +type failOnGetStore struct { |
| + Store |
| + failKey string |
| +} |
| + |
| +func (fs *failOnGetStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if key == fs.failKey { |
| + return nil, "", errors.New("bucket unreachable") |
| + } |
| + return fs.Store.Get(ctx, key) |
| +} |