diff --git a/cmd/nomos/assent.go b/cmd/nomos/assent.go index e1f3089..881a655 100644 --- a/cmd/nomos/assent.go +++ b/cmd/nomos/assent.go @@ -53,10 +53,18 @@ func extractPendingApprovals(calls []persistedCall) []pendingApproval { // don't restart it yet" contains neither "yes" nor "go ahead", but "wait" // alone should also block a stray "yes" a sentence later — checking negation // first and returning false errs toward re-confirming rather than assuming -// consent, per "when in doubt, escalate"). +// consent, per "when in doubt, escalate"). Includes contracted negatives +// ("haven't", "isn't", ...) alongside "don't"/"do not" — found live: "I +// haven't confirmed anything yet" was reading as an explicit confirmation +// because none of the contracted forms were covered, only "don't"/"do not". +// Deliberately does NOT include a bare "not": that's broad enough to false- +// negative ordinary assent ("go ahead, this is not risky") — the specific +// contracted-verb forms below are unambiguous negation on their own. var negationWords = []string{ "no", "nope", "don't", "do not", "stop", "wait", "hold on", "hold off", "not yet", "cancel", "nevermind", "never mind", "actually don't", "skip that", + "haven't", "hasn't", "isn't", "wasn't", "aren't", "can't", "cannot", + "won't", "wouldn't", "shouldn't", "didn't", "doesn't", } // assentWords, checked only if no negation matched. @@ -66,19 +74,56 @@ var assentWords = []string{ "lgtm", "run it", "execute", "ok go", "okay go", "please do", } +// wordTokenRe splits a message into lowercase word tokens. Apostrophes +// (straight ' and curly ’) stay attached to their word so "don't"/"haven't" +// tokenize as one token, not two. +var wordTokenRe = regexp.MustCompile(`[a-z0-9'’]+`) + +func tokenize(msg string) []string { + return wordTokenRe.FindAllString(strings.ToLower(strings.ReplaceAll(msg, "’", "'")), -1) +} + +// containsPhrase reports whether phrase (one or more words) appears as a +// consecutive run of WHOLE tokens in tokens — never a mid-word substring +// match. This is the fix for a real false positive found live: the old +// substring check (`strings.Contains(m, "yes")`) matched "yes" inside +// "yesterday", and "confirm" inside "confirmed"/"unconfirmed" without regard +// for word boundaries. Negation already used a word-boundary check +// (space-padded); assent/confirm words didn't — this brings both onto the +// same, more robust tokenized comparison instead of ad-hoc string padding. +func containsPhrase(tokens []string, phrase string) bool { + words := strings.Fields(phrase) + if len(words) == 0 || len(words) > len(tokens) { + return false + } + for i := 0; i+len(words) <= len(tokens); i++ { + match := true + for j, w := range words { + if tokens[i+j] != w { + match = false + break + } + } + if match { + return true + } + } + return false +} + // isAssent reports whether msg is a plain-language authorization of a // pending proposal. Deliberately simple and auditable: a fixed word list, // not a model judgment call, so behavior is predictable and can't be // prompt-injected via the pending action's own content. func isAssent(msg string) bool { - m := " " + strings.ToLower(strings.TrimSpace(msg)) + " " + tokens := tokenize(msg) for _, w := range negationWords { - if strings.Contains(m, " "+w+" ") || strings.HasPrefix(strings.TrimSpace(m), w+" ") || strings.HasPrefix(strings.TrimSpace(m), w+",") { + if containsPhrase(tokens, w) { return false } } for _, w := range assentWords { - if strings.Contains(m, w) { + if containsPhrase(tokens, w) { return true } } @@ -93,13 +138,13 @@ func isAssent(msg string) bool { // ("I confirm destroy 135"). Still negation-aware for the same reason as // isAssent: "don't confirm yet" must not accidentally match. func isTypedConfirmation(msg string) bool { - m := " " + strings.ToLower(strings.TrimSpace(msg)) + " " + tokens := tokenize(msg) for _, w := range negationWords { - if strings.Contains(m, " "+w+" ") || strings.HasPrefix(strings.TrimSpace(m), w+" ") || strings.HasPrefix(strings.TrimSpace(m), w+",") { + if containsPhrase(tokens, w) { return false } } - return strings.Contains(m, "confirm") + return containsPhrase(tokens, "confirm") || containsPhrase(tokens, "confirmed") } // approveExecution grants (or denies) a pending execution via the same HTTP diff --git a/cmd/nomos/assent_test.go b/cmd/nomos/assent_test.go index 64f4507..245fa05 100644 --- a/cmd/nomos/assent_test.go +++ b/cmd/nomos/assent_test.go @@ -45,6 +45,43 @@ func TestIsAssent_NegationBeatsAssentWord(t *testing.T) { } } +// TestIsAssent_WholeWordBoundary regression-tests a real false positive found +// live: the old substring check matched "yes" inside "yesterday" (and would +// equally match "confirm" inside "confirmed"/"unconfirmed" for +// isTypedConfirmation below) because only negation used a word-boundary +// check — assent/confirm words used a bare strings.Contains. Confirmed via a +// throwaway probe before being fixed; kept here permanently so a future +// change can't silently reintroduce it. +func TestIsAssent_WholeWordBoundary(t *testing.T) { + cases := []string{ + "not sure, maybe yesterday's logs show something useful", + "my eyesight isn't great, what does that say", + } + for _, c := range cases { + if isAssent(c) { + t.Errorf("isAssent(%q) = true, want false (word-boundary: 'yes' must not match inside 'yesterday'/'eyesight')", c) + } + } +} + +// TestIsTypedConfirmation_ContractedNegation regression-tests the other real +// false positive: isTypedConfirmation gates DESTRUCTIVE actions, and +// "confirm" matching inside "confirmed" combined with contracted negatives +// ("haven't") not being in negationWords meant a message that explicitly +// says the operator has NOT confirmed something could read as confirming it. +func TestIsTypedConfirmation_ContractedNegation(t *testing.T) { + cases := []string{ + "I haven't confirmed anything yet, let me think", + "that isn't confirmed on my end", + "we can't confirm that until tomorrow", + } + for _, c := range cases { + if isTypedConfirmation(c) { + t.Errorf("isTypedConfirmation(%q) = true, want false (contracted negation should block)", c) + } + } +} + func TestIsTypedConfirmation(t *testing.T) { positive := []string{ "I confirm destroy 135 in strong",