TUN-4118: Don't overwrite existing file with tunnel credentials. For ad-hoc tunnels, this means tunnel won't start if there's a file in the way.

Author: ipostelnik

CHANGES.md

@@ -12,7 +12,8 @@
 
 ### Improvements
 
-- none
+- Tunnel create command, as well as, running ad-hoc tunnels using `cloudflared tunnel -name NAME`, will not overwrite
+  existing files when writing tunnel credentials. 
 
 ### Bug Fixes
 

cmd/cloudflared/tunnel/subcommand_context.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/google/uuid"
@@ -147,7 +148,7 @@ func (sc *subcommandContext) readTunnelCredentials(credFinder CredFinder) (conne
 	return credentials, nil
 }
 
-func (sc *subcommandContext) create(name string, credentialsOutputPath string) (*tunnelstore.Tunnel, error) {
+func (sc *subcommandContext) create(name string, credentialsFilePath string) (*tunnelstore.Tunnel, error) {
 	client, err := sc.client()
 	if err != nil {
 		return nil, errors.Wrap(err, "couldn't create client to talk to Argo Tunnel backend")
@@ -173,27 +174,40 @@ func (sc *subcommandContext) create(name string, credentialsOutputPath string) (
 		TunnelID:     tunnel.ID,
 		TunnelName:   name,
 	}
-	filePath, writeFileErr := writeTunnelCredentials(credential.certPath, credentialsOutputPath, &tunnelCredentials)
+	usedCertPath := false
+	if credentialsFilePath == "" {
+		originCertDir := filepath.Dir(credential.certPath)
+		credentialsFilePath, err = tunnelFilePath(tunnelCredentials.TunnelID, originCertDir)
+		if err != nil {
+			return nil, err
+		}
+		usedCertPath = true
+	}
+	writeFileErr := writeTunnelCredentials(credentialsFilePath, &tunnelCredentials)
 	if writeFileErr != nil {
 		var errorLines []string
-		errorLines = append(errorLines, fmt.Sprintf("Your tunnel '%v' was created with ID %v. However, cloudflared couldn't write to the tunnel credentials file at %v.json.", tunnel.Name, tunnel.ID, tunnel.ID))
+		errorLines = append(errorLines, fmt.Sprintf("Your tunnel '%v' was created with ID %v. However, cloudflared couldn't write tunnel credentials to %s.", tunnel.Name, tunnel.ID, credentialsFilePath))
 		errorLines = append(errorLines, fmt.Sprintf("The file-writing error is: %v", writeFileErr))
 		if deleteErr := client.DeleteTunnel(tunnel.ID); deleteErr != nil {
 			errorLines = append(errorLines, fmt.Sprintf("Cloudflared tried to delete the tunnel for you, but encountered an error. You should use `cloudflared tunnel delete %v` to delete the tunnel yourself, because the tunnel can't be run without the tunnelfile.", tunnel.ID))
 			errorLines = append(errorLines, fmt.Sprintf("The delete tunnel error is: %v", deleteErr))
 		} else {
-			errorLines = append(errorLines, fmt.Sprintf("The tunnel was deleted, because the tunnel can't be run without the tunnelfile"))
+			errorLines = append(errorLines, fmt.Sprintf("The tunnel was deleted, because the tunnel can't be run without the credentials file"))
 		}
 		errorMsg := strings.Join(errorLines, "\n")
 		return nil, errors.New(errorMsg)
 	}
-	sc.log.Info().Msgf("Tunnel credentials written to %v. cloudflared chose this file based on where your origin certificate was found. Keep this file secret. To revoke these credentials, delete the tunnel.", filePath)
 
 	if outputFormat := sc.c.String(outputFormatFlag.Name); outputFormat != "" {
 		return nil, renderOutput(outputFormat, &tunnel)
 	}
 
-	sc.log.Info().Msgf("Created tunnel %s with id %s", tunnel.Name, tunnel.ID)
+	fmt.Printf("Tunnel credentials written to %v.", credentialsFilePath)
+	if usedCertPath {
+		fmt.Print(" cloudflared chose this file based on where your origin certificate was found.")
+	}
+	fmt.Println(" Keep this file secret. To revoke these credentials, delete the tunnel.")
+	fmt.Printf("\nCreated tunnel %s with id %s\n", tunnel.Name, tunnel.ID)
 	return tunnel, nil
 }
 

cmd/cloudflared/tunnel/subcommands.go

@@ -183,27 +183,20 @@ func tunnelFilePath(tunnelID uuid.UUID, directory string) (string, error) {
 	return homedir.Expand(filePath)
 }
 
-// If an `outputFile` is given, write the credentials there.
-// Otherwise, write it to the same directory as the originCert,
-// with the filename `<tunnel id>.json`.
-func writeTunnelCredentials(
-	originCertPath, outputFile string,
-	credentials *connection.Credentials,
-) (filePath string, err error) {
-	filePath = outputFile
-	if outputFile == "" {
-		originCertDir := filepath.Dir(originCertPath)
-		filePath, err = tunnelFilePath(credentials.TunnelID, originCertDir)
-	}
-	if err != nil {
-		return "", err
+// writeTunnelCredentials saves `credentials` as a JSON into `filePath`, only if
+// the file does not exist already
+func writeTunnelCredentials(filePath string, credentials *connection.Credentials) error {
+	if _, err := os.Stat(filePath); !os.IsNotExist(err) {
+		if err == nil {
+			return fmt.Errorf("%s already exists", filePath)
+		}
+		return err
 	}
-	// Write the name and ID to the file too
 	body, err := json.Marshal(credentials)
 	if err != nil {
-		return "", errors.Wrap(err, "Unable to marshal tunnel credentials to JSON")
+		return errors.Wrap(err, "Unable to marshal tunnel credentials to JSON")
 	}
-	return filePath, ioutil.WriteFile(filePath, body, 400)
+	return ioutil.WriteFile(filePath, body, 400)
 }
 
 func buildListCommand() *cli.Command {