mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix: harden thothctl pi lifecycle

Browse Source
This commit is contained in:
marcopan
2026-08-04 18:40:50 +02:00
parent 20e59b8d32
commit 8fde1f81c7
8 changed files with 308 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.dockerignore
+1
View File
@@ -7,6 +7,7 @@
**/*.pyc
.git
.worktrees
.thothctl
.gitignore
**/.env
**/.env.*
.gitignore
+1
View File
@@ -8,6 +8,7 @@ Thoth/
# === Visual companion brainstorming artifacts (local-only) ===
.superpowers/
.worktrees/
.thothctl/
# === Python ===
__pycache__/
tools/thothctl/cmd/thothctl/main.go
+43 -5
View File
@@ -30,9 +30,11 @@ Commands:
pi status Show the Pi version embedded in core.
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 Store non-secret Pi defaults (credentials stay in PI_AUTH_FILE).
pi update Rebuild or pull a pinned Pi image (requires --yes).
pi rollback --yes Restore the image recorded by the latest Pi update.
pi logs [--follow] Show sanitized core logs.
pi logs Show the latest sanitized core logs.
`
func main() {
@@ -156,13 +158,22 @@ func piCommand(ctx context.Context, installation config.Installation, runner com
fmt.Fprintln(stdout, "Pi/core smoke checks passed.")
return 0
case "logs":
logArgs, err := logsArgs(args[1:])
if err != nil {
return commandUsageError(stderr, "pi logs accepts only --follow")
if len(args) != 1 {
return commandUsageError(stderr, "pi logs does not support --follow; use bounded snapshots")
}
logArgs = append(logArgs, "core")
logArgs := []string{"logs", "--tail", "200", "core"}
result, err := controlled.Run(ctx, append([]string{"compose"}, logArgs...), nil)
return writeResult(result, err, secretValues, stdout, stderr)
case "configure":
defaults, err := parsePiConfigureArgs(args[1:])
if err != nil {
return commandUsageError(stderr, err.Error())
}
if err := pi.Configure(ctx, controlled, filepath.Join(installation.ProjectDirectory, ".thothctl", "pi-defaults.json"), defaults); err != nil {
return piFailure(stderr, err, secretValues)
}
fmt.Fprintln(stdout, "Pi defaults saved. Put credentials only in the configured PI_AUTH_FILE (mode 0600).")
return 0
case "update":
request, err := parsePiUpdateArgs(args[1:], filepath.Join(installation.ProjectDirectory, ".thothctl", "update-state.json"))
if err != nil {
@@ -193,6 +204,33 @@ func piCommand(ctx context.Context, installation config.Installation, runner com
}
}
func parsePiConfigureArgs(args []string) (pi.Defaults, error) {
var value pi.Defaults
for len(args) > 0 {
if len(args) < 2 {
return pi.Defaults{}, errors.New("configure options require values")
}
key, v := args[0], args[1]
args = args[2:]
switch key {
case "--provider":
value.Provider = v
case "--model":
value.Model = v
case "--thinking":
value.Thinking = v
case "--llm-url":
value.LLMURL = v
default:
return pi.Defaults{}, fmt.Errorf("unknown pi configure option %q", key)
}
}
if value.Provider == "" || value.Model == "" || value.Thinking == "" || value.LLMURL == "" {
return pi.Defaults{}, errors.New("pi configure requires --provider --model --thinking --llm-url")
}
return value, nil
}
func parsePiUpdateArgs(args []string, statePath string) (pi.Request, error) {
request := pi.Request{StatePath: statePath, Source: pi.BuildSource}
for len(args) > 0 {
tools/thothctl/internal/pi/commands.go
+83
View File
@@ -7,11 +7,94 @@ import (
"errors"
"fmt"
"io"
"net/url"
"os"
"path/filepath"
"regexp"
"strings"
"github.com/aritmolab/thothii/tools/thothctl/internal/compose"
)
var choicePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._/-]{0,127}$`)
type Defaults struct {
Provider string `json:"provider"`
Model string `json:"model"`
Thinking string `json:"thinking"`
LLMURL string `json:"llm_url"`
}
// Configure validates and atomically stores only local non-secret Pi defaults.
func Configure(ctx context.Context, runner Runner, path string, value Defaults) error {
if !choicePattern.MatchString(value.Provider) || !choicePattern.MatchString(value.Model) {
return errors.New("provider and model must be supported identifiers")
}
if value.Thinking != "low" && value.Thinking != "medium" && value.Thinking != "high" {
return errors.New("thinking must be low, medium, or high")
}
u, err := url.Parse(value.LLMURL)
if err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Host == "" || u.User != nil || u.RawQuery != "" || u.Fragment != "" {
return errors.New("LLM endpoint must be an http(s) URL without credentials, query, or fragment")
}
models, err := runCompose(ctx, runner, "exec", "-T", "core", "curl", "-fsS", "http://127.0.0.1:8787/models")
if err != nil {
return commandError("Pi options check", models, err)
}
var payload struct {
Models []struct {
Provider string `json:"provider"`
ID string `json:"id"`
} `json:"models"`
}
if json.Unmarshal([]byte(models.Stdout), &payload) != nil || len(payload.Models) == 0 {
return errors.New("Pi options response is invalid or empty")
}
found := false
for _, model := range payload.Models {
if model.Provider == value.Provider && model.ID == value.Model {
found = true
}
}
if !found {
return errors.New("provider/model is not in Pi options")
}
return writeJSON(path, value)
}
func writeJSON(path string, value any) error {
contents, err := json.MarshalIndent(value, "", " ")
if err != nil {
return err
}
contents = append(contents, '\n')
if err = os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
return errors.New("could not create Pi configuration directory")
}
temporary, err := os.CreateTemp(filepath.Dir(path), ".pi-defaults-*.tmp")
if err != nil {
return errors.New("could not write Pi configuration")
}
name := temporary.Name()
defer os.Remove(name)
if err = temporary.Chmod(0o600); err == nil {
_, err = temporary.Write(contents)
}
if err == nil {
err = temporary.Sync()
}
if closeErr := temporary.Close(); err == nil {
err = closeErr
}
if err == nil {
err = os.Rename(name, path)
}
if err != nil {
return errors.New("could not atomically write Pi configuration")
}
return nil
}
// Runner is the narrow, shell-free command boundary shared with thothctl.
type Runner interface {
Run(context.Context, []string, io.Reader) (compose.Result, error)
tools/thothctl/internal/pi/commands_test.go
+15
View File
@@ -2,6 +2,7 @@ package pi
import (
"context"
"path/filepath"
"strings"
"testing"
)
@@ -16,6 +17,20 @@ func TestDoctorRequiresExternalEndpointAuthPiStateAndHealth(t *testing.T) {
}
}
func TestConfigureValidatesBackendModelOptionsAndWritesNoSecrets(t *testing.T) {
fake := newFakeRunner()
path := filepath.Join(t.TempDir(), "pi-defaults.json")
if err := Configure(context.Background(), fake, path, Defaults{Provider: "provider", Model: "model", Thinking: "medium", LLMURL: "https://llm.example.invalid"}); err != nil {
t.Fatal(err)
}
if got := string(readStateBytes(t, path)); strings.Contains(got, "secret") || !strings.Contains(got, "llm.example.invalid") {
t.Fatalf("defaults=%q", got)
}
if err := Configure(context.Background(), fake, path, Defaults{Provider: "provider", Model: "unknown", Thinking: "medium", LLMURL: "https://llm.example.invalid"}); err == nil {
t.Fatal("expected unknown model rejection")
}
}
func TestTestUsesOnlySanitizedPiAndCoreProbes(t *testing.T) {
fake := newFakeRunner()
if err := Test(context.Background(), fake); err != nil {
tools/thothctl/internal/pi/state.go
+38
View File
@@ -7,6 +7,7 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"
"time"
)
@@ -30,9 +31,19 @@ type Image struct {
ID string `json:"id"`
Reference string `json:"reference"`
Volumes []string `json:"volumes"`
Mounts []Mount `json:"mounts"`
ConfigurationSHA string `json:"configuration_sha256,omitempty"`
}
// Mount is the complete persistence identity relevant to safe core recreation.
type Mount struct {
Type string `json:"type"`
Name string `json:"name,omitempty"`
Source string `json:"source"`
Destination string `json:"destination"`
RW bool `json:"rw"`
}
// Target records the immutable input selected by the operator. Source is either build or a
// digest-pinned image reference; it intentionally never contains credentials.
type Target struct {
@@ -96,11 +107,38 @@ func writeState(path string, state State) error {
temporary.Close()
return errors.New("could not write update recovery state")
}
if err := temporary.Sync(); err != nil {
temporary.Close()
return errors.New("could not durably write update recovery state")
}
if err := temporary.Close(); err != nil {
return errors.New("could not write update recovery state")
}
if err := os.Rename(temporaryName, path); err != nil {
return errors.New("could not finalize update recovery state")
}
if runtime.GOOS != "windows" {
if directoryHandle, err := os.Open(directory); err == nil {
_ = directoryHandle.Sync()
_ = directoryHandle.Close()
}
}
return nil
}
type updateLock struct{ path string }
func acquireLock(statePath string) (*updateLock, error) {
if err := os.MkdirAll(filepath.Dir(statePath), 0o700); err != nil {
return nil, errors.New("could not create Pi update recovery directory")
}
path := statePath + ".lock"
if err := os.Mkdir(path, 0o700); err != nil {
if errors.Is(err, os.ErrExist) {
return nil, errors.New("another Pi update or rollback is already in progress; recovery lock retained")
}
return nil, errors.New("could not acquire Pi update lock")
}
return &updateLock{path: path}, nil
}
func (l *updateLock) Release() { _ = os.Remove(l.path) }
tools/thothctl/internal/pi/update.go
+90 -13
View File
@@ -9,6 +9,7 @@ import (
"regexp"
"sort"
"strings"
"time"
)
var (
@@ -45,6 +46,11 @@ type Result struct {
// Update performs a recoverable core-only Pi update using the default Compose command layout.
func Update(ctx context.Context, runner Runner, request Request) (Result, error) {
lock, err := acquireLock(request.StatePath)
if err != nil {
return Result{StatePath: request.StatePath}, err
}
defer lock.Release()
if request.StatePath == "" {
return Result{}, errors.New("update state path is required")
}
@@ -73,14 +79,31 @@ func Update(ctx context.Context, runner Runner, request Request) (Result, error)
if err != nil {
return Result{StatePath: request.StatePath}, err
}
frontendStopped := false
if running {
if !request.Drain {
return Result{StatePath: request.StatePath}, ErrActiveSessions
}
stopped, stopErr := runCompose(ctx, runner, "stop", "frontend")
if stopErr != nil {
return Result{StatePath: request.StatePath}, commandError("frontend admission gate", stopped, stopErr)
}
frontendStopped = true
defer func() {
if frontendStopped {
_, _ = runCompose(context.Background(), runner, "up", "--detach", "frontend")
}
}()
for attempts := 0; attempts < 30; attempts++ {
running, err = activeSessions(ctx, runner)
if err != nil {
return Result{StatePath: request.StatePath}, err
}
if !running {
break
}
time.Sleep(time.Second)
}
if running {
return Result{StatePath: request.StatePath}, ErrActiveSessions
}
@@ -119,6 +142,17 @@ func Update(ctx context.Context, runner Runner, request Request) (Result, error)
_ = writeState(request.StatePath, state)
return Result{Phase: PhaseFailed, StatePath: request.StatePath}, err
}
if frontendStopped {
running, err = activeSessions(ctx, runner)
if err != nil {
return Result{Phase: PhaseFailed, StatePath: request.StatePath}, err
}
if running {
state.Phase, state.Error = PhaseFailed, "new session admitted while draining"
_ = writeState(request.StatePath, state)
return Result{Phase: PhaseFailed, StatePath: request.StatePath}, ErrActiveSessions
}
}
if err := recreateCore(ctx, runner); err != nil {
state.Phase, state.Error = PhaseFailed, "core recreation failed"
_ = writeState(request.StatePath, state)
@@ -129,7 +163,7 @@ func Update(ctx context.Context, runner Runner, request Request) (Result, error)
if err := writeState(request.StatePath, state); err != nil {
return Result{Phase: PhaseRecreated, StatePath: request.StatePath}, err
}
if err := verifyCandidate(ctx, runner, request.Version, previous.Volumes, previous.ConfigurationSHA); err != nil {
if err := verifyCandidate(ctx, runner, request.Version, previous); err != nil {
return rollbackAfterFailure(ctx, runner, request.StatePath, state, err)
}
state.Phase, state.Error = PhaseVerified, ""
@@ -141,6 +175,11 @@ func Update(ctx context.Context, runner Runner, request Request) (Result, error)
// 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, error) {
lock, err := acquireLock(statePath)
if err != nil {
return Result{StatePath: statePath}, err
}
defer lock.Release()
if !confirm {
return Result{StatePath: statePath}, ErrConfirmationRequired
}
@@ -208,15 +247,24 @@ func runningImage(ctx context.Context, runner Runner, reference string) (Image,
if err != nil || strings.TrimSpace(image.Stdout) == "" {
return Image{}, commandError("running core image check", image, err)
}
mounts, err := runner.Run(ctx, []string{"inspect", "--format", "{{range .Mounts}}{{if eq .Type \"volume\"}}{{.Name}}{{\"\\n\"}}{{end}}{{end}}", id}, nil)
mounts, err := runner.Run(ctx, []string{"inspect", "--format", "{{json .Mounts}}", id}, nil)
if err != nil {
return Image{}, commandError("core volume check", mounts, err)
}
volumes := nonEmptyLines(mounts.Stdout)
if len(volumes) == 0 {
return Image{}, errors.New("core has no named volumes to preserve")
var contract []Mount
if err := json.Unmarshal([]byte(mounts.Stdout), &contract); err != nil {
return Image{}, errors.New("core returned invalid persistence mount data")
}
return Image{ID: strings.TrimSpace(image.Stdout), Reference: reference, Volumes: volumes}, nil
if len(contract) == 0 {
return Image{}, errors.New("core has no persistence mounts to preserve")
}
volumes := make([]string, 0, len(contract))
for _, mount := range contract {
if mount.Type == "volume" && mount.Name != "" {
volumes = append(volumes, mount.Name)
}
}
return Image{ID: strings.TrimSpace(image.Stdout), Reference: reference, Volumes: volumes, Mounts: contract}, nil
}
func prepareCandidate(ctx context.Context, runner Runner, request Request, reference string) error {
@@ -239,14 +287,14 @@ func prepareCandidate(ctx context.Context, runner Runner, request Request, refer
}
func recreateCore(ctx context.Context, runner Runner) error {
result, err := runCompose(ctx, runner, "up", "--detach", "--no-deps", "--force-recreate", "core")
result, err := runCompose(ctx, runner, "up", "--detach", "--wait", "--wait-timeout", "45", "--no-deps", "--force-recreate", "core")
if err != nil {
return commandError("core recreation", result, err)
}
return nil
}
func verifyCandidate(ctx context.Context, runner Runner, wanted string, previousVolumes []string, configurationSHA string) error {
func verifyCandidate(ctx context.Context, runner Runner, wanted string, previous Image) error {
health, err := runCompose(ctx, runner, "exec", "-T", "core", "curl", "-fsS", "http://127.0.0.1:8787/health")
if err != nil {
return commandError("core health check", health, err)
@@ -265,15 +313,15 @@ func verifyCandidate(ctx context.Context, runner Runner, wanted string, previous
if err != nil {
return err
}
if configured.ConfigurationSHA != configurationSHA {
if configured.ConfigurationSHA != previous.ConfigurationSHA {
return errors.New("external endpoint configuration changed during Pi update")
}
after, err := runningImage(ctx, runner, configured.Reference)
if err != nil {
return err
}
if !sameStrings(previousVolumes, after.Volumes) {
return errors.New("core volume set changed during Pi update")
if !sameMounts(previous.Mounts, after.Mounts) {
return errors.New("core persistence mount contract changed during Pi update")
}
return nil
}
@@ -294,8 +342,20 @@ func restore(ctx context.Context, runner Runner, previous Image) error {
if err != nil {
return err
}
if !sameStrings(previous.Volumes, after.Volumes) {
return errors.New("core volume set changed during rollback")
if after.ID != previous.ID {
return errors.New("rollback core image does not match recorded previous image")
}
if configured.ConfigurationSHA != previous.ConfigurationSHA {
return errors.New("external endpoint configuration drift prevents rollback proof")
}
if !sameMounts(previous.Mounts, after.Mounts) {
return errors.New("core persistence mount contract changed during rollback")
}
if err := Doctor(ctx, runner); err != nil {
return err
}
if err := Test(ctx, runner); err != nil {
return err
}
return nil
}
@@ -316,3 +376,20 @@ func sameStrings(left, right []string) bool {
sort.Strings(right)
return strings.Join(left, "\x00") == strings.Join(right, "\x00")
}
func sameMounts(left, right []Mount) bool {
if len(left) != len(right) {
return false
}
key := func(m Mount) string {
return m.Type + "\x00" + m.Name + "\x00" + m.Source + "\x00" + m.Destination + "\x00" + fmt.Sprint(m.RW)
}
a, b := make([]string, len(left)), make([]string, len(right))
for i := range left {
a[i] = key(left[i])
}
for i := range right {
b[i] = key(right[i])
}
return sameStrings(a, b)
}
tools/thothctl/internal/pi/update_test.go
+34 -6
View File
@@ -28,7 +28,7 @@ func TestUpdateBuildsPinnedVersionRecreatesOnlyCoreAndPersistsRecoveryState(t *t
t.Fatalf("phase = %q, want %q", result.Phase, PhaseVerified)
}
assertCalled(t, fake.calls, "compose build --pull --build-arg PI_VERSION=0.81.0 core")
assertCalled(t, fake.calls, "compose up --detach --no-deps --force-recreate core")
assertCalled(t, fake.calls, "compose 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"`) {
t.Fatalf("state = %q, want credential-free verified metadata", got)
@@ -83,7 +83,7 @@ func TestUpdateRollsBackAfterPostRecreateFailures(t *testing.T) {
t.Fatalf("phase = %q, want %q", result.Phase, PhaseRolledBack)
}
assertCalled(t, fake.calls, "image tag sha256:old thothii-core:local")
assertCalled(t, fake.calls, "compose up --detach --no-deps --force-recreate core")
assertCalled(t, fake.calls, "compose up --detach --wait --wait-timeout 45 --no-deps --force-recreate core")
if strings.Join(fake.volumes, ",") != "settings,pi-state,sessions,workspace-registry" {
t.Fatalf("volumes changed: %v", fake.volumes)
}
@@ -138,7 +138,16 @@ func TestUpdateRequiresConfirmationAndDrainsActiveSessions(t *testing.T) {
func TestRollbackRestoresInterruptedOrPreviouslyRecordedState(t *testing.T) {
fake := newFakeRunner()
statePath := filepath.Join(t.TempDir(), "state.json")
writeStateForTest(t, statePath, State{Version: 1, Phase: PhaseRecreated, Previous: Image{ID: "sha256:old", Reference: "thothii-core:local", Volumes: []string{"settings", "pi-state", "sessions", "workspace-registry"}}})
configured, err := renderedCore(context.Background(), fake)
if err != nil {
t.Fatal(err)
}
previous, err := runningImage(context.Background(), fake, "thothii-core:local")
if err != nil {
t.Fatal(err)
}
previous.ConfigurationSHA = configured.ConfigurationSHA
writeStateForTest(t, statePath, State{Version: 1, Phase: PhaseRecreated, Previous: previous})
result, err := Rollback(context.Background(), fake, statePath, true)
if err != nil {
t.Fatalf("Rollback() error = %v", err)
@@ -147,7 +156,7 @@ func TestRollbackRestoresInterruptedOrPreviouslyRecordedState(t *testing.T) {
t.Fatalf("phase = %q, want %q", result.Phase, PhaseRolledBack)
}
assertCalled(t, fake.calls, "image tag sha256:old thothii-core:local")
assertCalled(t, fake.calls, "compose up --detach --no-deps --force-recreate core")
assertCalled(t, fake.calls, "compose up --detach --wait --wait-timeout 45 --no-deps --force-recreate core")
}
func TestUpdateRefusesToOverwriteInterruptedRecoveryState(t *testing.T) {
@@ -161,6 +170,18 @@ func TestUpdateRefusesToOverwriteInterruptedRecoveryState(t *testing.T) {
assertNotCalled(t, fake.calls, "compose")
}
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}]`
image, err := runningImage(context.Background(), fake, "thothii-core:local")
if err != nil {
t.Fatalf("runningImage() error = %v", err)
}
if len(image.Mounts) != 3 || image.Mounts[0].Type != "bind" || image.Mounts[0].Destination != "/data" {
t.Fatalf("mounts = %#v", image.Mounts)
}
}
type fakeRunner struct {
calls []string
fail string
@@ -168,6 +189,7 @@ type fakeRunner struct {
activeSessions bool
built bool
volumes []string
mountsJSON string
}
func newFakeRunner() *fakeRunner {
@@ -177,6 +199,9 @@ func newFakeRunner() *fakeRunner {
func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose.Result, error) {
call := strings.Join(args, " ")
f.calls = append(f.calls, call)
if strings.Contains(call, "image tag sha256:old") {
f.fail = ""
}
if f.fail == "preflight" && strings.Contains(call, "config --format json") {
return compose.Result{ExitCode: 1}, errors.New("provider token=secret")
}
@@ -199,8 +224,11 @@ func (f *fakeRunner) Run(_ context.Context, args []string, _ io.Reader) (compose
return compose.Result{Stdout: "core-container\n"}, nil
case strings.Contains(call, "inspect --format {{.Image}}"):
return compose.Result{Stdout: "sha256:old\n"}, nil
case strings.Contains(call, "inspect --format {{range .Mounts}}"):
return compose.Result{Stdout: strings.Join(f.volumes, "\n") + "\n"}, nil
case strings.Contains(call, "inspect --format {{json .Mounts}}"):
if f.mountsJSON != "" {
return compose.Result{Stdout: f.mountsJSON}, nil
}
return compose.Result{Stdout: `[{"Type":"volume","Name":"settings","Source":"settings","Destination":"/data/settings","RW":true},{"Type":"volume","Name":"pi-state","Source":"pi-state","Destination":"/home/thoth/.pi","RW":true},{"Type":"volume","Name":"sessions","Source":"sessions","Destination":"/data/sessions","RW":true},{"Type":"volume","Name":"workspace-registry","Source":"workspace-registry","Destination":"/data/workspace-registry","RW":true}]`}, nil
case strings.Contains(call, "tht session list --json"):
if f.activeSessions {
f.activeSessions = false