fix: fail closed thothctl secret sources

2026-08-04 17:17:39 +02:00
parent 4158990c10
commit 35a000222c
7 changed files with 366 additions and 49 deletions
@@ -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 {