fix: harden Pi lifecycle recovery
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
@@ -10,6 +11,7 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/aritmolab/thothii/tools/thothctl/internal/compose"
|
||||
@@ -23,7 +25,7 @@ const usage = `Usage: thothctl --installation <absolute-path>/thothii-installati
|
||||
Commands:
|
||||
status Show the Compose service state.
|
||||
doctor Validate Docker, Compose, rendered configuration, line endings, volumes, and health.
|
||||
logs Show the latest 200 sanitized service log lines.
|
||||
logs Show the latest 200 sanitized service log lines (bounded; no follow mode).
|
||||
start Start the installation in the background.
|
||||
stop Stop the installation.
|
||||
update --check-only Validate the current installation without changing containers.
|
||||
@@ -31,10 +33,18 @@ Commands:
|
||||
pi doctor Check Pi preconditions without changing the installation.
|
||||
pi test Run the temporary Pi/core smoke checks.
|
||||
pi check Alias for pi test.
|
||||
pi configure Apply non-secret provider/model/thinking defaults to core (credentials stay in PI_AUTH_FILE).
|
||||
pi update Rebuild or pull a pinned Pi image (--source build|pull and --yes required).
|
||||
pi configure [--provider P --model M --thinking low|medium|high]
|
||||
Select closed backend defaults interactively on a TTY; all flags are required otherwise.
|
||||
pi update --version V --source build --yes [--drain]
|
||||
Rebuild a pinned Pi version and recreate only core.
|
||||
pi update --version V --source pull --image IMAGE@sha256:DIGEST --yes [--drain]
|
||||
Pull an immutable candidate and recreate only core.
|
||||
pi rollback --yes Restore the image recorded by the latest Pi update.
|
||||
pi logs Show the latest sanitized core logs.
|
||||
pi maintenance status
|
||||
Show the durable core admission-gate state.
|
||||
pi maintenance recover --yes
|
||||
Verify a terminal installation, remove stale lifecycle files, and clear maintenance.
|
||||
pi logs Show the latest 200 sanitized core log lines (bounded; no follow mode).
|
||||
`
|
||||
|
||||
func main() {
|
||||
@@ -165,16 +175,18 @@ func piCommand(ctx context.Context, installation config.Installation, runner com
|
||||
result, err := controlled.Run(ctx, append([]string{"compose"}, logArgs...), nil)
|
||||
return writeResult(result, err, secretValues, stdout, stderr)
|
||||
case "configure":
|
||||
defaults, err := parsePiConfigureArgs(args[1:])
|
||||
authFile, authErr := installation.EnvironmentValue("PI_AUTH_FILE")
|
||||
if authErr != nil || strings.TrimSpace(authFile) == "" {
|
||||
return commandUsageError(stderr, "PI_AUTH_FILE must name the actual protected host credential file")
|
||||
}
|
||||
defaults, err := resolvePiConfigure(ctx, controlled, args[1:], os.Stdin, stdout, stdinIsTTY(os.Stdin))
|
||||
if err != nil {
|
||||
return commandUsageError(stderr, err.Error())
|
||||
}
|
||||
if err := pi.Configure(ctx, controlled, defaults); err != nil {
|
||||
return piFailure(stderr, err, secretValues)
|
||||
}
|
||||
authFile, _ := installation.EnvironmentValue("PI_AUTH_FILE")
|
||||
if authFile == "" { authFile = "the host path declared by PI_AUTH_FILE" }
|
||||
fmt.Fprintf(stdout, "Pi defaults applied and read back. Put credentials only in PI_AUTH_FILE=%s (mode 0600); expected variables/files are PI_AUTH_FILE and /home/thoth/.pi/agent/auth.json. Never pass credentials to thothctl.\n", authFile)
|
||||
fmt.Fprintf(stdout, "Pi defaults applied and read back. Provider credentials remain only in the host file %s (mode 0600). Never pass credentials to thothctl.\n", authFile)
|
||||
return 0
|
||||
case "update":
|
||||
request, err := parsePiUpdateArgs(args[1:], filepath.Join(installation.ProjectDirectory, ".thothctl", "update-state.json"))
|
||||
@@ -201,11 +213,108 @@ func piCommand(ctx context.Context, installation config.Installation, runner com
|
||||
}
|
||||
fmt.Fprintf(stdout, "Pi rollback restored the recorded core image. Recovery metadata: %s\n", result.StatePath)
|
||||
return 0
|
||||
case "maintenance":
|
||||
if len(args) == 2 && args[1] == "status" {
|
||||
status, err := pi.MaintenanceStatus(ctx, controlled)
|
||||
if err != nil {
|
||||
return piFailure(stderr, err, secretValues)
|
||||
}
|
||||
fmt.Fprintf(stdout, "Pi maintenance active: %t (admissions: %d)\n", status.Active, status.Admissions)
|
||||
return 0
|
||||
}
|
||||
if len(args) == 3 && args[1] == "recover" && args[2] == "--yes" {
|
||||
statePath := filepath.Join(installation.ProjectDirectory, ".thothctl", "update-state.json")
|
||||
if err := pi.RecoverMaintenance(ctx, controlled, statePath, true); err != nil {
|
||||
return piFailure(stderr, err, secretValues)
|
||||
}
|
||||
fmt.Fprintln(stdout, "Pi maintenance recovery verified; stale lifecycle files were removed and admissions are open.")
|
||||
return 0
|
||||
}
|
||||
return commandUsageError(stderr, "pi maintenance requires status or recover --yes")
|
||||
default:
|
||||
return commandUsageError(stderr, fmt.Sprintf("unknown pi command %q", args[0]))
|
||||
}
|
||||
}
|
||||
|
||||
func resolvePiConfigure(
|
||||
ctx context.Context,
|
||||
runner pi.Runner,
|
||||
args []string,
|
||||
input io.Reader,
|
||||
prompt io.Writer,
|
||||
isTTY bool,
|
||||
) (pi.Defaults, error) {
|
||||
if len(args) > 0 {
|
||||
return parsePiConfigureArgs(args)
|
||||
}
|
||||
if !isTTY {
|
||||
return pi.Defaults{}, errors.New("non-interactive pi configure requires --provider --model --thinking")
|
||||
}
|
||||
options, err := pi.ConfigurationOptions(ctx, runner)
|
||||
if err != nil {
|
||||
return pi.Defaults{}, err
|
||||
}
|
||||
providers := uniqueProviders(options)
|
||||
scanner := bufio.NewScanner(input)
|
||||
provider, err := numberedChoice(scanner, prompt, "provider", providers)
|
||||
if err != nil {
|
||||
return pi.Defaults{}, err
|
||||
}
|
||||
models := make([]string, 0)
|
||||
for _, option := range options {
|
||||
if option.Provider == provider {
|
||||
models = append(models, option.ID)
|
||||
}
|
||||
}
|
||||
model, err := numberedChoice(scanner, prompt, "model", models)
|
||||
if err != nil {
|
||||
return pi.Defaults{}, err
|
||||
}
|
||||
thinking, err := numberedChoice(scanner, prompt, "thinking level", []string{"low", "medium", "high"})
|
||||
if err != nil {
|
||||
return pi.Defaults{}, err
|
||||
}
|
||||
return pi.Defaults{Provider: provider, Model: model, Thinking: thinking}, nil
|
||||
}
|
||||
|
||||
func uniqueProviders(options []pi.ModelOption) []string {
|
||||
seen := make(map[string]bool)
|
||||
providers := make([]string, 0)
|
||||
for _, option := range options {
|
||||
if !seen[option.Provider] {
|
||||
seen[option.Provider] = true
|
||||
providers = append(providers, option.Provider)
|
||||
}
|
||||
}
|
||||
return providers
|
||||
}
|
||||
|
||||
func numberedChoice(scanner *bufio.Scanner, output io.Writer, label string, choices []string) (string, error) {
|
||||
if len(choices) == 0 {
|
||||
return "", fmt.Errorf("Pi returned no %s choices", label)
|
||||
}
|
||||
fmt.Fprintf(output, "Select %s:\n", label)
|
||||
for index, choice := range choices {
|
||||
fmt.Fprintf(output, " %d) %s\n", index+1, choice)
|
||||
}
|
||||
for {
|
||||
fmt.Fprintf(output, "Choice [1-%d]: ", len(choices))
|
||||
if !scanner.Scan() {
|
||||
return "", fmt.Errorf("interactive %s selection ended before a choice was entered", label)
|
||||
}
|
||||
selected, err := strconv.Atoi(strings.TrimSpace(scanner.Text()))
|
||||
if err == nil && selected >= 1 && selected <= len(choices) {
|
||||
return choices[selected-1], nil
|
||||
}
|
||||
fmt.Fprintln(output, "Enter one of the listed numbers.")
|
||||
}
|
||||
}
|
||||
|
||||
func stdinIsTTY(input *os.File) bool {
|
||||
info, err := input.Stat()
|
||||
return err == nil && info.Mode()&os.ModeCharDevice != 0
|
||||
}
|
||||
|
||||
func parsePiConfigureArgs(args []string) (pi.Defaults, error) {
|
||||
var value pi.Defaults
|
||||
for len(args) > 0 {
|
||||
@@ -264,17 +373,27 @@ func parsePiUpdateArgs(args []string, statePath string) (pi.Request, error) {
|
||||
return pi.Request{}, fmt.Errorf("unknown pi update option %q", args[0])
|
||||
}
|
||||
}
|
||||
if request.Version == "" { return pi.Request{}, errors.New("pi update requires --version <pinned-version>") }
|
||||
if request.Source == "" { return pi.Request{}, errors.New("pi update requires explicit --source build or pull") }
|
||||
if request.Source != pi.BuildSource && request.Source != pi.PullSource { return pi.Request{}, errors.New("--source requires build or pull") }
|
||||
if request.Source == pi.PullSource && request.Image == "" { return pi.Request{}, errors.New("--source pull requires --image <digest-reference>") }
|
||||
if request.Source == pi.BuildSource && request.Image != "" { return pi.Request{}, errors.New("--image is valid only with --source pull") }
|
||||
if request.Version == "" {
|
||||
return pi.Request{}, errors.New("pi update requires --version <pinned-version>")
|
||||
}
|
||||
if request.Source == "" {
|
||||
return pi.Request{}, errors.New("pi update requires explicit --source build or pull")
|
||||
}
|
||||
if request.Source != pi.BuildSource && request.Source != pi.PullSource {
|
||||
return pi.Request{}, errors.New("--source requires build or pull")
|
||||
}
|
||||
if request.Source == pi.PullSource && request.Image == "" {
|
||||
return pi.Request{}, errors.New("--source pull requires --image <digest-reference>")
|
||||
}
|
||||
if request.Source == pi.BuildSource && request.Image != "" {
|
||||
return pi.Request{}, errors.New("--image is valid only with --source pull")
|
||||
}
|
||||
return request, nil
|
||||
}
|
||||
|
||||
func piFailure(stderr io.Writer, err error, secretValues []string) int {
|
||||
code := 1
|
||||
if errors.Is(err, pi.ErrConfirmationRequired) || errors.Is(err, pi.ErrInvalidRequest) {
|
||||
if errors.Is(err, pi.ErrConfirmationRequired) || errors.Is(err, pi.ErrInvalidRequest) || errors.Is(err, pi.ErrActiveSessions) || errors.Is(err, pi.ErrInterruptedUpdate) {
|
||||
code = 2
|
||||
}
|
||||
var childExit interface{ ExitCode() int }
|
||||
@@ -299,10 +418,7 @@ func logsArgs(args []string) ([]string, error) {
|
||||
if len(args) == 0 {
|
||||
return []string{"logs", "--tail", "200"}, nil
|
||||
}
|
||||
if len(args) == 1 && args[0] == "--follow" {
|
||||
return []string{"logs", "--tail", "200", "--follow"}, nil
|
||||
}
|
||||
return nil, errors.New("logs accepts only --follow")
|
||||
return nil, errors.New("logs does not accept arguments; use bounded snapshots")
|
||||
}
|
||||
|
||||
func commandUsageError(stderr io.Writer, message string) int {
|
||||
|
||||
@@ -4,15 +4,91 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/aritmolab/thothii/tools/thothctl/internal/compose"
|
||||
"github.com/aritmolab/thothii/tools/thothctl/internal/pi"
|
||||
"github.com/aritmolab/thothii/tools/thothctl/internal/testsupport"
|
||||
)
|
||||
|
||||
func TestResolvePiConfigureUsesNumberedClosedChoicesOnlyForTTY(t *testing.T) {
|
||||
runner := &wizardRunner{}
|
||||
var prompt bytes.Buffer
|
||||
defaults, err := resolvePiConfigure(
|
||||
context.Background(), runner, nil, strings.NewReader("2\n1\n3\n"), &prompt, true,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
want := pi.Defaults{Provider: "zai", Model: "glm-5.2", Thinking: "high"}
|
||||
if defaults != want {
|
||||
t.Fatalf("defaults = %#v", defaults)
|
||||
}
|
||||
for _, expected := range []string{"1) deepseek", "2) zai", "1) glm-5.2", "3) high"} {
|
||||
if !strings.Contains(prompt.String(), expected) {
|
||||
t.Errorf("prompt %q missing %q", prompt.String(), expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolvePiConfigureRequiresExplicitFlagsWithoutTTY(t *testing.T) {
|
||||
runner := &wizardRunner{}
|
||||
_, err := resolvePiConfigure(context.Background(), runner, nil, strings.NewReader("1\n1\n1\n"), io.Discard, false)
|
||||
if err == nil || !strings.Contains(err.Error(), "non-interactive") {
|
||||
t.Fatalf("resolvePiConfigure() error = %v, want explicit non-interactive guidance", err)
|
||||
}
|
||||
if len(runner.calls) != 0 {
|
||||
t.Fatalf("Docker calls = %v, want none", runner.calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPiLifecycleContractErrorsExitTwo(t *testing.T) {
|
||||
for _, lifecycleErr := range []error{pi.ErrActiveSessions, pi.ErrInterruptedUpdate} {
|
||||
var stderr bytes.Buffer
|
||||
if code := piFailure(&stderr, lifecycleErr, nil); code != 2 {
|
||||
t.Errorf("piFailure(%v) = %d, want 2", lifecycleErr, code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLogsRejectsFollowAndOtherArguments(t *testing.T) {
|
||||
if _, err := logsArgs([]string{"--follow"}); err == nil {
|
||||
t.Fatal("logsArgs(--follow) error = nil, want bounded-log rejection")
|
||||
}
|
||||
if got, err := logsArgs(nil); err != nil || strings.Join(got, " ") != "logs --tail 200" {
|
||||
t.Fatalf("logsArgs(nil) = %v, %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUsageDocumentsClosedConfigureUpdateSourcesAndMaintenanceRecovery(t *testing.T) {
|
||||
if strings.Contains(usage, "--follow") {
|
||||
t.Fatal("usage still advertises unbounded log following")
|
||||
}
|
||||
for _, required := range []string{
|
||||
"--provider P --model M --thinking low|medium|high",
|
||||
"--source build",
|
||||
"--source pull --image IMAGE@sha256:DIGEST",
|
||||
"pi maintenance status",
|
||||
"pi maintenance recover --yes",
|
||||
} {
|
||||
if !strings.Contains(usage, required) {
|
||||
t.Errorf("usage missing %q", required)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
type wizardRunner struct{ calls []string }
|
||||
|
||||
func (r *wizardRunner) Run(_ context.Context, args []string, _ io.Reader) (compose.Result, error) {
|
||||
r.calls = append(r.calls, strings.Join(args, " "))
|
||||
return compose.Result{Stdout: `{"models":[{"provider":"deepseek","id":"deepseek-v4"},{"provider":"zai","id":"glm-5.2"}]}`}, nil
|
||||
}
|
||||
|
||||
func TestRunLogsRedactsAnUnlabelledDeclaredSecret(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "UNLABELLED_SECRET_FILE=%s\n")
|
||||
secretPath := filepath.Join(fixture.root, "operator-secret")
|
||||
@@ -379,6 +455,74 @@ func TestRunPiUpdateRequiresExplicitConfirmationWithoutInvokingDocker(t *testing
|
||||
assertDockerNotInvoked(t, fixture)
|
||||
}
|
||||
|
||||
func TestRunPiUpdateRequiresExplicitSourceWithoutInvokingDocker(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "THT_LLM_URL=https://llm.example.invalid\n")
|
||||
fixture.setEnvironment(t)
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := run(context.Background(), []string{
|
||||
"--installation", fixture.installationPath, "pi", "update", "--version", "0.81.0", "--yes",
|
||||
}, &stdout, &stderr)
|
||||
|
||||
if exitCode != 2 {
|
||||
t.Errorf("run() exit code = %d, want 2", exitCode)
|
||||
}
|
||||
if !strings.Contains(stderr.String(), "requires explicit --source build or pull") {
|
||||
t.Errorf("stderr = %q, want source guidance", stderr.String())
|
||||
}
|
||||
assertDockerNotInvoked(t, fixture)
|
||||
}
|
||||
|
||||
func TestRunPiConfigureReportsTheActualHostAuthFile(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "")
|
||||
authFile := filepath.Join(fixture.root, "pi-auth.json")
|
||||
if err := os.WriteFile(authFile, []byte(`{"provider":"credential"}`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fixture.setEnvContents(t, "THT_LLM_URL=https://llm.example.invalid\nPI_AUTH_FILE="+authFile+"\n")
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := run(context.Background(), []string{
|
||||
"--installation", fixture.installationPath, "pi", "configure",
|
||||
"--provider", "provider", "--model", "model", "--thinking", "medium",
|
||||
}, &stdout, &stderr)
|
||||
|
||||
if exitCode != 0 {
|
||||
t.Fatalf("run() exit = %d, stderr=%s", exitCode, stderr.String())
|
||||
}
|
||||
if !strings.Contains(stdout.String(), authFile) {
|
||||
t.Fatalf("stdout = %q, want host auth path", stdout.String())
|
||||
}
|
||||
if strings.Contains(stdout.String(), "/home/thoth/.pi") {
|
||||
t.Fatalf("stdout exposed container-only auth path: %q", stdout.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunPiMaintenanceStatusAndRecoverConfirmationContract(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "THT_LLM_URL=https://llm.example.invalid\n")
|
||||
fixture.setEnvironment(t)
|
||||
var stdout, stderr bytes.Buffer
|
||||
if code := run(context.Background(), []string{
|
||||
"--installation", fixture.installationPath, "pi", "maintenance", "status",
|
||||
}, &stdout, &stderr); code != 0 {
|
||||
t.Fatalf("maintenance status exit = %d, stderr = %s", code, stderr.String())
|
||||
}
|
||||
if stdout.String() != "Pi maintenance active: true (admissions: 0)\n" {
|
||||
t.Fatalf("maintenance status output = %q", stdout.String())
|
||||
}
|
||||
|
||||
second := newCLIFixture(t, "THT_LLM_URL=https://llm.example.invalid\n")
|
||||
second.setEnvironment(t)
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if code := run(context.Background(), []string{
|
||||
"--installation", second.installationPath, "pi", "maintenance", "recover",
|
||||
}, &stdout, &stderr); code != 2 {
|
||||
t.Fatalf("maintenance recover without --yes exit = %d, want 2", code)
|
||||
}
|
||||
assertDockerNotInvoked(t, second)
|
||||
}
|
||||
|
||||
func TestRunPiStatusPreservesDockerExitCodeAndRedactsDiagnostics(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "PI_TOKEN_FILE=%s\n")
|
||||
secretPath := filepath.Join(fixture.root, "pi-secret")
|
||||
@@ -448,6 +592,9 @@ case " $* " in
|
||||
*" config --format json "*) printf '%s\n' '{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}' ;;
|
||||
*" ps --format json "*) printf '%s\n' '[{"Service":"core","State":"running","Health":"healthy"},{"Service":"frontend","State":"running","Health":"healthy"}]' ;;
|
||||
*" pi --version "*) printf '%s\n' '0.80.3' ;;
|
||||
*"/models "*) printf '%s\n' '{"models":[{"provider":"provider","id":"model"}]}' ;;
|
||||
*"/settings "*) printf '%s\n' '{"provider":"provider","model":"model","thinking":"medium"}' ;;
|
||||
*"/internal/maintenance/status "*) printf '%s\n' '{"active":true,"admissions":0}' ;;
|
||||
*" logs "*) printf '%s\n' "$THOTHCTL_FAKE_LOG" ;;
|
||||
esac
|
||||
if [ "${THOTHCTL_FAKE_FAIL_ON:-}" = "version" ]; then
|
||||
|
||||
Reference in New Issue
Block a user