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 @@
 			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 @@
 	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 @@
 }
 
 func runMachinesAdd(cmd *cobra.Command, args []string) error {
-	var dumpFile string
 	var err error
 
 	flags := cmd.Flags()
@@ -200,7 +200,7 @@
 		return err
 	}
 
-	outputFile, err := flags.GetString("file")
+	dumpFile, err := flags.GetString("file")
 	if err != nil {
 		return err
 	}
@@ -220,7 +220,7 @@
 		return err
 	}
 
-	forceAdd, err := flags.GetBool("force")
+	force, err := flags.GetBool("force")
 	if err != nil {
 		return err
 	}
@@ -242,17 +242,28 @@
 	}
 
 	/*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) || force:
+			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 it, use "--force" 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 {
@@ -262,7 +273,7 @@
 		survey.AskOne(qs, &machinePassword)
 	}
 	password := strfmt.Password(machinePassword)
-	_, err = dbClient.CreateMachine(&machineID, &password, "", true, forceAdd, types.PasswordAuthType)
+	_, err = dbClient.CreateMachine(&machineID, &password, "", true, force, types.PasswordAuthType)
 	if err != nil {
 		return fmt.Errorf("unable to create machine: %s", err)
 	}
@@ -291,7 +302,7 @@
 		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

@@ -23,20 +23,24 @@ teardown() {
 
 #----------
 
-@test "can list machines as regular user" {
-    rune -0 cscli machines list
-}
-
 @test "we have exactly one machine" {
     rune -0 cscli machines list -o json
     rune -0 jq -c '[. | length, .[0].machineId[0:32], .[0].isValidated]' <(output)
     assert_output '[1,"githubciXXXXXXXXXXXXXXXXXXXXXXXX",true]'
 }
 
+@test "don't overwrite local credentials by default" {
+    rune -1 cscli machines add local -a -o json
+    rune -0 jq -r '.msg' <(stderr)
+    assert_output --partial 'already exists: please remove it, use "--force" or specify a different file with "-f"'
+    rune -0 cscli machines add local -a --force
+    assert_stderr --partial "Machine 'local' successfully added to the local API"
+}
+
 @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 +60,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

@@ -61,12 +61,12 @@ make_init_data() {
     ./instance-db config-yaml
     ./instance-db setup
 
+    ./bin/preload-hub-items
+
     # when installed packages are always using sqlite, so no need to regenerate
     # local credz for sqlite
 
-    ./bin/preload-hub-items
-
-    [[ "${DB_BACKEND}" == "sqlite" ]] || ${CSCLI} machines add --auto
+    [[ "${DB_BACKEND}" == "sqlite" ]] || ${CSCLI} machines add githubciXXXXXXXXXXXXXXXXXXXXXXXX --auto --force
 
     mkdir -p "$LOCAL_INIT_DIR"
 

test/lib/config/config-local

@@ -115,11 +115,12 @@ make_init_data() {
     ./instance-db config-yaml
     ./instance-db setup
 
-    "$CSCLI" --warning machines add githubciXXXXXXXXXXXXXXXXXXXXXXXX --auto
     "$CSCLI" --warning hub update
 
     ./bin/preload-hub-items
 
+    "$CSCLI" --warning machines add githubciXXXXXXXXXXXXXXXXXXXXXXXX --auto --force
+
     mkdir -p "$LOCAL_INIT_DIR"
 
     ./instance-db dump "${LOCAL_INIT_DIR}/database"