Refuse a glob as a unit, and make wanted-by say only what it saw

systemctl expands *, ? and [ in a unit argument, so reset-failed or stop
of "*" would have acted on every unit. wanted-by called a link dangling on
any stat error, followed a linked .wants anywhere, and missed most of the
manager's load path; it now says each of these, and says nothing names a
unit only of the directories it searched.

modules/systemd/cmd/systemd-tools/wanted_test.go

@@ -1,10 +1,14 @@
 package main
 
 // What wants a unit, and a failed record cleared (novox/hq issue 332): over a fake runner and a fake
-// directory listing, so no link is made to test them.
+// directory listing, so no link is made to test them (the mesh makes no symbolic links, its tests
+// included); the real listing is tested on what a temporary directory holds without one.
 
 import (
+	"errors"
 	"io/fs"
+	"os"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -72,7 +76,7 @@ func TestWantedByFindsADanglingEnableLinkInTheAccountsOwnConfiguration(t *testin
 	links := answer["links"].([]EnableLink)
 	want := []EnableLink{{Path: home + "/.config/systemd/user/default.target.wants/greenclip.service", Of: "default.target",
 		Dependency: "Wants", Target: "/usr/lib/systemd/user/greenclip.service", Dangles: true, Link: true,
-		Whose: "the operator account's own configuration"}}
+		Whose: "the operator account's own configuration", removable: true}}
 	if !reflect.DeepEqual(links, want) {
 		t.Fatalf("links %+v", links)
 	}
@@ -135,7 +139,7 @@ func TestWantedBySaysWhenNothingNamesTheUnit(t *testing.T) {
 		t.Fatalf("%v", answer["links"])
 	}
 	said := strings.Join(answer["findings"].([]string), "\n")
-	if !strings.Contains(said, "nothing names gone.service any more") {
+	if !strings.Contains(said, "nothing names gone.service in the directories searched:") {
 		t.Fatalf("%q", said)
 	}
 }
@@ -229,7 +233,7 @@ func TestResetFailedClearsOneUnitEscalatedOnlyInTheSystemScope(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !reflect.DeepEqual(calls[0].args, []string{"--user", "reset-failed", "greenclip.service"}) || calls[0].cmd != "systemctl" {
+	if !reflect.DeepEqual(calls[0].args, []string{"--user", "reset-failed", "--", "greenclip.service"}) || calls[0].cmd != "systemctl" {
 		t.Fatalf("%s %v", calls[0].cmd, calls[0].args)
 	}
 	answer := out.(map[string]any)
@@ -247,3 +251,194 @@ func TestResetFailedClearsOneUnitEscalatedOnlyInTheSystemScope(t *testing.T) {
 type stdioTool struct {
 	run func(map[string]any) (any, error)
 }
+
+// **A glob is not a unit** (the review of mesh-catalog #137): systemctl expands it to every loaded unit it
+// matches, so `reset-failed` or `stop` of "*" would act on all of them. Refused before anything runs, for
+// every verb taking a unit.
+func TestAGlobIsRefusedAsAUnitByEveryVerb(t *testing.T) {
+	for _, bad := range []string{"*", "*.service", "foo?.service", "a[bc].mount"} {
+		if unitArg(bad) == nil {
+			t.Errorf("%q accepted", bad)
+		}
+	}
+	var calls []call
+	m := withTree(operator(fake(func(call) Ran { return Ran{} }, &calls)), tree{})
+	for _, tool := range tools(m) {
+		verb := strings.TrimPrefix(tool.Name, seat+".")
+		if verb == "units" || verb == "failed" {
+			continue // a pattern, and no unit
+		}
+		_, err := tool.Run(map[string]any{"unit": "*", "scope": "user", "why": "test"})
+		if err == nil {
+			t.Errorf("%s accepted {\"unit\":\"*\"}", verb)
+		}
+	}
+	if len(calls) != 0 {
+		t.Fatalf("a glob reached the manager: %v", calls)
+	}
+}
+
+// The unit is given after "--" to show as well as to an act, so nothing after it is read as an option.
+func TestTheUnitComesAfterTheEndOfOptions(t *testing.T) {
+	var calls []call
+	m := operator(fake(func(call) Ran { return Ran{Stdout: "LoadState=loaded\n"} }, &calls))
+	if _, err := m.Status(System, "sshd.service"); err != nil {
+		t.Fatal(err)
+	}
+	a := calls[0].args
+	if a[0] != "show" || a[len(a)-2] != "--" || a[len(a)-1] != "sshd.service" {
+		t.Fatalf("%v", a)
+	}
+}
+
+// A target that could not be looked at, for any reason but its not existing, is neither dangling nor not:
+// it is said as the error it was, and the answer does not say nothing names the unit.
+func TestALinkWhoseTargetCannotBeLookedAtIsNotSaidToDangle(t *testing.T) {
+	user := home + "/.config/systemd/user"
+	fsys := tree{
+		user:                           {{Name: "default.target.wants", Dir: true}},
+		user + "/default.target.wants": {{Name: "x.service", Link: "/mnt/away/x.service", Error: "stat /mnt/away/x.service: permission denied"}},
+	}
+	answer, err := withTree(operator(fake(showAndReverse("LoadState=loaded\n", "x.service\n"), nil)), fsys).WantedBy(User, "x.service")
+	if err != nil {
+		t.Fatal(err)
+	}
+	links := answer["links"].([]EnableLink)
+	if len(links) != 1 || links[0].Dangles || links[0].Error == "" {
+		t.Fatalf("%+v", links)
+	}
+	said := strings.Join(answer["findings"].([]string), "\n")
+	if !strings.Contains(said, "could not be looked at: stat /mnt/away/x.service: permission denied") || strings.Contains(said, "does not exist") {
+		t.Fatalf("%q", said)
+	}
+}
+
+// A *.wants directory that is itself a link is followed only into the directories searched: one leading
+// elsewhere is said and not read, so neither the search nor a removal after it can be pointed anywhere.
+func TestALinkedDependencyDirectoryIsFollowedOnlyIntoTheSearchedDirectories(t *testing.T) {
+	user := home + "/.config/systemd/user"
+	fsys := tree{
+		user: {
+			{Name: "default.target.wants", Link: "/srv/elsewhere"},
+			{Name: "graphical-session.target.wants", Link: "/etc/systemd/user/shared.wants"},
+		},
+		"/srv/elsewhere":                 {{Name: "x.service", Link: "/gone/x.service", Dangles: true}},
+		"/etc/systemd/user/shared.wants": {{Name: "x.service", Link: "/gone/x.service", Dangles: true}},
+	}
+	read := []string{}
+	m := withTree(operator(fake(showAndReverse("LoadState=not-found\n", "x.service\n"), nil)), fsys)
+	inner := m.ReadDir
+	m.ReadDir = func(dir string) ([]Entry, error) { read = append(read, dir); return inner(dir) }
+	answer, err := m.WantedBy(User, "x.service")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, d := range read {
+		if d == "/srv/elsewhere" {
+			t.Fatal("a linked .wants outside the searched directories was followed")
+		}
+	}
+	links := answer["links"].([]EnableLink)
+	if len(links) != 1 || links[0].Path != user+"/graphical-session.target.wants/x.service" || links[0].removable {
+		t.Fatalf("%+v", links)
+	}
+	unread, _ := answer["unread"].([]string)
+	if len(unread) != 1 || !strings.Contains(unread[0], "outside the directories searched, not followed") {
+		t.Fatalf("%v", unread)
+	}
+}
+
+// "Nothing names it" is only ever said of the directories searched, and the account's own are not among
+// them when its home is not known — which is said.
+func TestNothingNamesItIsSaidOfTheDirectoriesSearchedAndTheHomeWhenUnknown(t *testing.T) {
+	m := withTree(operator(fake(showAndReverse("LoadState=not-found\n", "x.service\n"), nil)), tree{})
+	m.Home = ""
+	answer, err := m.WantedBy(User, "x.service")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, d := range answer["searched"].([]ConfigDir) {
+		if strings.Contains(d.Path, ".config") || strings.Contains(d.Path, ".local") || d.Path == "" {
+			t.Fatalf("searched %q without a home", d.Path)
+		}
+	}
+	said := strings.Join(answer["findings"].([]string), "\n")
+	if !strings.Contains(said, "nothing names x.service in the directories searched, which are not all of them") ||
+		!strings.Contains(said, "the account's home is not known") {
+		t.Fatalf("%q", said)
+	}
+	m.Home = home
+	answer, _ = m.WantedBy(User, "x.service")
+	said = strings.Join(answer["findings"].([]string), "\n")
+	if !strings.Contains(said, "nothing names x.service in the directories searched:") {
+		t.Fatalf("%q", said)
+	}
+}
+
+// Every directory systemd reads units from, in both scopes (systemd.unit(5)).
+func TestTheSearchedDirectoriesAreTheManagersLoadPath(t *testing.T) {
+	m := withTree(operator(nil), tree{})
+	paths := func(s Scope) string {
+		var out []string
+		for _, d := range m.configDirs(s) {
+			out = append(out, d.Path)
+		}
+		return strings.Join(out, "\n")
+	}
+	for _, want := range []string{"/etc/systemd/system.control", "/run/systemd/transient", "/run/systemd/generator.early",
+		"/etc/systemd/system", "/run/systemd/system", "/run/systemd/generator", "/usr/local/lib/systemd/system",
+		"/usr/lib/systemd/system", "/run/systemd/generator.late"} {
+		if !strings.Contains(paths(System)+"\n", want+"\n") {
+			t.Errorf("system scope does not search %s", want)
+		}
+	}
+	for _, want := range []string{home + "/.config/systemd/user.control", "/run/user/1000/systemd/user", home + "/.config/systemd/user",
+		"/etc/systemd/user", "/etc/xdg/systemd/user", home + "/.local/share/systemd/user", "/usr/local/lib/systemd/user", "/usr/lib/systemd/user"} {
+		if !strings.Contains(paths(User)+"\n", want+"\n") {
+			t.Errorf("user scope does not search %s", want)
+		}
+	}
+}
+
+// The real listing, by lstat, on what a temporary directory holds: a directory, a file, and a directory
+// that does not exist or cannot be read.
+func TestReadDirListsByLstat(t *testing.T) {
+	dir := t.TempDir()
+	if err := os.Mkdir(filepath.Join(dir, "default.target.wants"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "x.service"), []byte("[Unit]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	entries, err := readDir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := []Entry{{Name: "default.target.wants", Dir: true}, {Name: "x.service"}}
+	if !reflect.DeepEqual(entries, want) {
+		t.Fatalf("%+v", entries)
+	}
+	if _, err := readDir(filepath.Join(dir, "absent")); !errors.Is(err, fs.ErrNotExist) {
+		t.Fatalf("a missing directory: %v", err)
+	}
+	if os.Getuid() != 0 {
+		locked := filepath.Join(dir, "locked")
+		if err := os.Mkdir(locked, 0o000); err != nil {
+			t.Fatal(err)
+		}
+		defer os.Chmod(locked, 0o755)
+		if _, err := readDir(locked); err == nil || errors.Is(err, fs.ErrNotExist) {
+			t.Fatalf("an unreadable directory read as %v", err)
+		}
+	}
+	// And through the search: a real tree with no link in it names nothing, and says so.
+	m := operator(fake(showAndReverse("LoadState=not-found\n", "x.service\n"), nil))
+	m.Home, m.ReadDir = dir, readDir
+	answer, err := m.WantedBy(User, "x.service")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(answer["links"].([]EnableLink)) != 0 {
+		t.Fatalf("%v", answer["links"])
+	}
+}