fix: harden DynDNS recovery and publish v1.5.1

internal/updateagent/controller.go

@@ -32,10 +32,11 @@ func NewController(runner Runner, logger *slog.Logger) *Controller {
 
 func agentStatus(phase string) updateprotocol.Status {
 	return updateprotocol.Status{
-		Phase:        phase,
-		UpdatedAt:    time.Now().UTC(),
-		AgentVersion: buildinfo.Version,
-		Protocol:     buildinfo.UpdateProtocolVersion,
+		Phase:         phase,
+		AnonymousPull: true,
+		UpdatedAt:     time.Now().UTC(),
+		AgentVersion:  buildinfo.Version,
+		Protocol:      buildinfo.UpdateProtocolVersion,
 	}
 }
 
@@ -69,7 +70,9 @@ func (c *Controller) updateHandler(w http.ResponseWriter, r *http.Request) {
 	request.Version = strings.TrimSpace(request.Version)
 	request.CurrentVersion = strings.TrimSpace(request.CurrentVersion)
 	request.RegistryUsername = strings.TrimSpace(request.RegistryUsername)
-	if !updateprotocol.ValidVersion(request.Version) || !updateprotocol.ValidVersion(request.CurrentVersion) || request.RegistryUsername == "" || len(request.RegistryUsername) > 128 || len(request.RegistryToken) < 8 || len(request.RegistryToken) > 1024 {
+	anonymous := request.RegistryUsername == "" && request.RegistryToken == ""
+	credentialValid := request.RegistryUsername != "" && len(request.RegistryUsername) <= 128 && len(request.RegistryToken) >= 8 && len(request.RegistryToken) <= 1024
+	if !updateprotocol.ValidVersion(request.Version) || !updateprotocol.ValidVersion(request.CurrentVersion) || (!anonymous && !credentialValid) {
 		http.Error(w, "invalid update request", http.StatusBadRequest)
 		return
 	}
@@ -90,7 +93,7 @@ func (c *Controller) begin(version string) bool {
 		return false
 	}
 	now := time.Now().UTC()
-	c.status = updateprotocol.Status{Phase: updateprotocol.PhaseQueued, TargetVersion: version, Message: "Update eingereiht", StartedAt: now, UpdatedAt: now, AgentVersion: buildinfo.Version, Protocol: buildinfo.UpdateProtocolVersion}
+	c.status = updateprotocol.Status{Phase: updateprotocol.PhaseQueued, TargetVersion: version, Message: "Update eingereiht", StartedAt: now, UpdatedAt: now, AgentVersion: buildinfo.Version, Protocol: buildinfo.UpdateProtocolVersion, AnonymousPull: true}
 	return true
 }
 

internal/updateagent/controller_test.go

@@ -103,3 +103,30 @@ func TestControllerValidatesRequestsAndRunsOnlyOneUpdate(t *testing.T) {
 		t.Fatalf("update did not complete: %#v", controller.Status())
 	}
 }
+
+func TestControllerAcceptsAnonymousButRejectsPartialCredentials(t *testing.T) {
+	t.Parallel()
+	runner := &blockingRunner{started: make(chan updateprotocol.Request, 1), release: make(chan struct{})}
+	defer close(runner.release)
+	controller := NewController(runner, slog.New(slog.NewTextHandler(io.Discard, nil)))
+	for _, credentials := range []string{`,"registry_username":"user"`, `,"registry_token":"secret-token"`} {
+		response := httptest.NewRecorder()
+		controller.Handler().ServeHTTP(response, httptest.NewRequest(http.MethodPost, "/v1/update", strings.NewReader(`{"version":"v2.0.0","current_version":"v1.5.0"`+credentials+`}`)))
+		if response.Code != 400 {
+			t.Fatal("partial credentials accepted")
+		}
+	}
+	response := httptest.NewRecorder()
+	controller.Handler().ServeHTTP(response, httptest.NewRequest(http.MethodPost, "/v1/update", strings.NewReader(`{"version":"v2.0.0","current_version":"v1.5.0"}`)))
+	if response.Code != 202 || !controller.Status().AnonymousPull {
+		t.Fatalf("anonymous update rejected: %d", response.Code)
+	}
+	select {
+	case request := <-runner.started:
+		if request.RegistryToken != "" || request.RegistryUsername != "" {
+			t.Fatal("anonymous request changed")
+		}
+	case <-time.After(time.Second):
+		t.Fatal("anonymous runner not started")
+	}
+}

internal/updateagent/docker.go

@@ -104,6 +104,9 @@ func (r *DockerRunner) Update(ctx context.Context, request updateprotocol.Reques
 	targetReference := r.imageRepository + ":" + request.Version
 	progress(updateprotocol.PhasePulling, "Container-Image wird geladen und geprüft")
 	if err := r.docker.pullImage(ctx, r.imageRepository, request.Version, request.RegistryUsername, request.RegistryToken); err != nil {
+		if errors.Is(err, errRegistryAuthentication) {
+			return &updateError{message: "Container-Image ist nicht öffentlich zugänglich oder der Zugang wurde abgelehnt. Bitte einen gültigen Gitea-Zugang mit Paket-Leserechten hinterlegen; der Dienst läuft unverändert weiter", err: err}
+		}
 		return &updateError{message: "Image konnte nicht geladen werden; der Dienst läuft unverändert weiter", err: err}
 	}
 	targetImage, err := r.docker.inspectImage(ctx, targetReference)
@@ -137,10 +140,12 @@ func (r *DockerRunner) Update(ctx context.Context, request updateprotocol.Reques
 	}()
 
 	progress(updateprotocol.PhaseRestarting, "Anwendung wird kontrolliert neu gestartet")
+	// A lost Docker response does not mean the stop was not executed. Arm
+	// rollback first so a stopped application is restarted on this error path.
+	oldStopped = true
 	if err := r.docker.stopContainer(ctx, target.ID, 20); err != nil {
 		return err
 	}
-	oldStopped = true
 	if err := r.docker.renameContainer(ctx, target.ID, backupName); err != nil {
 		return err
 	}
@@ -315,9 +320,23 @@ func (c *dockerClient) findTarget(ctx context.Context, label string) (containerS
 	return running[0], nil
 }
 
+var errRegistryAuthentication = errors.New("registry authentication required")
+
 func (c *dockerClient) pullImage(ctx context.Context, repository, tag, username, credential string) error {
-	authJSON, _ := json.Marshal(map[string]string{"username": username, "password": credential, "serveraddress": strings.Split(repository, "/")[0]})
-	headers := http.Header{"X-Registry-Auth": {base64.URLEncoding.EncodeToString(authJSON)}}
+	// Public packages must work without credentials, including stale saved ones.
+	err := c.pullImageOnce(ctx, repository, tag, "", "")
+	if errors.Is(err, errRegistryAuthentication) && username != "" && credential != "" {
+		return c.pullImageOnce(ctx, repository, tag, username, credential)
+	}
+	return err
+}
+
+func (c *dockerClient) pullImageOnce(ctx context.Context, repository, tag, username, credential string) error {
+	var headers http.Header
+	if username != "" && credential != "" {
+		authJSON, _ := json.Marshal(map[string]string{"username": username, "password": credential, "serveraddress": strings.Split(repository, "/")[0]})
+		headers = http.Header{"X-Registry-Auth": {base64.URLEncoding.EncodeToString(authJSON)}}
+	}
 	query := url.Values{"fromImage": {repository}, "tag": {tag}}
 	response, err := c.request(ctx, http.MethodPost, "/images/create?"+query.Encode(), nil, headers, true)
 	if err != nil {
@@ -325,21 +344,49 @@ func (c *dockerClient) pullImage(ctx context.Context, repository, tag, username,
 	}
 	defer response.Body.Close()
 	if response.StatusCode != http.StatusOK {
-		return dockerStatusError(response)
+		err := dockerStatusError(response)
+		if response.StatusCode == http.StatusUnauthorized || response.StatusCode == http.StatusForbidden || registryAuthFailure(err.Error()) {
+			return fmt.Errorf("%w: %v", errRegistryAuthentication, err)
+		}
+		return err
 	}
 	scanner := bufio.NewScanner(response.Body)
 	scanner.Buffer(make([]byte, 64<<10), 1<<20)
 	for scanner.Scan() {
 		var event struct {
-			Error string `json:"error"`
+			Error       string `json:"error"`
+			ErrorDetail struct {
+				Code    int    `json:"code"`
+				Message string `json:"message"`
+			} `json:"errorDetail"`
 		}
-		if err := json.Unmarshal(scanner.Bytes(), &event); err == nil && event.Error != "" {
-			return errors.New(event.Error)
+		if err := json.Unmarshal(scanner.Bytes(), &event); err != nil {
+			return errors.New("Docker returned invalid pull progress")
+		}
+		message := event.Error
+		if message == "" {
+			message = event.ErrorDetail.Message
+		}
+		if message != "" {
+			if event.ErrorDetail.Code == http.StatusUnauthorized || event.ErrorDetail.Code == http.StatusForbidden || registryAuthFailure(message) {
+				return fmt.Errorf("%w: %s", errRegistryAuthentication, message)
+			}
+			return errors.New(message)
 		}
 	}
 	return scanner.Err()
 }
 
+func registryAuthFailure(message string) bool {
+	message = strings.ToLower(message)
+	for _, marker := range []string{"unauthorized", "authentication required", "access denied", "access to the resource is denied", "pull access denied", "no basic auth credentials"} {
+		if strings.Contains(message, marker) {
+			return true
+		}
+	}
+	return false
+}
+
 func (c *dockerClient) inspectContainer(ctx context.Context, id string) (containerInspect, error) {
 	response, err := c.request(ctx, http.MethodGet, "/containers/"+url.PathEscape(id)+"/json", nil, nil, true)
 	if err != nil {
@@ -805,9 +852,9 @@ func (c *dockerClient) removeContainer(ctx context.Context, id string, force boo
 }
 
 func (c *dockerClient) waitHealthy(ctx context.Context, id, expectedImageID, expectedVersion string, timeout time.Duration) error {
-	deadline := time.NewTimer(timeout)
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
 	ticker := time.NewTicker(time.Second)
-	defer deadline.Stop()
 	defer ticker.Stop()
 	consecutiveHealthy := 0
 	startedAt := time.Now()
@@ -815,8 +862,6 @@ func (c *dockerClient) waitHealthy(ctx context.Context, id, expectedImageID, exp
 		select {
 		case <-ctx.Done():
 			return ctx.Err()
-		case <-deadline.C:
-			return errors.New("replacement container did not become healthy in time")
 		case <-ticker.C:
 			inspected, err := c.inspectContainer(ctx, id)
 			if err != nil {
@@ -859,16 +904,14 @@ func (c *dockerClient) waitHealthy(ctx context.Context, id, expectedImageID, exp
 }
 
 func (c *dockerClient) waitCurrentHealthy(ctx context.Context, id, expectedImageID string, timeout, pollInterval time.Duration) (containerInspect, error) {
-	deadline := time.NewTimer(timeout)
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
 	ticker := time.NewTicker(pollInterval)
-	defer deadline.Stop()
 	defer ticker.Stop()
 	for {
 		select {
 		case <-ctx.Done():
 			return containerInspect{}, ctx.Err()
-		case <-deadline.C:
-			return containerInspect{}, errors.New("current container did not become healthy in time")
 		case <-ticker.C:
 			inspected, err := c.inspectContainer(ctx, id)
 			if err != nil {

internal/updateagent/docker_test.go

@@ -2,9 +2,11 @@ package updateagent
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"io"
+	"log/slog"
 	"net/http"
 	"strconv"
 	"strings"
@@ -13,8 +15,85 @@ import (
 	"time"
 
 	"github.com/root-kdw12/hetzner-api-dyndns-records-webinterface/internal/buildinfo"
+	"github.com/root-kdw12/hetzner-api-dyndns-records-webinterface/internal/updateprotocol"
 )
 
+func TestLostStopResponseRestartsPreviousContainer(t *testing.T) {
+	current := compatibleImage("v1.4.0", "v1.4.0")
+	current.ID = "sha256:current"
+	target := compatibleImage("v1.5.0", "v1.4.0")
+	var config map[string]json.RawMessage
+	if err := json.Unmarshal(current.Config, &config); err != nil {
+		t.Fatal(err)
+	}
+	config["Image"] = json.RawMessage(`"registry.example/app:v1.4.0"`)
+	containerConfig, _ := json.Marshal(config)
+	stopped, restarted := false, false
+	transport := roundTripFunc(func(r *http.Request) (*http.Response, error) {
+		var body any
+		status := http.StatusOK
+		switch strings.TrimPrefix(r.URL.Path, "/v1.0") {
+		case "/version":
+			body = map[string]string{"ApiVersion": "1.0"}
+		case "/containers/json":
+			body = []containerSummary{{ID: "old", State: "running"}}
+		case "/containers/old/json":
+			body = map[string]any{"Id": "old", "Name": "/app", "Image": current.ID, "Config": json.RawMessage(containerConfig), "HostConfig": map[string]any{}, "State": map[string]any{"Running": !stopped, "Health": map[string]string{"Status": "healthy"}}}
+		case "/images/sha256:current/json":
+			body = current
+		case "/images/registry.example/app:v1.5.0/json":
+			body = target
+		case "/images/create":
+			body = map[string]string{"status": "downloaded"}
+		case "/containers/old/stop":
+			stopped = true
+			return nil, io.ErrUnexpectedEOF
+		case "/containers/old/start":
+			stopped = false
+			restarted = true
+			status = http.StatusNoContent
+		default:
+			t.Errorf("unexpected Docker call: %s %s", r.Method, r.URL.Path)
+			status = http.StatusInternalServerError
+		}
+		encoded, err := json.Marshal(body)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return &http.Response{StatusCode: status, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(string(encoded)))}, nil
+	})
+	runner := &DockerRunner{docker: &dockerClient{client: &http.Client{Transport: transport}}, imageRepository: "registry.example/app", logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
+	err := runner.Update(context.Background(), updateprotocol.Request{Version: "v1.5.0", CurrentVersion: "v1.4.0"}, func(string, string) {})
+	if !errors.Is(err, io.ErrUnexpectedEOF) || !restarted || stopped {
+		t.Fatalf("lost stop response left service down: restarted=%v stopped=%v err=%v", restarted, stopped, err)
+	}
+}
+
+func TestHealthDeadlineCancelsStalledDockerRequest(t *testing.T) {
+	for _, replacement := range []bool{false, true} {
+		t.Run(strconv.FormatBool(replacement), func(t *testing.T) {
+			var requests atomic.Int32
+			client := &dockerClient{client: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
+				requests.Add(1)
+				<-r.Context().Done()
+				return nil, r.Context().Err()
+			})}}
+			// Parent outlives the health deadline; the request must use the latter.
+			ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+			defer cancel()
+			var err error
+			if replacement {
+				err = client.waitHealthy(ctx, "id", "", "", 1200*time.Millisecond)
+			} else {
+				_, err = client.waitCurrentHealthy(ctx, "id", "", 30*time.Millisecond, time.Millisecond)
+			}
+			if !errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil || requests.Load() != 1 {
+				t.Fatalf("health deadline did not cancel the Docker request: %v requests=%d", err, requests.Load())
+			}
+		})
+	}
+}
+
 type roundTripFunc func(*http.Request) (*http.Response, error)
 
 func (f roundTripFunc) RoundTrip(request *http.Request) (*http.Response, error) {
@@ -258,3 +337,57 @@ func dockerTestClient(health func() string) *dockerClient {
 	})
 	return &dockerClient{client: &http.Client{Transport: transport}, apiPrefix: "/v1.0"}
 }
+
+func TestImagePullUsesAnonymousThenOptionalCredentials(t *testing.T) {
+	t.Parallel()
+	for _, test := range []struct {
+		name, first string
+		status      int
+		credentials bool
+		wantCalls   int
+		wantErr     bool
+	}{
+		{"public", `{"status":"download complete"}`, 200, false, 1, false},
+		{"public with stale credential", `{"status":"download complete"}`, 200, true, 1, false},
+		{"private stream error", `{"error":"unauthorized: authentication required"}`, 200, true, 2, false},
+		{"private status", `{"message":"denied"}`, 401, true, 2, false},
+		{"private errorDetail", `{"errorDetail":{"code":403,"message":"denied"}}`, 200, true, 2, false},
+		{"private no access", `{"error":"unauthorized"}`, 200, false, 1, true},
+		{"network error", `{"error":"connection timed out"}`, 200, true, 1, true},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			calls := 0
+			client := &dockerClient{client: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
+				calls++
+				status, body := test.status, test.first
+				if calls == 1 {
+					if r.Header.Get("X-Registry-Auth") != "" {
+						t.Error("first pull sent credentials")
+					}
+				} else {
+					raw, err := base64.URLEncoding.DecodeString(r.Header.Get("X-Registry-Auth"))
+					if err != nil {
+						t.Fatal(err)
+					}
+					var auth map[string]string
+					if err = json.Unmarshal(raw, &auth); err != nil {
+						t.Fatal(err)
+					}
+					if auth["username"] != "user" || auth["password"] != "test-password" {
+						t.Error("missing private pull credentials")
+					}
+					status, body = 200, `{"status":"download complete"}`
+				}
+				return &http.Response{StatusCode: status, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(body))}, nil
+			})}}
+			username, password := "", ""
+			if test.credentials {
+				username, password = "user", "test-password"
+			}
+			err := client.pullImage(context.Background(), "registry.example/app", "v2.0.0", username, password)
+			if (err != nil) != test.wantErr || calls != test.wantCalls {
+				t.Fatalf("pull calls=%d err=%v", calls, err)
+			}
+		})
+	}
+}