cscli machines add: don't overwrite existing credential file

cmd/crowdsec-cli/capi.go

@@ -110,7 +110,7 @@ func NewCapiRegisterCmd() *cobra.Command {
 				if err != nil {
 					return fmt.Errorf("write api credentials in '%s' failed: %w", dumpFile, err)
 				}
-				log.Printf("Central API credentials dumped to '%s'", dumpFile)
+				log.Printf("Central API credentials written to '%s'", dumpFile)
 			} else {
 				fmt.Printf("%s\n", string(apiConfigDump))
 			}

cmd/crowdsec-cli/config_restore.go

@@ -183,7 +183,7 @@ func restoreConfigFromDirectory(dirPath string, oldBackup bool) error {
 			if csConfig.API.Server.OnlineClient != nil && csConfig.API.Server.OnlineClient.CredentialsFilePath != "" {
 				apiConfigDumpFile = csConfig.API.Server.OnlineClient.CredentialsFilePath
 			}
-			err = os.WriteFile(apiConfigDumpFile, apiConfigDump, 0o644)
+			err = os.WriteFile(apiConfigDumpFile, apiConfigDump, 0o600)
 			if err != nil {
 				return fmt.Errorf("write api credentials in '%s' failed: %s", apiConfigDumpFile, err)
 			}

cmd/crowdsec-cli/lapi.go

@@ -150,11 +150,11 @@ func runLapiRegister(cmd *cobra.Command, args []string) error {
 		log.Fatalf("unable to marshal api credentials: %s", err)
 	}
 	if dumpFile != "" {
-		err = os.WriteFile(dumpFile, apiConfigDump, 0644)
+		err = os.WriteFile(dumpFile, apiConfigDump, 0o600)
 		if err != nil {
 			log.Fatalf("write api credentials in '%s' failed: %s", dumpFile, err)
 		}
-		log.Printf("Local API credentials dumped to '%s'", dumpFile)
+		log.Printf("Local API credentials written to '%s'", dumpFile)
 	} else {
 		fmt.Printf("%s\n", string(apiConfigDump))
 	}

cmd/crowdsec-cli/machines.go

@@ -43,6 +43,7 @@ func generatePassword(length int) string {
 	charsetLength := len(charset)
 
 	buf := make([]byte, length)
+
 	for i := 0; i < length; i++ {
 		rInt, err := saferand.Int(saferand.Reader, big.NewInt(int64(charsetLength)))
 		if err != nil {
@@ -190,7 +191,6 @@ cscli machines add MyTestMachine --password MyPassword
 }
 
 func runMachinesAdd(cmd *cobra.Command, args []string) error {
-	var dumpFile string
 	var err error
 
 	flags := cmd.Flags()
@@ -200,7 +200,7 @@ func runMachinesAdd(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	outputFile, err := flags.GetString("file")
+	dumpFile, err := flags.GetString("file")
 	if err != nil {
 		return err
 	}
@@ -242,17 +242,28 @@ func runMachinesAdd(cmd *cobra.Command, args []string) error {
 	}
 
 	/*check if file already exists*/
-	if outputFile != "" {
-		dumpFile = outputFile
-	} else if csConfig.API.Client != nil && csConfig.API.Client.CredentialsFilePath != "" {
-		dumpFile = csConfig.API.Client.CredentialsFilePath
+	if dumpFile == "" && csConfig.API.Client != nil && csConfig.API.Client.CredentialsFilePath != "" {
+		credFile := csConfig.API.Client.CredentialsFilePath
+		// use the default only if the file does not exist
+		_, err := os.Stat(credFile)
+		switch {
+		case os.IsNotExist(err):
+			dumpFile = csConfig.API.Client.CredentialsFilePath
+		case err != nil:
+			return fmt.Errorf("unable to stat '%s': %s", credFile, err)
+		default:
+			return fmt.Errorf(`credentials file '%s' already exists, please remove or specify a different file with -f ("-f -" for standard output)`, credFile)
+		}
+	}
+
+	if dumpFile == "" {
+		return fmt.Errorf(`please specify a file to dump credentials to, with -f ("-f -" for standard output)`)
 	}
 
 	// create a password if it's not specified by user
 	if machinePassword == "" && !interactive {
 		if !autoAdd {
-			printHelp(cmd)
-			return nil
+			return fmt.Errorf("please specify a password with --password or use --auto")
 		}
 		machinePassword = generatePassword(passwordLength)
 	} else if machinePassword == "" && interactive {
@@ -291,7 +302,7 @@ func runMachinesAdd(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return fmt.Errorf("write api credentials in '%s' failed: %s", dumpFile, err)
 		}
-		log.Printf("API credentials dumped to '%s'", dumpFile)
+		log.Printf("API credentials written to '%s'", dumpFile)
 	} else {
 		fmt.Printf("%s\n", string(apiConfigDump))
 	}

test/bats/30_machines.bats

@@ -36,7 +36,7 @@ teardown() {
 @test "add a new machine and delete it" {
     rune -0 cscli machines add -a -f /dev/null CiTestMachine -o human
     assert_stderr --partial "Machine 'CiTestMachine' successfully added to the local API"
-    assert_stderr --partial "API credentials dumped to '/dev/null'"
+    assert_stderr --partial "API credentials written to '/dev/null'"
 
     # we now have two machines
     rune -0 cscli machines list -o json
@@ -56,7 +56,7 @@ teardown() {
 @test "register, validate and then remove a machine" {
     rune -0 cscli lapi register --machine CiTestMachineRegister -f /dev/null -o human
     assert_stderr --partial "Successfully registered to Local API (LAPI)"
-    assert_stderr --partial "Local API credentials dumped to '/dev/null'"
+    assert_stderr --partial "Local API credentials written to '/dev/null'"
 
     # the machine is not validated yet
     rune -0 cscli machines list -o json

test/lib/config/config-global

@@ -110,7 +110,7 @@ make_init_data() {
 
     preload_hub_items
 
-    [[ "${DB_BACKEND}" == "sqlite" ]] || ${CSCLI} machines add --auto
+    [[ "${DB_BACKEND}" == "sqlite" ]] || ${CSCLI} machines add --auto -f "$($CSCLI config show --key Config.API.Client.CredentialsFilePath)"
 
     mkdir -p "$LOCAL_INIT_DIR"
 

test/lib/config/config-local

@@ -160,7 +160,7 @@ make_init_data() {
     ./instance-db config-yaml
     ./instance-db setup
 
-    "$CSCLI" --warning machines add githubciXXXXXXXXXXXXXXXXXXXXXXXX --auto
+    "$CSCLI" --warning machines add githubciXXXXXXXXXXXXXXXXXXXXXXXX --auto -f "$($CSCLI config show --key Config.API.Client.CredentialsFilePath)"
     "$CSCLI" --warning hub update
 
     preload_hub_items