fix(deploy): address race conditions and improve code quality

- Add exclusive file creation with retry for deploy requests to prevent
  concurrent deploy commands from overwriting each other
- Use atomic rename for reading deploy requests to prevent race conditions
- Move output formatting to CLI layer (SendDeploySignal returns domain)
- Add security documentation for internal credentials file

commit: 0aa69fcc5e631961fc0f485da7a1c94102d8eb42
author: brice · 9mo ago · 2026-01-14 19:56 UTC
committer: brice · 9mo ago · 2026-01-14 17:42 UTC
parent: 56493a7
signature: unsigned

2 files · +67 -17

M internal/adapters/in/cli/deploy.go +8 -1

@@ -1,6 +1,8 @@
1	1		package cli
2	2
3	3		import (
	4	+	"fmt"
	5	+
4	6		"github.com/spf13/cobra"
5	7
6	8		"gordon/internal/app"

@@ -22,7 +24,12 @@ Examples:
22	24		gordon deploy api.example.com`,
23	25		Args: cobra.ExactArgs(1),
24	26		RunE: func(cmd *cobra.Command, args []string) error {
25		-	return app.SendDeploySignal(args[0])
	27	+	domain, err := app.SendDeploySignal(args[0])
	28	+	if err != nil {
	29	+	return err
	30	+	}
	31	+	fmt.Printf("Deploy signal sent for domain: %s\n", domain)
	32	+	return nil
26	33		},
27	34		}
28	35		}

 package cli
 import (
+	"fmt"
 	"github.com/spf13/cobra"
 	"gordon/internal/app"
   gordon deploy api.example.com`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return app.SendDeploySignal(args[0])
+			domain, err := app.SendDeploySignal(args[0])
+			if err != nil {
+				return err
+			}
+			fmt.Printf("Deploy signal sent for domain: %s\n", domain)
+			return nil
 		},
 	}
 }

M internal/app/run.go +59 -16


@@ -411,7 +411,10 @@ func getInternalCredentialsFile() string {
411	411		return filepath.Join(os.TempDir(), "gordon-internal-creds.json")
412	412		}
413	413
414		-	// persistInternalCredentials saves the internal registry credentials to a file.
	414	+	// persistInternalCredentials saves the internal registry credentials to a temp file.
	415	+	// Security note: Credentials are stored in the system temp directory with 0600 permissions.
	416	+	// The file is cleaned up on graceful shutdown but may persist if Gordon crashes.
	417	+	// These credentials are for internal loopback communication only and are regenerated on each start.
415	418		func persistInternalCredentials(username, password string) error {
416	419		creds := InternalCredentials{
417	420		Username: username,

@@ -892,56 +895,96 @@ func getDeployRequestFile() string {
892	895		return filepath.Join(os.TempDir(), "gordon-deploy-request")
893	896		}
894	897
	898	+	// writeDeployRequestFile creates the deploy request file exclusively with retry.
	899	+	// This prevents race conditions when multiple deploy commands run simultaneously.
	900	+	func writeDeployRequestFile(path string, data []byte, timeout time.Duration) error {
	901	+	deadline := time.Now().Add(timeout)
	902	+
	903	+	for {
	904	+	f, err := os.OpenFile(path, os.O_WRONLY\|os.O_CREATE\|os.O_EXCL, 0600)
	905	+	if err == nil {
	906	+	_, writeErr := f.Write(data)
	907	+	f.Close()
	908	+	if writeErr != nil {
	909	+	_ = os.Remove(path)
	910	+	return writeErr
	911	+	}
	912	+	return nil
	913	+	}
	914	+
	915	+	if os.IsExist(err) {
	916	+	if time.Now().After(deadline) {
	917	+	return fmt.Errorf("deploy request file still present after timeout; another deploy may be in progress")
	918	+	}
	919	+	time.Sleep(100 * time.Millisecond)
	920	+	continue
	921	+	}
	922	+
	923	+	return err
	924	+	}
	925	+	}
	926	+
895	927		// SendDeploySignal triggers a manual deploy for a specific route via SIGUSR2.
896		-	func SendDeploySignal(domain string) error {
897		-	// Write the domain to the deploy request file
	928	+	// Returns the domain name on success for the caller to display.
	929	+	func SendDeploySignal(domain string) (string, error) {
898	930		deployFile := getDeployRequestFile()
899		-	if err := os.WriteFile(deployFile, []byte(domain), 0600); err != nil {
900		-	return fmt.Errorf("failed to write deploy request: %w", err)
	931	+	if err := writeDeployRequestFile(deployFile, []byte(domain), 5*time.Second); err != nil {
	932	+	return "", fmt.Errorf("failed to write deploy request: %w", err)
901	933		}
902	934
903	935		// Find PID and send SIGUSR2
904	936		pidFile := findPidFile()
905	937		if pidFile == "" {
906		-	_ = os.Remove(deployFile) // Clean up on error
907		-	return fmt.Errorf("gordon PID file not found, is Gordon running?")
	938	+	_ = os.Remove(deployFile)
	939	+	return "", fmt.Errorf("gordon PID file not found, is Gordon running?")
908	940		}
909	941
910	942		pidBytes, err := os.ReadFile(pidFile)
911	943		if err != nil {
912	944		_ = os.Remove(deployFile)
913		-	return fmt.Errorf("failed to read PID file: %w", err)
	945	+	return "", fmt.Errorf("failed to read PID file: %w", err)
914	946		}
915	947
916	948		var pid int
917	949		if _, err := fmt.Sscanf(string(pidBytes), "%d", &pid); err != nil {
918	950		_ = os.Remove(deployFile)
919		-	return fmt.Errorf("failed to parse PID: %w", err)
	951	+	return "", fmt.Errorf("failed to parse PID: %w", err)
920	952		}
921	953
922	954		process, err := os.FindProcess(pid)
923	955		if err != nil {
924	956		_ = os.Remove(deployFile)
925		-	return fmt.Errorf("failed to find process: %w", err)
	957	+	return "", fmt.Errorf("failed to find process: %w", err)
926	958		}
927	959
928	960		if err := process.Signal(syscall.SIGUSR2); err != nil {
929	961		_ = os.Remove(deployFile)
930		-	return fmt.Errorf("failed to send deploy signal: %w", err)
	962	+	return "", fmt.Errorf("failed to send deploy signal: %w", err)
931	963		}
932	964
933		-	fmt.Printf("Deploy signal sent for domain: %s\n", domain)
934		-	return nil
	965	+	return domain, nil
935	966		}
936	967
937		-	// readDeployRequest reads and removes the deploy request file.
	968	+	// readDeployRequest reads and removes the deploy request file atomically.
	969	+	// Returns empty string if file doesn't exist (may have been consumed by another handler).
938	970		func readDeployRequest() (string, error) {
939	971		deployFile := getDeployRequestFile()
940		-	data, err := os.ReadFile(deployFile)
	972	+
	973	+	// Rename to a temp file first to make the read-and-delete atomic
	974	+	tmpFile := deployFile + ".processing"
	975	+	if err := os.Rename(deployFile, tmpFile); err != nil {
	976	+	if os.IsNotExist(err) {
	977	+	return "", fmt.Errorf("deploy request file not found (may have been processed already)")
	978	+	}
	979	+	return "", fmt.Errorf("failed to acquire deploy request: %w", err)
	980	+	}
	981	+
	982	+	data, err := os.ReadFile(tmpFile)
	983	+	_ = os.Remove(tmpFile) // Always clean up
941	984		if err != nil {
942	985		return "", fmt.Errorf("failed to read deploy request: %w", err)
943	986		}
944		-	_ = os.Remove(deployFile) // Clean up after reading
	987	+
945	988		return string(data), nil
946	989		}
947	990


@@ -411,7 +411,10 @@ func getInternalCredentialsFile() string {
411		return filepath.Join(os.TempDir(), "gordon-internal-creds.json")	411		return filepath.Join(os.TempDir(), "gordon-internal-creds.json")
412		}	412		}
413			413
414	-	// persistInternalCredentials saves the internal registry credentials to a file.	414	+	// persistInternalCredentials saves the internal registry credentials to a temp file.
			415	+	// Security note: Credentials are stored in the system temp directory with 0600 permissions.
			416	+	// The file is cleaned up on graceful shutdown but may persist if Gordon crashes.
			417	+	// These credentials are for internal loopback communication only and are regenerated on each start.
415		func persistInternalCredentials(username, password string) error {	418		func persistInternalCredentials(username, password string) error {
416		creds := InternalCredentials{	419		creds := InternalCredentials{
417		Username: username,	420		Username: username,

@@ -892,56 +895,96 @@ func getDeployRequestFile() string {
892		return filepath.Join(os.TempDir(), "gordon-deploy-request")	895		return filepath.Join(os.TempDir(), "gordon-deploy-request")
893		}	896		}
894			897
			898	+	// writeDeployRequestFile creates the deploy request file exclusively with retry.
			899	+	// This prevents race conditions when multiple deploy commands run simultaneously.
			900	+	func writeDeployRequestFile(path string, data []byte, timeout time.Duration) error {
			901	+	deadline := time.Now().Add(timeout)
			902	+
			903	+	for {
			904	+	f, err := os.OpenFile(path, os.O_WRONLY\|os.O_CREATE\|os.O_EXCL, 0600)
			905	+	if err == nil {
			906	+	_, writeErr := f.Write(data)
			907	+	f.Close()
			908	+	if writeErr != nil {
			909	+	_ = os.Remove(path)
			910	+	return writeErr
			911	+	}
			912	+	return nil
			913	+	}
			914	+
			915	+	if os.IsExist(err) {
			916	+	if time.Now().After(deadline) {
			917	+	return fmt.Errorf("deploy request file still present after timeout; another deploy may be in progress")
			918	+	}
			919	+	time.Sleep(100 * time.Millisecond)
			920	+	continue
			921	+	}
			922	+
			923	+	return err
			924	+	}
			925	+	}
			926	+
895		// SendDeploySignal triggers a manual deploy for a specific route via SIGUSR2.	927		// SendDeploySignal triggers a manual deploy for a specific route via SIGUSR2.
896	-	func SendDeploySignal(domain string) error {	928	+	// Returns the domain name on success for the caller to display.
897	-	// Write the domain to the deploy request file	929	+	func SendDeploySignal(domain string) (string, error) {
898		deployFile := getDeployRequestFile()	930		deployFile := getDeployRequestFile()
899	-	if err := os.WriteFile(deployFile, []byte(domain), 0600); err != nil {	931	+	if err := writeDeployRequestFile(deployFile, []byte(domain), 5*time.Second); err != nil {
900	-	return fmt.Errorf("failed to write deploy request: %w", err)	932	+	return "", fmt.Errorf("failed to write deploy request: %w", err)
901		}	933		}
902			934
903		// Find PID and send SIGUSR2	935		// Find PID and send SIGUSR2
904		pidFile := findPidFile()	936		pidFile := findPidFile()
905		if pidFile == "" {	937		if pidFile == "" {
906	-	_ = os.Remove(deployFile) // Clean up on error	938	+	_ = os.Remove(deployFile)
907	-	return fmt.Errorf("gordon PID file not found, is Gordon running?")	939	+	return "", fmt.Errorf("gordon PID file not found, is Gordon running?")
908		}	940		}
909			941
910		pidBytes, err := os.ReadFile(pidFile)	942		pidBytes, err := os.ReadFile(pidFile)
911		if err != nil {	943		if err != nil {
912		_ = os.Remove(deployFile)	944		_ = os.Remove(deployFile)
913	-	return fmt.Errorf("failed to read PID file: %w", err)	945	+	return "", fmt.Errorf("failed to read PID file: %w", err)
914		}	946		}
915			947
916		var pid int	948		var pid int
917		if _, err := fmt.Sscanf(string(pidBytes), "%d", &pid); err != nil {	949		if _, err := fmt.Sscanf(string(pidBytes), "%d", &pid); err != nil {
918		_ = os.Remove(deployFile)	950		_ = os.Remove(deployFile)
919	-	return fmt.Errorf("failed to parse PID: %w", err)	951	+	return "", fmt.Errorf("failed to parse PID: %w", err)
920		}	952		}
921			953
922		process, err := os.FindProcess(pid)	954		process, err := os.FindProcess(pid)
923		if err != nil {	955		if err != nil {
924		_ = os.Remove(deployFile)	956		_ = os.Remove(deployFile)
925	-	return fmt.Errorf("failed to find process: %w", err)	957	+	return "", fmt.Errorf("failed to find process: %w", err)
926		}	958		}
927			959
928		if err := process.Signal(syscall.SIGUSR2); err != nil {	960		if err := process.Signal(syscall.SIGUSR2); err != nil {
929		_ = os.Remove(deployFile)	961		_ = os.Remove(deployFile)
930	-	return fmt.Errorf("failed to send deploy signal: %w", err)	962	+	return "", fmt.Errorf("failed to send deploy signal: %w", err)
931		}	963		}
932			964
933	-	fmt.Printf("Deploy signal sent for domain: %s\n", domain)	965	+	return domain, nil
934	-	return nil
935		}	966		}
936			967
937	-	// readDeployRequest reads and removes the deploy request file.	968	+	// readDeployRequest reads and removes the deploy request file atomically.
			969	+	// Returns empty string if file doesn't exist (may have been consumed by another handler).
938		func readDeployRequest() (string, error) {	970		func readDeployRequest() (string, error) {
939		deployFile := getDeployRequestFile()	971		deployFile := getDeployRequestFile()
940	-	data, err := os.ReadFile(deployFile)	972	+
			973	+	// Rename to a temp file first to make the read-and-delete atomic
			974	+	tmpFile := deployFile + ".processing"
			975	+	if err := os.Rename(deployFile, tmpFile); err != nil {
			976	+	if os.IsNotExist(err) {
			977	+	return "", fmt.Errorf("deploy request file not found (may have been processed already)")
			978	+	}
			979	+	return "", fmt.Errorf("failed to acquire deploy request: %w", err)
			980	+	}
			981	+
			982	+	data, err := os.ReadFile(tmpFile)
			983	+	_ = os.Remove(tmpFile) // Always clean up
941		if err != nil {	984		if err != nil {
942		return "", fmt.Errorf("failed to read deploy request: %w", err)	985		return "", fmt.Errorf("failed to read deploy request: %w", err)
943		}	986		}
944	-	_ = os.Remove(deployFile) // Clean up after reading	987	+
945		return string(data), nil	988		return string(data), nil
946		}	989		}
947			990