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

fix: close server bypass edge cases

Browse Source
This commit is contained in:
marcopan
2026-08-05 12:09:32 +02:00
parent a94affd6ac
commit fc349e634c
9 changed files with 296 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/thothctl/cmd/thothctl/main.go
+1 -1
View File
@@ -175,7 +175,7 @@ func serverOperationFailure(stderr io.Writer, err error, secretValues []string)
message := output.Sanitize(err.Error(), secretValues)
var operationErr *serverops.OperationError
if errors.As(err, &operationErr) && operationErr.Detail() != "" {
detail := output.Sanitize(operationErr.Detail(), secretValues)
detail := output.SanitizeDetail(operationErr.Detail(), secretValues)
fmt.Fprintf(stderr, "thothctl: %s: %s\n", message, detail)
} else {
fmt.Fprintf(stderr, "thothctl: %s\n", message)
tools/thothctl/cmd/thothctl/main_test.go
+51 -25
View File
@@ -104,33 +104,59 @@ 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
func TestRunSessionsMigrateRedactsCompleteDetailBeforeDisplayBound(t *testing.T) {
longSecret := "long-secret-" + strings.Repeat("s", 700)
for _, spec := range []struct {
name string
secret string
failure string
leakedProbe string
}{
{
name: "secret longer than display limit",
secret: longSecret,
failure: longSecret + " rejected by TLS",
leakedProbe: longSecret[:64],
},
{
name: "secret crossing display boundary",
secret: "boundary-secret-value",
failure: strings.Repeat("p", 500) + "boundary-secret-value rejected",
leakedProbe: "boundary-sec",
},
} {
t.Run(spec.name, func(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(spec.secret), 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", spec.failure)
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)
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())
if code != 1 {
t.Fatalf("exit = %d, stderr = %q", code, stderr.String())
}
for _, required := range []string{"stage=session-migration", "class=nonzero-exit", "[REDACTED]"} {
if !strings.Contains(stderr.String(), required) {
t.Errorf("stderr = %q, missing %q", stderr.String(), required)
}
}
if strings.Contains(stderr.String(), spec.secret) || strings.Contains(stderr.String(), spec.leakedProbe) {
t.Fatalf("stderr exposed secret or prefix: %q", stderr.String())
}
if stderr.Len() > 640 {
t.Fatalf("stderr exceeded bounded display: %d bytes", stderr.Len())
}
})
}
}
tools/thothctl/internal/output/sanitize.go
+20
View File
@@ -18,6 +18,8 @@ const maxSecretSourceFiles = 32
const maxSecretSourceBytes = 256 * 1024
const maxDiagnosticDetailBytes = 512
// Sanitize redacts common credential fields and every supplied secret value.
func Sanitize(text string, secretValues []string) string {
text = credentialField.ReplaceAllString(text, "${1}[REDACTED]")
@@ -31,6 +33,24 @@ func Sanitize(text string, secretValues []string) string {
return text
}
// SanitizeDetail redacts the complete subprocess detail before normalizing and bounding the text
// that may be displayed at the CLI boundary.
func SanitizeDetail(text string, secretValues []string) string {
detail := strings.Join(strings.Fields(Sanitize(text, secretValues)), " ")
if len(detail) <= maxDiagnosticDetailBytes {
return detail
}
var bounded strings.Builder
for _, character := range detail {
encoded := string(character)
if bounded.Len()+len(encoded) > maxDiagnosticDetailBytes {
break
}
bounded.WriteString(encoded)
}
return bounded.String()
}
// SecretValuesFromFiles reads non-empty secret-file contents without exposing them to callers.
func SecretValuesFromFiles(paths []string) ([]string, error) {
if len(paths) > maxSecretSourceFiles {
tools/thothctl/internal/serverops/operations.go
+3 -20
View File
@@ -45,8 +45,8 @@ const (
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.
// OperationError reports only allowlisted operation metadata from Error. Complete subprocess
// detail is exposed separately so the CLI can redact it before applying its display bound.
type OperationError struct {
stage Stage
class ExitClass
@@ -384,24 +384,7 @@ func runDocker(ctx context.Context, runner Runner, stage Stage, args []string) (
if strings.TrimSpace(detail) == "" {
detail = err.Error()
}
return result, &OperationError{stage: stage, class: class, detail: boundedDetail(detail)}
return result, &OperationError{stage: stage, class: class, detail: 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()
}
tools/thothctl/internal/serverops/operations_test.go
+45 -31
View File
@@ -131,38 +131,52 @@ 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
}
}}
func TestMigrateSessionsPreservesCompleteFailureDetailBehindTypedMetadata(t *testing.T) {
longSecret := "long-secret-" + strings.Repeat("s", 700)
for _, spec := range []struct {
name string
secret string
stderr string
}{
{name: "secret longer than display limit", secret: longSecret, stderr: longSecret + " rejected"},
{name: "secret crossing display boundary", secret: "boundary-secret-value", stderr: strings.Repeat("p", 500) + "boundary-secret-value rejected"},
} {
t.Run(spec.name, func(t *testing.T) {
installation := testInstallation(t)
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: spec.stderr, 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)
_, 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 strings.Contains(operationErr.Error(), spec.secret[:12]) {
t.Fatalf("typed metadata exposed secret prefix: %q", operationErr.Error())
}
if detail := operationErr.Detail(); detail != spec.stderr || !strings.Contains(detail, spec.secret) {
t.Fatalf("detail was truncated before redaction: length=%d", len(detail))
}
if configCalls != 2 {
t.Fatalf("config calls = %d", configCalls)
}
})
}
}