fix(pi): isolate resolved package builds

tools/tht/internal/pi/build_contract_test.go

@@ -0,0 +1,31 @@
+package pi
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestCoreDockerfileBuildsAnIsolatedResolvedPiDependency(t *testing.T) {
+	path := filepath.Join("..", "..", "..", "..", "docker", "core.Dockerfile")
+	contents, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	dockerfile := string(contents)
+	for _, required := range []string{
+		"ARG PI_RUNTIME_PACKAGE_VERSION",
+		"if [ -n \"$PI_RUNTIME_PACKAGE_VERSION\" ]; then",
+		"npm install --package-lock-only --ignore-scripts --omit=dev \"$PI_PACKAGE_NAME@$PI_RUNTIME_PACKAGE_VERSION\"",
+		"npm ci --omit=dev",
+		"test \"$(./node_modules/.bin/pi --version)\" = \"$PI_VERSION\"",
+	} {
+		if !strings.Contains(dockerfile, required) {
+			t.Fatalf("docker/core.Dockerfile does not contain required isolated Pi build contract %q", required)
+		}
+	}
+	if strings.Contains(dockerfile, "COPY docker/pi-runtime/package.json docker/pi-runtime/package-lock.json ./\nRUN npm install @") {
+		t.Fatal("docker/core.Dockerfile mutates the checkout dependency state outside its isolated build stage")
+	}
+}

tools/tht/internal/pi/latest.go

@@ -43,7 +43,13 @@ func newNPMRegistryClient(client *http.Client) *npmRegistryClient {
 	if client == nil {
 		client = &http.Client{Timeout: registryTimeout}
 	}
-	return &npmRegistryClient{client: client}
+	bounded := *client
+	bounded.CheckRedirect = rejectRegistryRedirect
+	return &npmRegistryClient{client: &bounded}
+}
+
+func rejectRegistryRedirect(request *http.Request, _ []*http.Request) error {
+	return fmt.Errorf("Pi registry redirect to %s is not allowed", request.URL.Redacted())
 }
 
 // LatestStable returns the greatest released semantic version. npm's dist-tag is not trusted as
@@ -99,7 +105,10 @@ func (c *npmRegistryClient) LatestStable(ctx context.Context, packageName string
 			return "", errors.New("Pi registry response is invalid")
 		}
 		version, err := parseSemanticVersion(rawVersion)
-		if err != nil || version.prerelease != "" {
+		if err != nil {
+			return "", errors.New("Pi registry response contains an invalid semantic version key")
+		}
+		if version.prerelease != "" {
 			continue
 		}
 		if !found || selected.less(version) {

tools/tht/internal/pi/latest_test.go

@@ -53,6 +53,7 @@ func TestNPMRegistryClientRejectsInvalidResponses(t *testing.T) {
 	}{
 		{name: "prerelease only", status: http.StatusOK, body: `{"versions":{"1.0.0-rc.1":{}}}`, wantText: "no stable"},
 		{name: "malformed JSON", status: http.StatusOK, body: `{`, wantText: "invalid"},
+		{name: "malformed semantic version key", status: http.StatusOK, body: `{"versions":{"0.81.0":{},"latest":{}}}`, wantText: "invalid"},
 		{name: "no version data", status: http.StatusOK, body: `{"versions":{}}`, wantText: "no stable"},
 		{name: "package not found", status: http.StatusNotFound, body: `{"error":"not_found"}`, wantText: "not found"},
 	} {
@@ -68,6 +69,31 @@ func TestNPMRegistryClientRejectsInvalidResponses(t *testing.T) {
 	}
 }
 
+func TestNPMRegistryClientRejectsRedirectsOutsideTheAuthoritativePackageURL(t *testing.T) {
+	for _, destination := range []string{
+		"http://registry.npmjs.org/@earendil-works%2Fpi-coding-agent",
+		"https://mirror.example.invalid/@earendil-works%2Fpi-coding-agent",
+	} {
+		t.Run(destination, func(t *testing.T) {
+			calls := 0
+			client := newNPMRegistryClient(&http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) {
+				calls++
+				response := registryResponse(http.StatusFound, "")
+				response.Header.Set("Location", destination)
+				response.Request = request
+				return response, nil
+			})})
+			_, err := client.LatestStable(context.Background(), "@earendil-works/pi-coding-agent")
+			if err == nil || !strings.Contains(strings.ToLower(err.Error()), "redirect") {
+				t.Fatalf("LatestStable() redirect error = %v, want rejected redirect", err)
+			}
+			if calls != 1 {
+				t.Fatalf("LatestStable() followed rejected redirect %d times", calls-1)
+			}
+		})
+	}
+}
+
 func TestNPMRegistryClientRejectsAnOversizedResponse(t *testing.T) {
 	client := newNPMRegistryClient(&http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
 		return registryResponse(http.StatusOK, `{"versions":{"1.2.3":{}}}`+strings.Repeat(" ", registryBodyLimit+1)), nil
@@ -124,6 +150,7 @@ func TestUpdateWithResolvedVersionDoesNotMutateWhenDiscoveryFails(t *testing.T)
 		{name: "package missing", err: errors.New("package not found")},
 		{name: "malformed", err: errors.New("registry response is invalid")},
 		{name: "no stable version", err: errors.New("registry contains no stable version")},
+		{name: "malformed semantic version key", err: errors.New("registry version key is invalid")},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			fake := newFakeRunner()
@@ -144,6 +171,24 @@ func TestUpdateWithResolvedVersionDoesNotMutateWhenDiscoveryFails(t *testing.T)
 	}
 }
 
+func TestUpdateWithResolvedVersionDoesNotMutateForMalformedRegistryVersionKeys(t *testing.T) {
+	fake := newFakeRunner()
+	statePath := t.TempDir() + "/state/update.json"
+	registry := newNPMRegistryClient(&http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
+		return registryResponse(http.StatusOK, `{"versions":{"0.81.0":{},"not-a-semver":{}}}`), nil
+	})})
+	_, err := UpdateWithResolvedVersion(context.Background(), fake, Request{StatePath: statePath, Source: BuildSource, Confirm: true, Drain: true}, "@earendil-works/pi-coding-agent", registry)
+	if err == nil || !strings.Contains(err.Error(), "invalid") {
+		t.Fatalf("UpdateWithResolvedVersion() error = %v, want malformed registry version rejection", err)
+	}
+	if len(fake.calls) != 0 {
+		t.Fatalf("malformed registry version invoked Docker or lifecycle commands: %v", fake.calls)
+	}
+	if _, statErr := os.Stat(filepath.Dir(statePath)); !errors.Is(statErr, os.ErrNotExist) {
+		t.Fatalf("malformed registry version created update state directory: %v", statErr)
+	}
+}
+
 func TestReadRuntimePackageNameReadsThePiDependency(t *testing.T) {
 	root := t.TempDir()
 	path := filepath.Join(root, "docker", "pi-runtime")

tools/tht/internal/pi/update.go

@@ -633,7 +633,7 @@ func runningImage(ctx context.Context, runner Runner, reference string) (Image,
 
 func prepareCandidate(ctx context.Context, runner Runner, request Request, candidateReference string) error {
 	if request.Source == BuildSource {
-		result, err := runCompose(ctx, runner, "build", "--pull", "--build-arg", "PI_VERSION="+request.Version, "core")
+		result, err := runCompose(ctx, runner, "build", "--pull", "--build-arg", "PI_VERSION="+request.Version, "--build-arg", "PI_RUNTIME_PACKAGE_VERSION="+request.Version, "core")
 		if err != nil {
 			return commandError("Pi image build", result, err)
 		}

tools/tht/internal/pi/update_test.go

@@ -73,7 +73,7 @@ func TestUpdateBuildsRegistrySelectedVersionRecreatesOnlyCoreAndPersistsRecovery
 	if result.Version != "0.81.0" {
 		t.Fatalf("resolved version = %q, want 0.81.0", result.Version)
 	}
-	assertCalled(t, fake.calls, "build --pull --build-arg PI_VERSION=0.81.0 core")
+	assertCalled(t, fake.calls, "build --pull --build-arg PI_VERSION=0.81.0 --build-arg PI_RUNTIME_PACKAGE_VERSION=0.81.0 core")
 	assertCalled(t, fake.calls, "up --detach --wait --wait-timeout 45 --no-deps --force-recreate core")
 	assertNotCalled(t, fake.calls, "frontend")
 	if got := string(readStateBytes(t, result.StatePath)); strings.Contains(got, "secret") || !strings.Contains(got, `"phase": "verified"`) {