fix: make secrets and other kv arg parsing more rubust (#629)

* fix: make secrets and other kv arg parsing more rubust

* fix: use correct falg name for participant attribute

Author: rektdeckard

cmd/lk/agent.go

@@ -99,7 +99,10 @@ var (
 						silentFlag,
 						regionFlag,
 					},
-					ArgsUsage: "[working-dir]",
+					// NOTE: since secrets may contain commas, or indeed any special character we might want to treat as a flag separator,
+					// we disable it entirely here and require multiple --secrets flags to be used.
+					DisableSliceFlagSeparator: true,
+					ArgsUsage:                 "[working-dir]",
 				},
 				{
 					Name:   "config",
@@ -120,7 +123,10 @@ var (
 						secretsFlag,
 						secretsFileFlag,
 					},
-					ArgsUsage: "[working-dir]",
+					// NOTE: since secrets may contain commas, or indeed any special character we might want to treat as a flag separator,
+					// we disable it entirely here and require multiple --secrets flags to be used.
+					DisableSliceFlagSeparator: true,
+					ArgsUsage:                 "[working-dir]",
 				},
 				{
 					Name:   "status",
@@ -141,7 +147,10 @@ var (
 						secretsFlag,
 						secretsFileFlag,
 					},
-					ArgsUsage: "[working-dir]",
+					// NOTE: since secrets may contain commas, or indeed any special character we might want to treat as a flag separator,
+					// we disable it entirely here and require multiple --secrets flags to be used.
+					DisableSliceFlagSeparator: true,
+					ArgsUsage:                 "[working-dir]",
 				},
 				{
 					Name:   "restart",
@@ -237,7 +246,10 @@ var (
 							Value:    false,
 						},
 					},
-					ArgsUsage: "[working-dir]",
+					// NOTE: since secrets may contain commas, or indeed any special character we might want to treat as a flag separator,
+					// we disable it entirely here and require multiple --secrets flags to be used.
+					DisableSliceFlagSeparator: true,
+					ArgsUsage:                 "[working-dir]",
 				},
 			},
 		},
@@ -293,7 +305,7 @@ func createAgent(ctx context.Context, cmd *cli.Command) error {
 	}
 
 	// We have a configured project, but don't need to double-confirm if it was
-	// set via a command line flag, because intent it clear.
+	// set via a command line flag, because intent is clear.
 	if !cmd.IsSet("project") {
 		useProject := true
 		if err := huh.NewForm(huh.NewGroup(huh.NewConfirm().
@@ -1012,13 +1024,18 @@ func selectAgent(ctx context.Context, _ *cli.Command) (string, error) {
 func requireSecrets(_ context.Context, cmd *cli.Command, required, lazy bool) ([]*lkproto.AgentSecret, error) {
 	silent := cmd.Bool("silent")
 	secrets := make(map[string]*lkproto.AgentSecret)
-	for _, secret := range cmd.StringSlice("secrets") {
-		secret := strings.Split(secret, "=")
-		agentSecret := &lkproto.AgentSecret{
-			Name:  secret[0],
-			Value: []byte(secret[1]),
+
+	if values, err := parseKeyValuePairs(cmd, "secrets"); err != nil {
+		return nil, fmt.Errorf("failed to parse secrets: %w", err)
+	} else {
+		for key, val := range values {
+			agentSecret := &lkproto.AgentSecret{
+				Name:  key,
+				Value: []byte(val),
+			}
+			secrets[key] = agentSecret
 		}
-		secrets[secret[0]] = agentSecret
+
 	}
 
 	shouldReadFromDisk := cmd.IsSet("secrets-file") || !lazy || (required && len(secrets) == 0)

cmd/lk/room.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	"os"
 	"os/signal"
 	"regexp"
@@ -940,14 +941,9 @@ func joinRoom(ctx context.Context, cmd *cli.Command) error {
 		},
 	}
 
-	participantAttributes := make(map[string]string)
-
-	attrs := cmd.StringSlice("attribute")
-	for _, attr := range attrs {
-		kv := strings.Split(attr, "=")
-		if len(kv) == 2 {
-			participantAttributes[kv[0]] = kv[1]
-		}
+	participantAttributes, err := parseKeyValuePairs(cmd, "attribute")
+	if err != nil {
+		return fmt.Errorf("failed to parse participant attributes: %w", err)
 	}
 
 	// Read attributes from JSON file if specified
@@ -963,9 +959,7 @@ func joinRoom(ctx context.Context, cmd *cli.Command) error {
 		}
 
 		// Add attributes from file to the existing ones
-		for key, value := range fileAttrs {
-			participantAttributes[key] = value
-		}
+		maps.Copy(participantAttributes, fileAttrs)
 	}
 
 	room, err := lksdk.ConnectToRoom(project.URL, lksdk.ConnectInfo{

cmd/lk/utils.go

@@ -17,9 +17,11 @@ package main
 import (
 	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"strings"
 
+	"github.com/joho/godotenv"
 	"github.com/twitchtv/twirp"
 	"github.com/urfave/cli/v3"
 
@@ -161,6 +163,24 @@ func extractFlagOrArg(c *cli.Command, flag string) (string, error) {
 	return value, nil
 }
 
+func parseKeyValuePairs(c *cli.Command, flag string) (map[string]string, error) {
+	pairs := c.StringSlice(flag)
+	if len(pairs) == 0 {
+		return nil, nil
+	}
+
+	result := make(map[string]string, len(pairs))
+
+	for _, pair := range pairs {
+		if m, err := godotenv.Unmarshal(pair); err != nil {
+			return nil, fmt.Errorf("invalid key-value pair: %s: %w", pair, err)
+		} else {
+			maps.Copy(result, m)
+		}
+	}
+	return result, nil
+}
+
 type loadParams struct {
 	requireURL bool
 }

pkg/agentfs/secrets-file.go

@@ -15,11 +15,10 @@
 package agentfs
 
 import (
-	"bufio"
 	"os"
-	"strings"
 
 	"github.com/charmbracelet/huh"
+	"github.com/joho/godotenv"
 	"github.com/livekit/livekit-cli/v2/pkg/util"
 )
 
@@ -33,33 +32,12 @@ var knownEnvFiles = []string{
 }
 
 func ParseEnvFile(file string) (map[string]string, error) {
-	env := make(map[string]string)
 	f, err := os.Open(file)
 	if err != nil {
 		return nil, err
 	}
 	defer f.Close()
-
-	scanner := bufio.NewScanner(f)
-	for scanner.Scan() {
-		line := scanner.Text()
-		if strings.HasPrefix(line, "#") {
-			continue
-		}
-
-		parts := strings.SplitN(line, "=", 2)
-		if len(parts) < 2 {
-			continue
-		}
-
-		parts[0] = strings.TrimSpace(parts[0])
-		parts[1] = strings.TrimSpace(parts[1])
-		parts[1] = strings.Trim(parts[1], "\"'")
-		parts[1] = strings.Split(parts[1], "#")[0]
-		env[parts[0]] = parts[1]
-	}
-
-	return env, nil
+	return godotenv.Parse(f)
 }
 
 func DetectEnvFile(maybeFile string) (string, map[string]string, error) {