amadan
release: fail closed on store errors in program check
PreviousProgram now only skips missing manifests (os.ErrNotExist) and returns any other error to fail the check closed. A transient store failure no longer silently disables the wrong-program check. Added TestPreviousProgramFailsClosedOnStoreError to verify this behavior and added doc comments to ArtifactProgram, FileProgram, and ProgramMismatchError.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
2 files changed,
+67
−2
internal/release/program.go+17 −2internal/release/program_test.go+50 −0
diff --git a/internal/release/program.go b/internal/release/program.go| index 2bcf02525..4bc42d714 100644 |
| --- a/internal/release/program.go |
| +++ b/internal/release/program.go |
| @@ -13,11 +13,15 @@ import ( |
| "bytes" |
| "context" |
| "debug/buildinfo" |
| + "errors" |
| "fmt" |
| + "os" |
| "sort" |
| "strings" |
| ) |
| +// ArtifactProgram extracts the Go main package path from a binary's debug/buildinfo, |
| +// returning "" if the bytes are not a Go binary or buildinfo is unreadable. |
| func ArtifactProgram(b []byte) string { |
| bi, err := buildinfo.Read(bytes.NewReader(b)) |
| if err != nil { |
| @@ -26,6 +30,8 @@ func ArtifactProgram(b []byte) string { |
| return bi.Path |
| } |
| +// FileProgram extracts the Go main package path from a file at path, |
| +// returning "" if the file is not a Go binary or buildinfo is unreadable. |
| func FileProgram(path string) string { |
| bi, err := buildinfo.ReadFile(path) |
| if err != nil { |
| @@ -59,7 +65,8 @@ func ReleaseProgram(files map[string][]byte) (string, error) { |
| } |
| // PreviousProgram is the program of the newest release (among the 20 newest) |
| -// whose manifest declares one, and that release's version. |
| +// whose manifest declares one, and that release's version. It skips only missing |
| +// manifests (os.ErrNotExist), returning any other error to fail the check closed. |
| func PreviousProgram(ctx context.Context, s Store, acct, app string) (string, string, error) { |
| vers, _, err := VersionCandidates(ctx, s, acct, app, 20) |
| if err != nil { |
| @@ -68,7 +75,13 @@ func PreviousProgram(ctx context.Context, s Store, acct, app string) (string, st |
| var best *Manifest |
| for _, v := range vers { |
| m, err := GetManifest(ctx, s, acct, app, v) |
| - if err != nil || m == nil || m.Program == "" { |
| + if err != nil { |
| + if errors.Is(err, os.ErrNotExist) { |
| + continue |
| + } |
| + return "", "", err |
| + } |
| + if m == nil || m.Program == "" { |
| continue |
| } |
| if best == nil || m.BuiltAt > best.BuiltAt { |
| @@ -81,6 +94,8 @@ func PreviousProgram(ctx context.Context, s Store, acct, app string) (string, st |
| return best.Program, best.Version, nil |
| } |
| +// ProgramMismatchError is returned when a release attempts to ship a different program |
| +// than the app's previous releases without the --program-change override. |
| type ProgramMismatchError struct{ App, Got, Want, WantVersion string } |
| func (e *ProgramMismatchError) Error() string { |
diff --git a/internal/release/program_test.go b/internal/release/program_test.go| index a4f2269ea..6d3bac9e4 100644 |
| --- a/internal/release/program_test.go |
| +++ b/internal/release/program_test.go |
| @@ -109,3 +109,53 @@ func TestPreviousProgramSkipsReleasesWithoutOne(t *testing.T) { |
| t.Fatalf("PreviousProgram = %q %q %v, want example.com/console from v1", got, ver, err) |
| } |
| } |
| + |
| +// failingStore wraps a Store and fails Get for a specific key with a non-NotExist error. |
| +type failingStore struct { |
| + Store |
| + failKey string |
| +} |
| + |
| +func (fs *failingStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if key == fs.failKey { |
| + return nil, "", errors.New("simulated store failure") |
| + } |
| + return fs.Store.Get(ctx, key) |
| +} |
| + |
| +// A store error (not just missing manifest) must fail the check closed. |
| +func TestPreviousProgramFailsClosedOnStoreError(t *testing.T) { |
| + base := NewDirStore(t.TempDir()) |
| + console := tinyProgram(t, "example.com/console") |
| + |
| + // Ship v1 with console program, then v2 normally. |
| + if err := shipWithProgram(t, base, "v1", console, false); err != nil { |
| + t.Fatalf("ship v1: %v", err) |
| + } |
| + shipRelease(t, base, "bab", "console", "v2", adoptPlatform, []byte("v2 content")) |
| + |
| + // Wrap the store to fail on v1's manifest key. |
| + manifestKey := ManifestKey("bab", "console", "v1") |
| + s := &failingStore{Store: base, failKey: manifestKey} |
| + |
| + // PreviousProgram must return an error (not silently skip the failure). |
| + prog, ver, err := PreviousProgram(context.Background(), s, "bab", "console") |
| + if err == nil || prog != "" || ver != "" { |
| + t.Fatalf("PreviousProgram = %q %q %v, want non-nil error", prog, ver, err) |
| + } |
| + |
| + // CheckShipProgram must propagate the error. |
| + files := map[string][]byte{adoptPlatform: console} |
| + m := Manifest{Schema: 1, App: "console", Version: "v3", Kind: "binary", |
| + Artifacts: map[string]Artifact{adoptPlatform: {}}} |
| + checkErr := CheckShipProgram(context.Background(), s, "bab", &m, files, false) |
| + if checkErr == nil { |
| + t.Fatalf("CheckShipProgram returned nil, want error") |
| + } |
| + // The error should not be a ProgramMismatchError (which would indicate the check |
| + // wrongly disabled itself and allowed the program change). |
| + var pm *ProgramMismatchError |
| + if errors.As(checkErr, &pm) { |
| + t.Fatalf("CheckShipProgram = ProgramMismatchError, want other error: %v", checkErr) |
| + } |
| +} |