From 35a000222c8e4e872e568599fc5be77dc448e899 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 4 Aug 2026 17:17:39 +0200 Subject: [PATCH] fix: fail closed thothctl secret sources --- tools/thothctl/cmd/thothctl/main_test.go | 207 +++++++++++++++++- tools/thothctl/go.mod | 6 + tools/thothctl/go.sum | 21 ++ .../thothctl/internal/config/installation.go | 85 ++++--- tools/thothctl/internal/output/sanitize.go | 15 +- .../thothctl/internal/output/sanitize_test.go | 18 +- tools/thothctl/internal/safeio/files.go | 63 ++++++ 7 files changed, 366 insertions(+), 49 deletions(-) create mode 100644 tools/thothctl/internal/safeio/files.go diff --git a/tools/thothctl/cmd/thothctl/main_test.go b/tools/thothctl/cmd/thothctl/main_test.go index 37ae7b3d..41973010 100644 --- a/tools/thothctl/cmd/thothctl/main_test.go +++ b/tools/thothctl/cmd/thothctl/main_test.go @@ -32,6 +32,190 @@ func TestRunLogsRedactsAnUnlabelledDeclaredSecret(t *testing.T) { } } +func TestRunResolvesComposeDotenvCommentsQuotesAndInterpolationForSecretFiles(t *testing.T) { + fixture := newCLIFixture(t, "") + secretDirectory := filepath.Join(fixture.root, "secret directory") + if err := os.Mkdir(secretDirectory, 0o700); err != nil { + t.Fatal(err) + } + inlineSecret := filepath.Join(secretDirectory, "inline") + doubleQuotedSecret := filepath.Join(secretDirectory, "double quoted") + singleQuotedSecret := filepath.Join(secretDirectory, "single quoted") + interpolatedSecret := filepath.Join(secretDirectory, "interpolated") + for path, value := range map[string]string{ + inlineSecret: "inline-secret", + doubleQuotedSecret: "double-quoted-secret", + singleQuotedSecret: "single-quoted-secret", + interpolatedSecret: "interpolated-secret", + } { + if err := os.WriteFile(path, []byte(value), 0o600); err != nil { + t.Fatal(err) + } + } + fixture.setEnvContents(t, "SECRET_ROOT="+secretDirectory+"\n"+ + "INLINE_TOKEN_FILE="+inlineSecret+" # Compose comment\n"+ + "DOUBLE_TOKEN_FILE=\""+doubleQuotedSecret+"\" # Compose comment\n"+ + "SINGLE_TOKEN_FILE='"+singleQuotedSecret+"' # Compose comment\n"+ + "INTERPOLATED_TOKEN_SOURCE=\"${SECRET_ROOT}/interpolated\"\n") + t.Setenv("THOTHCTL_FAKE_LOG", "inline-secret double-quoted-secret single-quoted-secret interpolated-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()) + } + for _, secret := range []string{"inline-secret", "double-quoted-secret", "single-quoted-secret", "interpolated-secret"} { + if strings.Contains(stdout.String(), secret) { + t.Errorf("logs exposed %q: %q", secret, stdout.String()) + } + } +} + +func TestRunRedactsSecretSourceInBothStreams(t *testing.T) { + fixture := newCLIFixture(t, "") + secretPath := filepath.Join(fixture.root, "source-secret") + if err := os.WriteFile(secretPath, []byte("source-secret"), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvContents(t, "UNLABELLED_SECRET_SOURCE="+secretPath+"\n") + t.Setenv("THOTHCTL_FAKE_LOG", "stdout source-secret") + t.Setenv("THOTHCTL_FAKE_FAILURE", "stderr source-secret") + t.Setenv("THOTHCTL_FAKE_EXIT", "17") + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "logs"}, &stdout, &stderr) + + if exitCode != 17 { + t.Errorf("run() exit code = %d, want 17", exitCode) + } + if strings.Contains(stdout.String()+stderr.String(), "source-secret") { + t.Errorf("output exposed source secret: stdout=%q stderr=%q", stdout.String(), stderr.String()) + } +} + +func TestRunRedactsSecretWhenDoctorFails(t *testing.T) { + fixture := newCLIFixture(t, "") + secretPath := filepath.Join(fixture.root, "doctor-secret") + if err := os.WriteFile(secretPath, []byte("doctor-secret"), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvContents(t, "DOCTOR_SECRET_FILE="+secretPath+"\n") + t.Setenv("THOTHCTL_FAKE_FAIL_ON", "version") + t.Setenv("THOTHCTL_FAKE_FAILURE", "doctor saw doctor-secret") + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "doctor"}, &stdout, &stderr) + + if exitCode != 41 { + t.Errorf("run() exit code = %d, want 41", exitCode) + } + if strings.Contains(stdout.String()+stderr.String(), "doctor-secret") { + t.Errorf("doctor failure exposed secret: stdout=%q stderr=%q", stdout.String(), stderr.String()) + } +} + +func TestRunFailsClosedForUnresolvedSecretSourceInterpolation(t *testing.T) { + fixture := newCLIFixture(t, "MISSING_TOKEN_SOURCE=${MISSING_SECRET_ROOT}/token\n") + fixture.setEnvironment(t) + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "logs"}, &stdout, &stderr) + + if exitCode != 2 { + t.Errorf("run() exit code = %d, want 2", exitCode) + } + if !strings.Contains(stderr.String(), "installation secret declarations could not be read") { + t.Errorf("stderr = %q, want fail-closed declaration error", stderr.String()) + } + if _, err := os.Stat(fixture.argsFile); !os.IsNotExist(err) { + t.Errorf("Docker was invoked after unresolved interpolation: stat error = %v", err) + } +} + +func TestRunFailsClosedForTraversalAndParentSymlinkSecretSources(t *testing.T) { + for name, source := range map[string]func(*testing.T, cliFixture) string{ + "traversal": func(t *testing.T, fixture cliFixture) string { + secret := filepath.Join(fixture.root, "secret") + if err := os.WriteFile(secret, []byte("traversal-secret"), 0o600); err != nil { + t.Fatal(err) + } + return filepath.Join(fixture.root, "subdirectory") + string(filepath.Separator) + ".." + string(filepath.Separator) + "secret" + }, + "parent symlink": func(t *testing.T, fixture cliFixture) string { + realDirectory := filepath.Join(fixture.root, "real") + if err := os.Mkdir(realDirectory, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(realDirectory, "secret"), []byte("symlink-secret"), 0o600); err != nil { + t.Fatal(err) + } + linkDirectory := filepath.Join(fixture.root, "linked") + if err := os.Symlink(realDirectory, linkDirectory); err != nil { + t.Fatal(err) + } + return filepath.Join(linkDirectory, "secret") + }, + "final symlink": func(t *testing.T, fixture cliFixture) string { + realSecret := filepath.Join(fixture.root, "real-secret") + if err := os.WriteFile(realSecret, []byte("final-symlink-secret"), 0o600); err != nil { + t.Fatal(err) + } + linkSecret := filepath.Join(fixture.root, "linked-secret") + if err := os.Symlink(realSecret, linkSecret); err != nil { + t.Fatal(err) + } + return linkSecret + }, + } { + t.Run(name, func(t *testing.T) { + fixture := newCLIFixture(t, "") + unsafeSource := source(t, fixture) + fixture.setEnvContents(t, "UNSAFE_SECRET_SOURCE="+unsafeSource+"\n") + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "logs"}, &stdout, &stderr) + + if exitCode != 2 { + t.Errorf("run() exit code = %d, want 2", exitCode) + } + if !strings.Contains(stderr.String(), "could not be read") { + t.Errorf("stderr = %q, want sanitized unsafe-file error", stderr.String()) + } + if strings.Contains(stderr.String(), unsafeSource) { + t.Errorf("stderr revealed unsafe source path: %q", stderr.String()) + } + }) + } +} + +func TestRunFailsClosedForOversizedEnvAndSecretFiles(t *testing.T) { + t.Run("environment", func(t *testing.T) { + fixture := newCLIFixture(t, "") + fixture.setEnvContents(t, strings.Repeat("A", 1<<20+1)) + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "logs"}, &stdout, &stderr) + if exitCode != 2 || !strings.Contains(stderr.String(), "installation secret declarations could not be read") { + t.Errorf("exit=%d stderr=%q, want sanitized oversized-env failure", exitCode, stderr.String()) + } + }) + t.Run("secret", func(t *testing.T) { + fixture := newCLIFixture(t, "") + secretPath := filepath.Join(fixture.root, "large-secret") + if err := os.WriteFile(secretPath, make([]byte, 64*1024+1), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvContents(t, "LARGE_SECRET_FILE="+secretPath+"\n") + + var stdout, stderr bytes.Buffer + exitCode := run(context.Background(), []string{"--installation", fixture.installationPath, "logs"}, &stdout, &stderr) + if exitCode != 2 || !strings.Contains(stderr.String(), "declared secret file could not be read") { + t.Errorf("exit=%d stderr=%q, want sanitized oversized-secret failure", exitCode, stderr.String()) + } + }) +} + func TestRunStatusUsesStableComposeArguments(t *testing.T) { fixture := newCLIFixture(t, "SAFE_VALUE=1\n") fixture.setEnvironment(t) @@ -130,7 +314,15 @@ type cliFixture struct { func newCLIFixture(t *testing.T, envTemplate string) cliFixture { t.Helper() - root := t.TempDir() + temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) + if err != nil { + t.Fatal(err) + } + root, err := os.MkdirTemp(temporaryRoot, "thothctl-test-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) projectDirectory := filepath.Join(root, "project") if err := os.MkdirAll(filepath.Join(projectDirectory, "deploy"), 0o755); err != nil { t.Fatal(err) @@ -159,8 +351,12 @@ case " $* " in *" 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_FAIL_ON:-}" = "version" ]; then + printf '%s\n' "${THOTHCTL_FAKE_FAILURE:-fake Docker failure}" >&2 + exit 41 +fi if [ "${THOTHCTL_FAKE_EXIT:-0}" -ne 0 ]; then - printf '%s\n' 'fake Docker failure' >&2 + printf '%s\n' "${THOTHCTL_FAKE_FAILURE:-fake Docker failure}" >&2 fi exit "${THOTHCTL_FAKE_EXIT:-0}" ` @@ -176,6 +372,11 @@ func (f cliFixture) setEnvironment(t *testing.T, values ...string) { if len(values) > 0 { env = strings.Replace(env, "%s", values[0], 1) } + f.setEnvContents(t, env) +} + +func (f cliFixture) setEnvContents(t *testing.T, env string) { + t.Helper() if err := os.WriteFile(f.envFile, []byte(env), 0o600); err != nil { t.Fatal(err) } @@ -183,6 +384,8 @@ func (f cliFixture) setEnvironment(t *testing.T, values ...string) { t.Setenv("THOTHCTL_FAKE_ARGS", f.argsFile) t.Setenv("THOTHCTL_FAKE_EXIT", "0") t.Setenv("THOTHCTL_FAKE_LOG", "") + t.Setenv("THOTHCTL_FAKE_FAILURE", "") + t.Setenv("THOTHCTL_FAKE_FAIL_ON", "") } func (f cliFixture) invocations(t *testing.T) [][]string { diff --git a/tools/thothctl/go.mod b/tools/thothctl/go.mod index 33346ed5..72c13da5 100644 --- a/tools/thothctl/go.mod +++ b/tools/thothctl/go.mod @@ -3,3 +3,9 @@ module github.com/aritmolab/thothii/tools/thothctl go 1.24 require gopkg.in/yaml.v3 v3.0.1 + +require ( + github.com/compose-spec/compose-go/v2 v2.14.0 + github.com/sirupsen/logrus v1.9.0 + golang.org/x/sys v0.5.0 // indirect +) diff --git a/tools/thothctl/go.sum b/tools/thothctl/go.sum index a62c313c..593ae01e 100644 --- a/tools/thothctl/go.sum +++ b/tools/thothctl/go.sum @@ -1,4 +1,25 @@ +github.com/compose-spec/compose-go/v2 v2.14.0 h1:uaJeo5B3+OVlu+Rx2qLBcAdXPEUUzm5nQrRiGJafRAQ= +github.com/compose-spec/compose-go/v2 v2.14.0/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= +github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o= +gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g= diff --git a/tools/thothctl/internal/config/installation.go b/tools/thothctl/internal/config/installation.go index 36f7dae8..8da5f6cb 100644 --- a/tools/thothctl/internal/config/installation.go +++ b/tools/thothctl/internal/config/installation.go @@ -2,6 +2,7 @@ package config import ( + "bytes" "crypto/sha256" "errors" "fmt" @@ -9,7 +10,11 @@ import ( "os" "path/filepath" "strings" + "sync" + "github.com/aritmolab/thothii/tools/thothctl/internal/safeio" + "github.com/compose-spec/compose-go/v2/dotenv" + "github.com/sirupsen/logrus" "gopkg.in/yaml.v3" ) @@ -17,6 +22,8 @@ const installationFileName = "thothii-installation.yaml" const maxEnvironmentFileBytes = 1 << 20 +var dotenvParseMu sync.Mutex + type descriptor struct { Profile string `yaml:"profile"` ProjectDirectory string `yaml:"projectDirectory"` @@ -119,31 +126,27 @@ 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. +// SecretFiles returns canonical local secret paths declared through *_FILE or *_SOURCE variables. +// Compose's dotenv parser resolves comments, quotes, escapes, and interpolation. Unsupported or +// unresolved source interpolation is rejected before thothctl invokes Docker. func (i Installation) SecretFiles() ([]string, error) { - info, err := os.Stat(i.EnvFile) - if err != nil || info.Size() > maxEnvironmentFileBytes { + contents, err := safeio.ReadCanonicalRegular(i.EnvFile, maxEnvironmentFileBytes) + if err != nil { return nil, errors.New("installation secret declarations could not be read") } - contents, err := os.ReadFile(i.EnvFile) - if err != nil || len(contents) > maxEnvironmentFileBytes { + values, err := parseComposeDotenv(contents) + if err != nil { return nil, errors.New("installation secret declarations could not be read") } - files := make([]string, 0) + files := make([]string, 0, len(values)) 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) { + for key, value := range values { + key = strings.ToUpper(key) + if !strings.HasSuffix(key, "_FILE") && !strings.HasSuffix(key, "_SOURCE") { continue } - fileInfo, err := os.Lstat(value) - if errors.Is(err, os.ErrNotExist) { - continue - } - if err != nil || !fileInfo.Mode().IsRegular() { + if err := safeio.ValidateCanonicalPath(value); err != nil { return nil, errors.New("installation secret declarations could not be read") } if _, exists := seen[value]; !exists { @@ -154,25 +157,41 @@ func (i Installation) SecretFiles() ([]string, error) { return files, nil } -func environmentAssignment(line string) (string, string, bool) { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - return "", "", false +func parseComposeDotenv(contents []byte) (map[string]string, error) { + dotenvParseMu.Lock() + defer dotenvParseMu.Unlock() + + logger := logrus.StandardLogger() + previousOutput := logger.Out + previousHooks := logger.ReplaceHooks(make(logrus.LevelHooks)) + logger.SetOutput(io.Discard) + warnings := &dotenvWarnings{} + logger.AddHook(warnings) + defer func() { + logger.SetOutput(previousOutput) + logger.ReplaceHooks(previousHooks) + }() + + values, err := dotenv.ParseWithLookup(bytes.NewReader(contents), os.LookupEnv) + if err != nil || warnings.seen { + return nil, errors.New("dotenv parsing failed") } - line = strings.TrimPrefix(line, "export ") - key, value, found := strings.Cut(line, "=") - if !found { - return "", "", false + return values, nil +} + +type dotenvWarnings struct { + seen bool +} + +func (w *dotenvWarnings) Levels() []logrus.Level { + return logrus.AllLevels +} + +func (w *dotenvWarnings) Fire(entry *logrus.Entry) error { + if entry.Level == logrus.WarnLevel { + w.seen = true } - 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 + return nil } func ensureOnlyOneDocument(decoder *yaml.Decoder) error { diff --git a/tools/thothctl/internal/output/sanitize.go b/tools/thothctl/internal/output/sanitize.go index 83bb3ac1..80c60fbf 100644 --- a/tools/thothctl/internal/output/sanitize.go +++ b/tools/thothctl/internal/output/sanitize.go @@ -3,11 +3,11 @@ package output import ( "errors" - "io" - "os" "regexp" "sort" "strings" + + "github.com/aritmolab/thothii/tools/thothctl/internal/safeio" ) var credentialField = regexp.MustCompile(`(?im)(\b[\w.-]*(?:password|token|key)[\w.-]*\s*[:=]\s*)(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s,;]+)`) @@ -48,18 +48,9 @@ func SecretValuesFromFiles(paths []string) ([]string, error) { } 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) + contents, err := safeio.ReadCanonicalRegular(path, maxSecretFileBytes) 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 } diff --git a/tools/thothctl/internal/output/sanitize_test.go b/tools/thothctl/internal/output/sanitize_test.go index 0844c1d5..b16ec8a2 100644 --- a/tools/thothctl/internal/output/sanitize_test.go +++ b/tools/thothctl/internal/output/sanitize_test.go @@ -19,7 +19,7 @@ func TestSanitizeRedactsPasswordTokenAndKeyFields(t *testing.T) { func TestSanitizeRedactsSecretFileContents(t *testing.T) { t.Parallel() - secretFile := filepath.Join(t.TempDir(), "provider-token") + secretFile := filepath.Join(physicalTempDir(t), "provider-token") if err := os.WriteFile(secretFile, []byte("top-secret-value\n"), 0o600); err != nil { t.Fatal(err) } @@ -37,7 +37,7 @@ func TestSanitizeRedactsSecretFileContents(t *testing.T) { func TestSecretValuesFromFilesRejectsOversizedFiles(t *testing.T) { t.Parallel() - secretFile := filepath.Join(t.TempDir(), "oversized-token") + secretFile := filepath.Join(physicalTempDir(t), "oversized-token") if err := os.WriteFile(secretFile, make([]byte, 64*1024+1), 0o600); err != nil { t.Fatal(err) } @@ -46,3 +46,17 @@ func TestSecretValuesFromFilesRejectsOversizedFiles(t *testing.T) { t.Fatal("SecretValuesFromFiles() error = nil, want oversized-file error") } } + +func physicalTempDir(t *testing.T) string { + t.Helper() + root, err := filepath.EvalSymlinks(os.TempDir()) + if err != nil { + t.Fatal(err) + } + directory, err := os.MkdirTemp(root, "thothctl-output-test-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(directory) }) + return directory +} diff --git a/tools/thothctl/internal/safeio/files.go b/tools/thothctl/internal/safeio/files.go new file mode 100644 index 00000000..22cbc9c0 --- /dev/null +++ b/tools/thothctl/internal/safeio/files.go @@ -0,0 +1,63 @@ +// Package safeio reads installation files without following symlinked path components. +package safeio + +import ( + "errors" + "io" + "os" + "path/filepath" + "strings" +) + +var ErrUnsafeFile = errors.New("unsafe file") + +// ValidateCanonicalPath rejects relative or lexically non-canonical paths before they are opened. +func ValidateCanonicalPath(path string) error { + if !filepath.IsAbs(path) || filepath.Clean(path) != path || strings.Contains(path, string(filepath.Separator)+".."+string(filepath.Separator)) { + return ErrUnsafeFile + } + return nil +} + +// ReadCanonicalRegular opens a canonical regular file after rejecting symlinked parents, then +// bounds reads against the opened handle rather than a pre-open size check. +func ReadCanonicalRegular(path string, maximum int64) ([]byte, error) { + if err := ValidateCanonicalPath(path); err != nil { + return nil, err + } + if err := rejectSymlinkComponents(path); err != nil { + return nil, err + } + + file, err := os.Open(path) + if err != nil { + return nil, ErrUnsafeFile + } + defer file.Close() + info, err := file.Stat() + if err != nil || !info.Mode().IsRegular() { + return nil, ErrUnsafeFile + } + contents, err := io.ReadAll(io.LimitReader(file, maximum+1)) + if err != nil || int64(len(contents)) > maximum { + return nil, ErrUnsafeFile + } + return contents, nil +} + +func rejectSymlinkComponents(path string) error { + volume := filepath.VolumeName(path) + current := volume + string(filepath.Separator) + relative := strings.TrimPrefix(path, current) + for _, component := range strings.Split(relative, string(filepath.Separator)) { + if component == "" { + continue + } + current = filepath.Join(current, component) + info, err := os.Lstat(current) + if err != nil || info.Mode()&os.ModeSymlink != 0 { + return ErrUnsafeFile + } + } + return nil +}