install: match an entry's whole command, not a suffix of it

internal/install/install.go

@@ -221,9 +221,23 @@ func (s Spec) Entry(event string) (json.RawMessage, error) {
 	return b, nil
 }
 
+// legacyForms maps an event to older subcommands a previous release wrote
+// for it. Owner recognises a listed form as ours, for an entry a release
+// renames: it still refreshes and still detaches, not lost until the user's
+// next watch happens to touch it and not left behind for detach to miss
+// (LUI-3, LUI-4). Nothing has been renamed yet, so every list here is empty.
+var legacyForms = map[string][]string{}
+
 // Owner returns the install id an entry claims for an event, or "" for an
 // entry that is not ours. Ownership is carried in the command line itself,
 // which survives every round trip a settings file can take.
+//
+// A command matches only as a WHOLE: everything after its first shell word
+// (the executable, bare or quoted) must read exactly "<form> --install <id>"
+// for event's current subcommand or one of legacyForms[event] -- never
+// merely END with that text. A suffix check is the trap: "post" is also the
+// tail of "steer post", so a future entry installed under that longer
+// subcommand would have misread as this one's (LUI-3).
 func Owner(raw json.RawMessage, event string) string {
 	var g struct {
 		Hooks []struct {
@@ -233,17 +247,76 @@ func Owner(raw json.RawMessage, event string) string {
 	if json.Unmarshal(raw, &g) != nil {
 		return ""
 	}
-	suffix := " " + subcommand(event) + " " + Marker + " "
+	forms := append([]string{subcommand(event)}, legacyForms[event]...)
 	for _, h := range g.Hooks {
-		if i := strings.LastIndex(h.Command, suffix); i >= 0 {
-			if id := h.Command[i+len(suffix):]; isInstallID(id) {
-				return id
-			}
+		if id := ownerOf(h.Command, forms); id != "" {
+			return id
+		}
+	}
+	return ""
+}
+
+// ownerOf matches cmd against the whole of one of forms, not a suffix of it:
+// see Owner. It returns the claimed install id, or "" if none of forms
+// matches or the command does not parse as "<word> <rest>".
+func ownerOf(cmd string, forms []string) string {
+	rest, ok := afterFirstWord(cmd)
+	if !ok {
+		return ""
+	}
+	for _, form := range forms {
+		prefix := form + " " + Marker + " "
+		if !strings.HasPrefix(rest, prefix) {
+			continue
+		}
+		if id := rest[len(prefix):]; isInstallID(id) {
+			return id
 		}
 	}
 	return ""
 }
 
+// afterFirstWord splits cmd at the end of its first shell word -- the
+// executable, exactly as shellQuote writes it, bare or single-quoted -- and
+// returns what follows the one space after it. It does not reconstruct the
+// word itself: Owner does not care what the executable is (a moved binary is
+// what watch exists to refresh), only where it ends, so a caller can compare
+// everything after it exactly rather than searching for a trailing match.
+//
+// ok is false when cmd does not have the shape "<word> <rest>" at all: no
+// separating space for a bare word, or shellQuote's own escaping of an
+// embedded quote -- close quote, backslash, quote, open quote -- never
+// closes.
+func afterFirstWord(cmd string) (rest string, ok bool) {
+	if cmd == "" {
+		return "", false
+	}
+	if cmd[0] != '\'' {
+		i := strings.IndexByte(cmd, ' ')
+		if i < 0 {
+			return "", false
+		}
+		return cmd[i+1:], true
+	}
+	for i := 1; ; {
+		j := strings.IndexByte(cmd[i:], '\'')
+		if j < 0 {
+			return "", false
+		}
+		end := i + j
+		if strings.HasPrefix(cmd[end:], `'\''`) {
+			// An embedded quote in the executable, escaped as close,
+			// backslash, quote, open -- not the terminator.
+			i = end + 4
+			continue
+		}
+		if end+1 < len(cmd) && cmd[end+1] == ' ' {
+			return cmd[end+2:], true
+		}
+		return "", false
+	}
+}
+
 // Apply ensures exactly one entry of ours per event, refreshing it if the
 // binary has moved and collapsing duplicates. It reports whether the document
 // changed, so a no-op watch writes nothing. An entry of ours that is no longer

internal/install/install_test.go

@@ -45,6 +45,58 @@ func TestOwner(t *testing.T) {
 				`{"type":"command","command":"` + spec(oneID).Command(EventSessionStart) + `","timeout":5}]}`,
 			oneID,
 		},
+		// The four cases below exercise afterFirstWord's single-quoted
+		// branch, the one shellQuote sends every path through that is not
+		// made of bare-safe characters: a backslash (so every Windows
+		// install), an embedded single quote, or a space. Without these,
+		// the branch that every Windows install and any POSIX install
+		// under a path with a space or a quote actually runs had no
+		// coverage at all -- a break in it would have shown as "every
+		// watch adds a duplicate, every detach removes nothing" with no
+		// test going red first.
+		{
+			"quoted executable with an embedded single quote",
+			EventPreToolUse,
+			string(mustEntry(t, Spec{Executable: "/home/o'brien/bin/rashomon", InstallID: oneID}, EventPreToolUse)),
+			oneID,
+		},
+		{
+			"quoted executable with a space",
+			EventPreToolUse,
+			string(mustEntry(t, Spec{Executable: "/a b/rashomon", InstallID: oneID}, EventPreToolUse)),
+			oneID,
+		},
+		{
+			"quoted executable with Windows backslashes",
+			EventPreToolUse,
+			string(mustEntry(t, Spec{Executable: `C:\x\rashomon.exe`, InstallID: oneID}, EventPreToolUse)),
+			oneID,
+		},
+		{
+			// shellQuote never writes this shape -- it always closes the
+			// quote it opens -- but afterFirstWord has to resolve any
+			// command a settings file might actually hold, including one
+			// hand-edited into an invalid state. An unclosed quote has no
+			// "end of the first word" to split at, so Owner must read it
+			// as nobody's rather than guess.
+			"unterminated quote never resolves to an owner",
+			EventPreToolUse,
+			`{"hooks":[{"type":"command","command":"'/no/closing/quote post --install ` + oneID + `"}]}`,
+			"",
+		},
+		{
+			// LUI-2026-10-17: the narrowing to a whole-remainder match (see
+			// TestLUI3_) also means a hand-wrapped command -- a env-prefix
+			// here -- is no longer recognised as ours, even though the
+			// wrapped tail is otherwise byte-identical to an owned entry.
+			// Pinned here so the choice stays visible: a future change that
+			// restores suffix matching to recover this case would have to
+			// change this test, not just add one.
+			"a command wrapped in an env prefix is not ours",
+			EventPostToolUse,
+			`{"hooks":[{"type":"command","command":"env FOO=1 ` + spec(oneID).Command(EventPostToolUse) + `"}]}`,
+			"",
+		},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -55,6 +107,107 @@ func TestOwner(t *testing.T) {
 	}
 }
 
+// TestLUI3_OwnerDoesNotClaimSteerPostAsPost pins LUI-3: identity is read off
+// the whole of the command after its executable, not a suffix of it.
+//
+// "post" is PostToolUse's own subcommand, and it is also the tail of "steer
+// post" -- a longer subcommand a future entry under a different role
+// installs on the very same event. The old implementation searched for
+// " post --install " anywhere in the command, so a "steer post" entry would
+// have read, wrongly, as this install's post entry: detach would have
+// removed somebody's steer entry under the recorder's name, and a second
+// watch would have found "ours" already present and left a stray steer
+// entry untouched. The fix is a whole-remainder comparison, so a longer
+// subcommand that merely ends the same way is never mistaken for a shorter
+// one that is a true suffix of it.
+func TestLUI3_OwnerDoesNotClaimSteerPostAsPost(t *testing.T) {
+	notOurs := json.RawMessage(`{"matcher":"Bash","hooks":[{"type":"command",` +
+		`"command":"/usr/local/bin/rashomon steer post --install ` + oneID + `","timeout":5}]}`)
+	if got := Owner(notOurs, EventPostToolUse); got != "" {
+		t.Errorf("Owner(%s) = %q, want %q: a steer post entry is not the recorder's post entry",
+			notOurs, got, "")
+	}
+
+	// The positive twin, so the fix is a narrowing and not a break: the
+	// recorder's own post entry, same executable and id, is still read.
+	ours := json.RawMessage(mustEntry(t, spec(oneID), EventPostToolUse))
+	if got := Owner(ours, EventPostToolUse); got != oneID {
+		t.Errorf("Owner(%s) = %q, want %q", ours, got, oneID)
+	}
+}
+
+// TestLUI4_WatchThenDetachLeavesNoResidue pins LUI-4's legacy half: Owner
+// recognises a listed legacy subcommand as ours, so an entry a previous
+// release wrote under an older subcommand is refreshed to the current one in
+// place -- never duplicated -- and a later detach removes it exactly as it
+// would the current form. A foreign entry under the same event, and every
+// event watch never touches, stay byte for byte as found; that half of the
+// promise is TestH06_DetachLeavesTheFileByteForByteAsFound's and is
+// unchanged here.
+//
+// No real subcommand has been renamed yet (legacyForms is empty for every
+// production event), so this test supplies its own legacy form rather than
+// one shipped: it proves the mechanism a rename rides on, not a rename.
+func TestLUI4_WatchThenDetachLeavesNoResidue(t *testing.T) {
+	const legacy = "recap-legacy-v1"
+	legacyForms[EventStop] = []string{legacy}
+	t.Cleanup(func() { delete(legacyForms, EventStop) })
+
+	exe := "/usr/local/bin/rashomon"
+	legacyEntry := `{"hooks":[{"type":"command","command":"` +
+		exe + " " + legacy + " " + Marker + " " + oneID + `","timeout":` + fmt.Sprint(RecapTimeout) + `}]}`
+	doc, err := settings.Parse([]byte(`{"hooks":{"Stop":[` + foreignHook + `,` + legacyEntry + `]}}`))
+	if err != nil {
+		t.Fatalf("seeding: %v", err)
+	}
+
+	if _, err := Apply(doc, Spec{Executable: exe, InstallID: oneID}); err != nil {
+		t.Fatalf("Apply: %v", err)
+	}
+
+	stopEntries := entriesOf(t, doc, EventStop)
+	if len(stopEntries) != 2 {
+		t.Fatalf("Stop carries %d entries after refresh, want 2 (foreign, plus ours refreshed in place)",
+			len(stopEntries))
+	}
+	var refreshed json.RawMessage
+	for _, e := range stopEntries {
+		if Owner(e, EventStop) == oneID {
+			refreshed = e
+		}
+	}
+	if refreshed == nil {
+		t.Fatal("no entry under Stop reads as ours after Apply refreshed the legacy form")
+	}
+	if detail := intact(refreshed, EventStop); detail != "" {
+		t.Errorf("the refreshed entry is not intact: %s", detail)
+	}
+	if strings.Contains(string(refreshed), legacy) {
+		t.Errorf("the refreshed entry still carries the legacy subcommand %q: %s", legacy, refreshed)
+	}
+
+	removed, modified, err := Remove(doc, Spec{InstallID: oneID}, false)
+	if err != nil {
+		t.Fatalf("Remove: %v", err)
+	}
+	if removed == 0 {
+		t.Error("Remove removed nothing; the refreshed entry should have been recognised and removed")
+	}
+	if len(modified) != 0 {
+		t.Errorf("Remove reported entries it had to leave edited: %v", modified)
+	}
+	remainingStop := entriesOf(t, doc, EventStop)
+	if len(remainingStop) != 1 {
+		t.Fatalf("under Stop, %d entries remain, want 1 (the foreign one)", len(remainingStop))
+	}
+	if id := Owner(remainingStop[0], EventStop); id != "" {
+		t.Errorf("an owned entry survived detach under Stop, reading as %q", id)
+	}
+	if !strings.Contains(string(doc.Bytes()), foreignHook) {
+		t.Errorf("the foreign entry did not survive byte for byte:\n%s", doc.Bytes())
+	}
+}
+
 // TestRemoveIfAnyInstallID is the detach --all path. With the store gone there
 // is no id left to match against, so the predicate accepts every install -- and
 // the entry belonging to no install still has to survive it.

test/mutation/mutants/H-5_install.py

@@ -0,0 +1,25 @@
+# afterFirstWord's single-quoted branch has no requirement-row test of its
+# own -- TestLUI3_ and TestLUI4_ cover the whole-remainder comparison and the
+# legacy-form lookup, both downstream of this split, but neither one's
+# command is ever quoted. Every Windows install goes through this branch
+# (shellQuote quotes any path with a backslash), as does any POSIX install
+# under a path with a space or an embedded quote. This mutant breaks the
+# escaped-quote skip: resuming the search one byte past the closing quote,
+# instead of four, lands inside the backslash-quote-open sequence itself and
+# finds a false terminator there, so a quoted executable with an embedded
+# quote never resolves to an owner.
+m("afterFirstWord resumes inside the escaped-quote sequence instead of past it",
+  "internal/install/install.go",
+  "\t\tif strings.HasPrefix(cmd[end:], `'\\''`) {\n"
+  "\t\t\t// An embedded quote in the executable, escaped as close,\n"
+  "\t\t\t// backslash, quote, open -- not the terminator.\n"
+  "\t\t\ti = end + 4\n"
+  "\t\t\tcontinue\n"
+  "\t\t}",
+  "\t\tif strings.HasPrefix(cmd[end:], `'\\''`) {\n"
+  "\t\t\t// An embedded quote in the executable, escaped as close,\n"
+  "\t\t\t// backslash, quote, open -- not the terminator.\n"
+  "\t\t\ti = end + 1\n"
+  "\t\t\tcontinue\n"
+  "\t\t}",
+  "TestOwner/quoted_executable_with_an_embedded_single_quote")

test/mutation/mutants/LUI-3_install.py

@@ -0,0 +1,29 @@
+# LUI-3: entry identity is the whole command after the executable, not a
+# suffix of it. The old implementation searched h.Command for
+# " <subcommand> --install " anywhere, so a longer subcommand sharing a
+# shorter one's tail -- "steer post" ends in "post" -- would have read as the
+# shorter one's entry. TestLUI3_ pins the exact-match replacement directly;
+# TestLUI4_ also goes red, because the same suffix search never finds a
+# legacy form that is not itself a bare suffix of the command (e.g.
+# "recap-legacy-v1" against " recap --install ").
+m("LUI-3 Owner matches a suffix of the command instead of the whole of it",
+  "internal/install/install.go",
+  "\tforms := append([]string{subcommand(event)}, legacyForms[event]...)\n"
+  "\tfor _, h := range g.Hooks {\n"
+  "\t\tif id := ownerOf(h.Command, forms); id != \"\" {\n"
+  "\t\t\treturn id\n"
+  "\t\t}\n"
+  "\t}\n"
+  "\treturn \"\"\n"
+  "}",
+  "\tsuffix := \" \" + subcommand(event) + \" \" + Marker + \" \"\n"
+  "\tfor _, h := range g.Hooks {\n"
+  "\t\tif i := strings.LastIndex(h.Command, suffix); i >= 0 {\n"
+  "\t\t\tif id := h.Command[i+len(suffix):]; isInstallID(id) {\n"
+  "\t\t\t\treturn id\n"
+  "\t\t\t}\n"
+  "\t\t}\n"
+  "\t}\n"
+  "\treturn \"\"\n"
+  "}",
+  "TestLUI3_")

test/mutation/mutants/LUI-4_install.py

@@ -0,0 +1,10 @@
+# LUI-4: Owner recognises a listed legacy subcommand as ours, so a rename
+# refreshes an old entry in place instead of leaving it behind as residue
+# (detach would otherwise never recognise it either). Dropping legacyForms
+# from the candidate list is the whole regression: TestLUI4_ seeds a
+# legacy-shaped entry, so the refresh never fires, and Stop ends up with
+# three entries where it should have two.
+m("LUI-4 Owner never tries a listed legacy subcommand", "internal/install/install.go",
+  "\tforms := append([]string{subcommand(event)}, legacyForms[event]...)\n",
+  "\tforms := []string{subcommand(event)}\n",
+  "TestLUI4_")