diff --git a/.gitignore b/.gitignore index 0fadbab..187ae58 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ dist/ # test binary, built with `go test -c` *.test +*-test.yaml # output of the go coverage tool, specifically when used with LiteIDE *.out diff --git a/cmd/setup.go b/cmd/setup.go index 0fda757..e5dcbb3 100644 --- a/cmd/setup.go +++ b/cmd/setup.go @@ -4,11 +4,11 @@ import ( "bufio" "fmt" "os" - "strings" "github.com/borisdvlpr/gotail/internal/config" ierror "github.com/borisdvlpr/gotail/internal/error" "github.com/borisdvlpr/gotail/internal/file" + "github.com/borisdvlpr/gotail/internal/format" "github.com/borisdvlpr/gotail/internal/input" "github.com/borisdvlpr/gotail/internal/system" "github.com/spf13/afero" @@ -35,9 +35,8 @@ func NewSetupCmd(deps SetupCommand) *cobra.Command { Short: "Setup Tailscale on a new device", RunE: func(cmd *cobra.Command, args []string) (err error) { flags := []string{"tailscale", "up", "--ssh"} - initConfig := []string{ - "runcmd:\n", - ` - [ sh, -c, curl -fsSL https://tailscale.com/install.sh | sh ]` + "\n", + runcmds := [][]string{ + {"sh", "-c", "curl -fsSL https://tailscale.com/install.sh | sh"}, } cfg := &config.Config{} @@ -62,7 +61,6 @@ func NewSetupCmd(deps SetupCommand) *cobra.Command { if err = yaml.Unmarshal(configFile, &cfg); err != nil { return err } - } else { cfg.ExitNode, err = input.PromptUser("Setup device as an exit node?", []string{"y", "n"}) if err != nil { @@ -86,6 +84,11 @@ func NewSetupCmd(deps SetupCommand) *cobra.Command { return err } + cfg.Tags, err = input.PromptUser("Please enter tags for this device (comma separated):", nil) + if err != nil { + return err + } + cfg.AuthKey, err = input.PromptUser("Please enter your Tailscale authkey:", nil) if err != nil { return err @@ -106,19 +109,41 @@ func NewSetupCmd(deps SetupCommand) *cobra.Command { return err } - initConfig = append(initConfig, ` - [ sh, -c, echo 'net.ipv4.ip_forward = 1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf && echo 'net.ipv6.conf.all.forwarding = 1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf && sudo sysctl -p /etc/sysctl.d/99-tailscale.conf ]`+"\n") + runcmds = append(runcmds, []string{ + "sh", "-c", + "echo 'net.ipv4.ip_forward = 1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf && " + + "echo 'net.ipv6.conf.all.forwarding = 1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf && " + + "sudo sysctl -p /etc/sysctl.d/99-tailscale.conf", + }) flags = append(flags, fmt.Sprintf("--advertise-routes=%s", cfg.Subnets)) } if cfg.Hostname != "" { flags = append(flags, fmt.Sprintf("--hostname=%s", cfg.Hostname)) - initConfig = append(initConfig, fmt.Sprintf(` - [ sh, -c, sudo hostnamectl hostname %s ]`+"\n", cfg.Hostname)) + runcmds = append(runcmds, []string{ + "sh", "-c", fmt.Sprintf("sudo hostnamectl hostname %s", cfg.Hostname), + }) + } + + if cfg.Tags != "" { + normalized, tagErr := input.ValidateTags(cfg.Tags) + if tagErr != nil { + return tagErr + } + cfg.Tags = normalized + flags = append(flags, fmt.Sprintf("--advertise-tags=%s", cfg.Tags)) + cmd.Printf("This device will be tagged: %s.\n", cfg.Tags) } flags = append(flags, fmt.Sprintf("--authkey=%s", cfg.AuthKey)) + runcmds = append(runcmds, flags) + cmd.Println("Adding Tailscale to 'user-data' file.") - initConfig = append(initConfig, fmt.Sprintf(" - [ %s ]\n", strings.Join(flags, ", "))) + initConfig := []string{"runcmd:\n"} + for _, rc := range runcmds { + initConfig = append(initConfig, " - "+format.BuildRunCmd(rc)+"\n") + } initFile, err := deps.Fsys.OpenFile(filePath, os.O_APPEND|os.O_WRONLY, 0644) if err != nil { @@ -134,8 +159,7 @@ func NewSetupCmd(deps SetupCommand) *cobra.Command { writer := bufio.NewWriter(initFile) for _, conf := range initConfig { - _, err = writer.WriteString(conf) - if err != nil { + if _, err = writer.WriteString(conf); err != nil { return err } } diff --git a/cmd/setup_test.go b/cmd/setup_test.go index fe4cbdb..7786c40 100644 --- a/cmd/setup_test.go +++ b/cmd/setup_test.go @@ -63,7 +63,7 @@ func TestSetup_Success(t *testing.T) { contentBytes, _ := afero.ReadFile(mockFS, userDataPath) content := string(contentBytes) - if !strings.Contains(content, "- [ sh, -c, sudo hostnamectl hostname raspberrypi ]") { + if !strings.Contains(content, `- ["sh", "-c", "sudo hostnamectl hostname raspberrypi"]`) { t.Errorf("Expected user-data to contain hostname 'raspberrypi', got:\n%s", content) } } diff --git a/go.mod b/go.mod index abb565a..0439619 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,8 @@ require ( github.com/spf13/afero v1.15.0 github.com/spf13/cobra v1.10.2 github.com/spf13/viper v1.21.0 - golang.org/x/sys v0.44.0 + go.yaml.in/yaml/v3 v3.0.4 + golang.org/x/sys v0.47.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -14,11 +15,10 @@ require ( github.com/fsnotify/fsnotify v1.10.1 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/pelletier/go-toml/v2 v2.3.1 // indirect + github.com/pelletier/go-toml/v2 v2.4.3 // indirect github.com/sagikazarmark/locafero v0.12.0 // indirect github.com/spf13/cast v1.10.0 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect - go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/text v0.37.0 // indirect + golang.org/x/text v0.40.0 // indirect ) diff --git a/go.sum b/go.sum index 9d273b5..5eecac3 100644 --- a/go.sum +++ b/go.sum @@ -15,8 +15,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/pelletier/go-toml/v2 v2.3.1 h1:MYEvvGnQjeNkRF1qUuGolNtNExTDwct51yp7olPtrEc= -github.com/pelletier/go-toml/v2 v2.3.1/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= +github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= @@ -41,10 +41,10 @@ github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8 github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= -golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= -golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/config/config.go b/internal/config/config.go index 928f56d..89bc353 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,26 +1,27 @@ package config -import ierr "github.com/borisdvlpr/gotail/internal/error" +import ierror "github.com/borisdvlpr/gotail/internal/error" type Config struct { ExitNode string `yaml:"exit_node"` SubnetRouter string `yaml:"subnet_router"` Subnets string `yaml:"subnets"` Hostname string `yaml:"hostname"` + Tags string `yaml:"tags"` AuthKey string `yaml:"auth_key"` } func (c *Config) Validate() error { if c.AuthKey == "" { - return ierr.StatusError{Status: "auth key is required", StatusCode: 1} + return ierror.StatusError{Status: "auth key is required", StatusCode: 1} } if c.SubnetRouter == "y" && c.Subnets == "" { - return ierr.StatusError{Status: "subnets are required when subnet router is enabled", StatusCode: 1} + return ierror.StatusError{Status: "subnets are required when subnet router is enabled", StatusCode: 1} } if c.Hostname == "" { - return ierr.StatusError{Status: "hostname is required", StatusCode: 1} + return ierror.StatusError{Status: "hostname is required", StatusCode: 1} } return nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ed4036f..ccef7ea 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -4,7 +4,7 @@ import ( "errors" "testing" - ierr "github.com/borisdvlpr/gotail/internal/error" + ierror "github.com/borisdvlpr/gotail/internal/error" ) type ValidateConfigTestCase struct { @@ -16,40 +16,55 @@ type ValidateConfigTestCase struct { func TestConfigValidate(t *testing.T) { testCases := []ValidateConfigTestCase{ { - id: "case_01", + id: "exit_node_valid", config: Config{ExitNode: "y", SubnetRouter: "n", Hostname: "test-host", AuthKey: "tskey-abcd1234"}, expectedError: nil, }, { - id: "case_02", + id: "subnet_router_valid", config: Config{ExitNode: "n", SubnetRouter: "y", Subnets: "192.168.1.0/24", Hostname: "subnet-router", AuthKey: "tskey-abcd1234"}, expectedError: nil, }, { - id: "case_03", + id: "missing_auth_key", config: Config{ExitNode: "y", SubnetRouter: "n", Hostname: "test-host", AuthKey: ""}, - expectedError: ierr.StatusError{Status: "auth key is required", StatusCode: 1}, + expectedError: ierror.StatusError{Status: "auth key is required", StatusCode: 1}, }, { - id: "case_04", + id: "missing_hostname", config: Config{ExitNode: "y", SubnetRouter: "n", Hostname: "", AuthKey: "tskey-abcd1234"}, - expectedError: ierr.StatusError{Status: "hostname is required", StatusCode: 1}, + expectedError: ierror.StatusError{Status: "hostname is required", StatusCode: 1}, }, { - id: "case_05", + id: "subnet_router_without_subnets", config: Config{ExitNode: "n", SubnetRouter: "y", Subnets: "", Hostname: "subnet-router", AuthKey: "tskey-abcd1234"}, - expectedError: ierr.StatusError{Status: "subnets are required when subnet router is enabled", StatusCode: 1}, + expectedError: ierror.StatusError{Status: "subnets are required when subnet router is enabled", StatusCode: 1}, }, { - id: "case_06", + id: "simple_node_valid", config: Config{ExitNode: "n", SubnetRouter: "n", Hostname: "simple-node", AuthKey: "tskey-abcd1234"}, expectedError: nil, }, { - id: "case_07", + id: "fully_configured_valid", config: Config{ExitNode: "y", SubnetRouter: "y", Subnets: "192.168.1.0/24,10.0.0.0/16", Hostname: "fully-configured-node", AuthKey: "tskey-abcdefghijklmnop"}, expectedError: nil, }, + { + id: "tags_valid", + config: Config{ExitNode: "n", SubnetRouter: "n", Hostname: "tagged-node", Tags: "server,prod", AuthKey: "tskey-abcd1234"}, + expectedError: nil, + }, + { + id: "tags_do_not_mask_missing_auth_key", + config: Config{ExitNode: "n", SubnetRouter: "n", Hostname: "tagged-node", Tags: "server", AuthKey: ""}, + expectedError: ierror.StatusError{Status: "auth key is required", StatusCode: 1}, + }, + { + id: "tags_with_full_config_valid", + config: Config{ExitNode: "y", SubnetRouter: "y", Subnets: "192.168.1.0/24,10.0.0.0/16", Hostname: "full-node", Tags: "exit,router", AuthKey: "tskey-abcd1234"}, + expectedError: nil, + }, } for _, tc := range testCases { diff --git a/internal/file/file.go b/internal/file/file.go index 2bc03a6..ecb0785 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -17,6 +17,8 @@ import ( "github.com/spf13/afero" ) +var errFound = errors.New("file found") + // DriveSearcher is the platform-specific strategy for locating a file across // the drives or mountpoints visible on the current OS. Each platform provides // its own implementation via NewSystemSearcher; an unsupported operating system @@ -62,13 +64,13 @@ func GetFilePath(fsys afero.Fs, rootDir string, fileName string) (string, error) if !info.IsDir() && info.Name() == fileName { foundPath = path - return filepath.SkipDir + return errFound } return nil }) - if err != nil && !errors.Is(err, filepath.SkipDir) { + if err != nil && !errors.Is(err, errFound) { return "", fmt.Errorf("%w", err) } diff --git a/internal/file/file_test.go b/internal/file/file_test.go index e7a5645..41ee228 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -16,12 +16,12 @@ type GetFilePathTestCase struct { func TestGetFilePath(t *testing.T) { testCases := []GetFilePathTestCase{ { - id: "case_01", + id: "valid_path", file: "user-data", expectedPath: filepath.Join("file-test-dir", "user-data"), }, { - id: "case_02", + id: "empty_path", file: "", expectedPath: "", }, diff --git a/internal/file/linux_utils_test.go b/internal/file/linux_utils_test.go index 29737f2..0bc6f65 100644 --- a/internal/file/linux_utils_test.go +++ b/internal/file/linux_utils_test.go @@ -12,13 +12,34 @@ type IsMountpointSearchableTestCase struct { func TestIsMountpointSearchable(t *testing.T) { testCases := []IsMountpointSearchableTestCase{ - {id: "valid_media", mountpoint: "/media/usb", expected: true}, - {id: "valid_mnt", mountpoint: "/mnt/data", expected: true}, - {id: "valid_run_media", mountpoint: "/run/media/user/disk", expected: true}, - {id: "root", mountpoint: "/", expected: false}, - {id: "empty", mountpoint: "", expected: false}, - {id: "invalid_prefix", mountpoint: "/home/user", expected: false}, - {id: "invalid_chars", mountpoint: "/mnt/\x00bad", expected: false}, + { + id: "valid_media", + mountpoint: "/media/usb", + expected: true}, + { + id: "valid_mnt", + mountpoint: "/mnt/data", + expected: true}, + { + id: "valid_run_media", + mountpoint: "/run/media/user/disk", + expected: true}, + { + id: "root", + mountpoint: "/", + expected: false}, + { + id: "empty", + mountpoint: "", + expected: false}, + { + id: "invalid_prefix", + mountpoint: "/home/user", + expected: false}, + { + id: "invalid_chars", + mountpoint: "/mnt/\x00bad", + expected: false}, } for _, tc := range testCases { diff --git a/internal/file/windows_utils_test.go b/internal/file/windows_utils_test.go index 529ec5d..8d9c0e7 100644 --- a/internal/file/windows_utils_test.go +++ b/internal/file/windows_utils_test.go @@ -12,20 +12,62 @@ type IsDriveSearchableTestCase struct { func TestIsDriveSearchable(t *testing.T) { testCases := []IsDriveSearchableTestCase{ - {name: "removable_fat32", drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "FAT32"}, expected: true}, - {name: "removable_fat", drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "FAT"}, expected: true}, - {name: "removable_exfat", drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "exFAT"}, expected: true}, - {name: "fixed_fat32_internal_sd", drive: WinDrive{Root: `E:\`, Type: driveTypeFixed, FileSystem: "FAT32"}, expected: true}, - {name: "fixed_ntfs_os_volume", drive: WinDrive{Root: `C:\`, Type: driveTypeFixed, FileSystem: "NTFS"}, expected: false}, - {name: "removable_no_filesystem", drive: WinDrive{Root: `H:\`, Type: driveTypeRemovable, FileSystem: ""}, expected: false}, - {name: "removable_unknown_filesystem", drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "ext4"}, expected: false}, - {name: "cdrom_drive", drive: WinDrive{Root: `E:\`, Type: 5, FileSystem: "CDFS"}, expected: false}, - {name: "network_drive", drive: WinDrive{Root: `Z:\`, Type: 4, FileSystem: "NTFS"}, expected: false}, - {name: "empty_root", drive: WinDrive{Root: "", Type: driveTypeRemovable, FileSystem: "FAT32"}, expected: false}, - {name: "missing_backslash", drive: WinDrive{Root: "D:", Type: driveTypeRemovable, FileSystem: "FAT32"}, expected: false}, - {name: "letter_only", drive: WinDrive{Root: "D", Type: driveTypeRemovable, FileSystem: "FAT32"}, expected: false}, - {name: "linux_path", drive: WinDrive{Root: "/mnt/usb", Type: driveTypeRemovable, FileSystem: "FAT32"}, expected: false}, - {name: "forward_slash", drive: WinDrive{Root: `D:/`, Type: driveTypeRemovable, FileSystem: "FAT32"}, expected: false}, + { + name: "removable_fat32", + drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "FAT32"}, + expected: true}, + { + name: "removable_fat", + drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "FAT"}, + expected: true}, + { + name: "removable_exfat", + drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "exFAT"}, + expected: true}, + { + name: "fixed_fat32_internal_sd", + drive: WinDrive{Root: `E:\`, Type: driveTypeFixed, FileSystem: "FAT32"}, + expected: true}, + { + name: "fixed_ntfs_os_volume", + drive: WinDrive{Root: `C:\`, Type: driveTypeFixed, FileSystem: "NTFS"}, + expected: false}, + { + name: "removable_no_filesystem", + drive: WinDrive{Root: `H:\`, Type: driveTypeRemovable, FileSystem: ""}, + expected: false}, + { + name: "removable_unknown_filesystem", + drive: WinDrive{Root: `D:\`, Type: driveTypeRemovable, FileSystem: "ext4"}, + expected: false}, + { + name: "cdrom_drive", + drive: WinDrive{Root: `E:\`, Type: 5, FileSystem: "CDFS"}, + expected: false}, + { + name: "network_drive", + drive: WinDrive{Root: `Z:\`, Type: 4, FileSystem: "NTFS"}, + expected: false}, + { + name: "empty_root", + drive: WinDrive{Root: "", Type: driveTypeRemovable, FileSystem: "FAT32"}, + expected: false}, + { + name: "missing_backslash", + drive: WinDrive{Root: "D:", Type: driveTypeRemovable, FileSystem: "FAT32"}, + expected: false}, + { + name: "letter_only", + drive: WinDrive{Root: "D", Type: driveTypeRemovable, FileSystem: "FAT32"}, + expected: false}, + { + name: "linux_path", + drive: WinDrive{Root: "/mnt/usb", Type: driveTypeRemovable, FileSystem: "FAT32"}, + expected: false}, + { + name: "forward_slash", + drive: WinDrive{Root: `D:/`, Type: driveTypeRemovable, FileSystem: "FAT32"}, + expected: false}, } for _, tc := range testCases { diff --git a/internal/format/format.go b/internal/format/format.go new file mode 100644 index 0000000..e689c96 --- /dev/null +++ b/internal/format/format.go @@ -0,0 +1,19 @@ +package format + +import "strings" + +// BuildRunCmd renders args as a YAML flow sequence with each element +// double-quoted, e.g. ["tailscale", "up", "--advertise-routes=10.0.0.0/24,10.0.1.0/24"]. +// Quoting is required: cloud-init parses runcmd entries as YAML, so an unquoted +// element containing a comma (multiple subnets) would be split into separate +// arguments by the YAML parser. +func BuildRunCmd(args []string) string { + quoted := make([]string, len(args)) + for i, a := range args { + esc := strings.ReplaceAll(a, `\`, `\\`) + esc = strings.ReplaceAll(esc, `"`, `\"`) + quoted[i] = `"` + esc + `"` + } + + return "[" + strings.Join(quoted, ", ") + "]" +} diff --git a/internal/format/format_test.go b/internal/format/format_test.go new file mode 100644 index 0000000..de7feee --- /dev/null +++ b/internal/format/format_test.go @@ -0,0 +1,99 @@ +package format + +import ( + "reflect" + "testing" + + "go.yaml.in/yaml/v3" +) + +type BuildRunCmdTestCase struct { + id string + input []string + expected string +} + +func TestBuildRunCmd(t *testing.T) { + testCases := []BuildRunCmdTestCase{ + { + id: "typical_command", + input: []string{"tailscale", "up", "--ssh"}, + expected: `["tailscale", "up", "--ssh"]`, + }, + { + id: "single_element", + input: []string{"sh"}, + expected: `["sh"]`, + }, + { + id: "comma_in_value_preserved", + input: []string{"--advertise-routes=192.168.1.0/24,10.0.0.0/16"}, + expected: `["--advertise-routes=192.168.1.0/24,10.0.0.0/16"]`, + }, + { + id: "embedded_double_quote_escaped", + input: []string{`say "hi"`}, + expected: `["say \"hi\""]`, + }, + { + id: "embedded_backslash_escaped", + input: []string{`path\to\thing`}, + expected: `["path\\to\\thing"]`, + }, + { + id: "backslash_and_quote_escaped", + input: []string{`weird\"mix`}, + expected: `["weird\\\"mix"]`, + }, + { + id: "empty_string_element", + input: []string{""}, + expected: `[""]`, + }, + { + id: "empty_slice", + input: []string{}, + expected: `[]`, + }, + { + id: "nil_slice", + input: nil, + expected: `[]`, + }, + } + + for _, tc := range testCases { + output := BuildRunCmd(tc.input) + + if output != tc.expected { + t.Errorf("%v: BuildRunCmd(%q) = %q, expected %q", tc.id, tc.input, output, tc.expected) + } + } +} + +// TestBuildRunCmd_RoundTrip asserts the real contract: the output is a valid +// YAML flow sequence that parses back to the exact input args. This is what +// protects against the comma-splitting bug and catches any escaping error, +// independent of the exact spacing/quoting style. +func TestBuildRunCmd_RoundTrip(t *testing.T) { + inputs := [][]string{ + {"tailscale", "up", "--ssh", "--advertise-routes=192.168.1.0/24,10.0.0.0/16", "--authkey=tskey-abcd1234"}, + {"sh", "-c", "curl -fsSL https://tailscale.com/install.sh | sh"}, + {"sh", "-c", "echo 'net.ipv4.ip_forward = 1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf"}, + {`arg with "quotes"`, `arg\with\backslashes`, "value,with,commas"}, + } + + for i, args := range inputs { + out := BuildRunCmd(args) + + var parsed []string + if err := yaml.Unmarshal([]byte(out), &parsed); err != nil { + t.Errorf("case %d: output %q is not valid YAML: %v", i, out, err) + continue + } + + if !reflect.DeepEqual(parsed, args) { + t.Errorf("case %d: round-trip mismatch\n input: %q\n output: %s\n parsed: %q", i, args, out, parsed) + } + } +} diff --git a/internal/input/input_test.go b/internal/input/input_test.go index bebe033..53d3eff 100644 --- a/internal/input/input_test.go +++ b/internal/input/input_test.go @@ -23,21 +23,21 @@ func TestPromptUser(t *testing.T) { testCases := []PromptUserTestCase{ { - id: "case_01", + id: "valid_allowed_reply", input: "y\n", allowedInputs: []string{"y", "n"}, expectedAnswer: "y", expectedError: nil, }, { - id: "case_02", + id: "freeform_input_accepted", input: "tskey_test_1234_5678\n", allowedInputs: nil, expectedAnswer: "tskey_test_1234_5678", expectedError: nil, }, { - id: "case_03", + id: "disallowed_reply_aborts", input: "asdf\n", allowedInputs: []string{"y", "n"}, expectedAnswer: "y", diff --git a/internal/input/validation.go b/internal/input/validation.go index a4473f1..e8eaf0a 100644 --- a/internal/input/validation.go +++ b/internal/input/validation.go @@ -3,20 +3,46 @@ package input import ( "fmt" "net" + "regexp" "strings" ierror "github.com/borisdvlpr/gotail/internal/error" ) +var tagRegexp = regexp.MustCompile(`^[a-z0-9-]+$`) + // ValidateSubnets validates a comma-separated list of subnets. // Each subnet must be in CIDR notation. If any subnet is not in // the correct format, an error is returned indicating the invalid // subnet and a status code of 1. func ValidateSubnets(subnets string) error { for subnet := range strings.SplitSeq(subnets, ",") { - if _, _, err := net.ParseCIDR(subnet); err != nil { + if _, _, err := net.ParseCIDR(strings.TrimSpace(subnet)); err != nil { return ierror.StatusError{Status: fmt.Sprintf("%s: invalid subnet format", subnet), StatusCode: 1} } } return nil } + +// ValidateTags validates and normalizes a comma-separated list of Tailscale +// tag names. Users provide bare names (e.g. "server,prod-2"); each must contain +// only lowercase letters, digits, and hyphens. Tailscale lowercases tag names, +// so input is trimmed and lowercased before validation, and the "tag:" prefix +// is added afterward. Returns the normalized, comma-separated tags (e.g. +// "tag:server,tag:prod-2"), or a StatusError describing the first invalid tag. +func ValidateTags(tags string) (string, error) { + var normalized []string + for tag := range strings.SplitSeq(tags, ",") { + tag = strings.ToLower(strings.TrimSpace(tag)) + if !tagRegexp.MatchString(tag) { + return "", ierror.StatusError{ + Status: fmt.Sprintf("%q: invalid tag (expected letters, digits, and hyphens)", tag), + StatusCode: 1, + } + } + + normalized = append(normalized, "tag:"+tag) + } + + return strings.Join(normalized, ","), nil +} diff --git a/internal/input/validation_test.go b/internal/input/validation_test.go index d8f2951..2ac57f9 100644 --- a/internal/input/validation_test.go +++ b/internal/input/validation_test.go @@ -13,65 +13,72 @@ type ValidateSubnetsTestCase struct { expectedError error } +type ValidateTagsTestCase struct { + id string + input string + expectedOutput string + expectError bool +} + func TestValidateSubnet(t *testing.T) { testCases := []ValidateSubnetsTestCase{ { - id: "case_01", + id: "single_ipv4_valid", subnet: "192.168.1.1/24", expectedError: nil, }, { - id: "case_02", + id: "multiple_ipv4_valid", subnet: "192.168.1.1/24,192.168.2.2/24", expectedError: nil, }, { - id: "case_03", + id: "single_ipv6_valid", subnet: "2001:db8::/32", expectedError: nil, }, { - id: "case_04", + id: "multiple_ipv6_valid", subnet: "2001:db8::/32,2001:db8::/32", expectedError: nil, }, { - id: "case_05", + id: "empty_string", subnet: "", expectedError: ierror.StatusError{Status: ": invalid subnet format", StatusCode: 1}, }, { - id: "case_06", + id: "ipv4_missing_prefix_length", subnet: "192.168.1.1", expectedError: ierror.StatusError{Status: "192.168.1.1: invalid subnet format", StatusCode: 1}, }, { - id: "case_07", + id: "trailing_comma", subnet: "192.168.1.1/24,", expectedError: ierror.StatusError{Status: ": invalid subnet format", StatusCode: 1}, }, { - id: "case_08", + id: "leading_comma", subnet: ",192.168.1.1", expectedError: ierror.StatusError{Status: ": invalid subnet format", StatusCode: 1}, }, { - id: "case_09", + id: "second_ipv4_missing_prefix_length", subnet: "192.168.1.1/24,192.168.2.2", expectedError: ierror.StatusError{Status: "192.168.2.2: invalid subnet format", StatusCode: 1}, }, { - id: "case_10", + id: "incomplete_ipv4", subnet: "192.168.1.", expectedError: ierror.StatusError{Status: "192.168.1.: invalid subnet format", StatusCode: 1}, }, { - id: "case_11", + id: "ipv6_missing_prefix_length", subnet: "2001:db8::", expectedError: ierror.StatusError{Status: "2001:db8::: invalid subnet format", StatusCode: 1}, }, { - id: "case_12", + id: "second_ipv6_missing_prefix_length", subnet: "2001:db8::/32,2001:db8::", expectedError: ierror.StatusError{Status: "2001:db8::: invalid subnet format", StatusCode: 1}, }, @@ -84,3 +91,88 @@ func TestValidateSubnet(t *testing.T) { } } } +func TestValidateTags(t *testing.T) { + testCases := []ValidateTagsTestCase{ + { + id: "single_tag", + input: "server", + expectedOutput: "tag:server", + expectError: false, + }, + { + id: "multiple_tags", + input: "server,prod", + expectedOutput: "tag:server,tag:prod", + expectError: false, + }, + { + id: "hyphenated_name", + input: "prod-2", + expectedOutput: "tag:prod-2", + expectError: false}, + { + id: "uppercase_normalized", + input: "Server", + expectedOutput: "tag:server", + expectError: false}, + { + id: "whitespace_trimmed", + input: " server , prod ", + expectedOutput: "tag:server,tag:prod", + expectError: false}, + { + id: "prefix_included", + input: "tag:server", + expectError: true}, + { + id: "space_in_name", + input: "my server", + expectError: true}, + { + id: "underscore", + input: "prod_env", + expectError: true}, + { + id: "special_char", + input: "server!", + expectError: true}, + { + id: "trailing_comma", + input: "server,", + expectError: true}, + { + id: "whitespace_only", + input: " ", + expectError: true}, + } + + for _, tc := range testCases { + output, err := ValidateTags(tc.input) + + if tc.expectError { + if err == nil { + t.Errorf("%v: ValidateTags(%q) returned no error, expected one", tc.id, tc.input) + continue + } + + var statusErr ierror.StatusError + if !errors.As(err, &statusErr) { + t.Errorf("%v: ValidateTags(%q) returned %T, expected ierror.StatusError", tc.id, tc.input, err) + + } else if statusErr.StatusCode != 1 { + t.Errorf("%v: ValidateTags(%q) returned status code %d, expected 1", tc.id, tc.input, statusErr.StatusCode) + } + + continue + } + + if err != nil { + t.Errorf("%v: ValidateTags(%q) returned error %v, expected none", tc.id, tc.input, err) + continue + } + + if output != tc.expectedOutput { + t.Errorf("%v: ValidateTags(%q) = %q, expected %q", tc.id, tc.input, output, tc.expectedOutput) + } + } +} diff --git a/internal/system/system_unix_test.go b/internal/system/system_unix_test.go index ea50fa6..125d59b 100644 --- a/internal/system/system_unix_test.go +++ b/internal/system/system_unix_test.go @@ -31,17 +31,17 @@ type MockRootCheckerTestCase struct { func TestMockRootChecker(t *testing.T) { testCases := []MockRootCheckerTestCase{ { - id: "case_01", + id: "root_check_passes", checker: MockRootChecker{shouldError: false}, expectedError: nil, }, { - id: "case_02", + id: "root_check_fails_with_message", checker: MockRootChecker{shouldError: true, errorMsg: "error: permission denied"}, expectedError: ierror.StatusError{Status: "error: permission denied", StatusCode: 1}, }, { - id: "case_03", + id: "root_check_fails_without_message", checker: MockRootChecker{shouldError: true, errorMsg: ""}, expectedError: ierror.StatusError{Status: "", StatusCode: 1}, }, diff --git a/internal/system/system_windows_test.go b/internal/system/system_windows_test.go index af4ccd2..83ac53d 100644 --- a/internal/system/system_windows_test.go +++ b/internal/system/system_windows_test.go @@ -27,12 +27,12 @@ type CheckRootTestCase struct { func TestCheckRootWithChecker(t *testing.T) { testCases := []CheckRootTestCase{ { - id: "case_01", + id: "elevated_passes", checker: MockElevationChecker{elevated: true, err: nil}, expectedError: nil, }, { - id: "case_02", + id: "not_elevated_requires_admin", checker: MockElevationChecker{elevated: false, err: nil}, expectedError: ierror.StatusError{ Status: "setup must be run as administrator. please relaunch from an elevated shell (run as administrator).", @@ -40,7 +40,7 @@ func TestCheckRootWithChecker(t *testing.T) { }, }, { - id: "case_03", + id: "elevation_check_errors", checker: MockElevationChecker{elevated: false, err: errors.New("access denied")}, expectedError: ierror.StatusError{Status: "access denied", StatusCode: 1}, }, diff --git a/version.txt b/version.txt index 3eefcb9..9084fa2 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -1.0.0 +1.1.0