-
Notifications
You must be signed in to change notification settings - Fork 250
Description
🟡 medium - bug
File: cmd/root/root.go (line 222)
Code
func isFirstRun() bool {
configDir := paths.GetConfigDir()
markerFile := filepath.Join(configDir, ".cagent_first_run")
// Check if marker file exists
if _, err := os.Stat(markerFile); err == nil {
return false // File exists, not first run
}
// Create marker file to indicate this run has happened
if err := os.MkdirAll(configDir, 0o755); err != nil {
return false // Can't create config dir, assume not first run
}
if err := os.WriteFile(markerFile, []byte(""), 0o644); err != nil {
return false // Can't create marker file, assume not first run
}
return true // Successfully created marker, this is first run
}Problem
The isFirstRun function checks for the existence of a marker file and then creates it if it doesn't exist. This is a classic check-then-act race condition. If multiple cagent processes start concurrently, they might all see that the marker file does not exist, and then all try to create it. While os.WriteFile would likely handle concurrent writes gracefully (one would succeed, others might fail with "file exists" error), the "welcome message" and "telemetry message" could be printed multiple times, which is not ideal. Additionally, the os.MkdirAll call could also race.
Suggested Fix
Use a file lock or a more atomic operation to ensure only one process can perform the "first run" initialization. For example, use os.OpenFile with os.O_EXCL|os.O_CREATE to atomically create the file; if it fails due to the file already existing, then it's not the first run.
Here is an example of a more robust isFirstRun function using os.O_EXCL:
func isFirstRun() bool {
configDir := paths.GetConfigDir()
markerFile := filepath.Join(configDir, ".cagent_first_run")
// Ensure the config directory exists before trying to create the marker file
if err := os.MkdirAll(configDir, 0o755); err != nil {
slog.Warn("Failed to create config directory for first run marker", "error", err)
return false // Can't create config dir, assume not first run
}
// Atomically create the marker file. If it already exists, OpenFile will return an error.
f, err := os.OpenFile(markerFile, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o644)
if err != nil {
if os.IsExist(err) {
return false // File exists, not first run
}
slog.Warn("Failed to create first run marker file", "error", err)
return false // Other error creating marker file, assume not first run
}
f.Close() // Close the file immediately after creation
return true // Successfully created marker, this is first run
}Found by nightly codebase scan