diff --git a/docker/core.Dockerfile b/docker/core.Dockerfile index 8402f640..258a7148 100644 --- a/docker/core.Dockerfile +++ b/docker/core.Dockerfile @@ -10,9 +10,15 @@ FROM node:22-bookworm@sha256:7725a5c2c83eed1d36258c66efae14b1ceccd021db9ed1d9559 # ---- Stage 0: locked Pi runtime ---- FROM node:22-bookworm@sha256:7725a5c2c83eed1d36258c66efae14b1ceccd021db9ed1d9559d3335ed3d68ed AS pi-runtime-build ARG PI_VERSION +ARG PI_RUNTIME_PACKAGE_VERSION +ARG PI_PACKAGE_NAME=@earendil-works/pi-coding-agent WORKDIR /opt/pi-runtime COPY docker/pi-runtime/package.json docker/pi-runtime/package-lock.json ./ -RUN npm ci --omit=dev \ +RUN if [ -n "$PI_RUNTIME_PACKAGE_VERSION" ]; then \ + node -e 'const fs=require("node:fs"); const [name,version]=process.argv.slice(1); const manifest=JSON.parse(fs.readFileSync("package.json","utf8")); manifest.dependencies[name]=version; fs.writeFileSync("package.json",JSON.stringify(manifest,null,2)+"\\n");' "$PI_PACKAGE_NAME" "$PI_RUNTIME_PACKAGE_VERSION"; \ + npm install --package-lock-only --ignore-scripts --omit=dev "$PI_PACKAGE_NAME@$PI_RUNTIME_PACKAGE_VERSION"; \ + fi \ + && npm ci --omit=dev \ && test "$(./node_modules/.bin/pi --version)" = "$PI_VERSION" # ---- Stage 1: backend TypeScript -> dist ---- diff --git a/tools/tht/internal/pi/build_contract_test.go b/tools/tht/internal/pi/build_contract_test.go new file mode 100644 index 00000000..8ae62ad5 --- /dev/null +++ b/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") + } +} diff --git a/tools/tht/internal/pi/latest.go b/tools/tht/internal/pi/latest.go index df05c0df..69cb7b6d 100644 --- a/tools/tht/internal/pi/latest.go +++ b/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) { diff --git a/tools/tht/internal/pi/latest_test.go b/tools/tht/internal/pi/latest_test.go index fb0994dd..3e726cbe 100644 --- a/tools/tht/internal/pi/latest_test.go +++ b/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") diff --git a/tools/tht/internal/pi/update.go b/tools/tht/internal/pi/update.go index b9bfab85..50533f47 100644 --- a/tools/tht/internal/pi/update.go +++ b/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) } diff --git a/tools/tht/internal/pi/update_test.go b/tools/tht/internal/pi/update_test.go index b823628a..8b28434f 100644 --- a/tools/tht/internal/pi/update_test.go +++ b/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"`) {