feat(docker): pull the sandbox image when it is absent

Create went straight to ContainerCreate, so a host without the image
already on it failed with a bare "No such image: ...". That made every
deployment need a manual build or pull first, and made publishing an
image to a registry pointless — nothing would ever fetch it.

Pull-if-absent, not always-pull. The image is the guest's entire
userland, so re-resolving a mutable tag on every create would let the
code running in a sandbox change without anything in the caller's
configuration changing. Absent means fetch; present means use what is
here. A test asserts the present case does not re-resolve.

The pull body is drained rather than discarded early: the daemon does the
work as the stream is read, so returning at the first byte leaves the
image half-fetched and the create failing right behind it. The result is
then confirmed by inspect, because the pull API reports some failures
inside a 200 response.

ErrImageUnavailable is its own sentinel rather than ErrInvalid. The
reference may be perfectly well-formed and the registry simply down —
one is worth retrying and the other never is.

Also adds IsDigestPinned, since "pin a digest" is advice ARCHITECTURE.md
gives and nothing could previously check.

Author: Lutherwaves

ARCHITECTURE.md

@@ -154,6 +154,13 @@ Stated plainly, because a security section that only lists wins is marketing:
 - A gVisor escape. We inherit gVisor's threat model and its CVEs.
 - A malicious or backdoored sandbox **image**. Image supply chain is the caller's
   responsibility — pin digests.
+
+  openblox pulls an image when it is absent and otherwise uses what is already on
+  the host. Pull-if-absent, not always-pull, is the deliberate choice: the image
+  is the guest's entire userland, so re-resolving a mutable tag on every create
+  would let the code running in a sandbox change without anything in the caller's
+  configuration changing. The corollary is that a tag is only as trustworthy as
+  the moment it was first pulled, which is why `WithImage` says to pin a digest.
 - Side channels between co-resident sandboxes on the same host.
 - Anything the caller does with the results. openblox contains execution; it does
   not sanitise output.

pkg/docker/backend.go

@@ -109,6 +109,9 @@ func (b *Backend) Create(ctx context.Context, name string, opts ...sandbox.Creat
 	if err := b.assertRuntime(ctx, spec.Runtime); err != nil {
 		return nil, err
 	}
+	if err := b.ensureImage(ctx, spec.Image); err != nil {
+		return nil, err
+	}
 
 	cfg, hostCfg := buildConfig(name, spec)
 	created, err := b.cli.ContainerCreate(ctx, cfg, hostCfg, nil, nil, containerName(name))

pkg/docker/image.go

@@ -0,0 +1,61 @@
+package docker
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"strings"
+
+	cerrdefs "github.com/containerd/errdefs"
+	"github.com/docker/docker/api/types/image"
+
+	"github.com/blox-eng/openblox/pkg/sandbox"
+)
+
+// ensureImage makes the image available locally, pulling it if it is absent.
+//
+// Pull-if-absent rather than always-pull, deliberately. A sandbox image is the
+// guest's entire userland, so re-resolving a mutable tag on every Create would
+// mean the code running in the sandbox could change under the caller without
+// anything in their configuration changing. Absent means fetch; present means
+// use what is here.
+//
+// The corollary is that a tag is only as trustworthy as the moment it was first
+// pulled. Pin a digest — see [sandbox.WithImage] — and this becomes exact.
+func (b *Backend) ensureImage(ctx context.Context, ref string) error {
+	if _, err := b.cli.ImageInspect(ctx, ref); err == nil {
+		return nil
+	} else if !cerrdefs.IsNotFound(err) {
+		return fmt.Errorf("inspect image %q: %w", ref, err)
+	}
+
+	body, err := b.cli.ImagePull(ctx, ref, image.PullOptions{})
+	if err != nil {
+		return fmt.Errorf("%w: pull image %q: %w", sandbox.ErrImageUnavailable, ref, err)
+	}
+	defer body.Close()
+
+	// The pull is only complete once the response body is drained: the daemon
+	// streams progress and does the work as it is read, so returning early would
+	// leave the image half-fetched and the create failing right behind it.
+	if _, err := io.Copy(io.Discard, body); err != nil {
+		return fmt.Errorf("%w: pull image %q: %w", sandbox.ErrImageUnavailable, ref, err)
+	}
+
+	// Confirm rather than trust the stream: the pull API reports some failures
+	// inside the progress body with a 200 status.
+	if _, err := b.cli.ImageInspect(ctx, ref); err != nil {
+		return fmt.Errorf("%w: image %q is still absent after pulling", sandbox.ErrImageUnavailable, ref)
+	}
+	return nil
+}
+
+// IsDigestPinned reports whether ref names an image by digest rather than by a
+// tag. A tag can be repointed by whoever controls the registry; a digest cannot.
+func IsDigestPinned(ref string) bool {
+	at := strings.LastIndex(ref, "@")
+	if at < 0 {
+		return false
+	}
+	return strings.HasPrefix(ref[at+1:], "sha256:")
+}

pkg/docker/image_integration_test.go

@@ -0,0 +1,82 @@
+//go:build integration
+
+package docker
+
+import (
+	"context"
+	"errors"
+	"os/exec"
+	"testing"
+
+	"github.com/blox-eng/openblox/pkg/sandbox"
+)
+
+// pullTestImage is small, stable, and not the image the rest of the suite uses,
+// so removing it locally cannot disturb anything else.
+const pullTestImage = "alpine:3.19"
+
+// A sandbox host is not required to have the image already. Before this, Create
+// failed with a bare "No such image" and every deployment needed a manual build
+// or pull first.
+func TestCreatePullsAnAbsentImage(t *testing.T) {
+	b := newTestBackend(t)
+	ctx := context.Background()
+
+	// Start from a genuinely absent image, or the test proves nothing.
+	_ = exec.Command("docker", "rmi", "-f", pullTestImage).Run()
+	if err := exec.Command("docker", "image", "inspect", pullTestImage).Run(); err == nil {
+		t.Skipf("%s is still present (in use by another container?); cannot prove the pull", pullTestImage)
+	}
+
+	name := "openblox-test-pull"
+	_, err := b.Create(ctx, name, sandbox.WithImage(pullTestImage))
+	if err != nil {
+		t.Fatalf("Create with an absent image = %v, want it pulled", err)
+	}
+	t.Cleanup(func() { _ = b.Destroy(context.Background(), name) })
+
+	if err := exec.Command("docker", "image", "inspect", pullTestImage).Run(); err != nil {
+		t.Errorf("%s is still absent after a successful Create", pullTestImage)
+	}
+}
+
+// An image that cannot be obtained is its own failure, distinct from a malformed
+// request: the reference may be perfectly valid and the registry simply down.
+func TestCreateReportsAnUnobtainableImage(t *testing.T) {
+	b := newTestBackend(t)
+
+	_, err := b.Create(context.Background(), "openblox-test-noimage",
+		sandbox.WithImage("ghcr.io/blox-eng/openblox-does-not-exist:0.0.0"))
+
+	if !errors.Is(err, sandbox.ErrImageUnavailable) {
+		t.Fatalf("Create with an unobtainable image = %v, want ErrImageUnavailable", err)
+	}
+}
+
+// A present image must not be re-resolved on every create. The image is the
+// guest's whole userland, so a mutable tag silently changing under a caller is a
+// supply-chain surprise, not a convenience.
+func TestCreateDoesNotRePullAPresentImage(t *testing.T) {
+	b := newTestBackend(t)
+	ctx := context.Background()
+
+	before := imageID(t, testImage)
+	name := "openblox-test-nopull"
+	if _, err := b.Create(ctx, name, sandbox.WithImage(testImage)); err != nil {
+		t.Fatalf("Create = %v", err)
+	}
+	t.Cleanup(func() { _ = b.Destroy(context.Background(), name) })
+
+	if after := imageID(t, testImage); after != before {
+		t.Errorf("image id changed %s -> %s; a present image was re-resolved", before, after)
+	}
+}
+
+func imageID(t *testing.T, ref string) string {
+	t.Helper()
+	out, err := exec.Command("docker", "image", "inspect", "-f", "{{.Id}}", ref).Output()
+	if err != nil {
+		t.Fatalf("inspect %s = %v", ref, err)
+	}
+	return string(out)
+}

pkg/docker/image_test.go

@@ -0,0 +1,34 @@
+package docker
+
+import "testing"
+
+// A tag can be repointed by whoever controls the registry; a digest cannot. The
+// image is the guest's entire userland, so the distinction is the difference
+// between running what you reviewed and running whatever is there today.
+func TestIsDigestPinned(t *testing.T) {
+	pinned := []string{
+		"alpine@sha256:abc123",
+		"ghcr.io/blox-eng/blox-sandbox@sha256:deadbeef",
+		"registry.example.test:5000/team/img@sha256:0011",
+	}
+	for _, ref := range pinned {
+		if !IsDigestPinned(ref) {
+			t.Errorf("IsDigestPinned(%q) = false, want true", ref)
+		}
+	}
+
+	loose := []string{
+		"alpine",
+		"alpine:3.20",
+		"ghcr.io/blox-eng/blox-sandbox:0.2.0",
+		"registry.example.test:5000/team/img",
+		// A port in the host is not a digest, and neither is an empty one.
+		"example.test:5000/img@",
+		"example.test:5000/img@md5:abc",
+	}
+	for _, ref := range loose {
+		if IsDigestPinned(ref) {
+			t.Errorf("IsDigestPinned(%q) = true, want false", ref)
+		}
+	}
+}

pkg/sandbox/errors.go

@@ -25,6 +25,12 @@ var (
 	// isolation. A sandbox that is quietly less isolated than requested is worse
 	// than no sandbox, because the caller keeps trusting it.
 	ErrRuntimeUnavailable = errors.New("required runtime unavailable")
+
+	// ErrImageUnavailable means the sandbox image could not be obtained: it is
+	// absent locally and could not be pulled. Distinct from ErrInvalid because
+	// the request was well-formed and retrying may well succeed — a registry
+	// being unreachable is a different problem from a misspelled reference.
+	ErrImageUnavailable = errors.New("sandbox image unavailable")
 )
 
 func newInvalidError(format string, args ...any) error {