diff --git a/pkg/appview/handlers/upgrade_banner.go b/pkg/appview/handlers/upgrade_banner.go index e3f26f2..832ae59 100644 --- a/pkg/appview/handlers/upgrade_banner.go +++ b/pkg/appview/handlers/upgrade_banner.go @@ -129,9 +129,21 @@ func (h *UpgradeBannerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) w.WriteHeader(http.StatusNoContent) return } + } else if currentManifest.IsManifestList != newerManifest.IsManifestList { + // One side is a multi-arch index, the other a single-arch image. The two + // cannot be compared without resolving the index down to the child + // manifest matching the single-arch side's platform, and that is not + // implemented. Diffing the index itself is not a fallback: an index + // carries no layers and is not scanned, so every layer of the other side + // reads as removed and the vuln delta is computed against an absent scan, + // which invents a security improvement that did not happen. Show no + // banner rather than a wrong one. + slog.Debug("Upgrade banner: multi-arch/single-arch mismatch, cannot compare", + "currentIsManifestList", currentManifest.IsManifestList, + "newerIsManifestList", newerManifest.IsManifestList) + w.WriteHeader(http.StatusNoContent) + return } - // If one is multi-arch and the other isn't, we can't match platforms — - // fall through and show a basic banner without layer/vuln details. // Fetch layers for both currentDBLayers, _ := db.GetLayersForManifest(h.ReadOnlyDB, currentManifestForLayers.Key) diff --git a/pkg/appview/handlers/upgrade_banner_test.go b/pkg/appview/handlers/upgrade_banner_test.go new file mode 100644 index 0000000..2f529ac --- /dev/null +++ b/pkg/appview/handlers/upgrade_banner_test.go @@ -0,0 +1,282 @@ +package handlers + +import ( + "context" + "database/sql" + "fmt" + "html/template" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + "time" + + "atcr.io/pkg/appview/db" + "atcr.io/pkg/atproto" + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/go-chi/chi/v5" + _ "github.com/tursodatabase/go-libsql" +) + +const ( + upgradeTestDID = "did:plc:upgradebanner123" + upgradeTestHandle = "upgrade.test" + upgradeTestRepo = "app" + + ociIndexMediaType = "application/vnd.oci.image.index.v1+json" + ociManifestMediaType = "application/vnd.oci.image.manifest.v1+json" +) + +// upgradeStubDirectory resolves only the single DID used by these tests, so the +// handler's ResolveIdentity call never touches the network. +type upgradeStubDirectory struct{} + +func (d *upgradeStubDirectory) LookupDID(_ context.Context, did syntax.DID) (*identity.Identity, error) { + if did.String() != upgradeTestDID { + return nil, fmt.Errorf("%w: %s", identity.ErrDIDNotFound, did) + } + return &identity.Identity{ + DID: syntax.DID(upgradeTestDID), + Handle: syntax.Handle(upgradeTestHandle), + Services: map[string]identity.ServiceEndpoint{ + "atproto_pds": {Type: "AtprotoPersonalDataServer", URL: "https://pds.example.com"}, + }, + }, nil +} + +func (d *upgradeStubDirectory) LookupHandle(_ context.Context, handle syntax.Handle) (*identity.Identity, error) { + return nil, fmt.Errorf("%w: %s", identity.ErrHandleNotFound, handle) +} + +func (d *upgradeStubDirectory) Lookup(ctx context.Context, atid syntax.AtIdentifier) (*identity.Identity, error) { + if did, err := atid.AsDID(); err == nil { + return d.LookupDID(ctx, did) + } + return nil, fmt.Errorf("%w: %s", identity.ErrHandleResolutionFailed, atid) +} + +func (d *upgradeStubDirectory) Purge(_ context.Context, _ syntax.AtIdentifier) error { return nil } + +// newUpgradeBannerDB returns a file-backed database with the real schema plus +// the owning user. A file rather than ":memory:" because an in-memory libsql +// database is per-connection. +func newUpgradeBannerDB(t *testing.T) *sql.DB { + t.Helper() + database, err := db.InitDB(filepath.Join(t.TempDir(), "appview.db"), db.LibsqlConfig{}) + if err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = database.Close() }) + + if err := db.UpsertUser(database, &db.User{ + DID: upgradeTestDID, + Handle: upgradeTestHandle, + PDSEndpoint: "https://pds.example.com", + LastSeen: time.Now(), + }); err != nil { + t.Fatalf("UpsertUser: %v", err) + } + return database +} + +// insertUpgradeManifest writes a manifest row and returns its manifest key. +func insertUpgradeManifest(t *testing.T, database *sql.DB, digest, mediaType string, createdAt time.Time) string { + t.Helper() + key := "key-" + digest + _, err := database.Exec(` + INSERT INTO manifests (manifest_key, did, repository, digest, hold_endpoint, + schema_version, media_type, artifact_type, created_at) + VALUES (?, ?, ?, ?, '', 2, ?, 'container-image', ?) + `, key, upgradeTestDID, upgradeTestRepo, digest, mediaType, createdAt) + if err != nil { + t.Fatalf("insert manifest %s: %v", digest, err) + } + return key +} + +func insertUpgradeLayers(t *testing.T, database *sql.DB, manifestKey string, sizes ...int64) { + t.Helper() + for i, size := range sizes { + _, err := database.Exec(` + INSERT INTO layers (manifest_key, digest, size, media_type, layer_index) + VALUES (?, ?, ?, 'application/vnd.oci.image.layer.v1.tar+gzip', ?) + `, manifestKey, fmt.Sprintf("sha256:%s-layer%d", manifestKey, i), size, i) + if err != nil { + t.Fatalf("insert layer: %v", err) + } + } +} + +// insertUpgradeRef points a manifest list at a linux/amd64 child. +func insertUpgradeRef(t *testing.T, database *sql.DB, indexKey, childDigest string) { + t.Helper() + _, err := database.Exec(` + INSERT INTO manifest_references (manifest_key, digest, media_type, size, + platform_architecture, platform_os, platform_variant, is_attestation, reference_index) + VALUES (?, ?, ?, 100, 'amd64', 'linux', '', 0, 0) + `, indexKey, childDigest, ociManifestMediaType) + if err != nil { + t.Fatalf("insert manifest reference: %v", err) + } +} + +func insertUpgradeTag(t *testing.T, database *sql.DB, tag, digest string, createdAt time.Time) { + t.Helper() + _, err := database.Exec(` + INSERT INTO tags (did, repository, tag, digest, created_at) VALUES (?, ?, ?, ?, ?) + `, upgradeTestDID, upgradeTestRepo, tag, digest, createdAt) + if err != nil { + t.Fatalf("insert tag: %v", err) + } +} + +// serveUpgradeBanner runs the handler against the given database for a page +// showing currentDigest, and returns the recorder. +func serveUpgradeBanner(t *testing.T, database *sql.DB, currentDigest string) *httptest.ResponseRecorder { + t.Helper() + + atproto.SetDirectory(&upgradeStubDirectory{}) + t.Cleanup(func() { atproto.SetDirectory(nil) }) + + tmpl := template.Must(template.New("test").Parse( + `{{ define "upgrade-banner" }}BANNER {{ .NewerTag }} layers {{ .Summary.LayerCountFrom }}->{{ .Summary.LayerCountTo }}{{ end }}`)) + + h := &UpgradeBannerHandler{BaseUIHandler: BaseUIHandler{ + Templates: tmpl, + DB: database, + ReadOnlyDB: database, + }} + + req := httptest.NewRequest(http.MethodGet, "/upgrade-banner?digest="+currentDigest, nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("handle", upgradeTestDID) + rctx.URLParams.Add("*", "/"+upgradeTestRepo) + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + return rr +} + +// bothMultiArch builds an old and a new manifest list that share linux/amd64, +// with differing layer counts on the children. Returns the current digest. +func bothMultiArch(t *testing.T, database *sql.DB) string { + t.Helper() + old := time.Now().Add(-2 * time.Hour) + recent := time.Now().Add(-1 * time.Hour) + + oldChild := insertUpgradeManifest(t, database, "sha256:oldchild", ociManifestMediaType, old) + insertUpgradeLayers(t, database, oldChild, 100, 200) + oldIndex := insertUpgradeManifest(t, database, "sha256:oldindex", ociIndexMediaType, old) + insertUpgradeRef(t, database, oldIndex, "sha256:oldchild") + insertUpgradeTag(t, database, "v1", "sha256:oldindex", old) + + newChild := insertUpgradeManifest(t, database, "sha256:newchild", ociManifestMediaType, recent) + insertUpgradeLayers(t, database, newChild, 100) + newIndex := insertUpgradeManifest(t, database, "sha256:newindex", ociIndexMediaType, recent) + insertUpgradeRef(t, database, newIndex, "sha256:newchild") + insertUpgradeTag(t, database, "v2", "sha256:newindex", recent) + + return "sha256:oldindex" +} + +// bothSingleArch builds an old and a new single-arch image with differing layer +// counts. Returns the current digest. +func bothSingleArch(t *testing.T, database *sql.DB) string { + t.Helper() + old := time.Now().Add(-2 * time.Hour) + recent := time.Now().Add(-1 * time.Hour) + + oldKey := insertUpgradeManifest(t, database, "sha256:oldimage", ociManifestMediaType, old) + insertUpgradeLayers(t, database, oldKey, 100, 200) + insertUpgradeTag(t, database, "v1", "sha256:oldimage", old) + + newKey := insertUpgradeManifest(t, database, "sha256:newimage", ociManifestMediaType, recent) + insertUpgradeLayers(t, database, newKey, 100) + insertUpgradeTag(t, database, "v2", "sha256:newimage", recent) + + return "sha256:oldimage" +} + +func TestUpgradeBanner_BothMultiArchCommonPlatform_Renders(t *testing.T) { + database := newUpgradeBannerDB(t) + current := bothMultiArch(t, database) + + rr := serveUpgradeBanner(t, database, current) + + if rr.Code != http.StatusOK { + t.Fatalf("expected 200, got %d (body %q)", rr.Code, rr.Body.String()) + } + // The children were resolved, so the deltas describe the children's layers + // (2 -> 1), not the layer-less indexes. + if got, want := rr.Body.String(), "BANNER v2 layers 2->1"; got != want { + t.Errorf("expected body %q, got %q", want, got) + } +} + +func TestUpgradeBanner_BothSingleArch_Renders(t *testing.T) { + database := newUpgradeBannerDB(t) + current := bothSingleArch(t, database) + + rr := serveUpgradeBanner(t, database, current) + + if rr.Code != http.StatusOK { + t.Fatalf("expected 200, got %d (body %q)", rr.Code, rr.Body.String()) + } + if got, want := rr.Body.String(), "BANNER v2 layers 2->1"; got != want { + t.Errorf("expected body %q, got %q", want, got) + } +} + +// A single-arch image viewed while the newest tag is a multi-arch index. The +// index has no layers and no scan of its own, so comparing against it directly +// reports all of the current image's layers as removed. No banner is correct. +func TestUpgradeBanner_SingleArchCurrentMultiArchNewer_NoContent(t *testing.T) { + database := newUpgradeBannerDB(t) + old := time.Now().Add(-2 * time.Hour) + recent := time.Now().Add(-1 * time.Hour) + + oldKey := insertUpgradeManifest(t, database, "sha256:oldimage", ociManifestMediaType, old) + insertUpgradeLayers(t, database, oldKey, 100, 200, 300) + insertUpgradeTag(t, database, "v1", "sha256:oldimage", old) + + newChild := insertUpgradeManifest(t, database, "sha256:newchild", ociManifestMediaType, recent) + insertUpgradeLayers(t, database, newChild, 100, 200, 300) + newIndex := insertUpgradeManifest(t, database, "sha256:newindex", ociIndexMediaType, recent) + insertUpgradeRef(t, database, newIndex, "sha256:newchild") + insertUpgradeTag(t, database, "v2", "sha256:newindex", recent) + + rr := serveUpgradeBanner(t, database, "sha256:oldimage") + + if rr.Code != http.StatusNoContent { + t.Fatalf("expected 204 for single-arch current vs multi-arch newer, got %d (body %q)", + rr.Code, rr.Body.String()) + } +} + +// The mirror image: a multi-arch index viewed while the newest tag is a +// single-arch image. The index contributes no layers, so every layer of the +// newer image reads as added. +func TestUpgradeBanner_MultiArchCurrentSingleArchNewer_NoContent(t *testing.T) { + database := newUpgradeBannerDB(t) + old := time.Now().Add(-2 * time.Hour) + recent := time.Now().Add(-1 * time.Hour) + + oldChild := insertUpgradeManifest(t, database, "sha256:oldchild", ociManifestMediaType, old) + insertUpgradeLayers(t, database, oldChild, 100, 200, 300) + oldIndex := insertUpgradeManifest(t, database, "sha256:oldindex", ociIndexMediaType, old) + insertUpgradeRef(t, database, oldIndex, "sha256:oldchild") + insertUpgradeTag(t, database, "v1", "sha256:oldindex", old) + + newKey := insertUpgradeManifest(t, database, "sha256:newimage", ociManifestMediaType, recent) + insertUpgradeLayers(t, database, newKey, 100, 200, 300) + insertUpgradeTag(t, database, "v2", "sha256:newimage", recent) + + rr := serveUpgradeBanner(t, database, "sha256:oldindex") + + if rr.Code != http.StatusNoContent { + t.Fatalf("expected 204 for multi-arch current vs single-arch newer, got %d (body %q)", + rr.Code, rr.Body.String()) + } +}