fix: harden thothctl diagnostics
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
FROM golang:1.24 AS build
|
||||
FROM golang:1.24@sha256:d2d2bc1c84f7e60d7d2438a3836ae7d0c847f4888464e7ec9ba3a1339a1ee804 AS build
|
||||
|
||||
WORKDIR /src/tools/thothctl
|
||||
COPY tools/thothctl/go.mod tools/thothctl/go.sum ./
|
||||
|
||||
Executable
+27
@@ -0,0 +1,27 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
repository_root=$(cd "$(dirname "$0")/.." && pwd)
|
||||
dockerfile="$repository_root/docker/thothctl.Dockerfile"
|
||||
builder_image=$(awk '$1 == "FROM" && $3 == "AS" && $4 == "build" { print $2; exit }' "$dockerfile")
|
||||
|
||||
if [[ ! "$builder_image" =~ ^golang:1\.24@sha256:[0-9a-f]{64}$ ]]; then
|
||||
echo "thothctl builder must use a readable golang:1.24 tag with an immutable digest" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
manifest=$(docker buildx imagetools inspect "$builder_image")
|
||||
printf '%s\n' "$manifest" | grep -Eq 'Platform:[[:space:]]+linux/amd64'
|
||||
printf '%s\n' "$manifest" | grep -Eq 'Platform:[[:space:]]+linux/arm64'
|
||||
|
||||
temporary_output=$(mktemp -d)
|
||||
trap 'rm -rf "$temporary_output"' EXIT HUP INT TERM
|
||||
docker build --file "$dockerfile" --output "type=local,dest=$temporary_output" "$repository_root" >/dev/null
|
||||
|
||||
test -s "$temporary_output/thothctl-windows-amd64.exe"
|
||||
test -s "$temporary_output/thothctl-darwin-amd64"
|
||||
test -s "$temporary_output/thothctl-darwin-arm64"
|
||||
test -s "$temporary_output/thothctl-linux-amd64"
|
||||
test -s "$temporary_output/thothctl-linux-arm64"
|
||||
|
||||
echo "thothctl build contract passed."
|
||||
@@ -47,6 +47,16 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int {
|
||||
fmt.Fprintf(stderr, "thothctl: %s\n", output.Sanitize(err.Error(), nil))
|
||||
return 2
|
||||
}
|
||||
secretFiles, err := installation.SecretFiles()
|
||||
if err != nil {
|
||||
fmt.Fprintln(stderr, "thothctl: installation secret declarations could not be read")
|
||||
return 2
|
||||
}
|
||||
secretValues, err := output.SecretValuesFromFiles(secretFiles)
|
||||
if err != nil {
|
||||
fmt.Fprintln(stderr, "thothctl: declared secret file could not be read")
|
||||
return 2
|
||||
}
|
||||
|
||||
runner := compose.NewRunner("")
|
||||
var result compose.Result
|
||||
@@ -81,11 +91,11 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int {
|
||||
if len(commandArgs) != 0 {
|
||||
return commandUsageError(stderr, "doctor does not accept arguments")
|
||||
}
|
||||
return doctor(ctx, installation, runner, stdout, stderr)
|
||||
return doctor(ctx, installation, runner, secretValues, stdout, stderr)
|
||||
default:
|
||||
return commandUsageError(stderr, fmt.Sprintf("unknown command %q", command))
|
||||
}
|
||||
return writeResult(result, err, stdout, stderr)
|
||||
return writeResult(result, err, secretValues, stdout, stderr)
|
||||
}
|
||||
|
||||
func parseArgs(args []string) (string, string, []string, error) {
|
||||
@@ -113,12 +123,12 @@ func commandUsageError(stderr io.Writer, message string) int {
|
||||
return 2
|
||||
}
|
||||
|
||||
func writeResult(result compose.Result, err error, stdout, stderr io.Writer) int {
|
||||
func writeResult(result compose.Result, err error, secretValues []string, stdout, stderr io.Writer) int {
|
||||
if result.Stdout != "" {
|
||||
fmt.Fprint(stdout, output.Sanitize(result.Stdout, nil))
|
||||
fmt.Fprint(stdout, output.Sanitize(result.Stdout, secretValues))
|
||||
}
|
||||
if result.Stderr != "" {
|
||||
fmt.Fprint(stderr, output.Sanitize(result.Stderr, nil))
|
||||
fmt.Fprint(stderr, output.Sanitize(result.Stderr, secretValues))
|
||||
}
|
||||
if err == nil {
|
||||
return 0
|
||||
@@ -132,7 +142,7 @@ func writeResult(result compose.Result, err error, stdout, stderr io.Writer) int
|
||||
return 1
|
||||
}
|
||||
|
||||
func doctor(ctx context.Context, installation config.Installation, runner compose.Runner, stdout, stderr io.Writer) int {
|
||||
func doctor(ctx context.Context, installation config.Installation, runner compose.Runner, secretValues []string, stdout, stderr io.Writer) int {
|
||||
checks := [][]string{
|
||||
{"version", "--format", "{{.Client.Version}}"},
|
||||
{"compose", "version", "--short"},
|
||||
@@ -144,7 +154,7 @@ func doctor(ctx context.Context, installation config.Installation, runner compos
|
||||
for index, args := range checks {
|
||||
result, err := runner.Run(ctx, args, nil)
|
||||
if err != nil {
|
||||
return writeResult(result, err, stdout, stderr)
|
||||
return writeResult(result, err, secretValues, stdout, stderr)
|
||||
}
|
||||
if index == 3 {
|
||||
renderedConfig = result.Stdout
|
||||
|
||||
@@ -0,0 +1,205 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestRunLogsRedactsAnUnlabelledDeclaredSecret(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "UNLABELLED_SECRET_FILE=%s\n")
|
||||
secretPath := filepath.Join(fixture.root, "operator-secret")
|
||||
if err := os.WriteFile(secretPath, []byte("unlabelled-secret\r\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fixture.setEnvironment(t, secretPath)
|
||||
t.Setenv("THOTHCTL_FAKE_LOG", "fake Docker log: unlabelled-secret")
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "logs"}, &stdout, &stderr)
|
||||
|
||||
if exitCode != 0 {
|
||||
t.Fatalf("run() exit code = %d, stderr = %s", exitCode, stderr.String())
|
||||
}
|
||||
if strings.Contains(stdout.String(), "unlabelled-secret") {
|
||||
t.Fatalf("logs exposed an unlabelled secret: %q", stdout.String())
|
||||
}
|
||||
if stdout.String() != "fake Docker log: [REDACTED]\n" {
|
||||
t.Errorf("logs = %q, want redacted output", stdout.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunStatusUsesStableComposeArguments(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "SAFE_VALUE=1\n")
|
||||
fixture.setEnvironment(t)
|
||||
|
||||
for range 2 {
|
||||
var stdout, stderr bytes.Buffer
|
||||
if exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "status"}, &stdout, &stderr); exitCode != 0 {
|
||||
t.Fatalf("run() exit code = %d, stderr = %s", exitCode, stderr.String())
|
||||
}
|
||||
}
|
||||
|
||||
invocations := fixture.invocations(t)
|
||||
if len(invocations) != 2 {
|
||||
t.Fatalf("docker invocations = %d, want 2", len(invocations))
|
||||
}
|
||||
if strings.Join(invocations[0], "\x00") != strings.Join(invocations[1], "\x00") {
|
||||
t.Errorf("Compose arguments changed between identical status calls: %#v then %#v", invocations[0], invocations[1])
|
||||
}
|
||||
wantSuffix := []string{
|
||||
"--project-directory", fixture.projectDirectory,
|
||||
"--env-file", fixture.envFile,
|
||||
"-f", filepath.Join(fixture.projectDirectory, "compose.yaml"),
|
||||
"-f", filepath.Join(fixture.projectDirectory, "deploy", "compose.local.yaml"),
|
||||
"ps", "--format", "json",
|
||||
}
|
||||
got := invocations[0]
|
||||
if len(got) != len(wantSuffix)+3 || got[0] != "compose" || got[1] != "--project-name" || !strings.HasPrefix(got[2], "thothii-") {
|
||||
t.Fatalf("unexpected Compose prefix: %#v", got)
|
||||
}
|
||||
for index, want := range wantSuffix {
|
||||
if got[index+3] != want {
|
||||
t.Errorf("argument %d = %q, want %q", index+3, got[index+3], want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunExplainsWhenDockerIsNotAvailable(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "SAFE_VALUE=1\n")
|
||||
fixture.setEnvironment(t)
|
||||
t.Setenv("PATH", t.TempDir())
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "status"}, &stdout, &stderr)
|
||||
|
||||
if exitCode != 127 {
|
||||
t.Errorf("run() exit code = %d, want 127", exitCode)
|
||||
}
|
||||
if !strings.Contains(stderr.String(), "Docker is not installed or is not on PATH") {
|
||||
t.Errorf("stderr = %q, want Docker-not-found guidance", stderr.String())
|
||||
}
|
||||
if strings.Contains(stderr.String(), "executable file") {
|
||||
t.Errorf("stderr leaked a process implementation detail: %q", stderr.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunDoctorValidatesTheRenderedInstallation(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "SAFE_VALUE=1\n")
|
||||
fixture.setEnvironment(t)
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "doctor"}, &stdout, &stderr)
|
||||
|
||||
if exitCode != 0 {
|
||||
t.Fatalf("run() exit code = %d, stderr = %s", exitCode, stderr.String())
|
||||
}
|
||||
if stdout.String() != "Doctor checks passed.\n" {
|
||||
t.Errorf("stdout = %q, want doctor success", stdout.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunPreservesChildExitCodes(t *testing.T) {
|
||||
fixture := newCLIFixture(t, "SAFE_VALUE=1\n")
|
||||
fixture.setEnvironment(t)
|
||||
t.Setenv("THOTHCTL_FAKE_EXIT", "42")
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "update", "--check-only"}, &stdout, &stderr)
|
||||
|
||||
if exitCode != 42 {
|
||||
t.Errorf("run() exit code = %d, want 42", exitCode)
|
||||
}
|
||||
if stderr.String() != "fake Docker failure\n" {
|
||||
t.Errorf("stderr = %q, want child stderr", stderr.String())
|
||||
}
|
||||
}
|
||||
|
||||
type cliFixture struct {
|
||||
root string
|
||||
installationPath string
|
||||
projectDirectory string
|
||||
envFile string
|
||||
argsFile string
|
||||
pathDirectory string
|
||||
envTemplate string
|
||||
}
|
||||
|
||||
func newCLIFixture(t *testing.T, envTemplate string) cliFixture {
|
||||
t.Helper()
|
||||
root := t.TempDir()
|
||||
projectDirectory := filepath.Join(root, "project")
|
||||
if err := os.MkdirAll(filepath.Join(projectDirectory, "deploy"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, path := range []string{filepath.Join(projectDirectory, "compose.yaml"), filepath.Join(projectDirectory, "deploy", "compose.local.yaml")} {
|
||||
if err := os.WriteFile(path, []byte("services: {}\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
envFile := filepath.Join(root, "installation.env")
|
||||
installationPath := filepath.Join(root, "thothii-installation.yaml")
|
||||
contents := "profile: local\nprojectDirectory: " + projectDirectory + "\nenvFile: " + envFile + "\n"
|
||||
if err := os.WriteFile(installationPath, []byte(contents), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
pathDirectory := filepath.Join(root, "bin")
|
||||
if err := os.Mkdir(pathDirectory, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
argsFile := filepath.Join(root, "docker-args")
|
||||
fakeDocker := `#!/bin/sh
|
||||
printf '%s\n' "$@" >> "$THOTHCTL_FAKE_ARGS"
|
||||
printf '%s\n' -- >> "$THOTHCTL_FAKE_ARGS"
|
||||
case " $* " in
|
||||
*" config --format json "*) printf '%s\n' '{"volumes":{"settings":{}}}' ;;
|
||||
*" ps --format json "*) printf '%s\n' '[{"Service":"core","State":"running","Health":"healthy"},{"Service":"frontend","State":"running","Health":"healthy"}]' ;;
|
||||
*" logs "*) printf '%s\n' "$THOTHCTL_FAKE_LOG" ;;
|
||||
esac
|
||||
if [ "${THOTHCTL_FAKE_EXIT:-0}" -ne 0 ]; then
|
||||
printf '%s\n' 'fake Docker failure' >&2
|
||||
fi
|
||||
exit "${THOTHCTL_FAKE_EXIT:-0}"
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(pathDirectory, "docker"), []byte(fakeDocker), 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return cliFixture{root: root, installationPath: installationPath, projectDirectory: projectDirectory, envFile: envFile, argsFile: argsFile, pathDirectory: pathDirectory, envTemplate: envTemplate}
|
||||
}
|
||||
|
||||
func (f cliFixture) setEnvironment(t *testing.T, values ...string) {
|
||||
t.Helper()
|
||||
env := f.envTemplate
|
||||
if len(values) > 0 {
|
||||
env = strings.Replace(env, "%s", values[0], 1)
|
||||
}
|
||||
if err := os.WriteFile(f.envFile, []byte(env), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("PATH", f.pathDirectory)
|
||||
t.Setenv("THOTHCTL_FAKE_ARGS", f.argsFile)
|
||||
t.Setenv("THOTHCTL_FAKE_EXIT", "0")
|
||||
t.Setenv("THOTHCTL_FAKE_LOG", "")
|
||||
}
|
||||
|
||||
func (f cliFixture) invocations(t *testing.T) [][]string {
|
||||
t.Helper()
|
||||
contents, err := os.ReadFile(f.argsFile)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var invocations [][]string
|
||||
var invocation []string
|
||||
for _, line := range strings.Split(strings.TrimSuffix(string(contents), "\n"), "\n") {
|
||||
if line == "--" {
|
||||
invocations = append(invocations, invocation)
|
||||
invocation = nil
|
||||
continue
|
||||
}
|
||||
invocation = append(invocation, line)
|
||||
}
|
||||
return invocations
|
||||
}
|
||||
@@ -8,12 +8,15 @@ import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
const installationFileName = "thothii-installation.yaml"
|
||||
|
||||
const maxEnvironmentFileBytes = 1 << 20
|
||||
|
||||
type descriptor struct {
|
||||
Profile string `yaml:"profile"`
|
||||
ProjectDirectory string `yaml:"projectDirectory"`
|
||||
@@ -116,6 +119,62 @@ func (i Installation) ComposeArgs(command ...string) []string {
|
||||
return append(args, command...)
|
||||
}
|
||||
|
||||
// SecretFiles returns only existing, absolute regular files declared in the installation env file
|
||||
// through *_FILE or *_SOURCE variables. Missing paths are allowed because /run/secrets paths are
|
||||
// container-local declarations, not host files thothctl can read.
|
||||
func (i Installation) SecretFiles() ([]string, error) {
|
||||
info, err := os.Stat(i.EnvFile)
|
||||
if err != nil || info.Size() > maxEnvironmentFileBytes {
|
||||
return nil, errors.New("installation secret declarations could not be read")
|
||||
}
|
||||
contents, err := os.ReadFile(i.EnvFile)
|
||||
if err != nil || len(contents) > maxEnvironmentFileBytes {
|
||||
return nil, errors.New("installation secret declarations could not be read")
|
||||
}
|
||||
|
||||
files := make([]string, 0)
|
||||
seen := make(map[string]struct{})
|
||||
for _, line := range strings.Split(string(contents), "\n") {
|
||||
key, value, ok := environmentAssignment(line)
|
||||
if !ok || (!strings.HasSuffix(key, "_FILE") && !strings.HasSuffix(key, "_SOURCE")) || !filepath.IsAbs(value) {
|
||||
continue
|
||||
}
|
||||
fileInfo, err := os.Lstat(value)
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
continue
|
||||
}
|
||||
if err != nil || !fileInfo.Mode().IsRegular() {
|
||||
return nil, errors.New("installation secret declarations could not be read")
|
||||
}
|
||||
if _, exists := seen[value]; !exists {
|
||||
files = append(files, value)
|
||||
seen[value] = struct{}{}
|
||||
}
|
||||
}
|
||||
return files, nil
|
||||
}
|
||||
|
||||
func environmentAssignment(line string) (string, string, bool) {
|
||||
line = strings.TrimSpace(line)
|
||||
if line == "" || strings.HasPrefix(line, "#") {
|
||||
return "", "", false
|
||||
}
|
||||
line = strings.TrimPrefix(line, "export ")
|
||||
key, value, found := strings.Cut(line, "=")
|
||||
if !found {
|
||||
return "", "", false
|
||||
}
|
||||
key = strings.TrimSpace(key)
|
||||
if key == "" {
|
||||
return "", "", false
|
||||
}
|
||||
value = strings.TrimSpace(value)
|
||||
if len(value) >= 2 && ((value[0] == '"' && value[len(value)-1] == '"') || (value[0] == '\'' && value[len(value)-1] == '\'')) {
|
||||
value = value[1 : len(value)-1]
|
||||
}
|
||||
return strings.ToUpper(key), value, true
|
||||
}
|
||||
|
||||
func ensureOnlyOneDocument(decoder *yaml.Decoder) error {
|
||||
var extra any
|
||||
err := decoder.Decode(&extra)
|
||||
|
||||
@@ -2,7 +2,8 @@
|
||||
package output
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"errors"
|
||||
"io"
|
||||
"os"
|
||||
"regexp"
|
||||
"sort"
|
||||
@@ -11,6 +12,8 @@ import (
|
||||
|
||||
var credentialField = regexp.MustCompile(`(?im)(\b[\w.-]*(?:password|token|key)[\w.-]*\s*[:=]\s*)(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s,;]+)`)
|
||||
|
||||
const maxSecretFileBytes = 64 * 1024
|
||||
|
||||
// Sanitize redacts common credential fields and every supplied secret value.
|
||||
func Sanitize(text string, secretValues []string) string {
|
||||
text = credentialField.ReplaceAllString(text, "${1}[REDACTED]")
|
||||
@@ -27,14 +30,36 @@ func Sanitize(text string, secretValues []string) string {
|
||||
// SecretValuesFromFiles reads non-empty secret-file contents without exposing them to callers.
|
||||
func SecretValuesFromFiles(paths []string) ([]string, error) {
|
||||
values := make([]string, 0, len(paths))
|
||||
seen := make(map[string]struct{})
|
||||
for _, path := range paths {
|
||||
contents, err := os.ReadFile(path)
|
||||
value, err := readSecretFile(path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read secret file: %w", err)
|
||||
return nil, err
|
||||
}
|
||||
if value := strings.TrimSpace(string(contents)); value != "" {
|
||||
if value != "" {
|
||||
if _, exists := seen[value]; exists {
|
||||
continue
|
||||
}
|
||||
values = append(values, value)
|
||||
seen[value] = struct{}{}
|
||||
}
|
||||
}
|
||||
return values, nil
|
||||
}
|
||||
|
||||
func readSecretFile(path string) (string, error) {
|
||||
info, err := os.Lstat(path)
|
||||
if err != nil || !info.Mode().IsRegular() || info.Size() > maxSecretFileBytes {
|
||||
return "", errors.New("declared secret file could not be read")
|
||||
}
|
||||
file, err := os.Open(path)
|
||||
if err != nil {
|
||||
return "", errors.New("declared secret file could not be read")
|
||||
}
|
||||
defer file.Close()
|
||||
contents, err := io.ReadAll(io.LimitReader(file, maxSecretFileBytes+1))
|
||||
if err != nil || len(contents) > maxSecretFileBytes {
|
||||
return "", errors.New("declared secret file could not be read")
|
||||
}
|
||||
return strings.TrimRight(string(contents), "\r\n"), nil
|
||||
}
|
||||
|
||||
@@ -33,3 +33,16 @@ func TestSanitizeRedactsSecretFileContents(t *testing.T) {
|
||||
t.Errorf("Sanitize() = %q, want redacted secret", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSecretValuesFromFilesRejectsOversizedFiles(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
secretFile := filepath.Join(t.TempDir(), "oversized-token")
|
||||
if err := os.WriteFile(secretFile, make([]byte, 64*1024+1), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if _, err := SecretValuesFromFiles([]string{secretFile}); err == nil {
|
||||
t.Fatal("SecretValuesFromFiles() error = nil, want oversized-file error")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user