diff --git a/cmd/crowdsec-cli/capi.go b/cmd/crowdsec-cli/capi.go index 0261eab9c..e316abbc6 100644 --- a/cmd/crowdsec-cli/capi.go +++ b/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)) } diff --git a/cmd/crowdsec-cli/config_restore.go b/cmd/crowdsec-cli/config_restore.go index 56f628281..ab49836a9 100644 --- a/cmd/crowdsec-cli/config_restore.go +++ b/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) } diff --git a/cmd/crowdsec-cli/lapi.go b/cmd/crowdsec-cli/lapi.go index b2870cb20..755c2cb6d 100644 --- a/cmd/crowdsec-cli/lapi.go +++ b/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)) } diff --git a/cmd/crowdsec-cli/machines.go b/cmd/crowdsec-cli/machines.go index 6c97c0109..b7ceed254 100644 --- a/cmd/crowdsec-cli/machines.go +++ b/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 } @@ -220,7 +220,7 @@ func runMachinesAdd(cmd *cobra.Command, args []string) error { return err } - forceAdd, err := flags.GetBool("force") + force, err := flags.GetBool("force") 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) || 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 @@ func runMachinesAdd(cmd *cobra.Command, args []string) error { 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 @@ 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)) } diff --git a/test/bats/30_machines.bats b/test/bats/30_machines.bats index d5ddf840f..f321b8f3f 100644 --- a/test/bats/30_machines.bats +++ b/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 diff --git a/test/lib/config/config-global b/test/lib/config/config-global index 7814cefbb..22ac76df6 100755 --- a/test/lib/config/config-global +++ b/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" diff --git a/test/lib/config/config-local b/test/lib/config/config-local index 0941484ff..e3b7bc685 100755 --- a/test/lib/config/config-local +++ b/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"