From 302a4e3861818e8d9d9e00db157944a54acf946f Mon Sep 17 00:00:00 2001 From: chris lee Date: Thu, 8 Oct 2026 14:45:11 -0400 Subject: [PATCH 1/8] Add experimental macOS OCI machine image materialization --- docs/macos-experimental.md | 34 +++- lib/images/macos_import_darwin_test.go | 7 +- lib/images/macos_machine.go | 102 ++++++++++++ lib/images/macos_machine_test.go | 214 +++++++++++++++++++++++++ lib/images/manager.go | 66 +++++++- lib/images/platform.go | 4 - lib/images/tag.go | 4 +- 7 files changed, 411 insertions(+), 20 deletions(-) create mode 100644 lib/images/macos_machine.go create mode 100644 lib/images/macos_machine_test.go diff --git a/docs/macos-experimental.md b/docs/macos-experimental.md index 57e9f396d..187542f00 100644 --- a/docs/macos-experimental.md +++ b/docs/macos-experimental.md @@ -20,9 +20,37 @@ go run -tags containers_image_openpgp ./cmd/import-macos \ --name localhost/macos:spike ``` -The data directory needs APFS clonefile support. Images are `darwin/arm64`, not -OCI macOS containers. Registry pulls, manifest resolution and tagging/promotion -of macOS images are unsupported. Do not mutate the imported image files. +The local importer needs APFS clonefile support. Images are `darwin/arm64` +machine images, not OCI macOS containers. Registry pulls also support experimental +complete-machine bundles described below. Additional tags within the same +repository are supported; cross-repository promotion remains unsupported. +Do not mutate the imported image files. + +## Experimental OCI bundles + +The normal image API accepts a `darwin/arm64` OCI manifest whose configuration +labels describe a complete installed machine: + +| Label | Value | +|---|---| +| `io.hypeman.machine-image.version` | `1` | +| `io.hypeman.machine-image.kind` | `macos-image` | +| `io.hypeman.machine-image.disk-format` | `raw` | +| `io.hypeman.machine-image.disk-path` | Relative installed boot disk path | +| `io.hypeman.machine-image.aux-path` | Relative auxiliary storage path | +| `io.hypeman.machine-image.platform-path` | Relative macOS platform configuration JSON path | + +All three files must be distinct, nonempty regular files inside the unpacked +image. Platform configuration uses the local bundle's `MacOSImage` schema and +is limited to 64 KiB. Absolute paths, traversal and symlink escapes are rejected. +The installed disk is materialized directly rather than converted to a Linux +root filesystem. The matching auxiliary storage and platform identity are +retained. Do not package live mutable guest storage. + +This schema is experimental pending reconciliation with other machine-image +platforms. Complete bundles only: no base/delta chain, identity rebinding, or +concurrent-template guarantee. Ordinary OCI transport does not imply sparse +distribution efficiency, boot portability, or container execution semantics. ## API diff --git a/lib/images/macos_import_darwin_test.go b/lib/images/macos_import_darwin_test.go index 8518fbab2..f8cb4b660 100644 --- a/lib/images/macos_import_darwin_test.go +++ b/lib/images/macos_import_darwin_test.go @@ -40,12 +40,13 @@ func TestMacOSOfflineImport(t *testing.T) { _, err = ImportMacOSImage(ctx, p, "localhost/macos:cancelled", source) require.Error(t, err) } -func TestMacOSPlatformLocalOnly(t *testing.T) { +func TestMacOSPlatform(t *testing.T) { p, err := ParsePlatform("darwin/arm64") require.NoError(t, err) require.Equal(t, "darwin", p.OS) _, err = ParsePlatform("darwin/amd64") require.Error(t, err) - _, err = resolveManifestPlatform(&containerMetadata{OS: "darwin", Architecture: "arm64"}, "") - require.ErrorIs(t, err, ErrInvalidPlatform) + manifest, err := resolveManifestPlatform(&containerMetadata{OS: "darwin", Architecture: "arm64"}, "") + require.NoError(t, err) + require.Equal(t, "darwin/arm64", manifest.String()) } diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go new file mode 100644 index 000000000..a476e800b --- /dev/null +++ b/lib/images/macos_machine.go @@ -0,0 +1,102 @@ +package images + +import ( + "encoding/json" + "fmt" + "net" + "os" + "path/filepath" +) + +const ( + MacOSMachineVersionLabel = "io.hypeman.machine-image.version" + MacOSMachineKindLabel = "io.hypeman.machine-image.kind" + MacOSMachineDiskLabel = "io.hypeman.machine-image.disk-path" + MacOSMachineFormatLabel = "io.hypeman.machine-image.disk-format" + MacOSMachineAuxLabel = "io.hypeman.machine-image.aux-path" + MacOSMachinePlatformLabel = "io.hypeman.machine-image.platform-path" +) + +// macOSMachinePayload is a complete cold-boot bundle, not a container rootfs. +// This spike intentionally has no base/delta format or fork identity rebinding. +type macOSMachinePayload struct { + Disk string + Aux string + Platform *MacOSImage +} + +func machineBundleFile(root, relative string) (string, error) { + if relative == "" || !filepath.IsLocal(relative) { + return "", fmt.Errorf("machine payload path must be local and relative") + } + root, err := filepath.EvalSymlinks(root) + if err != nil { + return "", err + } + path, err := filepath.EvalSymlinks(filepath.Join(root, relative)) + if err != nil { + return "", err + } + rel, err := filepath.Rel(root, path) + if err != nil || !filepath.IsLocal(rel) { + return "", fmt.Errorf("machine payload escapes artifact root") + } + info, err := os.Stat(path) + if err != nil { + return "", err + } + if !info.Mode().IsRegular() || info.Size() == 0 { + return "", fmt.Errorf("machine payload must be a nonempty regular file") + } + return path, nil +} + +func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePayload, error) { + if meta.OS != "darwin" { + return nil, nil + } + if meta.Architecture != "arm64" || meta.Variant != "" { + return nil, fmt.Errorf("macOS machine requires darwin/arm64") + } + labels := meta.Labels + if labels[MacOSMachineVersionLabel] != "1" || labels[MacOSMachineKindLabel] != "macos-image" || labels[MacOSMachineFormatLabel] != "raw" { + return nil, fmt.Errorf("darwin artifact requires version 1 macos-image with raw disk, not an ordinary container image") + } + disk, err := machineBundleFile(root, labels[MacOSMachineDiskLabel]) + if err != nil { + return nil, err + } + aux, err := machineBundleFile(root, labels[MacOSMachineAuxLabel]) + if err != nil { + return nil, err + } + config, err := machineBundleFile(root, labels[MacOSMachinePlatformLabel]) + if err != nil { + return nil, err + } + if disk == aux || disk == config || aux == config { + return nil, fmt.Errorf("machine bundle files must be distinct") + } + info, err := os.Stat(config) + if err != nil { + return nil, err + } + if info.Size() > 64<<10 { + return nil, fmt.Errorf("machine platform metadata is too large") + } + b, err := os.ReadFile(config) + if err != nil { + return nil, err + } + var platform MacOSImage + if err := json.Unmarshal(b, &platform); err != nil { + return nil, err + } + if len(platform.HardwareModel) == 0 || len(platform.MachineIdentifier) == 0 || platform.CPUs < 2 || platform.Memory < 4<<30 { + return nil, fmt.Errorf("invalid macOS platform metadata") + } + if _, err := net.ParseMAC(platform.MAC); err != nil { + return nil, fmt.Errorf("invalid machine MAC: %w", err) + } + return &macOSMachinePayload{Disk: disk, Aux: aux, Platform: &platform}, nil +} diff --git a/lib/images/macos_machine_test.go b/lib/images/macos_machine_test.go new file mode 100644 index 000000000..fc31b3ab3 --- /dev/null +++ b/lib/images/macos_machine_test.go @@ -0,0 +1,214 @@ +package images + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/json" + "fmt" + "io" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func macOSFixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + c := MacOSImage{HardwareModel: []byte{1}, MachineIdentifier: []byte{2}, MAC: "02:00:00:00:00:01", CPUs: 4, Memory: 8 << 30} + b, err := json.Marshal(c) + require.NoError(t, err) + for f, data := range map[string][]byte{"config.json": b, "disk.img": []byte("not a real boot disk: synthetic OCI transport fixture"), "aux.img": []byte("synthetic aux")} { + require.NoError(t, os.WriteFile(filepath.Join(root, f), data, 0600)) + } + return root +} + +func macOSFixtureMetadata() *containerMetadata { + return &containerMetadata{OS: "darwin", Architecture: "arm64", Labels: map[string]string{ + MacOSMachineVersionLabel: "1", MacOSMachineKindLabel: "macos-image", MacOSMachineFormatLabel: "raw", + MacOSMachineDiskLabel: "disk.img", MacOSMachineAuxLabel: "aux.img", MacOSMachinePlatformLabel: "config.json", + }} +} + +func TestMacOSMachineValidation(t *testing.T) { + root := macOSFixture(t) + payload, err := parseMacOSMachine(root, macOSFixtureMetadata()) + require.NoError(t, err) + require.NotNil(t, payload) + for _, tc := range []struct{ name, key, value string }{ + {"unsupported version", MacOSMachineVersionLabel, "2"}, {"container not machine", MacOSMachineKindLabel, ""}, + {"wrong format", MacOSMachineFormatLabel, "qcow2"}, {"absolute path", MacOSMachineDiskLabel, "/etc/passwd"}, + {"traversal", MacOSMachineDiskLabel, "../disk.img"}, {"missing aux", MacOSMachineAuxLabel, "missing.img"}, + {"duplicate file", MacOSMachineAuxLabel, "disk.img"}, + } { + t.Run(tc.name, func(t *testing.T) { + meta := macOSFixtureMetadata() + meta.Labels[tc.key] = tc.value + _, err := parseMacOSMachine(root, meta) + require.Error(t, err) + }) + } + outside := filepath.Join(t.TempDir(), "outside") + require.NoError(t, os.WriteFile(outside, []byte("outside"), 0600)) + require.NoError(t, os.Symlink(outside, filepath.Join(root, "escape"))) + meta := macOSFixtureMetadata() + meta.Labels[MacOSMachineDiskLabel] = "escape" + _, err = parseMacOSMachine(root, meta) + require.Error(t, err) + meta = macOSFixtureMetadata() + meta.Architecture = "amd64" + _, err = parseMacOSMachine(root, meta) + require.Error(t, err) + require.NoError(t, os.WriteFile(filepath.Join(root, "config.json"), []byte(`{}`), 0600)) + _, err = parseMacOSMachine(root, macOSFixtureMetadata()) + require.Error(t, err) +} + +// Optional real-bundle run only reads a stopped source, never the live benchmark disk. +// It proves normal manager pull/materialization, not VZ boot or API server deployment. +func TestMacOSMachineOCIRoundTrip(t *testing.T) { + source := macOSFixture(t) + if realSource := os.Getenv("HYPEMAN_MACOS_OCI_SOURCE"); realSource != "" { + source = realSource + for _, f := range []string{"disk.img", "aux.img"} { + err := exec.Command("lsof", "-t", filepath.Join(source, f)).Run() + exit, ok := err.(*exec.ExitError) + require.True(t, ok && exit.ExitCode() == 1, "source storage must be verifiably closed") + } + } + work := t.TempDir() + if persistent := os.Getenv("HYPEMAN_MACOS_OCI_WORK"); persistent != "" { + work = persistent + require.NoError(t, os.MkdirAll(work, 0700)) + } + payload, err := parseMacOSMachine(source, macOSFixtureMetadata()) + require.NoError(t, err) + _ = payload + layerPath := filepath.Join(work, "bundle.tar.gz") + file, err := os.OpenFile(layerPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) + require.NoError(t, err) + gz, err := gzip.NewWriterLevel(file, gzip.BestSpeed) + require.NoError(t, err) + tw := tar.NewWriter(gz) + hashes := map[string]string{} + t.Log("packing stopped bundle", source) + for _, f := range []string{"disk.img", "aux.img", "config.json"} { + input, err := os.Open(filepath.Join(source, f)) + require.NoError(t, err) + info, err := input.Stat() + require.NoError(t, err) + require.NoError(t, tw.WriteHeader(&tar.Header{Name: f, Mode: 0600, Size: info.Size(), Typeflag: tar.TypeReg})) + h := sha256.New() + _, err = io.Copy(io.MultiWriter(tw, h), input) + require.NoError(t, err) + require.NoError(t, input.Close()) + hashes[f] = fmt.Sprintf("%x", h.Sum(nil)) + } + require.NoError(t, tw.Close()) + require.NoError(t, gz.Close()) + require.NoError(t, file.Close()) + t.Log("packed; streaming blobs into loopback registry storage") + layer, err := tarball.LayerFromFile(layerPath) + require.NoError(t, err) + image, err := mutate.AppendLayers(empty.Image, layer) + require.NoError(t, err) + cfg, err := image.ConfigFile() + require.NoError(t, err) + cfg.OS = "darwin" + cfg.Architecture = "arm64" + cfg.Config.Labels = macOSFixtureMetadata().Labels + image, err = mutate.ConfigFile(image, cfg) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(work, "registry-blobs"), 0700)) + // The test registry buffers PATCH uploads in memory even with a disk blob + // handler. Seed blobs through the streaming handler so real VM layers do + // not consume tens of GiB of RAM; remote.Write still publishes the manifest. + blobs := registry.NewDiskBlobHandler(filepath.Join(work, "registry-blobs")) + writer := blobs.(registry.BlobPutHandler) + layerDigest, err := layer.Digest() + require.NoError(t, err) + compressed, err := layer.Compressed() + require.NoError(t, err) + err = writer.Put(context.Background(), "macos", layerDigest, compressed) + closeErr := compressed.Close() + require.NoError(t, err) + require.NoError(t, closeErr) + configDigest, err := image.ConfigName() + require.NoError(t, err) + rawConfig, err := image.RawConfigFile() + require.NoError(t, err) + require.NoError(t, writer.Put(context.Background(), "macos", configDigest, io.NopCloser(bytes.NewReader(rawConfig)))) + server := httptest.NewServer(registry.New(registry.WithBlobHandler(blobs))) + defer server.Close() + ref, err := name.ParseReference(strings.TrimPrefix(server.URL, "http://") + "/macos:spike") + require.NoError(t, err) + require.NoError(t, remote.Write(ref, image)) + t.Log("published; pulling through normal image manager") + p := paths.New(filepath.Join(work, "hypeman-data")) + manager, err := NewManager(p, 1, nil) + require.NoError(t, err) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) + defer cancel() + pulled, err := manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "darwin/arm64"}) + require.NoError(t, err) + require.NoError(t, manager.WaitForReady(ctx, pulled.Name)) + pulled, err = manager.GetImage(ctx, pulled.Name) + require.NoError(t, err) + require.Equal(t, StatusReady, pulled.Status) + require.Equal(t, "darwin/arm64", pulled.Platform) + require.NotNil(t, pulled.MacOS) + disk, err := GetDiskPath(p, pulled.Name, pulled.Digest) + require.NoError(t, err) + for f, path := range map[string]string{"disk.img": disk, "aux.img": filepath.Join(filepath.Dir(disk), "aux.img")} { + input, err := os.Open(path) + require.NoError(t, err) + h := sha256.New() + _, err = io.Copy(h, input) + require.NoError(t, err) + require.NoError(t, input.Close()) + require.Equal(t, hashes[f], fmt.Sprintf("%x", h.Sum(nil)), f) + } + expected, err := os.ReadFile(filepath.Join(source, "config.json")) + require.NoError(t, err) + var config MacOSImage + require.NoError(t, json.Unmarshal(expected, &config)) + require.Equal(t, config, *pulled.MacOS) + digest, err := image.Digest() + require.NoError(t, err) + require.Equal(t, digest.String(), pulled.Digest) + reused, err := manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "darwin/arm64"}) + require.NoError(t, err) + require.Equal(t, StatusReady, reused.Status) + tagged, err := manager.TagImage(ctx, ref.Name(), ref.Context().Name()+":stable") + require.NoError(t, err) + require.Equal(t, pulled.Digest, tagged.Digest) + require.Equal(t, pulled.MacOS, tagged.MacOS) + tagDisk, err := GetDiskPath(p, tagged.Name, tagged.Digest) + require.NoError(t, err) + require.Equal(t, disk, tagDisk) + _, err = manager.TagImage(ctx, ref.Name(), strings.TrimPrefix(server.URL, "http://")+"/other:stable") + require.ErrorIs(t, err, ErrInvalidPlatform) + _, err = manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "linux/arm64"}) + require.Error(t, err) + report := map[string]any{"image": pulled.Name, "digest": pulled.Digest, "platform": pulled.Platform, "status": pulled.Status, "disk_path": disk, "source_hashes": hashes, "boot_tested": false} + b, err := json.MarshalIndent(report, "", " ") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(work, "roundtrip.json"), b, 0600)) + t.Log("round-trip verified", pulled.Digest) +} diff --git a/lib/images/manager.go b/lib/images/manager.go index 5ccd7b7e7..c9c746359 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -15,6 +15,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/uuid" + "github.com/kernel/hypeman/lib/forkvm" "github.com/kernel/hypeman/lib/paths" "github.com/kernel/hypeman/lib/queue" "github.com/kernel/hypeman/lib/tags" @@ -174,10 +175,6 @@ func (m *manager) CreateImage(ctx context.Context, req CreateImageRequest) (*Ima return nil, err } - if platform.OS == "darwin" { - return nil, fmt.Errorf("%w: macOS images must be imported locally with import-macos, not pulled as Linux containers", ErrInvalidPlatform) - } - // Parse and normalize normalized, err := ParseNormalizedRef(req.Name) if err != nil { @@ -223,6 +220,14 @@ func (m *manager) CreateImage(ctx context.Context, req CreateImageRequest) (*Ima m.createMu.Lock() defer m.createMu.Unlock() + if req.Platform != "" { + if cached, err := readMetadata(m.paths, ref.Repository(), ref.DigestHex()); err == nil && cached.Status == StatusReady { + actual, err := ParsePlatform(cached.Platform) + if err != nil || !platform.Matches(actual) { + return nil, fmt.Errorf("%w: requested %s but cached manifest is %s", ErrInvalidPlatform, platform, cached.Platform) + } + } + } if img, found, err := m.reuseExistingImage(ref, req.Credentials, req.Tags); found || err != nil { return img, err } @@ -514,9 +519,25 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials // atomic even when system/builds and images are on different filesystems. diskTempPath := diskPath + ".tmp-" + buildID defer os.Remove(diskTempPath) - // Use default image format (erofs on Linux, ext4 on Darwin) + payload, err := parseMacOSMachine(tempDir, result.Metadata) + if err != nil { + m.updateStatusByDigest(ref, StatusFailed, err, buildID) + return + } convertStart := time.Now() - diskSize, err := ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) + var diskSize int64 + if payload != nil { + err = forkvm.CopyRegularFile(payload.Disk, diskTempPath) + if err == nil { + var info os.FileInfo + info, err = os.Stat(diskTempPath) + if err == nil { + diskSize = info.Size() + } + } + } else { + diskSize, err = ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) + } m.recordImageBuildPhase(ctx, ref.Digest(), "filesystem_export", time.Since(convertStart), phaseStatus(err), "not_applicable") if err != nil { m.updateStatusByDigest(ref, StatusFailed, fmt.Errorf("convert to %s: %w", DefaultImageFormat, err), buildID) @@ -524,7 +545,7 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials } finalizeStart := time.Now() - err = m.finalizeImage(ref, result, diskSize, buildID, diskTempPath) + err = m.finalizeImage(ref, result, diskSize, buildID, diskTempPath, payload) m.recordImageBuildPhase(ctx, ref.Digest(), "finalize", time.Since(finalizeStart), phaseStatus(err), "not_applicable") if err != nil { if errors.Is(err, errStaleBuild) { @@ -537,7 +558,7 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials buildStatus = "success" } -func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize int64, buildID, diskTempPath string) error { +func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize int64, buildID, diskTempPath string, payloads ...*macOSMachinePayload) error { m.createMu.Lock() defer m.createMu.Unlock() @@ -559,6 +580,34 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i return err } + var payload *macOSMachinePayload + if len(payloads) > 0 { + payload = payloads[0] + } + if actualPlatform.OS == "darwin" && payload == nil { + return fmt.Errorf("macOS image requires a validated machine bundle") + } + auxCommitted := false + if payload != nil { + auxPath := filepath.Join(layout.dir, "aux.img") + auxTemp := auxPath + ".tmp-" + buildID + defer os.Remove(auxTemp) + if err := forkvm.CopyRegularFile(payload.Aux, auxTemp); err != nil { + return err + } + if err := os.Chmod(auxTemp, 0600); err != nil { + return err + } + if err := os.Rename(auxTemp, auxPath); err != nil { + return err + } + defer func() { + if !auxCommitted { + _ = os.Remove(auxPath) + } + }() + meta.MacOS = payload.Platform + } modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) diskInstalled := false modelWritten := false @@ -596,6 +645,7 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i return rollbackFinalization(layout, modelPath, diskInstalled, modelWritten, fmt.Errorf("write final metadata: %w", err)) } + auxCommitted = true m.notifyReady(ref.DigestHex(), StatusReady, nil) if !m.claimRequestedTags(ref, meta) { m.cleanupUnclaimedImage(ref) diff --git a/lib/images/platform.go b/lib/images/platform.go index 28e788b26..87b31df53 100644 --- a/lib/images/platform.go +++ b/lib/images/platform.go @@ -167,10 +167,6 @@ func resolveManifestPlatform(meta *containerMetadata, requested string) (Platfor Variant: meta.Variant, }.Normalize() - if actual.OS == "darwin" { - return Platform{}, fmt.Errorf("%w: macOS must be imported as a local disk bundle", ErrInvalidPlatform) - } - // An explicit request is authoritative for the match check and, when the // manifest omits its own platform, for the recorded value too. if strings.TrimSpace(requested) != "" { diff --git a/lib/images/tag.go b/lib/images/tag.go index 3a497a6a9..614ac4f14 100644 --- a/lib/images/tag.go +++ b/lib/images/tag.go @@ -28,8 +28,8 @@ func (m *manager) TagImage(ctx context.Context, source, target string) (*Image, if err != nil { return nil, err } - if meta.MacOS != nil { - return nil, fmt.Errorf("%w: tagging macOS bundles is not implemented", ErrInvalidPlatform) + if meta.MacOS != nil && sourceRef.Repository() != targetRef.Repository() { + return nil, fmt.Errorf("%w: cross-repository promotion of macOS bundles is not implemented", ErrInvalidPlatform) } if err := m.cancelPendingTag(targetRef.Repository(), targetRef.Tag()); err != nil { return nil, fmt.Errorf("cancel pending image tag: %w", err) From db9ac84e3f770efe01e864ca97f988f521025937 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:43:20 +0000 Subject: [PATCH 2/8] Harden macOS machine image import - Give the boot disk private permissions regardless of registry modes. - Stage auxiliary storage before taking the manager lock; finalization only renames it into place. - Count auxiliary storage in the image size used for accounting. - Require a 6-byte Ethernet machine MAC. - Reject bundle files that are hardlinks of one another. --- lib/images/macos_machine.go | 14 ++++++++-- lib/images/macos_machine_test.go | 27 +++++++++++++++++- lib/images/manager.go | 47 +++++++++++++++++++++----------- lib/images/manager_test.go | 2 +- 4 files changed, 69 insertions(+), 21 deletions(-) diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go index a476e800b..869ecd797 100644 --- a/lib/images/macos_machine.go +++ b/lib/images/macos_machine.go @@ -51,6 +51,12 @@ func machineBundleFile(root, relative string) (string, error) { return path, nil } +func sameFile(a, b string) bool { + ai, errA := os.Stat(a) + bi, errB := os.Stat(b) + return errA == nil && errB == nil && os.SameFile(ai, bi) +} + func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePayload, error) { if meta.OS != "darwin" { return nil, nil @@ -74,7 +80,8 @@ func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePaylo if err != nil { return nil, err } - if disk == aux || disk == config || aux == config { + // Compare inodes: hardlinks with different names must not satisfy distinctness. + if sameFile(disk, aux) || sameFile(disk, config) || sameFile(aux, config) { return nil, fmt.Errorf("machine bundle files must be distinct") } info, err := os.Stat(config) @@ -95,8 +102,9 @@ func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePaylo if len(platform.HardwareModel) == 0 || len(platform.MachineIdentifier) == 0 || platform.CPUs < 2 || platform.Memory < 4<<30 { return nil, fmt.Errorf("invalid macOS platform metadata") } - if _, err := net.ParseMAC(platform.MAC); err != nil { - return nil, fmt.Errorf("invalid machine MAC: %w", err) + // net.ParseMAC also accepts EUI-64 and InfiniBand addresses, which VZ rejects. + if mac, err := net.ParseMAC(platform.MAC); err != nil || len(mac) != 6 { + return nil, fmt.Errorf("invalid machine MAC %q: want a 6-byte Ethernet address", platform.MAC) } return &macOSMachinePayload{Disk: disk, Aux: aux, Platform: &platform}, nil } diff --git a/lib/images/macos_machine_test.go b/lib/images/macos_machine_test.go index fc31b3ab3..f88364df2 100644 --- a/lib/images/macos_machine_test.go +++ b/lib/images/macos_machine_test.go @@ -78,6 +78,23 @@ func TestMacOSMachineValidation(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(root, "config.json"), []byte(`{}`), 0600)) _, err = parseMacOSMachine(root, macOSFixtureMetadata()) require.Error(t, err) + + // EUI-64 and 20-octet InfiniBand addresses parse with net.ParseMAC but are not Ethernet. + for _, mac := range []string{"02:00:00:00:00:00:00:01", strings.Repeat("00:", 19) + "01"} { + b, err := json.Marshal(MacOSImage{HardwareModel: []byte{1}, MachineIdentifier: []byte{2}, MAC: mac, CPUs: 4, Memory: 8 << 30}) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(root, "config.json"), b, 0600)) + _, err = parseMacOSMachine(root, macOSFixtureMetadata()) + require.Error(t, err, mac) + } + + // A hardlink with a different name is the same file and must not satisfy distinctness. + linked := macOSFixture(t) + require.NoError(t, os.Link(filepath.Join(linked, "disk.img"), filepath.Join(linked, "alias.img"))) + meta = macOSFixtureMetadata() + meta.Labels[MacOSMachineAuxLabel] = "alias.img" + _, err = parseMacOSMachine(linked, meta) + require.ErrorContains(t, err, "distinct") } // Optional real-bundle run only reads a stopped source, never the live benchmark disk. @@ -113,7 +130,12 @@ func TestMacOSMachineOCIRoundTrip(t *testing.T) { require.NoError(t, err) info, err := input.Stat() require.NoError(t, err) - require.NoError(t, tw.WriteHeader(&tar.Header{Name: f, Mode: 0600, Size: info.Size(), Typeflag: tar.TypeReg})) + // The registry controls modes; a permissive disk must not become the canonical image mode. + mode := int64(0600) + if f == "disk.img" { + mode = 0666 + } + require.NoError(t, tw.WriteHeader(&tar.Header{Name: f, Mode: mode, Size: info.Size(), Typeflag: tar.TypeReg})) h := sha256.New() _, err = io.Copy(io.MultiWriter(tw, h), input) require.NoError(t, err) @@ -183,6 +205,9 @@ func TestMacOSMachineOCIRoundTrip(t *testing.T) { require.NoError(t, err) require.NoError(t, input.Close()) require.Equal(t, hashes[f], fmt.Sprintf("%x", h.Sum(nil)), f) + info, err := os.Stat(path) + require.NoError(t, err) + require.Equal(t, os.FileMode(0600), info.Mode().Perm(), f) } expected, err := os.ReadFile(filepath.Join(source, "config.json")) require.NoError(t, err) diff --git a/lib/images/manager.go b/lib/images/manager.go index c9c746359..faeb6de4e 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -514,7 +514,8 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials m.updateStatusByDigest(ref, StatusConverting, nil, buildID) - diskPath := resolveImageLayout(m.paths, ref.Repository(), ref.DigestHex()).disk + layout := resolveImageLayout(m.paths, ref.Repository(), ref.DigestHex()) + diskPath := layout.disk // Keep the temporary filesystem beside its final path so finalization stays // atomic even when system/builds and images are on different filesystems. diskTempPath := diskPath + ".tmp-" + buildID @@ -528,6 +529,10 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials var diskSize int64 if payload != nil { err = forkvm.CopyRegularFile(payload.Disk, diskTempPath) + if err == nil { + // Registry-supplied modes must not expose the canonical disk to other local users. + err = os.Chmod(diskTempPath, 0600) + } if err == nil { var info os.FileInfo info, err = os.Stat(diskTempPath) @@ -544,8 +549,30 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials return } + // Copy auxiliary storage before taking createMu; finalization only renames it. + auxTempPath := "" + if payload != nil { + auxTempPath = filepath.Join(layout.dir, "aux.img.tmp-"+buildID) + defer os.Remove(auxTempPath) + if err = forkvm.CopyRegularFile(payload.Aux, auxTempPath); err == nil { + err = os.Chmod(auxTempPath, 0600) + } + if err == nil { + var info os.FileInfo + info, err = os.Stat(auxTempPath) + if err == nil { + // Accounting covers every file the image occupies, not only the boot disk. + diskSize += info.Size() + } + } + if err != nil { + m.updateStatusByDigest(ref, StatusFailed, fmt.Errorf("stage macOS auxiliary storage: %w", err), buildID) + return + } + } + finalizeStart := time.Now() - err = m.finalizeImage(ref, result, diskSize, buildID, diskTempPath, payload) + err = m.finalizeImage(ref, result, diskSize, buildID, diskTempPath, auxTempPath, payload) m.recordImageBuildPhase(ctx, ref.Digest(), "finalize", time.Since(finalizeStart), phaseStatus(err), "not_applicable") if err != nil { if errors.Is(err, errStaleBuild) { @@ -558,7 +585,7 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials buildStatus = "success" } -func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize int64, buildID, diskTempPath string, payloads ...*macOSMachinePayload) error { +func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize int64, buildID, diskTempPath, auxTempPath string, payload *macOSMachinePayload) error { m.createMu.Lock() defer m.createMu.Unlock() @@ -580,25 +607,13 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i return err } - var payload *macOSMachinePayload - if len(payloads) > 0 { - payload = payloads[0] - } if actualPlatform.OS == "darwin" && payload == nil { return fmt.Errorf("macOS image requires a validated machine bundle") } auxCommitted := false if payload != nil { auxPath := filepath.Join(layout.dir, "aux.img") - auxTemp := auxPath + ".tmp-" + buildID - defer os.Remove(auxTemp) - if err := forkvm.CopyRegularFile(payload.Aux, auxTemp); err != nil { - return err - } - if err := os.Chmod(auxTemp, 0600); err != nil { - return err - } - if err := os.Rename(auxTemp, auxPath); err != nil { + if err := os.Rename(auxTempPath, auxPath); err != nil { return err } defer func() { diff --git a/lib/images/manager_test.go b/lib/images/manager_test.go index 13a6835e2..8bf2572e6 100644 --- a/lib/images/manager_test.go +++ b/lib/images/manager_test.go @@ -736,7 +736,7 @@ func TestDeleteAndRecreateDuringBuildTail(t *testing.T) { m.updateStatusByDigest(staleRef, StatusFailed, errors.New("stale build"), firstMeta.BuildID) staleBundle, err := m.ociClient.extractOCIImageBundle(digestHex) require.NoError(t, err) - require.ErrorIs(t, m.finalizeImage(staleRef, &pullResult{Metadata: staleBundle.Meta}, 1, firstMeta.BuildID, ""), errStaleBuild) + require.ErrorIs(t, m.finalizeImage(staleRef, &pullResult{Metadata: staleBundle.Meta}, 1, firstMeta.BuildID, "", "", nil), errStaleBuild) currentMeta, err = readMetadata(p, repo, digestHex) require.NoError(t, err) require.Equal(t, StatusPending, currentMeta.Status) From 5084f4955a0fb4bf8fb8fc6ebd23ec3ccdc00213 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:44:26 +0000 Subject: [PATCH 3/8] Normalize macOS machine platform before matching Compare the machine platform after the same normalization used for manifest matching, so aarch64 and case variants such as Darwin select the machine path instead of falling through to the rootfs path. --- lib/images/macos_machine.go | 7 +++++-- lib/images/macos_machine_test.go | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go index 869ecd797..7f305b961 100644 --- a/lib/images/macos_machine.go +++ b/lib/images/macos_machine.go @@ -58,10 +58,13 @@ func sameFile(a, b string) bool { } func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePayload, error) { - if meta.OS != "darwin" { + // Normalize as resolveManifestPlatform does, so aliases such as aarch64 and + // case variants such as Darwin select the machine path rather than rootfs. + normalized := Platform{OS: meta.OS, Architecture: meta.Architecture, Variant: meta.Variant}.Normalize() + if normalized.OS != "darwin" { return nil, nil } - if meta.Architecture != "arm64" || meta.Variant != "" { + if normalized.Architecture != "arm64" || normalized.Variant != "" { return nil, fmt.Errorf("macOS machine requires darwin/arm64") } labels := meta.Labels diff --git a/lib/images/macos_machine_test.go b/lib/images/macos_machine_test.go index f88364df2..eb8a7eafe 100644 --- a/lib/images/macos_machine_test.go +++ b/lib/images/macos_machine_test.go @@ -75,6 +75,15 @@ func TestMacOSMachineValidation(t *testing.T) { meta.Architecture = "amd64" _, err = parseMacOSMachine(root, meta) require.Error(t, err) + + // Platform aliases normalize before the machine check, as they do for manifest matching. + for _, alias := range []struct{ os, arch string }{{"darwin", "aarch64"}, {"Darwin", "arm64"}} { + meta = macOSFixtureMetadata() + meta.OS, meta.Architecture = alias.os, alias.arch + payload, err := parseMacOSMachine(root, meta) + require.NoError(t, err, alias) + require.NotNil(t, payload, alias) + } require.NoError(t, os.WriteFile(filepath.Join(root, "config.json"), []byte(`{}`), 0600)) _, err = parseMacOSMachine(root, macOSFixtureMetadata()) require.Error(t, err) From aa10052faa060b4ef315a0c005f87fa7300ac941 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:04:48 +0000 Subject: [PATCH 4/8] Stage machine image files in one helper before finalization Copy and validate the boot disk and auxiliary storage in a single helper, and hand finalization one staged-files value (paths, payload, size) instead of separately correlated paths and a variadic payload. Finalization still only renames staged files and commits metadata under the manager lock. --- lib/images/macos_machine.go | 32 ++++++++++++++++++ lib/images/manager.go | 67 ++++++++++++------------------------- lib/images/manager_test.go | 2 +- 3 files changed, 55 insertions(+), 46 deletions(-) diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go index 7f305b961..e1b7f0ac0 100644 --- a/lib/images/macos_machine.go +++ b/lib/images/macos_machine.go @@ -6,6 +6,8 @@ import ( "net" "os" "path/filepath" + + "github.com/kernel/hypeman/lib/forkvm" ) const ( @@ -57,6 +59,36 @@ func sameFile(a, b string) bool { return errA == nil && errB == nil && os.SameFile(ai, bi) } +// stageMacOSMachine copies a validated bundle's boot disk and auxiliary storage to +// build-private paths, outside the manager lock. It returns the bytes both files +// occupy, which is the size recorded for accounting. +func stageMacOSMachine(payload *macOSMachinePayload, diskTemp, auxTemp string) (int64, error) { + diskSize, err := stageMachineFile(payload.Disk, diskTemp) + if err != nil { + return 0, fmt.Errorf("stage boot disk: %w", err) + } + auxSize, err := stageMachineFile(payload.Aux, auxTemp) + if err != nil { + return 0, fmt.Errorf("stage auxiliary storage: %w", err) + } + return diskSize + auxSize, nil +} + +func stageMachineFile(src, dst string) (int64, error) { + if err := forkvm.CopyRegularFile(src, dst); err != nil { + return 0, err + } + // Registry-supplied modes must not expose the canonical files to other local users. + if err := os.Chmod(dst, 0600); err != nil { + return 0, err + } + info, err := os.Stat(dst) + if err != nil { + return 0, err + } + return info.Size(), nil +} + func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePayload, error) { // Normalize as resolveManifestPlatform does, so aliases such as aarch64 and // case variants such as Darwin select the machine path rather than rootfs. diff --git a/lib/images/manager.go b/lib/images/manager.go index faeb6de4e..8680d0b2e 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -15,7 +15,6 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/uuid" - "github.com/kernel/hypeman/lib/forkvm" "github.com/kernel/hypeman/lib/paths" "github.com/kernel/hypeman/lib/queue" "github.com/kernel/hypeman/lib/tags" @@ -526,22 +525,13 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials return } convertStart := time.Now() - var diskSize int64 + staged := stagedImageFiles{disk: diskTempPath, machine: payload} if payload != nil { - err = forkvm.CopyRegularFile(payload.Disk, diskTempPath) - if err == nil { - // Registry-supplied modes must not expose the canonical disk to other local users. - err = os.Chmod(diskTempPath, 0600) - } - if err == nil { - var info os.FileInfo - info, err = os.Stat(diskTempPath) - if err == nil { - diskSize = info.Size() - } - } + staged.aux = filepath.Join(layout.dir, "aux.img.tmp-"+buildID) + defer os.Remove(staged.aux) + staged.sizeBytes, err = stageMacOSMachine(payload, diskTempPath, staged.aux) } else { - diskSize, err = ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) + staged.sizeBytes, err = ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) } m.recordImageBuildPhase(ctx, ref.Digest(), "filesystem_export", time.Since(convertStart), phaseStatus(err), "not_applicable") if err != nil { @@ -549,30 +539,8 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials return } - // Copy auxiliary storage before taking createMu; finalization only renames it. - auxTempPath := "" - if payload != nil { - auxTempPath = filepath.Join(layout.dir, "aux.img.tmp-"+buildID) - defer os.Remove(auxTempPath) - if err = forkvm.CopyRegularFile(payload.Aux, auxTempPath); err == nil { - err = os.Chmod(auxTempPath, 0600) - } - if err == nil { - var info os.FileInfo - info, err = os.Stat(auxTempPath) - if err == nil { - // Accounting covers every file the image occupies, not only the boot disk. - diskSize += info.Size() - } - } - if err != nil { - m.updateStatusByDigest(ref, StatusFailed, fmt.Errorf("stage macOS auxiliary storage: %w", err), buildID) - return - } - } - finalizeStart := time.Now() - err = m.finalizeImage(ref, result, diskSize, buildID, diskTempPath, auxTempPath, payload) + err = m.finalizeImage(ref, result, buildID, staged) m.recordImageBuildPhase(ctx, ref.Digest(), "finalize", time.Since(finalizeStart), phaseStatus(err), "not_applicable") if err != nil { if errors.Is(err, errStaleBuild) { @@ -585,7 +553,16 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials buildStatus = "success" } -func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize int64, buildID, diskTempPath, auxTempPath string, payload *macOSMachinePayload) error { +// stagedImageFiles are the build outputs written before finalization takes createMu. +// Finalization only renames them into place and commits metadata. +type stagedImageFiles struct { + disk string // staged disk, beside its final path + aux string // staged auxiliary storage; macOS machine images only + machine *macOSMachinePayload // nil for rootfs images + sizeBytes int64 // bytes the staged files occupy, recorded for accounting +} + +func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID string, staged stagedImageFiles) error { m.createMu.Lock() defer m.createMu.Unlock() @@ -607,13 +584,13 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i return err } - if actualPlatform.OS == "darwin" && payload == nil { + if actualPlatform.OS == "darwin" && staged.machine == nil { return fmt.Errorf("macOS image requires a validated machine bundle") } auxCommitted := false - if payload != nil { + if staged.machine != nil { auxPath := filepath.Join(layout.dir, "aux.img") - if err := os.Rename(auxTempPath, auxPath); err != nil { + if err := os.Rename(staged.aux, auxPath); err != nil { return err } defer func() { @@ -621,14 +598,14 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i _ = os.Remove(auxPath) } }() - meta.MacOS = payload.Platform + meta.MacOS = staged.machine.Platform } modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) diskInstalled := false modelWritten := false if err := installAtomically(layout.disk, func(path string) error { - return os.Rename(diskTempPath, path) + return os.Rename(staged.disk, path) }); err != nil { return fmt.Errorf("install image disk: %w", err) } @@ -649,7 +626,7 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, diskSize i meta.Status = StatusReady meta.Error = nil meta.Platform = actualPlatform.String() - meta.SizeBytes = diskSize + meta.SizeBytes = staged.sizeBytes meta.Entrypoint = result.Metadata.Entrypoint meta.Cmd = result.Metadata.Cmd meta.Env = result.Metadata.Env diff --git a/lib/images/manager_test.go b/lib/images/manager_test.go index 8bf2572e6..23404d5a8 100644 --- a/lib/images/manager_test.go +++ b/lib/images/manager_test.go @@ -736,7 +736,7 @@ func TestDeleteAndRecreateDuringBuildTail(t *testing.T) { m.updateStatusByDigest(staleRef, StatusFailed, errors.New("stale build"), firstMeta.BuildID) staleBundle, err := m.ociClient.extractOCIImageBundle(digestHex) require.NoError(t, err) - require.ErrorIs(t, m.finalizeImage(staleRef, &pullResult{Metadata: staleBundle.Meta}, 1, firstMeta.BuildID, "", "", nil), errStaleBuild) + require.ErrorIs(t, m.finalizeImage(staleRef, &pullResult{Metadata: staleBundle.Meta}, firstMeta.BuildID, stagedImageFiles{sizeBytes: 1}), errStaleBuild) currentMeta, err = readMetadata(p, repo, digestHex) require.NoError(t, err) require.Equal(t, StatusPending, currentMeta.Status) From 87d4e6968878ffe04e98b7fabe27c46f8ffc7314 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:06:08 +0000 Subject: [PATCH 5/8] Scope cached-platform check to macOS and unify finalize rollback - Compare the requested platform with cached records only for macOS images. Linux records from before platform tracking have no platform, and the check rejected every explicit-platform create that reached one. - Track everything finalization installs (aux, disk, manifest model) in one list, removed together if finalization fails before the metadata commit. - Use MacOSImage.Validate for the OCI machine parser, sharing the import rules. - Drop the env-gated real-bundle branch from the unit test. It shelled to lsof and wrote a report file; the synthetic round trip covers the same pull path. --- lib/images/macos_machine.go | 9 ++----- lib/images/macos_machine_test.go | 20 +--------------- lib/images/manager.go | 41 +++++++++++++------------------- 3 files changed, 20 insertions(+), 50 deletions(-) diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go index e1b7f0ac0..ab95ca6dc 100644 --- a/lib/images/macos_machine.go +++ b/lib/images/macos_machine.go @@ -3,7 +3,6 @@ package images import ( "encoding/json" "fmt" - "net" "os" "path/filepath" @@ -134,12 +133,8 @@ func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePaylo if err := json.Unmarshal(b, &platform); err != nil { return nil, err } - if len(platform.HardwareModel) == 0 || len(platform.MachineIdentifier) == 0 || platform.CPUs < 2 || platform.Memory < 4<<30 { - return nil, fmt.Errorf("invalid macOS platform metadata") - } - // net.ParseMAC also accepts EUI-64 and InfiniBand addresses, which VZ rejects. - if mac, err := net.ParseMAC(platform.MAC); err != nil || len(mac) != 6 { - return nil, fmt.Errorf("invalid machine MAC %q: want a 6-byte Ethernet address", platform.MAC) + if err := platform.Validate(); err != nil { + return nil, err } return &macOSMachinePayload{Disk: disk, Aux: aux, Platform: &platform}, nil } diff --git a/lib/images/macos_machine_test.go b/lib/images/macos_machine_test.go index eb8a7eafe..7261ee0ca 100644 --- a/lib/images/macos_machine_test.go +++ b/lib/images/macos_machine_test.go @@ -11,7 +11,6 @@ import ( "io" "net/http/httptest" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -110,22 +109,9 @@ func TestMacOSMachineValidation(t *testing.T) { // It proves normal manager pull/materialization, not VZ boot or API server deployment. func TestMacOSMachineOCIRoundTrip(t *testing.T) { source := macOSFixture(t) - if realSource := os.Getenv("HYPEMAN_MACOS_OCI_SOURCE"); realSource != "" { - source = realSource - for _, f := range []string{"disk.img", "aux.img"} { - err := exec.Command("lsof", "-t", filepath.Join(source, f)).Run() - exit, ok := err.(*exec.ExitError) - require.True(t, ok && exit.ExitCode() == 1, "source storage must be verifiably closed") - } - } work := t.TempDir() - if persistent := os.Getenv("HYPEMAN_MACOS_OCI_WORK"); persistent != "" { - work = persistent - require.NoError(t, os.MkdirAll(work, 0700)) - } - payload, err := parseMacOSMachine(source, macOSFixtureMetadata()) + _, err := parseMacOSMachine(source, macOSFixtureMetadata()) require.NoError(t, err) - _ = payload layerPath := filepath.Join(work, "bundle.tar.gz") file, err := os.OpenFile(layerPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) require.NoError(t, err) @@ -240,9 +226,5 @@ func TestMacOSMachineOCIRoundTrip(t *testing.T) { require.ErrorIs(t, err, ErrInvalidPlatform) _, err = manager.CreateImage(ctx, CreateImageRequest{Name: ref.Name(), Platform: "linux/arm64"}) require.Error(t, err) - report := map[string]any{"image": pulled.Name, "digest": pulled.Digest, "platform": pulled.Platform, "status": pulled.Status, "disk_path": disk, "source_hashes": hashes, "boot_tested": false} - b, err := json.MarshalIndent(report, "", " ") - require.NoError(t, err) - require.NoError(t, os.WriteFile(filepath.Join(work, "roundtrip.json"), b, 0600)) t.Log("round-trip verified", pulled.Digest) } diff --git a/lib/images/manager.go b/lib/images/manager.go index 8680d0b2e..2442e2a62 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -220,7 +220,8 @@ func (m *manager) CreateImage(ctx context.Context, req CreateImageRequest) (*Ima defer m.createMu.Unlock() if req.Platform != "" { - if cached, err := readMetadata(m.paths, ref.Repository(), ref.DigestHex()); err == nil && cached.Status == StatusReady { + // Only locally imported or pulled macOS records carry a platform the request can contradict. + if cached, err := readMetadata(m.paths, ref.Repository(), ref.DigestHex()); err == nil && cached.Status == StatusReady && cached.MacOS != nil { actual, err := ParsePlatform(cached.Platform) if err != nil || !platform.Matches(actual) { return nil, fmt.Errorf("%w: requested %s but cached manifest is %s", ErrInvalidPlatform, platform, cached.Platform) @@ -587,29 +588,22 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID st if actualPlatform.OS == "darwin" && staged.machine == nil { return fmt.Errorf("macOS image requires a validated machine bundle") } - auxCommitted := false + // Files installed before the metadata commit. Finalization failure removes them. + var installed []string if staged.machine != nil { auxPath := filepath.Join(layout.dir, "aux.img") if err := os.Rename(staged.aux, auxPath); err != nil { - return err + return rollbackFinalization(installed, err) } - defer func() { - if !auxCommitted { - _ = os.Remove(auxPath) - } - }() + installed = append(installed, auxPath) meta.MacOS = staged.machine.Platform } - modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) - diskInstalled := false - modelWritten := false - if err := installAtomically(layout.disk, func(path string) error { return os.Rename(staged.disk, path) }); err != nil { - return fmt.Errorf("install image disk: %w", err) + return rollbackFinalization(installed, fmt.Errorf("install image disk: %w", err)) } - diskInstalled = true + installed = append(installed, layout.disk) // Persist the manifest content model beside the shared content so later // stages can recompose the image from per-layer artifacts and GC can tell @@ -617,10 +611,11 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID st if result.Manifest != nil { model := *result.Manifest model.Platform = actualPlatform.String() + modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) if err := writeManifestModelAt(modelPath, ref.DigestHex(), &model); err != nil { - return rollbackFinalization(layout, modelPath, diskInstalled, modelWritten, fmt.Errorf("write manifest model: %w", err)) + return rollbackFinalization(append(installed, modelPath), fmt.Errorf("write manifest model: %w", err)) } - modelWritten = true + installed = append(installed, modelPath) } meta.Status = StatusReady @@ -634,10 +629,10 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID st meta.WorkingDir = result.Metadata.WorkingDir if err := writeMetadataFile(layout.metadata, meta); err != nil { - return rollbackFinalization(layout, modelPath, diskInstalled, modelWritten, fmt.Errorf("write final metadata: %w", err)) + return rollbackFinalization(installed, fmt.Errorf("write final metadata: %w", err)) } - auxCommitted = true + installed = nil // committed: nothing below removes installed files m.notifyReady(ref.DigestHex(), StatusReady, nil) if !m.claimRequestedTags(ref, meta) { m.cleanupUnclaimedImage(ref) @@ -653,13 +648,11 @@ func manifestModelPath(p *paths.Paths, layout imageLayout, digestHex string) str return filepath.Join(layout.dir, "manifest.json") } -func rollbackFinalization(layout imageLayout, modelPath string, diskInstalled, modelWritten bool, cause error) error { +// rollbackFinalization removes the files finalization installed before it failed. +func rollbackFinalization(installed []string, cause error) error { var rollbackErr error - if modelWritten { - rollbackErr = errors.Join(rollbackErr, os.Remove(modelPath)) - } - if diskInstalled { - rollbackErr = errors.Join(rollbackErr, os.Remove(layout.disk)) + for _, path := range installed { + rollbackErr = errors.Join(rollbackErr, os.Remove(path)) } if rollbackErr != nil { return errors.Join(cause, fmt.Errorf("rollback finalization: %w", rollbackErr)) From 42199f6ca7a65439e4da9b8056bc6fb34cc5cd86 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:16:48 +0000 Subject: [PATCH 6/8] Carry only the platform in the staged machine image value finalize reads just the platform from the machine payload, so stagedImageFiles holds that platform rather than the whole payload. The payload's source paths are only needed while staging. --- lib/images/manager.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/images/manager.go b/lib/images/manager.go index 2442e2a62..57b39a001 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -526,10 +526,11 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials return } convertStart := time.Now() - staged := stagedImageFiles{disk: diskTempPath, machine: payload} + staged := stagedImageFiles{disk: diskTempPath} if payload != nil { staged.aux = filepath.Join(layout.dir, "aux.img.tmp-"+buildID) defer os.Remove(staged.aux) + staged.macos = payload.Platform staged.sizeBytes, err = stageMacOSMachine(payload, diskTempPath, staged.aux) } else { staged.sizeBytes, err = ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) @@ -557,10 +558,10 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials // stagedImageFiles are the build outputs written before finalization takes createMu. // Finalization only renames them into place and commits metadata. type stagedImageFiles struct { - disk string // staged disk, beside its final path - aux string // staged auxiliary storage; macOS machine images only - machine *macOSMachinePayload // nil for rootfs images - sizeBytes int64 // bytes the staged files occupy, recorded for accounting + disk string // staged disk, beside its final path + aux string // staged auxiliary storage; macOS machine images only + macos *MacOSImage // platform of a macOS machine image; nil for rootfs images + sizeBytes int64 // bytes the staged files occupy, recorded for accounting } func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID string, staged stagedImageFiles) error { @@ -585,18 +586,18 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID st return err } - if actualPlatform.OS == "darwin" && staged.machine == nil { + if actualPlatform.OS == "darwin" && staged.macos == nil { return fmt.Errorf("macOS image requires a validated machine bundle") } // Files installed before the metadata commit. Finalization failure removes them. var installed []string - if staged.machine != nil { + if staged.macos != nil { auxPath := filepath.Join(layout.dir, "aux.img") if err := os.Rename(staged.aux, auxPath); err != nil { return rollbackFinalization(installed, err) } installed = append(installed, auxPath) - meta.MacOS = staged.machine.Platform + meta.MacOS = staged.macos } if err := installAtomically(layout.disk, func(path string) error { return os.Rename(staged.disk, path) From ed2c34eed4b2e0a3ae27f649db4ba995d48e9d2b Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:47:36 +0000 Subject: [PATCH 7/8] Fold the cached-platform check into image reuse and return staged files whole - Check an explicit platform against a ready macOS record inside reuseExistingImage, which already reads that record under the create lock, instead of reading it a second time in CreateImage. - Resolve an explicit platform against the manifest through validateDigestPlatform, which already holds the same check and message. - stageMacOSMachine returns the staged files value, so buildImage assigns one value instead of assembling the aux path, platform and size separately. --- lib/images/macos_machine.go | 8 ++++---- lib/images/manager.go | 34 +++++++++++++++++++--------------- lib/images/platform.go | 4 ++-- lib/images/tag_test.go | 2 +- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go index ab95ca6dc..a6814001e 100644 --- a/lib/images/macos_machine.go +++ b/lib/images/macos_machine.go @@ -61,16 +61,16 @@ func sameFile(a, b string) bool { // stageMacOSMachine copies a validated bundle's boot disk and auxiliary storage to // build-private paths, outside the manager lock. It returns the bytes both files // occupy, which is the size recorded for accounting. -func stageMacOSMachine(payload *macOSMachinePayload, diskTemp, auxTemp string) (int64, error) { +func stageMacOSMachine(payload *macOSMachinePayload, diskTemp, auxTemp string) (stagedImageFiles, error) { diskSize, err := stageMachineFile(payload.Disk, diskTemp) if err != nil { - return 0, fmt.Errorf("stage boot disk: %w", err) + return stagedImageFiles{}, fmt.Errorf("stage boot disk: %w", err) } auxSize, err := stageMachineFile(payload.Aux, auxTemp) if err != nil { - return 0, fmt.Errorf("stage auxiliary storage: %w", err) + return stagedImageFiles{}, fmt.Errorf("stage auxiliary storage: %w", err) } - return diskSize + auxSize, nil + return stagedImageFiles{disk: diskTemp, aux: auxTemp, macos: payload.Platform, sizeBytes: diskSize + auxSize}, nil } func stageMachineFile(src, dst string) (int64, error) { diff --git a/lib/images/manager.go b/lib/images/manager.go index 57b39a001..d567d5fe6 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -219,16 +219,11 @@ func (m *manager) CreateImage(ctx context.Context, req CreateImageRequest) (*Ima m.createMu.Lock() defer m.createMu.Unlock() + var requested *Platform if req.Platform != "" { - // Only locally imported or pulled macOS records carry a platform the request can contradict. - if cached, err := readMetadata(m.paths, ref.Repository(), ref.DigestHex()); err == nil && cached.Status == StatusReady && cached.MacOS != nil { - actual, err := ParsePlatform(cached.Platform) - if err != nil || !platform.Matches(actual) { - return nil, fmt.Errorf("%w: requested %s but cached manifest is %s", ErrInvalidPlatform, platform, cached.Platform) - } - } + requested = &platform } - if img, found, err := m.reuseExistingImage(ref, req.Credentials, req.Tags); found || err != nil { + if img, found, err := m.reuseExistingImage(ref, req.Credentials, req.Tags, requested); found || err != nil { return img, err } return m.createAndQueueImage(ref, req, platform) @@ -260,13 +255,15 @@ func (m *manager) ImportLocalImage(ctx context.Context, repo, reference, digest m.createMu.Lock() defer m.createMu.Unlock() - if img, found, err := m.reuseExistingImage(ref, nil, nil); found || err != nil { + if img, found, err := m.reuseExistingImage(ref, nil, nil, nil); found || err != nil { return img, err } return m.createAndQueueImage(ref, CreateImageRequest{Name: imageRef}, hostPlatform()) } -func (m *manager) reuseExistingImage(ref *ResolvedRef, credentials *authn.AuthConfig, resourceTags tags.Tags) (*Image, bool, error) { +// reuseExistingImage returns a record already on disk for ref. requested is the +// caller's explicit platform, or nil when the request named none. +func (m *manager) reuseExistingImage(ref *ResolvedRef, credentials *authn.AuthConfig, resourceTags tags.Tags, requested *Platform) (*Image, bool, error) { meta, err := readMetadata(m.paths, ref.Repository(), ref.DigestHex()) if err != nil { return nil, false, nil @@ -284,6 +281,13 @@ func (m *manager) reuseExistingImage(ref *ResolvedRef, credentials *authn.AuthCo return nil, true, fmt.Errorf("%w: retry after the current pull completes", ErrCredentialConflict) } } + // Only ready macOS records carry a platform the request can contradict. + if requested != nil && meta.Status == StatusReady && meta.MacOS != nil { + actual, err := ParsePlatform(meta.Platform) + if err != nil || !requested.Matches(actual) { + return nil, true, fmt.Errorf("%w: requested %s but cached manifest is %s", ErrInvalidPlatform, *requested, meta.Platform) + } + } if resourceTags != nil { meta.Tags = tags.Clone(resourceTags) if err := writeMetadata(m.paths, ref.Repository(), ref.DigestHex(), meta); err != nil { @@ -526,13 +530,13 @@ func (m *manager) buildImage(ctx context.Context, ref *ResolvedRef, credentials return } convertStart := time.Now() - staged := stagedImageFiles{disk: diskTempPath} + var staged stagedImageFiles if payload != nil { - staged.aux = filepath.Join(layout.dir, "aux.img.tmp-"+buildID) - defer os.Remove(staged.aux) - staged.macos = payload.Platform - staged.sizeBytes, err = stageMacOSMachine(payload, diskTempPath, staged.aux) + auxTemp := filepath.Join(layout.dir, "aux.img.tmp-"+buildID) + defer os.Remove(auxTemp) + staged, err = stageMacOSMachine(payload, diskTempPath, auxTemp) } else { + staged = stagedImageFiles{disk: diskTempPath} staged.sizeBytes, err = ExportRootfs(tempDir, diskTempPath, DefaultImageFormat) } m.recordImageBuildPhase(ctx, ref.Digest(), "filesystem_export", time.Since(convertStart), phaseStatus(err), "not_applicable") diff --git a/lib/images/platform.go b/lib/images/platform.go index 87b31df53..3ff14acf6 100644 --- a/lib/images/platform.go +++ b/lib/images/platform.go @@ -180,8 +180,8 @@ func resolveManifestPlatform(meta *containerMetadata, requested string) (Platfor if err := actual.validate(); err != nil { return Platform{}, fmt.Errorf("image platform: %w", err) } - if !want.Matches(actual) { - return Platform{}, fmt.Errorf("%w: requested %s but manifest is %s", ErrInvalidPlatform, want, actual) + if err := validateDigestPlatform(requested, want, actual); err != nil { + return Platform{}, err } return actual, nil } diff --git a/lib/images/tag_test.go b/lib/images/tag_test.go index 9a6d6a605..5475c33d7 100644 --- a/lib/images/tag_test.go +++ b/lib/images/tag_test.go @@ -297,7 +297,7 @@ func TestReuseExistingImageUpdatesResourceTags(t *testing.T) { ref, err := ParseNormalizedRef(repository + "@sha256:" + digest) require.NoError(t, err) resolved := NewResolvedRef(ref, "sha256:"+digest) - img, found, err := m.reuseExistingImage(resolved, nil, tags.Tags{"team": "payments"}) + img, found, err := m.reuseExistingImage(resolved, nil, tags.Tags{"team": "payments"}, nil) require.NoError(t, err) require.True(t, found) require.Equal(t, "payments", img.Tags["team"]) From 4888603e6008ce7a38a2f59322b1979c9b249051 Mon Sep 17 00:00:00 2001 From: chris lee Date: Fri, 9 Oct 2026 16:23:34 -0400 Subject: [PATCH 8/8] Preserve uninstalled manifests on rollback and reuse bundle validation --- lib/images/finalization_rollback_test.go | 31 ++++++++++ lib/images/macos_machine.go | 78 +----------------------- lib/images/manager.go | 3 +- 3 files changed, 33 insertions(+), 79 deletions(-) create mode 100644 lib/images/finalization_rollback_test.go diff --git a/lib/images/finalization_rollback_test.go b/lib/images/finalization_rollback_test.go new file mode 100644 index 000000000..65d936b22 --- /dev/null +++ b/lib/images/finalization_rollback_test.go @@ -0,0 +1,31 @@ +package images + +import ( + "os" + "path/filepath" + "testing" + + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestFinalizationRollbackPreservesUninstalledManifest(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + ref, err := ParseNormalizedRef("localhost/qa@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + require.NoError(t, err) + resolved := NewResolvedRef(ref, ref.Digest()) + require.NoError(t, writeMetadata(p, ref.Repository(), ref.DigestHex(), &imageMetadata{BuildID: "qa-build", Status: StatusPending})) + layout := resolveImageLayout(p, ref.Repository(), ref.DigestHex()) + modelPath := manifestModelPath(p, layout, ref.DigestHex()) + require.NoError(t, os.MkdirAll(filepath.Dir(modelPath), 0700)) + require.NoError(t, os.WriteFile(modelPath, []byte("preexisting-manifest"), 0600)) + stagedDisk := filepath.Join(layout.dir, "qa-staged-disk") + require.NoError(t, os.WriteFile(stagedDisk, []byte("synthetic-disk"), 0600)) + // Manifest validation fails before any model file is installed. + err = m.finalizeImage(resolved, &pullResult{Metadata: &containerMetadata{OS: "linux", Architecture: "arm64"}, Manifest: &imageManifestModel{}}, "qa-build", stagedImageFiles{disk: stagedDisk, sizeBytes: 14}) + require.ErrorContains(t, err, "write manifest model") + data, err := os.ReadFile(modelPath) + require.NoError(t, err, "rollback must not remove a manifest that this attempt did not install") + require.Equal(t, "preexisting-manifest", string(data)) +} diff --git a/lib/images/macos_machine.go b/lib/images/macos_machine.go index a6814001e..ed95911dc 100644 --- a/lib/images/macos_machine.go +++ b/lib/images/macos_machine.go @@ -1,10 +1,8 @@ package images import ( - "encoding/json" "fmt" "os" - "path/filepath" "github.com/kernel/hypeman/lib/forkvm" ) @@ -18,46 +16,6 @@ const ( MacOSMachinePlatformLabel = "io.hypeman.machine-image.platform-path" ) -// macOSMachinePayload is a complete cold-boot bundle, not a container rootfs. -// This spike intentionally has no base/delta format or fork identity rebinding. -type macOSMachinePayload struct { - Disk string - Aux string - Platform *MacOSImage -} - -func machineBundleFile(root, relative string) (string, error) { - if relative == "" || !filepath.IsLocal(relative) { - return "", fmt.Errorf("machine payload path must be local and relative") - } - root, err := filepath.EvalSymlinks(root) - if err != nil { - return "", err - } - path, err := filepath.EvalSymlinks(filepath.Join(root, relative)) - if err != nil { - return "", err - } - rel, err := filepath.Rel(root, path) - if err != nil || !filepath.IsLocal(rel) { - return "", fmt.Errorf("machine payload escapes artifact root") - } - info, err := os.Stat(path) - if err != nil { - return "", err - } - if !info.Mode().IsRegular() || info.Size() == 0 { - return "", fmt.Errorf("machine payload must be a nonempty regular file") - } - return path, nil -} - -func sameFile(a, b string) bool { - ai, errA := os.Stat(a) - bi, errB := os.Stat(b) - return errA == nil && errB == nil && os.SameFile(ai, bi) -} - // stageMacOSMachine copies a validated bundle's boot disk and auxiliary storage to // build-private paths, outside the manager lock. It returns the bytes both files // occupy, which is the size recorded for accounting. @@ -102,39 +60,5 @@ func parseMacOSMachine(root string, meta *containerMetadata) (*macOSMachinePaylo if labels[MacOSMachineVersionLabel] != "1" || labels[MacOSMachineKindLabel] != "macos-image" || labels[MacOSMachineFormatLabel] != "raw" { return nil, fmt.Errorf("darwin artifact requires version 1 macos-image with raw disk, not an ordinary container image") } - disk, err := machineBundleFile(root, labels[MacOSMachineDiskLabel]) - if err != nil { - return nil, err - } - aux, err := machineBundleFile(root, labels[MacOSMachineAuxLabel]) - if err != nil { - return nil, err - } - config, err := machineBundleFile(root, labels[MacOSMachinePlatformLabel]) - if err != nil { - return nil, err - } - // Compare inodes: hardlinks with different names must not satisfy distinctness. - if sameFile(disk, aux) || sameFile(disk, config) || sameFile(aux, config) { - return nil, fmt.Errorf("machine bundle files must be distinct") - } - info, err := os.Stat(config) - if err != nil { - return nil, err - } - if info.Size() > 64<<10 { - return nil, fmt.Errorf("machine platform metadata is too large") - } - b, err := os.ReadFile(config) - if err != nil { - return nil, err - } - var platform MacOSImage - if err := json.Unmarshal(b, &platform); err != nil { - return nil, err - } - if err := platform.Validate(); err != nil { - return nil, err - } - return &macOSMachinePayload{Disk: disk, Aux: aux, Platform: &platform}, nil + return readMacOSMachineBundle(root, labels[MacOSMachineDiskLabel], labels[MacOSMachineAuxLabel], labels[MacOSMachinePlatformLabel], false) } diff --git a/lib/images/manager.go b/lib/images/manager.go index d567d5fe6..58922fb0d 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -618,7 +618,7 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID st model.Platform = actualPlatform.String() modelPath := manifestModelPath(m.paths, layout, ref.DigestHex()) if err := writeManifestModelAt(modelPath, ref.DigestHex(), &model); err != nil { - return rollbackFinalization(append(installed, modelPath), fmt.Errorf("write manifest model: %w", err)) + return rollbackFinalization(installed, fmt.Errorf("write manifest model: %w", err)) } installed = append(installed, modelPath) } @@ -637,7 +637,6 @@ func (m *manager) finalizeImage(ref *ResolvedRef, result *pullResult, buildID st return rollbackFinalization(installed, fmt.Errorf("write final metadata: %w", err)) } - installed = nil // committed: nothing below removes installed files m.notifyReady(ref.DigestHex(), StatusReady, nil) if !m.claimRequestedTags(ref, meta) { m.cleanupUnclaimedImage(ref)