fix(setup): validate endpoints and secret files

2026-08-15 22:53:04 +02:00
parent 6dc9e80564
commit 2856964654
2 changed files with 241 additions and 6 deletions
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -20,6 +21,7 @@ import (
 const (
 	descriptorName  = "thothii-installation.yaml"
 	environmentName = "operator.env"
+	maxSecretBytes  = 64 << 10
 )

 var installationIDPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]*$`)
@@ -182,7 +184,10 @@ func collectAnswers(request Request, input io.Reader, output io.Writer, root str
 			return answers{}, err
 		}
 	}
-	missing := missingSecretFiles(value)
+	missing, missingErr := missingSecretFiles(value)
+	if missingErr != nil {
+		return answers{}, missingErr
+	}
 	if len(missing) > 0 {
 		answer, promptErr := prompt(scanner, output, "Create blank secret-file templates for the missing locations? Type yes to confirm", "no")
 		if promptErr != nil {
@@ -251,6 +256,11 @@ func validateAnswers(value answers) error {
 	if value.workspaceAccess != "ssh" && value.workspaceAccess != "https" {
 		return errors.New("workspace repository access must be ssh or https")
 	}
+	for name, endpoint := range map[string]string{"DWH API": value.dwhRESTURL, "LLM API": value.llmURL} {
+		if err := validateServiceEndpoint(name, endpoint); err != nil {
+			return err
+		}
+	}
 	for name, path := range map[string]string{
 		"secret file location": value.secretsFile, "Pi credentials file location": value.piAuthFile,
 		"workspace Git credentials file location": value.gitCredentialsFile, "workspace Git CA file location": value.gitCAFile,
@@ -358,20 +368,39 @@ func writeIfAbsent(path string, contents []byte, created *[]string) error {
 	return nil
 }

-func missingSecretFiles(value answers) []string {
+func validateServiceEndpoint(name, endpoint string) error {
+	if endpoint == "" {
+		return nil
+	}
+	parsed, err := url.Parse(endpoint)
+	if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Host == "" ||
+		parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" {
+		return fmt.Errorf("%s endpoint must be an http(s) URL without user information, password, query, or fragment", name)
+	}
+	return nil
+}
+
+func missingSecretFiles(value answers) ([]string, error) {
 	paths := configuredSecretPaths(value)
 	missing := make([]string, 0, len(paths))
 	for _, path := range paths {
-		if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) {
+		exists, err := inspectExistingSecretFile(path)
+		if err != nil {
+			return nil, err
+		}
+		if !exists {
 			missing = append(missing, path)
 		}
 	}
 	sort.Strings(missing)
-	return missing
+	return missing, nil
 }

 func validateOrCreateSecretFiles(value answers, output io.Writer) error {
-	missing := missingSecretFiles(value)
+	missing, err := missingSecretFiles(value)
+	if err != nil {
+		return err
+	}
 	if len(missing) == 0 {
 		return nil
 	}
@@ -390,6 +419,23 @@ func validateOrCreateSecretFiles(value answers, output io.Writer) error {
 	return nil
 }

+func inspectExistingSecretFile(path string) (bool, error) {
+	before, err := os.Lstat(path)
+	if errors.Is(err, os.ErrNotExist) {
+		return false, nil
+	}
+	if err != nil {
+		return false, fmt.Errorf("secret file %s could not be inspected; choose a readable regular file", path)
+	}
+	if !before.Mode().IsRegular() || before.Mode()&os.ModeSymlink != 0 {
+		return false, fmt.Errorf("secret file %s must be a readable regular file, not a directory, symlink, or special file", path)
+	}
+	if _, err := safeio.ReadCanonicalRegular(path, maxSecretBytes); err != nil {
+		return false, fmt.Errorf("secret file %s must be a canonical readable regular file", path)
+	}
+	return true, nil
+}
+
 func configuredSecretPaths(value answers) []string {
 	paths := []string{value.secretsFile, value.piAuthFile}
 	if value.workspaceAccess == "ssh" {
@@ -4,11 +4,14 @@ import (
 	"bytes"
 	"errors"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"

 	"github.com/aritmolab/thothii/tools/tht/internal/config"
+	"github.com/aritmolab/thothii/tools/tht/internal/testsupport"
 )

 func TestEnsureFilesCreatesDiscoverableConfigurationInProjectWithSpaces(t *testing.T) {
@@ -211,6 +214,183 @@ func TestEnsureFilesIncludesServerStorageLocations(t *testing.T) {
 	}
 }

+func TestEnsureFilesRejectsUnsafeServiceEndpointsBeforeWritingConfiguration(t *testing.T) {
+	for _, test := range []struct {
+		name, environment, value string
+	}{
+		{"DWH user info", "THT_SETUP_DWH_REST_URL", "https://operator@dwh.example.invalid/api"},
+		{"DWH password", "THT_SETUP_DWH_REST_URL", "https://operator:password@dwh.example.invalid/api"},
+		{"DWH query", "THT_SETUP_DWH_REST_URL", "https://dwh.example.invalid/api?token=secret"},
+		{"LLM fragment", "THT_SETUP_LLM_URL", "https://llm.example.invalid/api#secret"},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			root := newProject(t, "unsafe endpoint")
+			setNonInteractiveAnswers(t, newExternalSecrets(t, root))
+			t.Setenv(test.environment, test.value)
+			id := "rejected"
+			_, err := EnsureFiles(Request{ProjectRoot: root, InstallationID: id, Profile: "local", NonInteractive: true}, strings.NewReader(""), ioDiscard{})
+			if err == nil || !strings.Contains(err.Error(), "endpoint") {
+				t.Fatalf("EnsureFiles() error = %v, want endpoint rejection", err)
+			}
+			assertNoConfigurationOutput(t, root, id)
+		})
+	}
+}
+
+func TestEnsureFilesRejectsUnsafeExistingSecretFiles(t *testing.T) {
+	for _, test := range []struct {
+		name   string
+		mutate func(t *testing.T, paths *secretPaths)
+	}{
+		{
+			name: "directory",
+			mutate: func(t *testing.T, paths *secretPaths) {
+				paths.piAuth = filepath.Dir(paths.piAuth)
+			},
+		},
+		{
+			name: "symlink",
+			mutate: func(t *testing.T, paths *secretPaths) {
+				link := paths.piAuth + ".link"
+				testsupport.SymlinkOrSkip(t, paths.piAuth, link)
+				paths.piAuth = link
+			},
+		},
+		{
+			name: "path beneath a symlinked directory",
+			mutate: func(t *testing.T, paths *secretPaths) {
+				link := filepath.Join(filepath.Dir(paths.piAuth), "parent-link")
+				testsupport.SymlinkOrSkip(t, filepath.Dir(paths.piAuth), link)
+				paths.piAuth = filepath.Join(link, filepath.Base(paths.piAuth))
+			},
+		},
+		{
+			name: "unreadable file",
+			mutate: func(t *testing.T, paths *secretPaths) {
+				if runtime.GOOS == "windows" {
+					t.Skip("POSIX read permissions are not portable to Windows")
+				}
+				if err := os.Chmod(paths.piAuth, 0o000); err != nil {
+					t.Fatal(err)
+				}
+				t.Cleanup(func() { _ = os.Chmod(paths.piAuth, 0o600) })
+				file, err := os.Open(paths.piAuth)
+				if err == nil {
+					_ = file.Close()
+					t.Skip("effective user can read mode 000 files")
+				}
+			},
+		},
+		{
+			name: "stat error",
+			mutate: func(t *testing.T, paths *secretPaths) {
+				paths.knownHosts = filepath.Join(paths.sshKey, "not-a-directory")
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			root := newProject(t, "unsafe secret")
+			paths := newExternalSecrets(t, root)
+			test.mutate(t, &paths)
+			setNonInteractiveAnswers(t, paths)
+			_, err := EnsureFiles(Request{ProjectRoot: root, InstallationID: "rejected", Profile: "local", NonInteractive: true}, strings.NewReader(""), ioDiscard{})
+			if err == nil || (!strings.Contains(err.Error(), "readable regular file") && !strings.Contains(err.Error(), "could not be inspected")) {
+				t.Fatalf("EnsureFiles() error = %v, want unsafe secret-file rejection", err)
+			}
+			assertNoConfigurationOutput(t, root, "rejected")
+		})
+	}
+}
+
+func TestEnsureFilesRejectsUnwritableDeploymentDirectory(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX directory write permissions are not portable to Windows")
+	}
+	root := newProject(t, "unwritable deployment")
+	setNonInteractiveAnswers(t, newExternalSecrets(t, root))
+	deploy := filepath.Join(root, "deploy")
+	if err := os.Chmod(deploy, 0o500); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(deploy, 0o700) })
+	_, err := EnsureFiles(Request{ProjectRoot: root, InstallationID: "rejected", Profile: "local", NonInteractive: true}, strings.NewReader(""), ioDiscard{})
+	if err == nil {
+		t.Skip("effective user can create files in a mode 0500 directory")
+	}
+	assertNoConfigurationOutput(t, root, "rejected")
+}
+
+func TestEnsureFilesIgnoresGeneratedFilesInActualLinkedGitWorktree(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git is unavailable")
+	}
+	worktree := newLinkedGitWorktree(t)
+	secrets := newExternalSecretsAt(t, filepath.Join(t.TempDir(), "external secrets"))
+	setNonInteractiveAnswers(t, secrets)
+	result, err := EnsureFiles(Request{ProjectRoot: worktree, InstallationID: "linked", Profile: "local", NonInteractive: true}, strings.NewReader(""), ioDiscard{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, path := range []string{result.DescriptorPath, result.EnvironmentPath} {
+		gitRun(t, worktree, "check-ignore", "--quiet", path)
+		if err := exec.Command("git", "-C", worktree, "ls-files", "--error-unmatch", "--", path).Run(); err == nil {
+			t.Fatalf("generated path %s was added to the Git index", path)
+		}
+	}
+	status := gitOutput(t, worktree, "status", "--porcelain", "--untracked-files=all")
+	if status != "" {
+		t.Fatalf("generated configuration appears in linked-worktree Git status: %q", status)
+	}
+}
+
+func assertNoConfigurationOutput(t *testing.T, root, id string) {
+	t.Helper()
+	directory := filepath.Join(root, "deploy", id)
+	for _, name := range []string{"thothii-installation.yaml", "operator.env"} {
+		if _, err := os.Lstat(filepath.Join(directory, name)); !errors.Is(err, os.ErrNotExist) {
+			t.Fatalf("unexpected configuration output %s: %v", name, err)
+		}
+	}
+}
+
+func newLinkedGitWorktree(t *testing.T) string {
+	t.Helper()
+	repository := newProject(t, "source repository")
+	if err := os.WriteFile(filepath.Join(repository, ".gitignore"), []byte("deploy/*/thothii-installation.yaml\ndeploy/*/operator.env\ndeploy/*/secrets/*\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	gitRun(t, repository, "init")
+	gitRun(t, repository, "config", "user.email", "tests@example.invalid")
+	gitRun(t, repository, "config", "user.name", "ThothII tests")
+	gitRun(t, repository, "add", ".")
+	gitRun(t, repository, "commit", "-m", "fixture")
+	worktree := filepath.Join(t.TempDir(), "linked worktree")
+	gitRun(t, repository, "worktree", "add", "-b", "linked-fixture", worktree)
+	canonical, err := filepath.EvalSymlinks(worktree)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return canonical
+}
+
+func gitRun(t *testing.T, directory string, args ...string) {
+	t.Helper()
+	command := exec.Command("git", append([]string{"-C", directory}, args...)...)
+	if output, err := command.CombinedOutput(); err != nil {
+		t.Fatalf("git %s: %v: %s", strings.Join(args, " "), err, output)
+	}
+}
+
+func gitOutput(t *testing.T, directory string, args ...string) string {
+	t.Helper()
+	command := exec.Command("git", append([]string{"-C", directory}, args...)...)
+	output, err := command.Output()
+	if err != nil {
+		t.Fatalf("git %s: %v", strings.Join(args, " "), err)
+	}
+	return string(output)
+}
+
 func newProject(t *testing.T, name string) string {
 	t.Helper()
 	root := filepath.Join(t.TempDir(), name)
@@ -251,10 +431,19 @@ type secretPaths struct {

 func newExternalSecrets(t *testing.T, root string) secretPaths {
 	t.Helper()
-	directory := filepath.Join(root, "external secrets")
+	return newExternalSecretsAt(t, filepath.Join(root, "external secrets"))
+}
+
+func newExternalSecretsAt(t *testing.T, directory string) secretPaths {
+	t.Helper()
 	if err := os.Mkdir(directory, 0o700); err != nil {
 		t.Fatal(err)
 	}
+	canonical, err := filepath.EvalSymlinks(directory)
+	if err != nil {
+		t.Fatal(err)
+	}
+	directory = canonical
 	paths := secretPaths{
 		secrets: filepath.Join(directory, "thothii.secrets"), piAuth: filepath.Join(directory, "pi-auth.json"),
 		sshKey: filepath.Join(directory, "git-key"), knownHosts: filepath.Join(directory, "known-hosts"),