diff --git a/docs/contracts/thothctl-pi.md b/docs/contracts/thothctl-pi.md index 4c4e09bf..9ff37c46 100644 --- a/docs/contracts/thothctl-pi.md +++ b/docs/contracts/thothctl-pi.md @@ -78,10 +78,13 @@ maintenance gate before it checks sessions. Without `--drain`, active open sessi command. With `--drain`, the command polls the authenticated session inventory until no active sessions remain; it never terminates sessions and the wait is bounded. -Restart retains the exact current image and does not build, pull, select, tag, or upgrade an image. -It recreates only `core` with `--no-deps --force-recreate`; `frontend` and named volumes are not +Restart retains the exact current image and never builds, pulls, or upgrades an image. Before any +core mutation, it tags the captured running image ID with a transaction-scoped reference and +selects that reference through a lifecycle-only Compose override. A configured mutable tag moving +after capture therefore cannot change the restarted image. It recreates only `core` with +`--no-deps --force-recreate --no-build --pull never`; `frontend` and named volumes are not recreated. Before reopening admission, it verifies health, the unchanged Pi version, the -provider/model/settings smoke, unchanged non-secret rendered configuration, the current image +provider/model/settings smoke, unchanged non-secret rendered configuration, the captured image identity, and the complete persistence-mount fingerprint. Restart and update keep separate recovery state: @@ -91,10 +94,13 @@ Restart and update keep separate recovery state: /.thothctl//update-state.json ``` -The files are mode `0600` and share one installation lifecycle lock, so a restart cannot race an -update. A restart refuses an incomplete update or restart state. After core mutation, a failure -leaves admission gated and preserves `restart-state.json`; the operator must use status/logs and -maintenance recovery rather than deleting state files. +The files are mode `0600` and share one installation lifecycle lock, so restart, update, and +rollback cannot race. Every mutating lifecycle command checks both files. Malformed or non-terminal +restart recovery state blocks update and rollback; malformed or incomplete update recovery state +blocks restart. A verified terminal restart state is cleaned up safely before a later mutation. +After core mutation, a restart failure leaves admission gated and preserves both +`restart-state.json` and its exact-image override; the operator must use status/logs and maintenance +recovery rather than deleting recovery material. ## Updating Pi @@ -139,7 +145,7 @@ files and never deletes the durable selector. Only `core` is recreated, with `--no-deps --force-recreate`; `frontend` is not recreated and no volume-replacement flags are used. Verification checks health; exact requested Pi version at all three declared boundaries (the candidate executable, `PI_VERSION` environment, and -`org.opencontainers.image.version` image label); the provider/model/settings smoke; unchanged +`io.thothii.pi.version` image label); the provider/model/settings smoke; unchanged non-secret rendered configuration; and the complete persistence-mount fingerprint. ## Recovery, rollback, and maintenance cleanup @@ -175,8 +181,9 @@ For a failed update with `update-state.json`, first run: thothctl --installation /absolute/path/thothii-installation.yaml pi rollback --yes ``` -Rollback operates on update state only. A failed restart with `restart-state.json` retains its -current image and has no candidate image to roll back; first inspect status and logs, repair the +Rollback restores the image recorded in update state, but it checks restart state before making any +change. A failed, pending, or malformed restart state rejects rollback. A failed restart retains its +captured image and has no candidate image to roll back; first inspect status and logs, repair the reported problem, then use maintenance recovery. Inspect and clean a stale durable gate with: @@ -186,14 +193,15 @@ thothctl --installation /absolute/path/thothii-installation.yaml pi maintenance thothctl --installation /absolute/path/thothii-installation.yaml pi maintenance recover --yes ``` -`maintenance recover` verifies and removes an interrupted restart state before it processes update -state. It completes an interrupted verified-image promotion, safely finalizes a preparation -interrupted before core mutation, and refuses other pending mutations. For terminal or absent -recovery state, it removes only a stale transaction override, verifies the running installation -when the gate is active, and only then removes the durable marker and reopens admission. It never -removes `current-image.yaml`. If rollback or recovery fails, leave the marker in place, preserve -the relevant recovery state, repair the reported Docker/configuration issue, and rerun rollback or -maintenance recovery. +`maintenance recover` restores the captured restart image pin and lifecycle override when needed, +verifies and removes interrupted restart recovery material, and only then processes update state. +It completes an interrupted verified-image promotion, safely finalizes a preparation interrupted +before core mutation, and refuses other pending mutations. For terminal or absent recovery state, +it removes only a stale transaction override, verifies the running installation when the gate is +active, and only then removes the durable marker and reopens admission. It never removes +`current-image.yaml`. If rollback or recovery fails, leave the marker in place, preserve the +relevant recovery state and override, repair the reported Docker/configuration issue, and rerun +rollback or maintenance recovery. Missing confirmation, invalid arguments, active sessions, and an interrupted transaction exit `2`. Docker and verification failures exit nonzero with concise, redacted guidance. Direct diff --git a/docs/install/pi-management.md b/docs/install/pi-management.md index b86fa0d0..e604bc36 100644 --- a/docs/install/pi-management.md +++ b/docs/install/pi-management.md @@ -77,9 +77,12 @@ running application with one confirmed restart: `--yes` confirms that core will be recreated. Without `--drain`, restart refuses active sessions; with it, ThothII closes admission and waits for active sessions to finish without terminating them. -The wait is bounded. Restart retains the current image: it does not build, pull, select, or upgrade -an image. It will restart only core, then verifies health, Pi version, settings/model smoke, -non-secret rendered configuration, and persistence mounts before reopening admission. +The wait is bounded. Restart retains the exact captured running image: it does not build, pull, or +upgrade an image. Before recreating core, it pins that image through transaction-scoped Compose +override material so a configured tag moving during the operation cannot change the selected +image, and Compose is explicitly told never to build or pull. It will restart only core, then +verifies health, Pi version, settings/model smoke, non-secret rendered configuration, and +persistence mounts before reopening admission. Use `pi restart --yes` when there are already no active sessions. Do not substitute `thothctl stop` and `thothctl start` or raw Compose commands for this reload workflow. @@ -110,8 +113,8 @@ volumes. ## Recover a failed lifecycle operation If a restart or update fails after core recreation, leave maintenance enabled and preserve the -reported recovery state. Do not delete `.thothctl`, state files, containers, or volumes. Inspect -status and sanitized logs: +reported recovery state and transaction override. Do not delete `.thothctl`, state files, +containers, or volumes. Inspect status and sanitized logs: ```sh "$THTCTL" --installation "$INSTALLATION" pi maintenance status diff --git a/frontend/src/shell/PiManagement.test.tsx b/frontend/src/shell/PiManagement.test.tsx index 8d053348..f633df11 100644 --- a/frontend/src/shell/PiManagement.test.tsx +++ b/frontend/src/shell/PiManagement.test.tsx @@ -252,6 +252,9 @@ test("shows a seven-step host-terminal workflow in scrollable platform tabs", as expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi restart --yes --drain"); expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source build --yes --drain"); expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source pull --image @sha256: --yes --drain"); + expect(linux).toHaveTextContent(" is a placeholder. Replace it with the Pi release/version you want to install."); + expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance status"); + expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi logs"); expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi rollback --yes"); expect(linux).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance recover --yes"); expect(linux).not.toHaveTextContent("~/.pi/agent/"); @@ -274,6 +277,9 @@ test("shows a seven-step host-terminal workflow in scrollable platform tabs", as expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi restart --yes --drain"); expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source build --yes --drain"); expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source pull --image @sha256: --yes --drain"); + expect(macos).toHaveTextContent(" is a placeholder. Replace it with the Pi release/version you want to install."); + expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance status"); + expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi logs"); expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi rollback --yes"); expect(macos).toHaveTextContent("~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance recover --yes"); @@ -295,6 +301,9 @@ test("shows a seven-step host-terminal workflow in scrollable platform tabs", as expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi restart --yes --drain'); expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi update --version --source build --yes --drain'); expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi update --version --source pull --image @sha256: --yes --drain'); + expect(windows).toHaveTextContent(" is a placeholder. Replace it with the Pi release/version you want to install."); + expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi maintenance status'); + expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi logs'); expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi rollback --yes'); expect(windows).toHaveTextContent('& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi maintenance recover --yes'); }); diff --git a/frontend/src/shell/PiManagement.tsx b/frontend/src/shell/PiManagement.tsx index ebe0e713..98226c4e 100644 --- a/frontend/src/shell/PiManagement.tsx +++ b/frontend/src/shell/PiManagement.tsx @@ -126,7 +126,7 @@ const piPlatforms: Array<{ id: PiPlatform; label: string; details: PiPlatformDet restartCommand: "~/bin/thothctl --installation ~/thothii-installation.yaml pi restart --yes --drain", updateCommand: "~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source build --yes --drain", pullCommand: "~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source pull --image @sha256: --yes --drain", - recoveryCommands: "~/bin/thothctl --installation ~/thothii-installation.yaml pi rollback --yes\n~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance recover --yes", + recoveryCommands: "~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance status\n~/bin/thothctl --installation ~/thothii-installation.yaml pi logs\n~/bin/thothctl --installation ~/thothii-installation.yaml pi rollback --yes\n~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance recover --yes", }, }, { @@ -140,7 +140,7 @@ const piPlatforms: Array<{ id: PiPlatform; label: string; details: PiPlatformDet restartCommand: "~/bin/thothctl --installation ~/thothii-installation.yaml pi restart --yes --drain", updateCommand: "~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source build --yes --drain", pullCommand: "~/bin/thothctl --installation ~/thothii-installation.yaml pi update --version --source pull --image @sha256: --yes --drain", - recoveryCommands: "~/bin/thothctl --installation ~/thothii-installation.yaml pi rollback --yes\n~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance recover --yes", + recoveryCommands: "~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance status\n~/bin/thothctl --installation ~/thothii-installation.yaml pi logs\n~/bin/thothctl --installation ~/thothii-installation.yaml pi rollback --yes\n~/bin/thothctl --installation ~/thothii-installation.yaml pi maintenance recover --yes", }, }, { @@ -154,7 +154,7 @@ const piPlatforms: Array<{ id: PiPlatform; label: string; details: PiPlatformDet restartCommand: '& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi restart --yes --drain', updateCommand: '& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi update --version --source build --yes --drain', pullCommand: '& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi update --version --source pull --image @sha256: --yes --drain', - recoveryCommands: '& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi rollback --yes\n& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi maintenance recover --yes', + recoveryCommands: '& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi maintenance status\n& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi logs\n& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi rollback --yes\n& (Resolve-Path "~\\bin\\thothctl-windows-amd64.exe") --installation (Resolve-Path "~\\thothii-installation.yaml") pi maintenance recover --yes', }, }, ]; @@ -199,13 +199,14 @@ function PiInstructionSteps({ details }: { details: PiPlatformDetails }) {

  • Update the Pi version

    Use a build update only when changing the bundled Pi version.

    +

    {" is a placeholder. Replace it with the Pi release/version you want to install."}

    {details.updateCommand}

    Advanced: pull an immutable, digest-pinned image.

    {details.pullCommand}

  • Recover a failed update

    -

    For a failed update, use rollback. For restart or update maintenance recovery after repairing the reported problem, use maintenance recovery.

    +

    Check maintenance status and bounded sanitized Pi logs first. For a failed update, use rollback. For restart or update maintenance recovery after repairing the reported problem, use maintenance recovery.

    {details.recoveryCommands}
    • ; diff --git a/scripts/verify-workspace-install-docs.sh b/scripts/verify-workspace-install-docs.sh index a44aaf43..651b326d 100755 --- a/scripts/verify-workspace-install-docs.sh +++ b/scripts/verify-workspace-install-docs.sh @@ -1131,10 +1131,21 @@ NODE verify_pi_management_guide() { local guide="$root/docs/install/pi-management.md" + local lifecycle_contract="$root/docs/contracts/thothctl-pi.md" [[ -f "$guide" ]] || { echo "missing Pi management guide: docs/install/pi-management.md" >&2 return 1 } + [[ -f "$lifecycle_contract" ]] || { + echo "missing Pi lifecycle contract: docs/contracts/thothctl-pi.md" >&2 + return 1 + } + require_text "$lifecycle_contract" "Pi lifecycle contract" \ + "io.thothii.pi.version" + if grep -Fq 'org.opencontainers.image.version' "$lifecycle_contract"; then + echo "Pi lifecycle contract must use io.thothii.pi.version, not org.opencontainers.image.version" >&2 + return 1 + fi require_headings "$guide" "Pi management guide" \ "Choose application defaults" \ "Edit the provider catalog and enabled-model policy" \ diff --git a/tools/thothctl/cmd/thothctl/main.go b/tools/thothctl/cmd/thothctl/main.go index 3cb32bea..362b07db 100644 --- a/tools/thothctl/cmd/thothctl/main.go +++ b/tools/thothctl/cmd/thothctl/main.go @@ -333,7 +333,11 @@ func piCommand(ctx context.Context, installation config.Installation, runner com fmt.Fprintf(stdout, "Pi core restarted with the existing image; version %s readiness and smoke checks passed.\n", output.Sanitize(result.Version, secretValues)) return 0 case "update": - request, err := parsePiUpdateArgs(args[1:], installation.UpdateStatePath()) + request, err := parsePiUpdateArgs( + args[1:], + installation.UpdateStatePath(), + installation.RestartStatePath(), + ) if err != nil { return commandUsageError(stderr, err.Error()) } @@ -351,7 +355,13 @@ func piCommand(ctx context.Context, installation config.Installation, runner com if len(args) != 2 || args[1] != "--yes" { return commandUsageError(stderr, "pi rollback requires --yes") } - result, err := pi.Rollback(ctx, controlled, installation.UpdateStatePath(), true) + result, err := pi.Rollback( + ctx, + controlled, + installation.UpdateStatePath(), + installation.RestartStatePath(), + true, + ) if err != nil { return piFailure(stderr, err, secretValues) } @@ -489,8 +499,8 @@ func parsePiConfigureArgs(args []string) (pi.Defaults, error) { return value, nil } -func parsePiUpdateArgs(args []string, statePath string) (pi.Request, error) { - request := pi.Request{StatePath: statePath} +func parsePiUpdateArgs(args []string, statePath, restartStatePath string) (pi.Request, error) { + request := pi.Request{StatePath: statePath, RestartStatePath: restartStatePath} for len(args) > 0 { switch args[0] { case "--version": @@ -566,7 +576,7 @@ func parsePiRestartArgs(args []string, restartStatePath, updateStatePath string) func piFailure(stderr io.Writer, err error, secretValues []string) int { code := 1 - if errors.Is(err, pi.ErrConfirmationRequired) || errors.Is(err, pi.ErrInvalidRequest) || errors.Is(err, pi.ErrActiveSessions) || errors.Is(err, pi.ErrInterruptedUpdate) { + if errors.Is(err, pi.ErrConfirmationRequired) || errors.Is(err, pi.ErrInvalidRequest) || errors.Is(err, pi.ErrActiveSessions) || errors.Is(err, pi.ErrInterruptedUpdate) || errors.Is(err, pi.ErrInterruptedRestart) { code = 2 } var childExit interface{ ExitCode() int } diff --git a/tools/thothctl/cmd/thothctl/main_test.go b/tools/thothctl/cmd/thothctl/main_test.go index 2be78950..736f1fe5 100644 --- a/tools/thothctl/cmd/thothctl/main_test.go +++ b/tools/thothctl/cmd/thothctl/main_test.go @@ -274,6 +274,59 @@ func TestRunLogsRedactsAnUnlabelledDeclaredSecret(t *testing.T) { } } +func TestRunPiLogsRedactsNestedAuthAndBundleScalars(t *testing.T) { + fixture := newCLIFixture(t, "") + authFile := filepath.Join(fixture.root, "pi-auth.json") + bundleFile := filepath.Join(fixture.root, "thothii.secrets") + if err := os.WriteFile(authFile, []byte(`{ + "providers": { + "dummy-provider": { + "auth": { + "key": "dummy-canary-pi-log-json", + "nested": {"access": "dummy-canary-pi-log-nested"} + } + } + } +}`), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bundleFile, []byte( + "MODEL_API_KEY=dummy-canary-pi-log-model\n"+ + "DWH_PASSWORD=dummy-canary-pi-log-dwh\n", + ), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvContents(t, "PI_AUTH_FILE="+authFile+"\nTHT_SECRETS_FILE="+bundleFile+"\n") + t.Setenv( + "THOTHCTL_FAKE_LOG", + "dummy-canary-pi-log-json dummy-canary-pi-log-nested "+ + "dummy-canary-pi-log-model dummy-canary-pi-log-dwh", + ) + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{ + "--installation", fixture.installationPath, "pi", "logs", + }, &stdout, &stderr) + + if exitCode != 0 { + t.Fatalf("run() exit code = %d, stderr = %q", exitCode, stderr.String()) + } + for _, canary := range []string{ + "dummy-canary-pi-log-json", + "dummy-canary-pi-log-nested", + "dummy-canary-pi-log-model", + "dummy-canary-pi-log-dwh", + } { + if strings.Contains(stdout.String()+stderr.String(), canary) { + t.Fatalf("pi logs exposed scalar %q: stdout=%q stderr=%q", canary, stdout.String(), stderr.String()) + } + } + if strings.Count(stdout.String(), "[REDACTED]") != 4 { + t.Fatalf("pi logs = %q, want four scalar redactions", stdout.String()) + } + assertInvocationContains(t, fixture.invocations(t), "logs", "--tail", "200", "core") +} + func TestRunResolvesComposeDotenvCommentsQuotesAndInterpolationForSecretFiles(t *testing.T) { fixture := newCLIFixture(t, "") secretDirectory := filepath.Join(fixture.root, "secret directory") diff --git a/tools/thothctl/internal/output/sanitize.go b/tools/thothctl/internal/output/sanitize.go index 9c360fa0..24ae6547 100644 --- a/tools/thothctl/internal/output/sanitize.go +++ b/tools/thothctl/internal/output/sanitize.go @@ -2,15 +2,21 @@ package output import ( + "bytes" + "encoding/json" "errors" + "io" "regexp" "sort" "strings" "github.com/aritmolab/thothii/tools/thothctl/internal/safeio" + "github.com/compose-spec/compose-go/v2/dotenv" ) -var credentialField = regexp.MustCompile(`(?im)(\b[\w.-]*(?:password|token|key)[\w.-]*\s*[:=]\s*)(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s,;]+)`) +var credentialField = regexp.MustCompile(`(?im)((?:"|')?[\w.-]*(?:password|token|key|secret|credential)[\w.-]*(?:"|')?\s*[:=]\s*)(?:"(?:\\.|[^"\\\r\n])*"|'[^'\r\n]*'|[^\s,;}]+)`) + +var dotenvAssignment = regexp.MustCompile(`(?m)^\s*(?:export\s+)?[A-Za-z_][A-Za-z0-9_.-]*\s*=`) const maxSecretFileBytes = 64 * 1024 @@ -18,6 +24,12 @@ const maxSecretSourceFiles = 32 const maxSecretSourceBytes = 256 * 1024 +const maxSecretValuesPerFile = 1024 + +const maxSecretValues = 4096 + +const maxJSONSecretDepth = 32 + const maxDiagnosticDetailBytes = 512 // Sanitize redacts common credential fields and every supplied secret value. @@ -28,6 +40,9 @@ func Sanitize(text string, secretValues []string) string { for _, value := range values { if value != "" { text = strings.ReplaceAll(text, value, "[REDACTED]") + if encoded, err := json.Marshal(value); err == nil { + text = strings.ReplaceAll(text, string(encoded), "[REDACTED]") + } } } return text @@ -60,7 +75,7 @@ func SecretValuesFromFiles(paths []string) ([]string, error) { seen := make(map[string]struct{}) var totalBytes int64 for _, path := range paths { - value, size, err := readSecretFile(path) + contents, size, err := readSecretFile(path) if err != nil { return nil, err } @@ -68,21 +83,116 @@ func SecretValuesFromFiles(paths []string) ([]string, error) { if totalBytes > maxSecretSourceBytes { return nil, errors.New("declared secret file could not be read") } - if value != "" { + extracted, err := extractSecretValues(contents) + if err != nil { + return nil, errors.New("declared secret file could not be read") + } + for _, value := range extracted { + if value == "" { + continue + } if _, exists := seen[value]; exists { continue } values = append(values, value) seen[value] = struct{}{} + if len(values) > maxSecretValues { + return nil, errors.New("declared secret file could not be read") + } } } return values, nil } -func readSecretFile(path string) (string, int64, error) { +func readSecretFile(path string) ([]byte, int64, error) { contents, err := safeio.ReadCanonicalRegular(path, maxSecretFileBytes) if err != nil { - return "", 0, errors.New("declared secret file could not be read") + return nil, 0, errors.New("declared secret file could not be read") } - return strings.TrimRight(string(contents), "\r\n"), int64(len(contents)), nil + return contents, int64(len(contents)), nil +} + +func extractSecretValues(contents []byte) ([]string, error) { + whole := strings.TrimRight(string(contents), "\r\n") + trimmed := bytes.TrimSpace(contents) + if len(trimmed) == 0 { + return nil, nil + } + values := make([]string, 0, 8) + if whole != "" { + values = append(values, whole) + } + + if trimmed[0] == '{' || trimmed[0] == '[' { + var document any + decoder := json.NewDecoder(bytes.NewReader(trimmed)) + decoder.UseNumber() + if err := decoder.Decode(&document); err != nil { + return nil, err + } + var extra any + if err := decoder.Decode(&extra); !errors.Is(err, io.EOF) { + if err == nil { + return nil, errors.New("secret JSON contains multiple documents") + } + return nil, err + } + count := 0 + if err := collectJSONSecretValues(document, 0, &count, &values); err != nil { + return nil, err + } + return values, nil + } + + if dotenvAssignment.Match(trimmed) { + parsed, err := dotenv.Parse(bytes.NewReader(contents)) + if err != nil || len(parsed) > maxSecretValuesPerFile { + return nil, errors.New("secret dotenv bundle is invalid") + } + keys := make([]string, 0, len(parsed)) + for key := range parsed { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if parsed[key] != "" { + values = append(values, parsed[key]) + } + } + } + return values, nil +} + +func collectJSONSecretValues(value any, depth int, count *int, values *[]string) error { + if depth > maxJSONSecretDepth { + return errors.New("secret JSON nesting is too deep") + } + switch typed := value.(type) { + case map[string]any: + keys := make([]string, 0, len(typed)) + for key := range typed { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + if err := collectJSONSecretValues(typed[key], depth+1, count, values); err != nil { + return err + } + } + case []any: + for _, item := range typed { + if err := collectJSONSecretValues(item, depth+1, count, values); err != nil { + return err + } + } + default: + *count++ + if *count > maxSecretValuesPerFile { + return errors.New("secret JSON contains too many scalar values") + } + if scalar, ok := typed.(string); ok && scalar != "" { + *values = append(*values, scalar) + } + } + return nil } diff --git a/tools/thothctl/internal/output/sanitize_test.go b/tools/thothctl/internal/output/sanitize_test.go index b16ec8a2..2e227fa0 100644 --- a/tools/thothctl/internal/output/sanitize_test.go +++ b/tools/thothctl/internal/output/sanitize_test.go @@ -3,6 +3,7 @@ package output import ( "os" "path/filepath" + "strings" "testing" ) @@ -34,6 +35,113 @@ func TestSanitizeRedactsSecretFileContents(t *testing.T) { } } +func TestSecretValuesFromFilesRedactsNestedJSONScalars(t *testing.T) { + t.Parallel() + + secretFile := filepath.Join(physicalTempDir(t), "pi-auth.json") + contents := `{ + "providers": { + "dummy-provider": { + "auth": { + "key": "dummy-canary-json-key", + "tokens": { + "access": "dummy-canary-json-access", + "refresh": "dummy-canary-json-refresh" + } + } + } + } +}` + if err := os.WriteFile(secretFile, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + + secrets, err := SecretValuesFromFiles([]string{secretFile}) + if err != nil { + t.Fatalf("SecretValuesFromFiles() error = %v", err) + } + got := Sanitize( + "unlabelled dummy-canary-json-key dummy-canary-json-access dummy-canary-json-refresh", + secrets, + ) + for _, canary := range []string{ + "dummy-canary-json-key", + "dummy-canary-json-access", + "dummy-canary-json-refresh", + } { + if strings.Contains(got, canary) { + t.Fatalf("Sanitize() exposed nested JSON scalar %q: %q", canary, got) + } + } +} + +func TestSecretValuesFromFilesRedactsEveryDotenvBundleValue(t *testing.T) { + t.Parallel() + + secretFile := filepath.Join(physicalTempDir(t), "thothii.secrets") + contents := "MODEL_API_KEY=dummy-canary-bundle-model\n" + + "DWH_PASSWORD='dummy-canary-bundle-dwh'\n" + + "SESSION_TOKEN=\"dummy-canary-bundle-session\"\n" + if err := os.WriteFile(secretFile, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + + secrets, err := SecretValuesFromFiles([]string{secretFile}) + if err != nil { + t.Fatalf("SecretValuesFromFiles() error = %v", err) + } + got := Sanitize( + "unlabelled dummy-canary-bundle-model dummy-canary-bundle-dwh dummy-canary-bundle-session", + secrets, + ) + for _, canary := range []string{ + "dummy-canary-bundle-model", + "dummy-canary-bundle-dwh", + "dummy-canary-bundle-session", + } { + if strings.Contains(got, canary) { + t.Fatalf("Sanitize() exposed dotenv bundle scalar %q: %q", canary, got) + } + } +} + +func TestSanitizeRecognizesQuotedCredentialKeys(t *testing.T) { + t.Parallel() + + got := Sanitize(`{"key":"dummy-canary-quoted-key","safe":"visible"}`, nil) + if strings.Contains(got, "dummy-canary-quoted-key") || !strings.Contains(got, `"safe":"visible"`) { + t.Fatalf("Sanitize() = %q, want only the quoted credential field redacted", got) + } +} + +func TestSecretValuesFromFilesRejectsMalformedJSONAndExcessiveScalars(t *testing.T) { + t.Parallel() + + t.Run("malformed", func(t *testing.T) { + secretFile := filepath.Join(physicalTempDir(t), "malformed-auth.json") + if err := os.WriteFile(secretFile, []byte(`{"auth":{"key":"dummy-canary-malformed"}`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := SecretValuesFromFiles([]string{secretFile}); err == nil { + t.Fatal("SecretValuesFromFiles() error = nil, want malformed-JSON failure") + } + }) + + t.Run("scalar bound", func(t *testing.T) { + secretFile := filepath.Join(physicalTempDir(t), "many-auth-values.json") + values := make([]string, 1025) + for index := range values { + values[index] = `"dummy-canary-value"` + } + if err := os.WriteFile(secretFile, []byte("["+strings.Join(values, ",")+"]"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := SecretValuesFromFiles([]string{secretFile}); err == nil { + t.Fatal("SecretValuesFromFiles() error = nil, want scalar-count failure") + } + }) +} + func TestSecretValuesFromFilesRejectsOversizedFiles(t *testing.T) { t.Parallel() diff --git a/tools/thothctl/internal/pi/restart.go b/tools/thothctl/internal/pi/restart.go index 28c2f052..be15cf6a 100644 --- a/tools/thothctl/internal/pi/restart.go +++ b/tools/thothctl/internal/pi/restart.go @@ -9,7 +9,7 @@ import ( ) var ( - errInterruptedRestart = errors.New("a previous Pi restart is incomplete; recover lifecycle maintenance before restarting again") + ErrInterruptedRestart = errors.New("a previous Pi restart is incomplete; recover lifecycle maintenance before another Pi lifecycle operation") errRestartImageDrift = errors.New("core image changed during Pi restart") errRestartConfigurationDrift = errors.New("external endpoint configuration changed during Pi restart") errRestartMountDrift = errors.New("core persistence mount contract changed during Pi restart") @@ -70,14 +70,7 @@ func restartWithHooks( } else if err != nil && !errors.Is(err, os.ErrNotExist) { return RestartResult{StatePath: request.StatePath}, err } - if state, err := readState(request.StatePath); err == nil { - if err := validateRestartRecoveryState(state); err != nil { - return RestartResult{StatePath: request.StatePath}, err - } - if state.MutationStarted { - return RestartResult{StatePath: request.StatePath}, errInterruptedRestart - } - } else if err != nil && !errors.Is(err, os.ErrNotExist) { + if err := prepareLifecycleMutation(request.StatePath, hooks.removeFile); err != nil { return RestartResult{StatePath: request.StatePath}, err } clearMaintenance := true @@ -124,8 +117,13 @@ func restartWithHooks( return RestartResult{StatePath: request.StatePath, Version: version}, err } previous.ConfigurationSHA = configured.ConfigurationSHA + transaction := lifecycleTransaction(request.StatePath) + previous.Reference = lifecycleImageTag(transaction, "restart") + if err := tagImage(ctx, runner, previous.ID, previous.Reference, "restart image pin"); err != nil { + return RestartResult{StatePath: request.StatePath, Version: version}, err + } state = State{ - Transaction: lifecycleTransaction(request.StatePath), + Transaction: transaction, Phase: PhasePreflight, Target: Target{Version: version, Source: "restart"}, Previous: previous, @@ -133,6 +131,11 @@ func restartWithHooks( if err := hooks.writeState(request.StatePath, state); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, err } + overridePath := lifecycleOverridePath(request.StatePath, transaction) + if err := writeLifecycleOverride(overridePath, previous.Reference); err != nil { + return RestartResult{StatePath: request.StatePath, Version: version}, err + } + lifecycle := composeOverrideRunner{Runner: runner, path: overridePath} if running, err := activeSessions(ctx, runner); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, err } else if running { @@ -144,23 +147,26 @@ func restartWithHooks( } mutationStarted = true clearMaintenance = false - if err := recreateCore(ctx, runner); err != nil { + if err := recreateCoreWithoutImageChanges(ctx, lifecycle); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart core recreation failed", err) } - if err := ensureMaintenance(ctx, runner); err != nil { + if err := ensureMaintenance(ctx, lifecycle); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart maintenance proof failed", err) } state.Phase = PhaseRecreated if err := hooks.writeState(request.StatePath, state); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart recreation state could not be recorded", err) } - if err := verifyRestart(ctx, runner, version, previous); err != nil { + if err := verifyRestart(ctx, lifecycle, version, previous); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart verification failed", err) } state.Phase = PhaseVerified if err := hooks.writeState(request.StatePath, state); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart verification state could not be recorded", err) } + if err := hooks.removeFile(overridePath); err != nil { + return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart image override could not be removed", err) + } if err := hooks.removeFile(request.StatePath); err != nil { return RestartResult{StatePath: request.StatePath, Version: version}, recoveryRequired("Pi restart recovery state could not be removed", err) } @@ -168,6 +174,28 @@ func restartWithHooks( return RestartResult{StatePath: request.StatePath, Version: version}, nil } +func recreateCoreWithoutImageChanges(ctx context.Context, runner Runner) error { + result, err := runCompose( + ctx, + runner, + "up", + "--detach", + "--wait", + "--wait-timeout", + "45", + "--no-deps", + "--force-recreate", + "--no-build", + "--pull", + "never", + "core", + ) + if err != nil { + return commandError("core recreation", result, err) + } + return nil +} + func verifyRestart(ctx context.Context, runner Runner, wanted string, previous Image) error { if err := Doctor(ctx, runner); err != nil { return err @@ -223,14 +251,25 @@ func RecoverLifecycleMaintenance( if err := validateRestartRecoveryState(restartState); err != nil { return err } + restartOverride := lifecycleOverridePath(restartStatePath, restartState.Transaction) if restartState.MutationStarted { - if err := ensureMaintenance(ctx, runner); err != nil { + if err := tagImage(ctx, runner, restartState.Previous.ID, restartState.Previous.Reference, "restart recovery image pin"); err != nil { + return recoveryRequired("Pi restart recovery image pin could not be restored", err) + } + if err := writeLifecycleOverride(restartOverride, restartState.Previous.Reference); err != nil { + return recoveryRequired("Pi restart recovery image override could not be restored", err) + } + lifecycle := composeOverrideRunner{Runner: runner, path: restartOverride} + if err := ensureMaintenance(ctx, lifecycle); err != nil { return recoveryRequired("Pi restart maintenance recovery failed", err) } - if err := verifyRestart(ctx, runner, restartState.Target.Version, restartState.Previous); err != nil { + if err := verifyRestart(ctx, lifecycle, restartState.Target.Version, restartState.Previous); err != nil { return recoveryRequired("Pi restart recovery verification failed", err) } } + if err := durableRemove(restartOverride); err != nil { + return recoveryRequired("Pi restart recovery image override could not be removed", err) + } if err := durableRemove(restartStatePath); err != nil { return recoveryRequired("Pi restart recovery state could not be removed", err) } @@ -292,3 +331,30 @@ func validateRestartStatePaths(restartStatePath, updateStatePath string) error { } return nil } + +func pairedRestartStatePath(updateStatePath string) string { + return filepath.Join(filepath.Dir(updateStatePath), "restart-state.json") +} + +func prepareLifecycleMutation(restartStatePath string, removeFile func(string) error) error { + state, err := readState(restartStatePath) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("restart recovery state could not be validated: %w", err) + } + if err := validateRestartRecoveryState(state); err != nil { + return err + } + if state.Phase != PhaseVerified || !state.MutationStarted { + return ErrInterruptedRestart + } + if err := removeFile(lifecycleOverridePath(restartStatePath, state.Transaction)); err != nil { + return recoveryRequired("verified restart override could not be cleaned up", err) + } + if err := removeFile(restartStatePath); err != nil { + return recoveryRequired("verified restart state could not be cleaned up", err) + } + return nil +} diff --git a/tools/thothctl/internal/pi/restart_test.go b/tools/thothctl/internal/pi/restart_test.go index 809100c9..2c5651b5 100644 --- a/tools/thothctl/internal/pi/restart_test.go +++ b/tools/thothctl/internal/pi/restart_test.go @@ -55,15 +55,51 @@ func TestRestartDrainsRecreatesOnlyCoreAndRetainsImage(t *testing.T) { if sleepCalls != 1 { t.Fatalf("drain sleep calls = %d, want 1", sleepCalls) } - assertCalled(t, fake.calls, "up --detach --wait --wait-timeout 45 --no-deps --force-recreate core") - assertNotCalled(t, fake.calls, "build --pull") - assertNotCalled(t, fake.calls, "pull ") + assertCalled(t, fake.calls, "up --detach --wait --wait-timeout 45 --no-deps --force-recreate --no-build --pull never core") + assertNotCalled(t, fake.calls, "compose build --pull") + for _, call := range fake.calls { + if strings.HasPrefix(call, "pull ") { + t.Fatalf("restart invoked direct image pull: %s", call) + } + } assertNotCalled(t, fake.calls, "frontend") if _, err := os.Stat(result.StatePath); !errors.Is(err, os.ErrNotExist) { t.Fatalf("successful restart state still exists: %v", err) } } +func TestRestartPinsCapturedImageWhenConfiguredTagMovesBeforeRecreate(t *testing.T) { + dir := t.TempDir() + fake := newFakeRunner() + hooks := defaultLifecycleHooks + write := hooks.writeState + hooks.writeState = func(path string, state State) error { + if state.Phase == PhasePreflight && state.MutationStarted { + fake.tags[fake.configuredImage] = "sha256:moved-configured-tag" + fake.imageVersions["sha256:moved-configured-tag"] = "9.99.0" + } + return write(path, state) + } + restartStatePath := filepath.Join(dir, "restart-state.json") + + result, err := restartWithHooks(context.Background(), fake, RestartRequest{ + StatePath: restartStatePath, + UpdateStatePath: filepath.Join(dir, "update-state.json"), + Confirm: true, + }, hooks) + if err != nil { + t.Fatalf("restartWithHooks() error = %v", err) + } + if result.Version != "0.80.3" || fake.currentImage != "sha256:old" { + t.Fatalf("restart result=%+v image=%q; want captured 0.80.3 / sha256:old", result, fake.currentImage) + } + assertCalled(t, fake.calls, "image tag sha256:old thothii-core:thothctl-") + assertCalled(t, fake.calls, "pi-lifecycle-") + if matches, globErr := filepath.Glob(filepath.Join(dir, "pi-lifecycle-*.yaml")); globErr != nil || len(matches) != 0 { + t.Fatalf("successful restart overrides = %v, error = %v; want safe cleanup", matches, globErr) + } +} + func TestRestartRefusesActiveSessionsWithoutDrain(t *testing.T) { dir := t.TempDir() fake := newFakeRunner() @@ -193,6 +229,11 @@ func TestRestartPostRecreateFailureKeepsMaintenanceAndRecoveryState(t *testing.T if stateErr != nil || !state.MutationStarted { t.Fatalf("restart recovery state = %+v, %v; want durable mutation state", state, stateErr) } + overridePath := lifecycleOverridePath(statePath, state.Transaction) + selected, overrideErr := readLifecycleOverride(overridePath) + if overrideErr != nil || selected != state.Previous.Reference || fake.tags[selected] != state.Previous.ID { + t.Fatalf("restart override = %q, %v; want retained exact image %q", selected, overrideErr, state.Previous.ID) + } } func TestRestartMaintenanceClearFailureRestoresRecoveryState(t *testing.T) { @@ -233,6 +274,16 @@ func TestRecoverLifecycleMaintenanceVerifiesAndClearsRestartState(t *testing.T) Previous: previous, MutationStarted: true, }) + restartState, err := readState(restartStatePath) + if err != nil { + t.Fatal(err) + } + restartOverride := lifecycleOverridePath(restartStatePath, restartState.Transaction) + if err := writeLifecycleOverride(restartOverride, restartState.Previous.Reference); err != nil { + t.Fatal(err) + } + delete(fake.tags, restartState.Previous.Reference) + fake.tags[fake.configuredImage] = "sha256:moved-before-recovery" candidate := previous candidate.Reference = "thothii-core:thothctl-recover-candidate" writeStateForTest(t, updateStatePath, State{ @@ -254,6 +305,13 @@ func TestRecoverLifecycleMaintenanceVerifiesAndClearsRestartState(t *testing.T) if _, err := os.Stat(restartStatePath); !errors.Is(err, os.ErrNotExist) { t.Fatalf("restart recovery state still exists: %v", err) } + if _, err := os.Stat(restartOverride); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("restart recovery override still exists: %v", err) + } + if fake.tags[restartState.Previous.Reference] != restartState.Previous.ID { + t.Fatalf("restart recovery pin = %q, want %q", fake.tags[restartState.Previous.Reference], restartState.Previous.ID) + } + assertCalled(t, fake.calls, restartOverride) if fake.maintenance { t.Fatal("maintenance remained active after both lifecycle states were verified") } diff --git a/tools/thothctl/internal/pi/update.go b/tools/thothctl/internal/pi/update.go index 8466d55e..edb33f04 100644 --- a/tools/thothctl/internal/pi/update.go +++ b/tools/thothctl/internal/pi/update.go @@ -37,12 +37,13 @@ const ( // Request contains only non-secret operator inputs. type Request struct { - StatePath string - Version string - Source Source - Image string - Confirm bool - Drain bool + StatePath string + RestartStatePath string + Version string + Source Source + Image string + Confirm bool + Drain bool } // Result summarizes the completed, failed, or recovered transaction without command output. @@ -72,6 +73,12 @@ func updateWithHooks(ctx context.Context, runner Runner, request Request, hooks if request.StatePath == "" { return Result{}, errors.New("update state path is required") } + if request.RestartStatePath == "" { + request.RestartStatePath = pairedRestartStatePath(request.StatePath) + } + if err := validateRestartStatePaths(request.RestartStatePath, request.StatePath); err != nil { + return Result{StatePath: request.StatePath}, err + } lock, err := acquireLock(request.StatePath) if err != nil { return Result{StatePath: request.StatePath}, err @@ -96,6 +103,9 @@ func updateWithHooks(ctx context.Context, runner Runner, request Request, hooks } request.Image = canonical } + if err := prepareLifecycleMutation(request.RestartStatePath, hooks.removeFile); err != nil { + return Result{StatePath: request.StatePath}, err + } if old, err := readState(request.StatePath); err == nil && stateNeedsRecovery(old) { return Result{StatePath: request.StatePath}, ErrInterruptedUpdate } else if err != nil && !errors.Is(err, os.ErrNotExist) { @@ -254,11 +264,17 @@ func failPreparation(statePath, overridePath string, state State, cause error, h } // Rollback restores the image recorded in durable update state. It is safe for interrupted runs. -func Rollback(ctx context.Context, runner Runner, statePath string, confirm bool) (result Result, retErr error) { - return rollbackWithHooks(ctx, runner, statePath, confirm, defaultLifecycleHooks) +func Rollback(ctx context.Context, runner Runner, statePath, restartStatePath string, confirm bool) (result Result, retErr error) { + return rollbackWithHooks(ctx, runner, statePath, restartStatePath, confirm, defaultLifecycleHooks) } -func rollbackWithHooks(ctx context.Context, runner Runner, statePath string, confirm bool, hooks lifecycleHooks) (result Result, retErr error) { +func rollbackWithHooks(ctx context.Context, runner Runner, statePath, restartStatePath string, confirm bool, hooks lifecycleHooks) (result Result, retErr error) { + if restartStatePath == "" { + restartStatePath = pairedRestartStatePath(statePath) + } + if err := validateRestartStatePaths(restartStatePath, statePath); err != nil { + return Result{StatePath: statePath}, err + } lock, err := acquireLock(statePath) if err != nil { return Result{StatePath: statePath}, err @@ -267,6 +283,9 @@ func rollbackWithHooks(ctx context.Context, runner Runner, statePath string, con if !confirm { return Result{StatePath: statePath}, ErrConfirmationRequired } + if err := prepareLifecycleMutation(restartStatePath, hooks.removeFile); err != nil { + return Result{StatePath: statePath}, err + } maintenanceErr := ensureMaintenance(ctx, runner) clearMaintenance := maintenanceErr == nil defer func() { diff --git a/tools/thothctl/internal/pi/update_test.go b/tools/thothctl/internal/pi/update_test.go index 389b459c..2923d275 100644 --- a/tools/thothctl/internal/pi/update_test.go +++ b/tools/thothctl/internal/pi/update_test.go @@ -114,7 +114,7 @@ func TestSuccessfulUpdateAndRollbackRemainSelectedOnFreshRecreate(t *testing.T) if fake.currentImage != "sha256:candidate" { t.Fatalf("fresh recreate image = %q, want verified candidate", fake.currentImage) } - if _, err := Rollback(context.Background(), fake, statePath, true); err != nil { + if _, err := Rollback(context.Background(), fake, statePath, pairedRestartStatePath(statePath), true); err != nil { t.Fatal(err) } fake.currentImage = "sha256:candidate" @@ -209,7 +209,7 @@ func TestDigestPinnedConfiguredImageIsNeverUsedAsARollbackTagTarget(t *testing.T if _, err := Update(context.Background(), fake, Request{StatePath: statePath, Version: "0.81.0", Source: BuildSource, Confirm: true}); err != nil { t.Fatal(err) } - if _, err := Rollback(context.Background(), fake, statePath, true); err != nil { + if _, err := Rollback(context.Background(), fake, statePath, pairedRestartStatePath(statePath), true); err != nil { t.Fatal(err) } assertNotCalled(t, fake.calls, "image tag sha256:old "+fake.configuredImage) @@ -306,7 +306,7 @@ func TestSuccessfulLifecycleCommandsPreserveTypedRecoveryErrorFromMaintenanceCle Confirm: true, }) } else { - result, err = Rollback(context.Background(), fake, statePath, true) + result, err = Rollback(context.Background(), fake, statePath, pairedRestartStatePath(statePath), true) } if !fake.maintenance { @@ -382,7 +382,7 @@ func TestManualRollbackSurvivesADeadCandidateCore(t *testing.T) { fake.coreRunning = false fake.maintenance = false - result, err := Rollback(context.Background(), fake, statePath, true) + result, err := Rollback(context.Background(), fake, statePath, pairedRestartStatePath(statePath), true) if err != nil || result.Phase != PhaseRolledBack { t.Fatalf("Rollback() = %+v, %v; want restored previous core", result, err) @@ -657,7 +657,7 @@ func TestRollbackFinalStateWriteFailureKeepsMaintenanceAndOverrideForRecovery(t hooks := defaultLifecycleHooks hooks.writeState = func(string, State) error { return errors.New("injected rollback state write failure") } - result, err := rollbackWithHooks(context.Background(), fake, statePath, true, hooks) + result, err := rollbackWithHooks(context.Background(), fake, statePath, pairedRestartStatePath(statePath), true, hooks) if err == nil || result.Phase != PhaseFailed { t.Fatalf("rollbackWithHooks() = %+v, %v; want failed durable finalization", result, err) } @@ -783,7 +783,7 @@ func TestRollbackRestoresInterruptedOrPreviouslyRecordedState(t *testing.T) { previous.Reference = "thothii-core:thothctl-test-previous" fake.tags[previous.Reference] = previous.ID writeStateForTest(t, statePath, State{Version: 1, Phase: PhaseRecreated, Previous: previous}) - result, err := Rollback(context.Background(), fake, statePath, true) + result, err := Rollback(context.Background(), fake, statePath, pairedRestartStatePath(statePath), true) if err != nil { t.Fatalf("Rollback() error = %v", err) } @@ -805,6 +805,100 @@ func TestUpdateRefusesToOverwriteInterruptedRecoveryState(t *testing.T) { assertNotCalled(t, fake.calls, "compose") } +func TestFailedRestartBlocksUpdateAndRollbackAndMalformedStateAlsoRejects(t *testing.T) { + for _, operation := range []string{"update", "rollback"} { + for _, restartState := range []string{"failed", "malformed"} { + t.Run(operation+"_"+restartState, func(t *testing.T) { + dir := t.TempDir() + fake := newFakeRunner() + updateStatePath := filepath.Join(dir, "update-state.json") + restartStatePath := filepath.Join(dir, "restart-state.json") + if restartState == "failed" { + fake.fail = "health" + _, restartErr := Restart(context.Background(), fake, RestartRequest{ + StatePath: restartStatePath, + UpdateStatePath: updateStatePath, + Confirm: true, + }) + var recovery *RecoveryRequiredError + if !errors.As(restartErr, &recovery) { + t.Fatalf("Restart() error = %v, want failed restart recovery state", restartErr) + } + fake.fail = "" + } else if err := os.WriteFile(restartStatePath, []byte("{malformed"), 0o600); err != nil { + t.Fatal(err) + } + if operation == "rollback" { + previous := stateImageForTest(t, fake) + writeStateForTest(t, updateStatePath, State{ + Transaction: "rollback-target", + Phase: PhaseRecreated, + Target: Target{Version: fake.version, Source: string(BuildSource)}, + Previous: previous, + MutationStarted: true, + }) + } + fake.calls = nil + + var err error + if operation == "update" { + _, err = Update(context.Background(), fake, Request{ + StatePath: updateStatePath, + RestartStatePath: restartStatePath, + Version: "0.81.0", + Source: BuildSource, + Confirm: true, + }) + } else { + _, err = Rollback(context.Background(), fake, updateStatePath, restartStatePath, true) + } + if err == nil { + t.Fatalf("%s accepted %s restart recovery state", operation, restartState) + } + if restartState == "failed" && !errors.Is(err, ErrInterruptedRestart) { + t.Fatalf("%s error = %v, want ErrInterruptedRestart", operation, err) + } + assertNotCalled(t, fake.calls, "compose") + }) + } + } +} + +func TestUpdateCleansVerifiedRestartStateBeforeNormalLifecycleWork(t *testing.T) { + dir := t.TempDir() + fake := newFakeRunner() + updateStatePath := filepath.Join(dir, "update-state.json") + restartStatePath := filepath.Join(dir, "restart-state.json") + state := State{ + Transaction: "verified-restart", + Phase: PhaseVerified, + Target: Target{Version: fake.version, Source: "restart"}, + Previous: stateImageForTest(t, fake), + MutationStarted: true, + } + writeStateForTest(t, restartStatePath, state) + overridePath := lifecycleOverridePath(restartStatePath, state.Transaction) + if err := writeLifecycleOverride(overridePath, state.Previous.Reference); err != nil { + t.Fatal(err) + } + + result, err := Update(context.Background(), fake, Request{ + StatePath: updateStatePath, + RestartStatePath: restartStatePath, + Version: fake.version, + Source: BuildSource, + Confirm: true, + }) + if err != nil || result.Phase != PhaseNoop { + t.Fatalf("Update() = %+v, %v; want normal no-op after verified restart", result, err) + } + for _, path := range []string{restartStatePath, overridePath} { + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("terminal restart artifact %s still exists: %v", path, err) + } + } +} + func TestRunningImageCapturesServerBindAndNamedMountIdentity(t *testing.T) { fake := newFakeRunner() fake.mountsJSON = `[{"Type":"bind","Source":"/srv/thothii/data","Destination":"/data","RW":true},{"Type":"bind","Source":"/srv/thothii/pi","Destination":"/home/thoth/.pi","RW":true},{"Type":"volume","Name":"sessions","Source":"/var/lib/docker/volumes/sessions/_data","Destination":"/data/sessions","RW":true}]`