From 26d899ff4af5966de99a7034ab13b28cc83c8a4c Mon Sep 17 00:00:00 2001 From: luqueee Date: Fri, 4 Sep 2026 11:47:00 +0200 Subject: [PATCH 1/4] fix: harden clip bind defaults --- README.md | 20 +++++- cmd/mole/clip.go | 27 +++++++-- cmd/mole/clip_test.go | 17 ++++++ cmd/mole/init.go | 56 +++++++++++++---- cmd/mole/init_test.go | 107 +++++++++++++++++++++++++++++++++ examples/mole.yaml | 8 ++- internal/clip/client.go | 4 +- internal/clip/server.go | 3 +- internal/config/config.go | 50 +++++++++++++-- internal/config/config_test.go | 31 +++++++--- 10 files changed, 286 insertions(+), 37 deletions(-) create mode 100644 cmd/mole/init_test.go diff --git a/README.md b/README.md index 0e1fe1c..c6e8fe9 100644 --- a/README.md +++ b/README.md @@ -300,10 +300,24 @@ desktop. Clipboard watching is macOS-only; other platforms can still serve explicit pushes and pull. ```bash -mole clip serve # on the source machine (binds clip_listen, default 0.0.0.0:7777) +mole clip serve # on the source machine (default: loopback 127.0.0.1:7777) mole clip pull # on the target; uses clip_url from the config, or -url ``` +The clip endpoint has no authentication. The default loopback bind keeps it +off the network; for a remote pull, bind to the source machine's private +WireGuard or Tailscale address explicitly and use the same address in +`clip_url`: + +```yaml +clip_url: http://100.64.0.10:7777 +clip_listen: 100.64.0.10:7777 +``` + +Binding to `0.0.0.0:7777`, `:7777`, or `[::]:7777` is supported for controlled +networks, but `mole clip serve` emits a warning because every interface can +reach the unauthenticated endpoint. + ### Generate the config with `mole init` @@ -363,7 +377,7 @@ mole ports remove|rm [-config PATH] mole ports list|ls [-config PATH] mole config edit [-config PATH] [-editor CMD] mole init [flags] -mole clip serve [-listen 0.0.0.0:7777] [-watch] [-config PATH] [-log-level L] +mole clip serve [-listen 127.0.0.1:7777] [-watch] [-config PATH] [-log-level L] mole clip pull [-url URL] [-config PATH] [-log-level L] mole update [-version REF] [-dry-run] [-no-verify] mole version · mole help @@ -383,7 +397,7 @@ mole version · mole help | `ssh_port` | int | `22` | SSH port on the remote | | `insecure` | bool | `false` | Disable SSH host key verification (UNSAFE; dev only) | | `clip_url` | string | — | Clip server URL used by `mole clip pull` | -| `clip_listen` | string | `0.0.0.0:7777` | Bind address for `mole clip serve` | +| `clip_listen` | string | `127.0.0.1:7777` | Bind address; use a private Tailscale/WireGuard IP for remote access | | `clip_interval_ms` | int | — | Clipboard poll interval for `clip serve -watch` | Fallback `discover_ports`: diff --git a/cmd/mole/clip.go b/cmd/mole/clip.go index 22f751d..9c15769 100644 --- a/cmd/mole/clip.go +++ b/cmd/mole/clip.go @@ -1,5 +1,5 @@ // mole clip — share clipboard images between a Mac and a remote -// host (typically an LXC) over a WireGuard link. +// host (typically an LXC) over a private WireGuard or Tailscale link. // // Two sub-modes: // @@ -58,7 +58,7 @@ func runClip(args []string) int { func printClipUsage(w *os.File) { color := cliColor(w) - fmt.Fprintf(w, "%s\n\n", cBold("mole clip — share clipboard images over a WireGuard link", color)) + fmt.Fprintf(w, "%s\n\n", cBold("mole clip — share clipboard images over a private WireGuard or Tailscale link", color)) fmt.Fprintf(w, " %s\n", cBold("USAGE", color)) fmt.Fprintf(w, " mole clip %s\n", cDim(" [flags]", color)) fmt.Println() @@ -67,8 +67,8 @@ func printClipUsage(w *os.File) { fmt.Fprintf(w, " %s %s\n", cGreen("pull", color), "Run on the remote; fetch the latest image and print its path.") fmt.Println() fmt.Fprintf(w, " %s\n", cBold("NOTES", color)) - fmt.Fprintf(w, " %s\n", cDim(" serve binds -listen (default 0.0.0.0:7777). On a WireGuard-only host,", color)) - fmt.Fprintf(w, " %s\n", cDim(" the link's perimeter is the security boundary. There is no auth.", color)) + fmt.Fprintf(w, " %s\n", cDim(" serve binds -listen (default "+config.DefaultClipListen+"). The endpoint has no auth;", color)) + fmt.Fprintf(w, " %s\n", cDim(" use a private Tailscale/WireGuard address when remote access is required.", color)) fmt.Fprintf(w, " %s\n", cDim(" pull prints the image path on stdout and exits. Exit code 3 means", color)) fmt.Fprintf(w, " %s\n", cDim(" 'no image on the server yet'.", color)) } @@ -80,7 +80,7 @@ func runClipServe(args []string) int { fs := flag.NewFlagSet("clip serve", flag.ExitOnError) var ( configPath = fs.String("config", "", "path to YAML config (default: ./mole.yaml, then user-global)") - listen = fs.String("listen", "0.0.0.0:7777", "address to bind the clip HTTP server") + listen = fs.String("listen", config.DefaultClipListen, "address to bind the clip HTTP server") watch = fs.Bool("watch", true, "poll the macOS clipboard and auto-push new images") interval = fs.Duration("interval", 500*time.Millisecond, "clipboard poll cadence (only used with -watch)") logLevel = fs.String("log-level", "", "debug|info|warn|error") @@ -93,7 +93,7 @@ polls the macOS pasteboard to keep the latest image cached. Flags: -config path to YAML config (default: ./mole.yaml, then user-global) - -listen bind address (default 0.0.0.0:7777) + -listen bind address (default 127.0.0.1:7777) -watch poll the macOS clipboard and push new images (default true) -interval clipboard poll cadence (default 500ms) -log-level debug|info|warn|error`) @@ -123,6 +123,9 @@ Flags: *logLevel = cfg.LogLevel } log := newLogger(*logLevel) + if isBroadClipBind(*listen) { + log.Warn("clip server has no authentication and is listening on all interfaces", "addr", *listen, "hint", "use loopback or a private Tailscale/WireGuard address") + } ln, err := net.Listen("tcp", *listen) if err != nil { @@ -171,6 +174,18 @@ Flags: return 0 } +func isBroadClipBind(addr string) bool { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return false + } + if host == "" { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsUnspecified() +} + // runClipPull fetches the latest image from the clip server and // prints its path to stdout. Exits 0 on success, 3 on // ErrNoImage, 1 on any other error. diff --git a/cmd/mole/clip_test.go b/cmd/mole/clip_test.go index 2d74c28..b415527 100644 --- a/cmd/mole/clip_test.go +++ b/cmd/mole/clip_test.go @@ -117,6 +117,23 @@ func TestClipPull_NoImage_ExitCodeMapping(t *testing.T) { } } +func TestClipBindScope(t *testing.T) { + cases := map[string]bool{ + "127.0.0.1:7777": false, + "100.64.0.10:7777": false, + "10.0.0.10:7777": false, + "[::1]:7777": false, + "0.0.0.0:7777": true, + ":7777": true, + "[::]:7777": true, + } + for addr, wantBroad := range cases { + if got := isBroadClipBind(addr); got != wantBroad { + t.Errorf("isBroadClipBind(%q) = %v, want %v", addr, got, wantBroad) + } + } +} + // silentLogger returns a logger that throws away every record. Keeps // the test output clean. func silentLogger() *slog.Logger { diff --git a/cmd/mole/init.go b/cmd/mole/init.go index fa46620..43ba917 100644 --- a/cmd/mole/init.go +++ b/cmd/mole/init.go @@ -18,6 +18,7 @@ import ( "fmt" "io" "net" + "net/url" "os" "os/exec" "path/filepath" @@ -38,8 +39,8 @@ type initAnswers struct { PrintOnly bool // true → write to stdout instead of a file Global bool // true → ~/.config/mole/config.yaml ClipEnabled bool // true → also write clip_* keys for `mole clip` - ClipURL string // URL the LXC pulls from; e.g. http://10.0.0.1:7777 - ClipListen string // address the Mac serves on; defaults to 0.0.0.0:7777 + ClipURL string // URL the LXC pulls from; e.g. http://100.64.0.10:7777 + ClipListen string // address the Mac serves on; defaults to 127.0.0.1:7777 ClipIntervalMs int // clipboard poll cadence on the Mac } @@ -59,7 +60,7 @@ func runInit(args []string) int { force = fs.Bool("force", false, "overwrite the config file if it already exists") clip = fs.Bool("clip", false, "configure clip_* keys so `mole clip` works (skip the interactive prompt)") clipURL = fs.String("clip-url", "", "URL the LXC pulls from (only used with -clip); e.g. http://10.0.0.1:7777") - clipListen = fs.String("clip-listen", "0.0.0.0:7777", "address the Mac serves on (only used with -clip)") + clipListen = fs.String("clip-listen", "", "address the Mac serves on (only used with -clip; defaults to the clip URL or 127.0.0.1:7777)") clipInterval = fs.Int("clip-interval-ms", 500, "clipboard poll cadence in ms (only used with -clip)") ) fs.Usage = func() { @@ -244,7 +245,11 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { in.ClipURL = envDefault("MOLE_CLIP_URL", "") } if in.ClipListen == "" { - in.ClipListen = envDefault("MOLE_CLIP_LISTEN", "0.0.0.0:7777") + in.ClipListen = envDefault("MOLE_CLIP_LISTEN", "") + } + clipListenProvided := strings.TrimSpace(in.ClipListen) != "" + if in.ClipListen == "" { + in.ClipListen = config.DefaultClipListen } ans := &initAnswers{ @@ -259,6 +264,9 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { ClipIntervalMs: in.ClipIntervalMs, } ans.Ports = config.ParsePorts(in.PortsCSV) + if ans.ClipEnabled && ans.ClipURL != "" && !clipListenProvided { + ans.ClipListen = clipListenForURL(ans.ClipURL) + } // In non-interactive mode, all required values must already be set. if !opt.Interactive { @@ -341,24 +349,30 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { default: ans.AutoDiscover = false ans.Ports = nil + } + // Clipboard sharing: ask once whether to wire it up. Default is // "no" so an unsuspecting user doesn't open a port they don't // need. We only ask in interactive mode; -clip / -clip-url on // the command line skip the question entirely. if !ans.ClipEnabled && in.ClipURL == "" { - raw := prompt(opt.In, opt.Out, "Sync clipboard screenshots over WireGuard? [y/N]", "n") + raw := prompt(opt.In, opt.Out, "Sync clipboard screenshots over Tailscale/WireGuard? [y/N]", "n") ans.ClipEnabled = strings.HasPrefix(strings.ToLower(strings.TrimSpace(raw)), "y") if ans.ClipEnabled { // Ask for the URL the LXC will pull from. We default to - // 10.0.0.1:7777 (a common WireGuard /24) but the user - // almost certainly needs to override this with their - // actual wg IP. - urlRaw := prompt(opt.In, opt.Out, "Mac WireGuard IP the LXC will pull from (e.g. 10.0.0.1:7777)", "10.0.0.1:7777") + // 100.64.0.10:7777 (a representative Tailscale address), + // but the user almost certainly needs to override this with + // their actual Tailscale or WireGuard IP. + urlRaw := prompt(opt.In, opt.Out, "Mac private IP the LXC will pull from (e.g. 100.64.0.10:7777)", "100.64.0.10:7777") if !strings.HasPrefix(urlRaw, "http://") && !strings.HasPrefix(urlRaw, "https://") { urlRaw = "http://" + urlRaw } ans.ClipURL = urlRaw - ans.ClipListen = prompt(opt.In, opt.Out, "Address mole clip serve binds on the Mac", "0.0.0.0:7777") + listenDefault := clipListenForURL(ans.ClipURL) + if clipListenProvided { + listenDefault = ans.ClipListen + } + ans.ClipListen = prompt(opt.In, opt.Out, "Address mole clip serve binds on the Mac (use a private Tailscale/WireGuard IP for remote access)", listenDefault) intervalRaw := prompt(opt.In, opt.Out, "Clipboard poll interval (ms)", "500") if n, perr := strconv.Atoi(strings.TrimSpace(intervalRaw)); perr == nil && n > 0 { ans.ClipIntervalMs = n @@ -367,7 +381,6 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { } } } -} // Save location. If the user already pinned a path via -config / // -global / env var, respect that and don't ask. @@ -434,7 +447,7 @@ func renderYAML(ans *initAnswers) string { } } if ans.ClipEnabled { - b.WriteString("\n# Clipboard sharing over a WireGuard link. `mole clip serve`\n") + b.WriteString("\n# Clipboard sharing over a private WireGuard or Tailscale link. `mole clip serve`\n") b.WriteString("# on the Mac binds clip_listen; `mole clip pull` on the remote\n") b.WriteString("# reaches clip_url. Both commands auto-read these keys.\n") b.WriteString("clip_url: ") @@ -456,6 +469,25 @@ func renderYAML(ans *initAnswers) string { // Helpers // --------------------------------------------------------------------------- +func clipListenForURL(raw string) string { + endpoint := strings.TrimSpace(raw) + if endpoint == "" { + return config.DefaultClipListen + } + if !strings.Contains(endpoint, "://") { + endpoint = "http://" + endpoint + } + parsed, err := url.Parse(endpoint) + if err != nil || parsed.Hostname() == "" { + return config.DefaultClipListen + } + port := parsed.Port() + if port == "" { + port = "7777" + } + return net.JoinHostPort(parsed.Hostname(), port) +} + func validateRemote(r string) error { r = strings.TrimSpace(r) if r == "" { diff --git a/cmd/mole/init_test.go b/cmd/mole/init_test.go new file mode 100644 index 0000000..eff092b --- /dev/null +++ b/cmd/mole/init_test.go @@ -0,0 +1,107 @@ +package main + +import ( + "io" + "strings" + "testing" + + "github.com/Luqueee/mole/internal/config" +) + +func TestClipListenForURL(t *testing.T) { + cases := map[string]string{ + "http://100.64.0.10:7777": "100.64.0.10:7777", + "100.64.0.10:8888": "100.64.0.10:8888", + "https://[fd00::10]:9000": "[fd00::10]:9000", + "http://100.64.0.10": "100.64.0.10:7777", + "": config.DefaultClipListen, + "not a URL": config.DefaultClipListen, + } + for raw, want := range cases { + if got := clipListenForURL(raw); got != want { + t.Errorf("clipListenForURL(%q) = %q, want %q", raw, got, want) + } + } +} + +func TestGatherAnswers_ClipListenEnvironmentOverridesDefault(t *testing.T) { + t.Setenv("MOLE_CLIP_LISTEN", "100.64.0.20:7777") + + ans, err := gatherAnswers(initInputs{ + Remote: "dev", + PortsCSV: "3000", + ClipEnabled: true, + ClipURL: "http://100.64.0.10:7777", + ClipIntervalMs: 500, + }, initOptions{}) + if err != nil { + t.Fatalf("gatherAnswers() error = %v", err) + } + if ans.ClipListen != "100.64.0.20:7777" { + t.Fatalf("ClipListen = %q, want environment value", ans.ClipListen) + } + + t.Setenv("MOLE_CLIP_LISTEN", "") + ans, err = gatherAnswers(initInputs{ + Remote: "dev", + PortsCSV: "3000", + ClipEnabled: true, + ClipURL: "http://100.64.0.10:8888", + ClipIntervalMs: 500, + }, initOptions{}) + if err != nil { + t.Fatalf("gatherAnswers() error = %v", err) + } + if ans.ClipListen != "100.64.0.10:8888" { + t.Fatalf("ClipListen = %q, want address derived from URL", ans.ClipListen) + } +} + +func TestGatherAnswers_InteractiveClipAfterEveryPortChoice(t *testing.T) { + t.Setenv("MOLE_CLIP", "") + t.Setenv("MOLE_CLIP_URL", "") + t.Setenv("MOLE_CLIP_LISTEN", "") + + cases := map[string]string{ + "auto-discover": "dev\n1\ny\n\n\n\n", + "explicit": "dev\n2\n3000\ny\n\n\n\n", + "skip": "dev\n3\ny\n\n\n\n", + } + for name, input := range cases { + t.Run(name, func(t *testing.T) { + ans, err := gatherAnswers(initInputs{}, initOptions{ + Interactive: true, + In: &oneLineReader{lines: strings.Split(input, "\n")}, + Out: &strings.Builder{}, + }) + if err != nil { + t.Fatalf("gatherAnswers() error = %v", err) + } + if !ans.ClipEnabled { + t.Fatal("ClipEnabled = false, want true") + } + if ans.ClipURL != "http://100.64.0.10:7777" { + t.Fatalf("ClipURL = %q, want default private endpoint", ans.ClipURL) + } + if ans.ClipListen != "100.64.0.10:7777" { + t.Fatalf("ClipListen = %q, want address derived from ClipURL", ans.ClipListen) + } + }) + } +} + +// oneLineReader prevents readLine's short-lived scanners from buffering the +// answers for later prompts in an interactive test. +type oneLineReader struct { + lines []string +} + +func (r *oneLineReader) Read(p []byte) (int, error) { + if len(r.lines) == 0 { + return 0, io.EOF + } + line := r.lines[0] + "\n" + r.lines = r.lines[1:] + copy(p, line) + return len(line), nil +} diff --git a/examples/mole.yaml b/examples/mole.yaml index 6e7d6eb..f2ccbfb 100644 --- a/examples/mole.yaml +++ b/examples/mole.yaml @@ -33,4 +33,10 @@ ssh_port: 22 # Disable SSH host key verification. UNSAFE — leaves the tunnel open to # man-in-the-middle. Off by default; only for throwaway dev hosts. -# insecure: false \ No newline at end of file +# insecure: false + +# Optional clipboard sharing. The endpoint has no authentication and defaults +# to loopback. For remote access, use the same private Tailscale or WireGuard +# address in both fields (replace 100.64.0.10 with the source machine's IP). +# clip_url: http://100.64.0.10:7777 +# clip_listen: 100.64.0.10:7777 diff --git a/internal/clip/client.go b/internal/clip/client.go index 80ac3b3..c88b290 100644 --- a/internal/clip/client.go +++ b/internal/clip/client.go @@ -19,8 +19,8 @@ import ( var ErrNoImage = errors.New("clip: no image on server") // Client talks to a clip.Server over HTTP. The URL is whatever the -// server is reachable at on the WireGuard link — typically -// http://:7777. +// server is reachable on the private WireGuard or Tailscale link — typically +// http://:7777. type Client struct { endpoint string log *slog.Logger diff --git a/internal/clip/server.go b/internal/clip/server.go index 252bae7..3bf5ddb 100644 --- a/internal/clip/server.go +++ b/internal/clip/server.go @@ -2,7 +2,8 @@ // Package clip shares a single clipboard image between two mole // processes over HTTP. The Mac runs Server; the LXC runs Client. // -// The transport is a plain HTTP endpoint reachable over a WireGuard +// The transport is a plain HTTP endpoint reachable over a private WireGuard +// or Tailscale // link, not over mole's SSH tunnel: an earlier version of this code // tried to wire a reverse forward through tunnel.Manager, but // golang.org/x/crypto/ssh does not expose the ListenOn primitive needed diff --git a/internal/config/config.go b/internal/config/config.go index d4f2a47..2daac34 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,6 +5,8 @@ package config import ( "bytes" "fmt" + "net" + "net/url" "os" "path/filepath" "runtime" @@ -14,6 +16,11 @@ import ( "gopkg.in/yaml.v3" ) +// DefaultClipListen is the least-exposure bind address for the unauthenticated +// clipboard endpoint. Use an explicit private interface address for remote +// access over Tailscale or WireGuard. +const DefaultClipListen = "127.0.0.1:7777" + // Config holds the runtime configuration for mole. type Config struct { // Remote is the SSH target in the form user@host[:port]. @@ -54,13 +61,14 @@ type Config struct { Insecure bool `yaml:"insecure"` // ClipURL is the HTTP endpoint of the clip server on the Mac - // (e.g. http://10.0.0.1:7777), as reachable from the LXC over - // WireGuard. Read by `mole clip pull`. + // (e.g. http://100.64.0.10:7777), as reachable from the LXC over + // Tailscale or WireGuard. Read by `mole clip pull`. ClipURL string `yaml:"clip_url"` // ClipListen is the address the clip server binds on the Mac. - // Default 0.0.0.0:7777; the WireGuard perimeter is the security - // boundary, so no auth is enforced. Read by `mole clip serve`. + // It defaults to loopback because the endpoint has no authentication. + // Set it to an explicit private Tailscale or WireGuard address for + // remote access. Read by `mole clip serve`. ClipListen string `yaml:"clip_listen"` // ClipIntervalMs controls the clipboard poll cadence on the Mac. @@ -88,7 +96,7 @@ func Default() *Config { }, LogLevel: "info", SSHPort: 22, - ClipListen: "0.0.0.0:7777", + ClipListen: DefaultClipListen, } } @@ -181,9 +189,41 @@ func Load(path string) (*Config, error) { return nil, fmt.Errorf("parse config %q: %w", path, err) } + // Keep existing clip_url-only configurations usable after the safer + // loopback default was introduced. An explicit clip_listen, including + // loopback, always wins over this compatibility fallback. + var clipConfig struct { + ClipURL string `yaml:"clip_url"` + ClipListen *string `yaml:"clip_listen"` + } + if err := yaml.Unmarshal(data, &clipConfig); err == nil && clipConfig.ClipListen == nil && strings.TrimSpace(clipConfig.ClipURL) != "" { + if listen := clipListenForURL(clipConfig.ClipURL); listen != "" { + cfg.ClipListen = listen + } + } + return cfg, nil } +func clipListenForURL(raw string) string { + endpoint := strings.TrimSpace(raw) + if endpoint == "" { + return "" + } + if !strings.Contains(endpoint, "://") { + endpoint = "http://" + endpoint + } + parsed, err := url.Parse(endpoint) + if err != nil || parsed.Hostname() == "" { + return "" + } + port := parsed.Port() + if port == "" { + port = "7777" + } + return net.JoinHostPort(parsed.Hostname(), port) +} + // Save writes cfg back to path as YAML, preserving any comments and // non-default keys that were already in the file. The strategy is: // diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a1976c4..e4b3f9b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -54,8 +54,8 @@ func TestDefault_DiscoverPortsContainsExpected(t *testing.T) { func TestClipDefaults(t *testing.T) { cfg := Default() - if cfg.ClipListen != "0.0.0.0:7777" { - t.Errorf("ClipListen = %q, want 0.0.0.0:7777", cfg.ClipListen) + if cfg.ClipListen != "127.0.0.1:7777" { + t.Errorf("ClipListen = %q, want 127.0.0.1:7777", cfg.ClipListen) } if cfg.ClipURL != "" { t.Errorf("ClipURL = %q, want empty (no default URL — user must set it)", cfg.ClipURL) @@ -68,9 +68,9 @@ func TestClipDefaults(t *testing.T) { func TestLoad_ClipPartialYAML(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "cfg.yaml") - // Only override clip_url — listen and interval should fall back - // to defaults. This is the realistic migration path: an existing - // mole.yaml gains a single clip_url key and the rest just works. + // Only override clip_url — listen should be derived from the endpoint + // so an existing mole.yaml remains reachable after the safer loopback + // default was introduced. content := "clip_url: http://10.0.0.1:7777\n" if err := os.WriteFile(path, []byte(content), 0o644); err != nil { t.Fatalf("write config: %v", err) @@ -82,14 +82,31 @@ func TestLoad_ClipPartialYAML(t *testing.T) { if cfg.ClipURL != "http://10.0.0.1:7777" { t.Errorf("ClipURL = %q, want http://10.0.0.1:7777", cfg.ClipURL) } - if cfg.ClipListen != "0.0.0.0:7777" { - t.Errorf("ClipListen = %q, want default 0.0.0.0:7777", cfg.ClipListen) + if cfg.ClipListen != "10.0.0.1:7777" { + t.Errorf("ClipListen = %q, want address derived from ClipURL", cfg.ClipListen) } if cfg.ClipIntervalMs != 0 { t.Errorf("ClipIntervalMs = %d, want default 0", cfg.ClipIntervalMs) } } +func TestLoad_ExplicitClipListen(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "cfg.yaml") + content := "clip_url: http://100.64.0.10:7777\nclip_listen: 127.0.0.1:7777\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.ClipListen != "127.0.0.1:7777" { + t.Errorf("ClipListen = %q, want explicit loopback interface", cfg.ClipListen) + } +} + func TestLoad_EmptyPath(t *testing.T) { cfg, err := Load("") if err != nil { From f9680a390d3a1c69563da7ee4300364ce4f4511b Mon Sep 17 00:00:00 2001 From: luqueee Date: Fri, 4 Sep 2026 11:59:55 +0200 Subject: [PATCH 2/4] fix: apply CodeRabbit auto-fixes --- cmd/mole/clip.go | 11 +++++++---- cmd/mole/clip_test.go | 1 + cmd/mole/init.go | 31 +++++++++++++++++++++++-------- cmd/mole/init_test.go | 26 ++++++++++++++++++++++++-- internal/config/config.go | 24 ++++++++++++++++-------- internal/config/config_test.go | 15 +++++++++++++++ 6 files changed, 86 insertions(+), 22 deletions(-) diff --git a/cmd/mole/clip.go b/cmd/mole/clip.go index 9c15769..58ccb4d 100644 --- a/cmd/mole/clip.go +++ b/cmd/mole/clip.go @@ -123,16 +123,16 @@ Flags: *logLevel = cfg.LogLevel } log := newLogger(*logLevel) - if isBroadClipBind(*listen) { - log.Warn("clip server has no authentication and is listening on all interfaces", "addr", *listen, "hint", "use loopback or a private Tailscale/WireGuard address") - } - ln, err := net.Listen("tcp", *listen) if err != nil { log.Error("clip serve: listen failed", "addr", *listen, "err", err) return 1 } defer ln.Close() + effectiveAddr := ln.Addr().String() + if isBroadClipBind(effectiveAddr) { + log.Warn("clip server has no authentication and is listening on all interfaces", "addr", effectiveAddr, "hint", "use loopback or a private Tailscale/WireGuard address") + } srv := &http.Server{ Handler: clip.New(log).Handler(), @@ -175,6 +175,9 @@ Flags: } func isBroadClipBind(addr string) bool { + if addr == "" { + return true + } host, _, err := net.SplitHostPort(addr) if err != nil { return false diff --git a/cmd/mole/clip_test.go b/cmd/mole/clip_test.go index b415527..72e12c1 100644 --- a/cmd/mole/clip_test.go +++ b/cmd/mole/clip_test.go @@ -126,6 +126,7 @@ func TestClipBindScope(t *testing.T) { "0.0.0.0:7777": true, ":7777": true, "[::]:7777": true, + "": true, } for addr, wantBroad := range cases { if got := isBroadClipBind(addr); got != wantBroad { diff --git a/cmd/mole/init.go b/cmd/mole/init.go index 43ba917..a80086f 100644 --- a/cmd/mole/init.go +++ b/cmd/mole/init.go @@ -81,11 +81,16 @@ Flags: -test after writing, test the SSH connection -force overwrite the config file if it already exists -up start mole (mole up) immediately after writing + -clip configure clipboard sharing options + -clip-url URL the remote LXC pulls clipboard images from + -clip-listen address the local clipboard server binds on + -clip-interval-ms clipboard polling interval in milliseconds -h, --help show this help Environment (read when the corresponding flag is empty): MOLE_REMOTE, MOLE_PORTS, MOLE_AUTO_DISCOVER, - MOLE_CONFIG_PATH, MOLE_GLOBAL`) + MOLE_CONFIG_PATH, MOLE_GLOBAL, MOLE_CLIP, + MOLE_CLIP_URL, MOLE_CLIP_LISTEN`) } if err := fs.Parse(args); err != nil { return 2 @@ -265,7 +270,11 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { } ans.Ports = config.ParsePorts(in.PortsCSV) if ans.ClipEnabled && ans.ClipURL != "" && !clipListenProvided { - ans.ClipListen = clipListenForURL(ans.ClipURL) + listen, err := clipListenForURL(ans.ClipURL) + if err != nil { + return nil, err + } + ans.ClipListen = listen } // In non-interactive mode, all required values must already be set. @@ -368,7 +377,10 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { urlRaw = "http://" + urlRaw } ans.ClipURL = urlRaw - listenDefault := clipListenForURL(ans.ClipURL) + listenDefault, err := clipListenForURL(ans.ClipURL) + if err != nil { + return nil, err + } if clipListenProvided { listenDefault = ans.ClipListen } @@ -469,23 +481,26 @@ func renderYAML(ans *initAnswers) string { // Helpers // --------------------------------------------------------------------------- -func clipListenForURL(raw string) string { +func clipListenForURL(raw string) (string, error) { endpoint := strings.TrimSpace(raw) if endpoint == "" { - return config.DefaultClipListen + return config.DefaultClipListen, nil } if !strings.Contains(endpoint, "://") { endpoint = "http://" + endpoint } parsed, err := url.Parse(endpoint) - if err != nil || parsed.Hostname() == "" { - return config.DefaultClipListen + if err != nil { + return "", fmt.Errorf("invalid clip URL %q: %w", raw, err) + } + if parsed.Hostname() == "" { + return "", fmt.Errorf("invalid clip URL %q: missing host", raw) } port := parsed.Port() if port == "" { port = "7777" } - return net.JoinHostPort(parsed.Hostname(), port) + return net.JoinHostPort(parsed.Hostname(), port), nil } func validateRemote(r string) error { diff --git a/cmd/mole/init_test.go b/cmd/mole/init_test.go index eff092b..012de74 100644 --- a/cmd/mole/init_test.go +++ b/cmd/mole/init_test.go @@ -15,13 +15,35 @@ func TestClipListenForURL(t *testing.T) { "https://[fd00::10]:9000": "[fd00::10]:9000", "http://100.64.0.10": "100.64.0.10:7777", "": config.DefaultClipListen, - "not a URL": config.DefaultClipListen, } for raw, want := range cases { - if got := clipListenForURL(raw); got != want { + got, err := clipListenForURL(raw) + if err != nil { + t.Errorf("clipListenForURL(%q) error = %v", raw, err) + } + if got != want { t.Errorf("clipListenForURL(%q) = %q, want %q", raw, got, want) } } + if _, err := clipListenForURL("not a URL"); err == nil { + t.Fatal("clipListenForURL() accepted a malformed URL") + } + if _, err := clipListenForURL("http://100.64.0.10:abc"); err == nil { + t.Fatal("clipListenForURL() accepted a nonnumeric port") + } +} + +func TestGatherAnswers_RejectsMalformedClipURL(t *testing.T) { + _, err := gatherAnswers(initInputs{ + Remote: "dev", + PortsCSV: "3000", + ClipEnabled: true, + ClipURL: "http://100.64.0.10:abc", + ClipIntervalMs: 500, + }, initOptions{}) + if err == nil || !strings.Contains(err.Error(), "invalid clip URL") { + t.Fatalf("gatherAnswers() error = %v, want invalid clip URL", err) + } } func TestGatherAnswers_ClipListenEnvironmentOverridesDefault(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 2daac34..700f798 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -196,32 +196,40 @@ func Load(path string) (*Config, error) { ClipURL string `yaml:"clip_url"` ClipListen *string `yaml:"clip_listen"` } - if err := yaml.Unmarshal(data, &clipConfig); err == nil && clipConfig.ClipListen == nil && strings.TrimSpace(clipConfig.ClipURL) != "" { - if listen := clipListenForURL(clipConfig.ClipURL); listen != "" { - cfg.ClipListen = listen + if err := yaml.Unmarshal(data, &clipConfig); err != nil { + return nil, fmt.Errorf("parse config %q: %w", path, err) + } + if clipConfig.ClipListen == nil && strings.TrimSpace(clipConfig.ClipURL) != "" { + listen, err := clipListenForURL(clipConfig.ClipURL) + if err != nil { + return nil, fmt.Errorf("parse config %q: %w", path, err) } + cfg.ClipListen = listen } return cfg, nil } -func clipListenForURL(raw string) string { +func clipListenForURL(raw string) (string, error) { endpoint := strings.TrimSpace(raw) if endpoint == "" { - return "" + return "", nil } if !strings.Contains(endpoint, "://") { endpoint = "http://" + endpoint } parsed, err := url.Parse(endpoint) - if err != nil || parsed.Hostname() == "" { - return "" + if err != nil { + return "", fmt.Errorf("invalid clip URL %q: %w", raw, err) + } + if parsed.Hostname() == "" { + return "", fmt.Errorf("invalid clip URL %q: missing host", raw) } port := parsed.Port() if port == "" { port = "7777" } - return net.JoinHostPort(parsed.Hostname(), port) + return net.JoinHostPort(parsed.Hostname(), port), nil } // Save writes cfg back to path as YAML, preserving any comments and diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e4b3f9b..7dcc7e9 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "reflect" "sort" + "strings" "testing" ) @@ -107,6 +108,20 @@ func TestLoad_ExplicitClipListen(t *testing.T) { } } +func TestLoad_RejectsMalformedClipURL(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "cfg.yaml") + content := "clip_url: http://100.64.0.10:abc\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + _, err := Load(path) + if err == nil || !strings.Contains(err.Error(), "invalid clip URL") { + t.Fatalf("Load() error = %v, want invalid clip URL", err) + } +} + func TestLoad_EmptyPath(t *testing.T) { cfg, err := Load("") if err != nil { From a6883bc811e1fdea86f515eae8c78883bdce21ac Mon Sep 17 00:00:00 2001 From: luqueee Date: Fri, 4 Sep 2026 12:11:18 +0200 Subject: [PATCH 3/4] fix: sanitize clip URL validation errors --- cmd/mole/init.go | 10 ++++++---- cmd/mole/init_test.go | 11 +++++++++++ internal/config/config.go | 11 +++++++---- internal/config/config_test.go | 5 ++++- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cmd/mole/init.go b/cmd/mole/init.go index a80086f..ac64fc3 100644 --- a/cmd/mole/init.go +++ b/cmd/mole/init.go @@ -269,12 +269,14 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { ClipIntervalMs: in.ClipIntervalMs, } ans.Ports = config.ParsePorts(in.PortsCSV) - if ans.ClipEnabled && ans.ClipURL != "" && !clipListenProvided { + if ans.ClipEnabled && ans.ClipURL != "" { listen, err := clipListenForURL(ans.ClipURL) if err != nil { return nil, err } - ans.ClipListen = listen + if !clipListenProvided { + ans.ClipListen = listen + } } // In non-interactive mode, all required values must already be set. @@ -491,10 +493,10 @@ func clipListenForURL(raw string) (string, error) { } parsed, err := url.Parse(endpoint) if err != nil { - return "", fmt.Errorf("invalid clip URL %q: %w", raw, err) + return "", errors.New("invalid clip URL: malformed syntax or port") } if parsed.Hostname() == "" { - return "", fmt.Errorf("invalid clip URL %q: missing host", raw) + return "", errors.New("invalid clip URL: missing host") } port := parsed.Port() if port == "" { diff --git a/cmd/mole/init_test.go b/cmd/mole/init_test.go index 012de74..57d7357 100644 --- a/cmd/mole/init_test.go +++ b/cmd/mole/init_test.go @@ -30,6 +30,13 @@ func TestClipListenForURL(t *testing.T) { } if _, err := clipListenForURL("http://100.64.0.10:abc"); err == nil { t.Fatal("clipListenForURL() accepted a nonnumeric port") + } else if strings.Contains(err.Error(), "abc") { + t.Fatalf("clipListenForURL() exposed malformed port in error: %v", err) + } + if _, err := clipListenForURL("http://user:sensitive-value@host:abc"); err == nil { + t.Fatal("clipListenForURL() accepted credentials with a malformed port") + } else if strings.Contains(err.Error(), "sensitive-value") { + t.Fatalf("clipListenForURL() exposed URL credentials in error: %v", err) } } @@ -39,11 +46,15 @@ func TestGatherAnswers_RejectsMalformedClipURL(t *testing.T) { PortsCSV: "3000", ClipEnabled: true, ClipURL: "http://100.64.0.10:abc", + ClipListen: "127.0.0.1:7777", ClipIntervalMs: 500, }, initOptions{}) if err == nil || !strings.Contains(err.Error(), "invalid clip URL") { t.Fatalf("gatherAnswers() error = %v, want invalid clip URL", err) } + if strings.Contains(err.Error(), "100.64.0.10") { + t.Fatalf("gatherAnswers() exposed URL in error: %v", err) + } } func TestGatherAnswers_ClipListenEnvironmentOverridesDefault(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 700f798..4872e32 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ package config import ( "bytes" + "errors" "fmt" "net" "net/url" @@ -199,12 +200,14 @@ func Load(path string) (*Config, error) { if err := yaml.Unmarshal(data, &clipConfig); err != nil { return nil, fmt.Errorf("parse config %q: %w", path, err) } - if clipConfig.ClipListen == nil && strings.TrimSpace(clipConfig.ClipURL) != "" { + if strings.TrimSpace(clipConfig.ClipURL) != "" { listen, err := clipListenForURL(clipConfig.ClipURL) if err != nil { return nil, fmt.Errorf("parse config %q: %w", path, err) } - cfg.ClipListen = listen + if clipConfig.ClipListen == nil { + cfg.ClipListen = listen + } } return cfg, nil @@ -220,10 +223,10 @@ func clipListenForURL(raw string) (string, error) { } parsed, err := url.Parse(endpoint) if err != nil { - return "", fmt.Errorf("invalid clip URL %q: %w", raw, err) + return "", errors.New("invalid clip URL: malformed syntax or port") } if parsed.Hostname() == "" { - return "", fmt.Errorf("invalid clip URL %q: missing host", raw) + return "", errors.New("invalid clip URL: missing host") } port := parsed.Port() if port == "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7dcc7e9..a2417f7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -111,7 +111,7 @@ func TestLoad_ExplicitClipListen(t *testing.T) { func TestLoad_RejectsMalformedClipURL(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "cfg.yaml") - content := "clip_url: http://100.64.0.10:abc\n" + content := "clip_url: http://user:sensitive-value@100.64.0.10:abc\nclip_listen: 127.0.0.1:7777\n" if err := os.WriteFile(path, []byte(content), 0o644); err != nil { t.Fatalf("write config: %v", err) } @@ -120,6 +120,9 @@ func TestLoad_RejectsMalformedClipURL(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "invalid clip URL") { t.Fatalf("Load() error = %v, want invalid clip URL", err) } + if strings.Contains(err.Error(), "sensitive-value") || strings.Contains(err.Error(), "100.64.0.10") { + t.Fatalf("Load() exposed URL details in error: %v", err) + } } func TestLoad_EmptyPath(t *testing.T) { From e4e7ba52097bbffd9b016ccbf8ce3e4e92bf0d78 Mon Sep 17 00:00:00 2001 From: luqueee Date: Fri, 4 Sep 2026 12:33:28 +0200 Subject: [PATCH 4/4] fix: validate clip listen port ranges --- cmd/mole/init.go | 9 ++++++-- cmd/mole/init_test.go | 42 ++++++++++++++++++++++++++++++++++ internal/config/config.go | 4 ++++ internal/config/config_test.go | 21 +++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/cmd/mole/init.go b/cmd/mole/init.go index ac64fc3..d49ea63 100644 --- a/cmd/mole/init.go +++ b/cmd/mole/init.go @@ -252,8 +252,9 @@ func gatherAnswers(in initInputs, opt initOptions) (*initAnswers, error) { if in.ClipListen == "" { in.ClipListen = envDefault("MOLE_CLIP_LISTEN", "") } - clipListenProvided := strings.TrimSpace(in.ClipListen) != "" - if in.ClipListen == "" { + in.ClipListen = strings.TrimSpace(in.ClipListen) + clipListenProvided := in.ClipListen != "" + if !clipListenProvided { in.ClipListen = config.DefaultClipListen } @@ -502,6 +503,10 @@ func clipListenForURL(raw string) (string, error) { if port == "" { port = "7777" } + portNumber, err := strconv.Atoi(port) + if err != nil || portNumber < 1 || portNumber > 65535 { + return "", errors.New("invalid clip URL: port must be between 1 and 65535") + } return net.JoinHostPort(parsed.Hostname(), port), nil } diff --git a/cmd/mole/init_test.go b/cmd/mole/init_test.go index 57d7357..ebb235c 100644 --- a/cmd/mole/init_test.go +++ b/cmd/mole/init_test.go @@ -38,6 +38,14 @@ func TestClipListenForURL(t *testing.T) { } else if strings.Contains(err.Error(), "sensitive-value") { t.Fatalf("clipListenForURL() exposed URL credentials in error: %v", err) } + for _, raw := range []string{ + "http://100.64.0.10:0", + "http://100.64.0.10:65536", + } { + if _, err := clipListenForURL(raw); err == nil { + t.Errorf("clipListenForURL(%q) accepted an out-of-range port", raw) + } + } } func TestGatherAnswers_RejectsMalformedClipURL(t *testing.T) { @@ -57,6 +65,25 @@ func TestGatherAnswers_RejectsMalformedClipURL(t *testing.T) { } } +func TestGatherAnswers_RejectsOutOfRangeClipPort(t *testing.T) { + for _, raw := range []string{ + "http://100.64.0.10:0", + "http://100.64.0.10:65536", + } { + _, err := gatherAnswers(initInputs{ + Remote: "dev", + PortsCSV: "3000", + ClipEnabled: true, + ClipURL: raw, + ClipListen: "127.0.0.1:7777", + ClipIntervalMs: 500, + }, initOptions{}) + if err == nil || !strings.Contains(err.Error(), "port must be between 1 and 65535") { + t.Errorf("gatherAnswers(%q) error = %v, want port range validation", raw, err) + } + } +} + func TestGatherAnswers_ClipListenEnvironmentOverridesDefault(t *testing.T) { t.Setenv("MOLE_CLIP_LISTEN", "100.64.0.20:7777") @@ -88,6 +115,21 @@ func TestGatherAnswers_ClipListenEnvironmentOverridesDefault(t *testing.T) { if ans.ClipListen != "100.64.0.10:8888" { t.Fatalf("ClipListen = %q, want address derived from URL", ans.ClipListen) } + + t.Setenv("MOLE_CLIP_LISTEN", " ") + ans, err = gatherAnswers(initInputs{ + Remote: "dev", + PortsCSV: "3000", + ClipEnabled: true, + ClipURL: "http://100.64.0.10:9999", + ClipIntervalMs: 500, + }, initOptions{}) + if err != nil { + t.Fatalf("gatherAnswers() with whitespace environment value error = %v", err) + } + if ans.ClipListen != "100.64.0.10:9999" { + t.Fatalf("ClipListen = %q, want address derived from URL after whitespace environment value", ans.ClipListen) + } } func TestGatherAnswers_InteractiveClipAfterEveryPortChoice(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index 4872e32..c44bda0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -232,6 +232,10 @@ func clipListenForURL(raw string) (string, error) { if port == "" { port = "7777" } + portNumber, err := strconv.Atoi(port) + if err != nil || portNumber < 1 || portNumber > 65535 { + return "", errors.New("invalid clip URL: port must be between 1 and 65535") + } return net.JoinHostPort(parsed.Hostname(), port), nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a2417f7..9e960aa 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -125,6 +125,27 @@ func TestLoad_RejectsMalformedClipURL(t *testing.T) { } } +func TestLoad_RejectsClipURLPortRange(t *testing.T) { + for name, port := range map[string]string{ + "zero": "0", + "above maximum": "65536", + } { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "cfg.yaml") + content := "clip_url: http://100.64.0.10:" + port + "\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + _, err := Load(path) + if err == nil || !strings.Contains(err.Error(), "port must be between 1 and 65535") { + t.Fatalf("Load() error = %v, want port range validation", err) + } + }) + } +} + func TestLoad_EmptyPath(t *testing.T) { cfg, err := Load("") if err != nil {