runtime: select BuildKit explicitly + fix tar symlink encoding

feature/dockerfile.go

@@ -108,7 +108,6 @@ func GenerateBuildContext(plan BuildPlan, dst string) error {
 // generateDockerfile returns the contents of the Dockerfile that layers
 // the configured features on top of plan.BaseImage. Structure:
 //
-//	# syntax=docker/dockerfile:1.4
 //	ARG _DEV_CONTAINERS_BASE_IMAGE=...
 //	FROM $_DEV_CONTAINERS_BASE_IMAGE AS devcontainer_target
 //	USER root
@@ -119,9 +118,14 @@ func GenerateBuildContext(plan BuildPlan, dst string) error {
 //	LABEL devcontainer.metadata='[...]'
 //	ARG _DEV_CONTAINERS_IMAGE_USER=root
 //	USER $_DEV_CONTAINERS_IMAGE_USER
+//
+// Intentionally no `# syntax=docker/dockerfile:X` directive: nothing
+// in the emitted file needs a non-builtin frontend, and declaring one
+// forces buildkit to pull `docker/dockerfile:*` from a registry —
+// which requires a session to forward credentials (we don't open
+// one) and hangs indefinitely behind broken registry mirrors.
 func generateDockerfile(plan BuildPlan) (string, error) {
 	var b strings.Builder
-	b.WriteString("# syntax=docker/dockerfile:1.4\n")
 	fmt.Fprintf(&b, "ARG _DEV_CONTAINERS_BASE_IMAGE=%s\n", plan.BaseImage)
 	b.WriteString("FROM $_DEV_CONTAINERS_BASE_IMAGE AS devcontainer_target\n\n")
 

feature/dockerfile_test.go

@@ -58,7 +58,6 @@ func TestGenerateBuildContext_WritesExpectedLayout(t *testing.T) {
 	}
 
 	wantSubstrings := []string{
-		"# syntax=docker/dockerfile:1.4",
 		"ARG _DEV_CONTAINERS_BASE_IMAGE=alpine:3.20",
 		"FROM $_DEV_CONTAINERS_BASE_IMAGE",
 		"COPY ./build-context/ /tmp/dc-features/",
@@ -76,6 +75,12 @@ func TestGenerateBuildContext_WritesExpectedLayout(t *testing.T) {
 			t.Errorf("Dockerfile missing %q", want)
 		}
 	}
+	// Frontend directive must not be reintroduced: buildkit treats it as
+	// a hard frontend-pull requirement, which we don't supply credentials
+	// for (no session) and which hangs behind broken registry mirrors.
+	if strings.Contains(string(df), "# syntax=") {
+		t.Errorf("Dockerfile must not declare a syntax= frontend; built-in is sufficient")
+	}
 
 	// Per-feature dirs populated with run.sh, feature.env, install.sh.
 	for _, idx := range []string{"0", "1"} {

runtime/docker/build.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/moby/moby/api/types/build"
 	"github.com/moby/moby/client"
 
 	"github.com/crunchloop/devcontainer/runtime"
@@ -22,6 +23,15 @@ import (
 // Streaming progress messages are mapped onto the events channel as
 // runtime.BuildEvents (drop-on-full). Build failures surface as a
 // non-nil error including any structured error returned by the daemon.
+//
+// BuildKit is required. The classic builder synthesizes one
+// intermediate container per Dockerfile step and routes every
+// container API through the daemon's authorization pipeline, which
+// — behind an authz plugin — turns a sub-second build into a
+// multi-minute one (~140× slowdown observed in production). BuildKit
+// uses a single streaming session and is unaffected. Docker Engine
+// has shipped with BuildKit enabled by default since 23.0 (Feb 2023);
+// requiring it here is in line with the lib's modern-spec stance.
 func (r *Runtime) BuildImage(ctx context.Context, spec runtime.BuildSpec, events chan<- runtime.BuildEvent) (runtime.ImageRef, error) {
 	if spec.ContextPath == "" {
 		return runtime.ImageRef{}, fmt.Errorf("BuildImage: spec.ContextPath required")
@@ -57,6 +67,7 @@ func (r *Runtime) BuildImage(ctx context.Context, spec runtime.BuildSpec, events
 		CacheFrom:  spec.CacheFrom,
 		NoCache:    spec.NoCache,
 		Remove:     true,
+		Version:    build.BuilderBuildKit,
 	})
 	if err != nil {
 		return runtime.ImageRef{}, fmt.Errorf("ImageBuild: %w", err)
@@ -81,6 +92,22 @@ func (r *Runtime) BuildImage(ctx context.Context, spec runtime.BuildSpec, events
 // streamBuildOutput parses the JSON-line stream from ImageBuild,
 // emitting BuildEvents and returning a non-nil error if the daemon
 // reports a build failure. Closes body before returning.
+//
+// Two response shapes are handled:
+//
+//   - Classic-builder-style records with `stream` (log lines) and
+//     `status` (layer/pull progress) fields. Pre-BuildKit format.
+//
+//   - BuildKit records of the form `{"id":"moby.buildkit.trace",
+//     "aux":"<base64-protobuf>"}` for per-step progress and
+//     `{"id":"moby.image.id","aux":{"ID":"sha256:..."}}` for the
+//     final image. The aux protobuf is buildkit's `SolveStatus` —
+//     decoding requires the buildkit module. We intentionally don't
+//     pull that dep in: per-step progress events are silently dropped
+//     under BuildKit; BuildStart / BuildCompleted (emitted by the
+//     caller and at the end of BuildImage) still fire correctly, and
+//     errors still propagate via `errorDetail` / `error` fields.
+//     A future PR can revisit if vertex-level progress is needed.
 func streamBuildOutput(ctx context.Context, body io.ReadCloser, events chan<- runtime.BuildEvent) error {
 	defer body.Close()
 
@@ -129,12 +156,15 @@ func streamBuildOutput(ctx context.Context, body io.ReadCloser, events chan<- ru
 }
 
 // tarDirectory writes the contents of dir (recursively) into w as a
-// non-gzipped tar archive. Symlinks are followed (their targets are
-// included as regular files) — the build daemon doesn't need our
-// engineering tmp dirs to preserve link semantics.
+// non-gzipped tar archive. Symlinks are preserved as tar TypeSymlink
+// entries with their original target text; the daemon-side BuildKit
+// frontend handles the resolution.
 //
-// Empty / unreadable files are best-effort logged via the returned
-// error rather than silently dropped.
+// Previously this passed an empty link argument to tar.FileInfoHeader
+// for symlinks, producing tar entries with TypeSymlink + empty
+// Linkname. Some downstream tar readers reject those as malformed and
+// abort the build mid-stream — common in compose-primary contexts
+// containing node_modules/.bin/* or similar bin-symlinks.
 func tarDirectory(dir string, w io.Writer) error {
 	tw := tar.NewWriter(w)
 	defer func() { _ = tw.Close() }()
@@ -157,7 +187,16 @@ func tarDirectory(dir string, w io.Writer) error {
 		if err != nil {
 			return err
 		}
-		hdr, err := tar.FileInfoHeader(info, "")
+
+		var link string
+		isSymlink := info.Mode()&os.ModeSymlink != 0
+		if isSymlink {
+			link, err = os.Readlink(path)
+			if err != nil {
+				return fmt.Errorf("readlink %s: %w", path, err)
+			}
+		}
+		hdr, err := tar.FileInfoHeader(info, link)
 		if err != nil {
 			return fmt.Errorf("tar header for %s: %w", path, err)
 		}
@@ -169,7 +208,7 @@ func tarDirectory(dir string, w io.Writer) error {
 		if err := tw.WriteHeader(hdr); err != nil {
 			return err
 		}
-		if d.IsDir() || (info.Mode()&os.ModeSymlink != 0 && hdr.Typeflag != tar.TypeReg) {
+		if d.IsDir() || isSymlink {
 			return nil
 		}
 		f, err := os.Open(path)

test/integration/build_context_symlink_test.go

@@ -0,0 +1,82 @@
+//go:build integration
+
+package integration
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	devcontainer "github.com/crunchloop/devcontainer"
+)
+
+// TestBuildContext_SurvivesSymlinks builds a Dockerfile-source workspace
+// whose context directory contains a symlink that is not referenced by
+// any COPY instruction. Regression guard for a tar-packing bug where
+// symlink entries were written with TypeSymlink + empty Linkname; some
+// tar readers (including dockerd's older path) reject those as
+// malformed and abort the build with an opaque tar error.
+//
+// The presence of a symlink in node_modules/.bin/* or vendored
+// dependencies is the canonical real-world case — Dockerfile usually
+// only COPYs ./pkg/, but the daemon still has to stream the whole
+// context tar past its reader before it can prune.
+func TestBuildContext_SurvivesSymlinks(t *testing.T) {
+	if testing.Short() {
+		t.Skip("integration tests skipped with -short")
+	}
+
+	eng, rt := newEngine(t)
+	defer rt.Close()
+
+	dir := t.TempDir()
+
+	// Real file in the context that the symlink will point at.
+	mustWrite(t, filepath.Join(dir, "target.txt"), "real-content\n")
+
+	// Relative symlink target.txt → ./target.txt, mimicking
+	// node_modules/.bin/* link layout.
+	if err := os.Symlink("target.txt", filepath.Join(dir, "link.txt")); err != nil {
+		t.Fatalf("symlink: %v", err)
+	}
+
+	// Trivial Dockerfile that only COPYs the regular file. The symlink
+	// is unreferenced — the test is that the tar stream survives, not
+	// that COPY of the symlink works.
+	mustWrite(t, filepath.Join(dir, "Dockerfile"), `
+FROM alpine:3.20
+COPY target.txt /etc/target.txt
+RUN echo built > /etc/symlink-build-marker
+`)
+
+	mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{
+		"build": { "dockerfile": "Dockerfile", "context": ".." }
+	}`)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	wsObj, err := eng.Up(ctx, devcontainer.UpOptions{
+		LocalWorkspaceFolder: dir,
+		Recreate:             true,
+		SkipLifecycle:        true,
+	})
+	if err != nil {
+		t.Fatalf("Up with symlinked context: %v", err)
+	}
+	defer func() { _ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{Remove: true}) }()
+
+	res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{
+		Cmd: []string{"cat", "/etc/symlink-build-marker"},
+	})
+	if err != nil {
+		t.Fatalf("Exec marker: %v", err)
+	}
+	if res.ExitCode != 0 || !strings.Contains(res.Stdout, "built") {
+		t.Errorf("marker not present (build did not complete?): exit=%d stdout=%q stderr=%q",
+			res.ExitCode, res.Stdout, res.Stderr)
+	}
+}