fix: enforce server trust boundaries
This commit is contained in:
@@ -172,7 +172,14 @@ func writeRemovalTargets(outputWriter io.Writer, project string, targets []serve
|
||||
}
|
||||
|
||||
func serverOperationFailure(stderr io.Writer, err error, secretValues []string) int {
|
||||
fmt.Fprintf(stderr, "thothctl: %s\n", output.Sanitize(err.Error(), secretValues))
|
||||
message := output.Sanitize(err.Error(), secretValues)
|
||||
var operationErr *serverops.OperationError
|
||||
if errors.As(err, &operationErr) && operationErr.Detail() != "" {
|
||||
detail := output.Sanitize(operationErr.Detail(), secretValues)
|
||||
fmt.Fprintf(stderr, "thothctl: %s: %s\n", message, detail)
|
||||
} else {
|
||||
fmt.Fprintf(stderr, "thothctl: %s\n", message)
|
||||
}
|
||||
if errors.Is(err, serverops.ErrConfirmationRequired) || errors.Is(err, serverops.ErrUnsafeState) {
|
||||
return 2
|
||||
}
|
||||
|
||||
@@ -104,6 +104,36 @@ func TestRunSessionsMigrateRequiresExplicitConfirmationBeforeDocker(t *testing.T
|
||||
assertDockerNotInvoked(t, fixture)
|
||||
}
|
||||
|
||||
func TestRunSessionsMigrateReportsSanitizedStageAndExitClass(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "MIGRATION_PASSWORD_FILE=%s\n")
|
||||
fixture.setProfile(t, "server")
|
||||
secretPath := filepath.Join(fixture.root, "migration-password")
|
||||
if err := os.WriteFile(secretPath, []byte("migration-secret-value"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fixture.setEnvironment(t, secretPath)
|
||||
t.Setenv("THOTHCTL_FAKE_CONFIG", `{"services":{"core":{"image":"thothii-core:local"},"session-migrate":{"image":"thothii-core:local"}}}`)
|
||||
t.Setenv("THOTHCTL_FAKE_MIGRATION_FAILURE", "TLS rejected migration-secret-value")
|
||||
t.Setenv("THOTHCTL_FAKE_MIGRATION_EXIT", "23")
|
||||
var stdout, stderr bytes.Buffer
|
||||
|
||||
code := run(context.Background(), []string{
|
||||
"--installation", fixture.installationPath, "sessions", "migrate", "--yes",
|
||||
}, &stdout, &stderr)
|
||||
|
||||
if code != 1 {
|
||||
t.Fatalf("exit = %d, stderr = %q", code, stderr.String())
|
||||
}
|
||||
for _, required := range []string{"stage=session-migration", "class=nonzero-exit", "TLS rejected [REDACTED]"} {
|
||||
if !strings.Contains(stderr.String(), required) {
|
||||
t.Errorf("stderr = %q, missing %q", stderr.String(), required)
|
||||
}
|
||||
}
|
||||
if strings.Contains(stderr.String(), "migration-secret-value") {
|
||||
t.Fatalf("stderr exposed secret: %q", stderr.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunRemoveDisplaysExactInstallationTargetsBeforeConfirmation(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "")
|
||||
fixture.setProfile(t, "server")
|
||||
@@ -660,7 +690,18 @@ printf '%s\n' "$@" >> "$THOTHCTL_FAKE_ARGS"
|
||||
printf '%s\n' -- >> "$THOTHCTL_FAKE_ARGS"
|
||||
case " $* " in
|
||||
*" ps --all --format json core frontend "*) printf '%s\n' "${THOTHCTL_FAKE_STOPPED_PS:-[]}" ;;
|
||||
*" config --format json "*) printf '%s\n' '{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}' ;;
|
||||
*" config --format json "*)
|
||||
if [ -n "${THOTHCTL_FAKE_CONFIG:-}" ]; then
|
||||
printf '%s\n' "$THOTHCTL_FAKE_CONFIG"
|
||||
else
|
||||
printf '%s\n' '{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}'
|
||||
fi ;;
|
||||
*" run --rm --no-deps --no-TTY session-migrate "*)
|
||||
if [ "${THOTHCTL_FAKE_MIGRATION_EXIT:-0}" -ne 0 ]; then
|
||||
printf '%s\n' "$THOTHCTL_FAKE_MIGRATION_FAILURE" >&2
|
||||
exit "$THOTHCTL_FAKE_MIGRATION_EXIT"
|
||||
fi
|
||||
printf '%s\n' '{"applied":[],"drifted":[],"pending":[]}' ;;
|
||||
*" ps --format json "*) printf '%s\n' '[{"Service":"core","State":"running","Health":"healthy"},{"Service":"frontend","State":"running","Health":"healthy"}]' ;;
|
||||
*"io.thothii.pi.version"*) printf '%s\n' '0.80.3' ;;
|
||||
*"PI_VERSION"*) printf '%s\n' '0.80.3' ;;
|
||||
@@ -707,6 +748,9 @@ func (f cliFixture) setEnvContents(t *testing.T, env string) {
|
||||
t.Setenv("THOTHCTL_FAKE_FAILURE", "")
|
||||
t.Setenv("THOTHCTL_FAKE_FAIL_ON", "")
|
||||
t.Setenv("THOTHCTL_FAKE_STOPPED_PS", "[]")
|
||||
t.Setenv("THOTHCTL_FAKE_CONFIG", "")
|
||||
t.Setenv("THOTHCTL_FAKE_MIGRATION_FAILURE", "")
|
||||
t.Setenv("THOTHCTL_FAKE_MIGRATION_EXIT", "0")
|
||||
}
|
||||
|
||||
func (f cliFixture) setProfile(t *testing.T, profile string) {
|
||||
|
||||
@@ -25,6 +25,50 @@ type Runner interface {
|
||||
Run(context.Context, []string, io.Reader) (compose.Result, error)
|
||||
}
|
||||
|
||||
type Stage string
|
||||
|
||||
const (
|
||||
StageContainerInspection Stage = "container-inspection"
|
||||
StageMigrationConfig Stage = "migration-config"
|
||||
StageMigrationVerification Stage = "migration-config-verification"
|
||||
StageSessionMigration Stage = "session-migration"
|
||||
StageContainerRemoval Stage = "container-removal"
|
||||
StageRemovalVerification Stage = "removal-verification"
|
||||
)
|
||||
|
||||
type ExitClass string
|
||||
|
||||
const (
|
||||
ExitClassNonzero ExitClass = "nonzero-exit"
|
||||
ExitClassUnavailable ExitClass = "unavailable"
|
||||
ExitClassTimeout ExitClass = "timeout"
|
||||
ExitClassInvocation ExitClass = "invocation-failure"
|
||||
)
|
||||
|
||||
// OperationError reports only allowlisted operation metadata from Error. Detail remains bounded
|
||||
// and is exposed separately so the CLI can redact declared secrets before displaying it.
|
||||
type OperationError struct {
|
||||
stage Stage
|
||||
class ExitClass
|
||||
detail string
|
||||
}
|
||||
|
||||
func (e *OperationError) Error() string {
|
||||
return fmt.Sprintf("stage=%s class=%s", e.stage, e.class)
|
||||
}
|
||||
|
||||
func (e *OperationError) Stage() Stage {
|
||||
return e.stage
|
||||
}
|
||||
|
||||
func (e *OperationError) Class() ExitClass {
|
||||
return e.class
|
||||
}
|
||||
|
||||
func (e *OperationError) Detail() string {
|
||||
return e.detail
|
||||
}
|
||||
|
||||
type MigrationStatus struct {
|
||||
Applied []string `json:"applied"`
|
||||
Drifted []string `json:"drifted"`
|
||||
@@ -51,7 +95,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn
|
||||
if installation.Profile != "server" {
|
||||
return MigrationStatus{}, fmt.Errorf("%w: session migration requires a server installation", ErrUnsafeState)
|
||||
}
|
||||
containers, err := inspectContainers(ctx, installation, runner)
|
||||
containers, err := inspectContainers(ctx, installation, runner, StageContainerInspection)
|
||||
if err != nil {
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
@@ -59,7 +103,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
|
||||
rendered, err := runCompose(ctx, runner, installation.ComposeArgs("--profile", "session-migrate", "config", "--format", "json"))
|
||||
rendered, err := runCompose(ctx, runner, StageMigrationConfig, installation.ComposeArgs("--profile", "session-migrate", "config", "--format", "json"))
|
||||
if err != nil {
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
@@ -77,7 +121,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn
|
||||
if err != nil {
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
finalConfig, err := runCompose(ctx, runner, configArgs)
|
||||
finalConfig, err := runCompose(ctx, runner, StageMigrationVerification, configArgs)
|
||||
if err != nil {
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
@@ -90,7 +134,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn
|
||||
if err != nil {
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
result, err := runCompose(ctx, runner, runArgs)
|
||||
result, err := runCompose(ctx, runner, StageSessionMigration, runArgs)
|
||||
if err != nil {
|
||||
return MigrationStatus{}, err
|
||||
}
|
||||
@@ -110,7 +154,7 @@ func Remove(ctx context.Context, installation config.Installation, runner Runner
|
||||
if installation.Profile != "server" {
|
||||
return RemovalResult{}, fmt.Errorf("%w: removal requires a server installation", ErrUnsafeState)
|
||||
}
|
||||
targets, err := inspectContainers(ctx, installation, runner)
|
||||
targets, err := inspectContainers(ctx, installation, runner, StageContainerInspection)
|
||||
result := RemovalResult{Targets: targets}
|
||||
if err != nil {
|
||||
return result, err
|
||||
@@ -137,11 +181,11 @@ func Remove(ctx context.Context, installation config.Installation, runner Runner
|
||||
for _, target := range targets {
|
||||
args = append(args, target.ID)
|
||||
}
|
||||
if _, err := runDocker(ctx, runner, args); err != nil {
|
||||
if _, err := runDocker(ctx, runner, StageContainerRemoval, args); err != nil {
|
||||
return result, err
|
||||
}
|
||||
}
|
||||
remaining, err := inspectContainers(ctx, installation, runner)
|
||||
remaining, err := inspectContainers(ctx, installation, runner, StageRemovalVerification)
|
||||
if err != nil {
|
||||
return result, err
|
||||
}
|
||||
@@ -177,8 +221,8 @@ func sameTargetIDs(targets []Container, confirmed []string) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func inspectContainers(ctx context.Context, installation config.Installation, runner Runner) ([]Container, error) {
|
||||
result, err := runCompose(ctx, runner, installation.ComposeArgs("ps", "--all", "--format", "json", "core", "frontend"))
|
||||
func inspectContainers(ctx context.Context, installation config.Installation, runner Runner, stage Stage) ([]Container, error) {
|
||||
result, err := runCompose(ctx, runner, stage, installation.ComposeArgs("ps", "--all", "--format", "json", "core", "frontend"))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -320,14 +364,44 @@ func verifySnapshots(snapshots []pathSnapshot) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func runCompose(ctx context.Context, runner Runner, args []string) (compose.Result, error) {
|
||||
return runDocker(ctx, runner, args)
|
||||
func runCompose(ctx context.Context, runner Runner, stage Stage, args []string) (compose.Result, error) {
|
||||
return runDocker(ctx, runner, stage, args)
|
||||
}
|
||||
|
||||
func runDocker(ctx context.Context, runner Runner, args []string) (compose.Result, error) {
|
||||
func runDocker(ctx context.Context, runner Runner, stage Stage, args []string) (compose.Result, error) {
|
||||
result, err := runner.Run(ctx, args, nil)
|
||||
if err != nil {
|
||||
return result, errors.New("Docker operation failed")
|
||||
class := ExitClassInvocation
|
||||
switch {
|
||||
case errors.Is(ctx.Err(), context.DeadlineExceeded):
|
||||
class = ExitClassTimeout
|
||||
case result.ExitCode == 127:
|
||||
class = ExitClassUnavailable
|
||||
case result.ExitCode != 0:
|
||||
class = ExitClassNonzero
|
||||
}
|
||||
detail := result.Stderr
|
||||
if strings.TrimSpace(detail) == "" {
|
||||
detail = err.Error()
|
||||
}
|
||||
return result, &OperationError{stage: stage, class: class, detail: boundedDetail(detail)}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
func boundedDetail(detail string) string {
|
||||
detail = strings.Join(strings.Fields(detail), " ")
|
||||
const maximumBytes = 512
|
||||
if len(detail) <= maximumBytes {
|
||||
return detail
|
||||
}
|
||||
var bounded strings.Builder
|
||||
for _, character := range detail {
|
||||
encoded := string(character)
|
||||
if bounded.Len()+len(encoded) > maximumBytes {
|
||||
break
|
||||
}
|
||||
bounded.WriteString(encoded)
|
||||
}
|
||||
return bounded.String()
|
||||
}
|
||||
|
||||
@@ -131,6 +131,41 @@ func TestMigrateSessionsFailsClosedBeforeMutation(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestMigrateSessionsReturnsTypedBoundedRunFailure(t *testing.T) {
|
||||
installation := testInstallation(t)
|
||||
secret := "database-password-in-stderr"
|
||||
configCalls := 0
|
||||
runner := &fakeRunner{run: func(args []string) (compose.Result, error) {
|
||||
switch {
|
||||
case contains(args, "ps", "--all"):
|
||||
return compose.Result{Stdout: `[]`}, nil
|
||||
case contains(args, "config", "--format", "json"):
|
||||
configCalls++
|
||||
return compose.Result{Stdout: `{"services":{"core":{"image":"thothii-core:local"},"session-migrate":{"image":"thothii-core:local"}}}`}, nil
|
||||
case contains(args, "run", "--rm", "--no-deps", "--no-TTY", "session-migrate"):
|
||||
return compose.Result{Stderr: "TLS connection for " + secret + ": " + strings.Repeat("x", 2048), ExitCode: 23}, errors.New("exit status 23")
|
||||
default:
|
||||
t.Fatalf("unexpected Docker invocation: %#v", args)
|
||||
return compose.Result{}, nil
|
||||
}
|
||||
}}
|
||||
|
||||
_, err := MigrateSessions(context.Background(), installation, runner, true)
|
||||
var operationErr *OperationError
|
||||
if !errors.As(err, &operationErr) {
|
||||
t.Fatalf("MigrateSessions() error = %T %v, want OperationError", err, err)
|
||||
}
|
||||
if operationErr.Stage() != StageSessionMigration || operationErr.Class() != ExitClassNonzero {
|
||||
t.Fatalf("operation error = %#v", operationErr)
|
||||
}
|
||||
if detail := operationErr.Detail(); !strings.Contains(detail, secret) || len(detail) > 512 {
|
||||
t.Fatalf("bounded detail length=%d value=%q", len(detail), detail)
|
||||
}
|
||||
if configCalls != 2 {
|
||||
t.Fatalf("config calls = %d", configCalls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemovePreservesEveryDeclaredBindSecretAndBackup(t *testing.T) {
|
||||
installation, preserved := removalInstallation(t)
|
||||
psCalls := 0
|
||||
|
||||
Reference in New Issue
Block a user