• X
• Mastodon
• LinkedIn
• Facebook

migrator/main.go (2)

30-41: Consider adding timeouts to the profiling HTTP server.

The HTTP server on :6060 lacks read/write timeouts, which could make it vulnerable to slowloris-style attacks. While this is an internal debug server, adding timeouts is a defensive practice.

💡 Suggested improvement

 func startProfilingServer() {
 	handler := http.NewServeMux()
 	for path, debugHandler := range routes.DebugRoutes {
 		handler.Handle(path, debugHandler)
 	}
-	srv := &http.Server{Addr: ":6060", Handler: handler}
+	srv := &http.Server{
+		Addr:         ":6060",
+		Handler:      handler,
+		ReadTimeout:  30 * time.Second,
+		WriteTimeout: 30 * time.Second,
+	}
 	go func() {
 		if err := srv.ListenAndServe(); err != nil {
 			log.WriteToStderrf("Closing profiling server: %v", err)
 		}
 	}()
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@migrator/main.go` around lines 30 - 41, The profiling server created in
startProfilingServer currently constructs srv := &http.Server{Addr: ":6060",
Handler: handler} without timeouts; update the server configuration (the srv
variable) to include sensible ReadTimeout, ReadHeaderTimeout, WriteTimeout (and
optionally IdleTimeout) values to mitigate slowloris-style attacks, then start
it the same way (ListenAndServe) while keeping the existing error handling and
use of routes.DebugRoutes and handler.

---

118-132: Redundant external database check.

The pgconfig.IsExternalDatabase() check on line 124 is redundant since ensureDatabaseExists() is only called when !pgconfig.IsExternalDatabase() on line 62. The function always returns nil for external databases due to the check at line 124, but this code path is unreachable.

♻️ Suggested simplification

 func ensureDatabaseExists() error {
 	sourceMap, adminConfig, err := pgconfig.GetPostgresConfig()
 	if err != nil {
 		return err
 	}

-	if !pgconfig.IsExternalDatabase() {
-		return retry.WithRetry(func() error {
-			return dbCheck(sourceMap, adminConfig)
-		}, retry.Tries(60), retry.BetweenAttempts(func(_ int) {
-			time.Sleep(5 * time.Second)
-		}))
-	}
-	return nil
+	return retry.WithRetry(func() error {
+		return dbCheck(sourceMap, adminConfig)
+	}, retry.Tries(60), retry.BetweenAttempts(func(_ int) {
+		time.Sleep(5 * time.Second)
+	}))
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@migrator/main.go` around lines 118 - 132, The function ensureDatabaseExists
currently re-checks pgconfig.IsExternalDatabase() redundantly and returns nil
for external DBs; remove that internal IsExternalDatabase() conditional and
always perform the retry.WithRetry call that invokes dbCheck using the sourceMap
and adminConfig from pgconfig.GetPostgresConfig(), returning any error from the
retry; in short, delete the inner external-database branch so
ensureDatabaseExists simply gets config (sourceMap, adminConfig) and runs
retry.WithRetry(... dbCheck ...) and returns its result.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes