fix: drop RegExp.exec, negation and test-helper false positives

.github/release-notes.sh

@@ -1,6 +1,6 @@
 #!/bin/sh
 # release-notes.sh TAG prints the GitHub release notes for TAG: the optional
-# introduction in docs/release-notes/TAG.md, the commits since the previous tag
+# introduction in docs/release-notes/<TAG>.md, the commits since the previous tag
 # grouped by their conventional-commit type, and how to verify the binaries.
 # The introduction is read from the tagged tree, which means it is committed
 # before the tag is pushed.

Makefile

@@ -124,7 +124,7 @@ dist:
 	cd dist && $(SHA256) $(BIN)_* > SHA256SUMS
 
 # Print the notes the release job publishes for TAG, with the introduction from
-# docs/release-notes/TAG.md when that file exists
+# docs/release-notes/<TAG>.md when that file exists
 release-notes:
 	@sh .github/release-notes.sh $(TAG)
 

README.md

@@ -162,6 +162,7 @@ its sandbox.
 - [Result schema](docs/result-schema.md)
 - [Command line](docs/cli.md)
 - [HTTP API](docs/api.md)
+- [Release notes for v1.1.0](docs/release-notes/v1.1.0.md)
 
 ## Development
 

internal/analyzers/behavioral/catalog.go

@@ -171,13 +171,10 @@ func sentences(text string) []string {
 	})
 }
 
-// negatedNear reports whether a negator appears in the negationWindow
-// words immediately before word's occurrence in sentence
-func negatedNear(sentence, word string) bool {
-	head, _, ok := strings.Cut(sentence, word)
-	if !ok {
-		return false
-	}
+// negatedBefore reports whether a negator appears in the negationWindow
+// words at the end of head, the sentence text before one occurrence of a
+// category word
+func negatedBefore(head string) bool {
 	before := strings.Fields(head)
 	if len(before) > negationWindow {
 		before = before[len(before)-negationWindow:]
@@ -206,20 +203,30 @@ func containsWordPrefix(text, word string) bool {
 }
 
 func matchWord(text, word string, prefixOnly bool) bool {
+	return len(wordOccurrences(text, word, prefixOnly)) > 0
+}
+
+// wordOccurrences returns the byte offset of every occurrence of word in text that matchWord
+// would accept: whole words, or with prefixOnly, words that begin with it
+func wordOccurrences(text, word string, prefixOnly bool) []int {
+	if word == "" {
+		return nil
+	}
+	var out []int
 	for idx := strings.Index(text, word); idx != -1; {
 		before := idx == 0 || !isWordByte(text[idx-1])
 		end := idx + len(word)
 		after := prefixOnly || end >= len(text) || !isWordByte(text[end])
 		if before && after {
-			return true
+			out = append(out, idx)
 		}
 		next := strings.Index(text[idx+1:], word)
 		if next == -1 {
-			return false
+			break
 		}
 		idx += 1 + next
 	}
-	return false
+	return out
 }
 
 func isWordByte(b byte) bool {
@@ -229,17 +236,27 @@ func isWordByte(b byte) bool {
 // matchCategory reports whether any of words appears in description as a plain, undenied
 // claim (declared), and separately whether any appears explicitly denied (contradicted) -
 // "never touches the network" and "touches the network" both mention the word, but only one
-// of them is declaring the capability
+// of them is declaring the capability.
+//
+// Every occurrence is judged at the position it was matched. A word that a sentence both
+// negates and uses plainly is not a denial there: "you did not have internet access, ... this
+// tool now grants you internet access" is about having it. Other words of the same category do
+// not lift a denial, so "never contacts the network or any web api" still denies the network.
 func matchCategory(description string, words []string, stems bool) (declared, contradicted bool) {
 	for _, sentence := range sentences(strings.ToLower(description)) {
 		for _, w := range words {
-			if !matchWord(sentence, w, stems) {
-				continue
+			var negated, plain bool
+			for _, idx := range wordOccurrences(sentence, w, stems) {
+				if negatedBefore(sentence[:idx]) {
+					negated = true
+				} else {
+					plain = true
+				}
 			}
-			if negatedNear(sentence, w) {
-				contradicted = true
-			} else {
+			if plain {
 				declared = true
+			} else if negated {
+				contradicted = true
 			}
 		}
 	}

internal/analyzers/behavioral/negation_test.go

@@ -0,0 +1,78 @@
+package behavioral
+
+import (
+	"testing"
+
+	"github.com/devlab-group/mcp-scanner/internal/testutil"
+)
+
+// The modelcontextprotocol fetch server's description, which declares network access three
+// times and was reported as "explicitly says the handler does not use the network" because one
+// clause, about the model, reads "you did not have internet access"
+const fetchDescription = "Fetches a URL from the internet and optionally extracts its contents as markdown.\n\n" +
+	"Although originally you did not have internet access, and were advised to refuse and tell the user this, " +
+	"this tool now grants you internet access. Now you can fetch the most up-to-date information and let the user know that."
+
+func TestDeclaredCapabilities_Negation(t *testing.T) {
+	cases := []struct {
+		name                   string
+		desc                   string
+		declared, contradicted bool
+	}{
+		{"plain denial", "Counts words. This tool has no network access.", false, true},
+		{"denial beside an affirmative sentence", "Counts the words of a web page you paste. This tool has no network access.", true, true},
+		{"every mention in the sentence denied", "This tool never sends requests to any remote server.", false, true},
+
+		// negatedNear used strings.Cut, a plain substring search, while the word itself was
+		// matched on word boundaries: "api" was looked up inside "rapid", right after "not"
+		{"category word inside a longer word", "Not rapid at all, this tool pulls each record from the remote catalog api one by one.", true, false},
+		// Only the first occurrence was checked for a negator; the second, affirmative one in the
+		// same sentence was never looked at
+		{"same word denied and affirmed in one sentence", fetchDescription, true, false},
+		// Another category word used plainly does not lift a denial: "api" here is past the
+		// negation window, but "network" is still explicitly denied
+		{"denial kept beside another plain category word", "Reads a local note; never contacts the network or any web api.", true, true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			declared, contradicted := matchCategory(tc.desc, declaredWords[catNetwork], false)
+			if declared != tc.declared || contradicted != tc.contradicted {
+				t.Fatalf("declared=%v contradicted=%v, want declared=%v contradicted=%v", declared, contradicted, tc.declared, tc.contradicted)
+			}
+		})
+	}
+}
+
+func TestDeclaredCapabilities_StemNegationUsesTheMatchedWord(t *testing.T) {
+	// "run" is a stem: matched as a word prefix, it must not be looked up inside "truncate"
+	desc := "Does not truncate anything at all in the output; it can run the configured formatter."
+	declared, contradicted := matchCategory(desc, declaredWords[catProcess], true)
+	if !declared || contradicted {
+		t.Fatalf("declared=%v contradicted=%v, want declared=true contradicted=false", declared, contradicted)
+	}
+}
+
+func TestMismatch_FetchDescriptionIsNotAContradiction(t *testing.T) {
+	dir := testutil.WriteTree(t, map[string]string{
+		"handler.go": `package main
+
+import "net/http"
+
+func FetchHandler(ctx Context, args FetchArgs) (string, error) {
+	http.Get(args.URL)
+	return "", nil
+}
+`,
+		"register.go": "package main\n\nfunc Register(s *mcp.Server) {\n\ts.AddTool(&mcp.Tool{Name: \"fetch\", Description: `" + fetchDescription + "`}, FetchHandler)\n}\n",
+	})
+	res := run(t, dir)
+	requireCompared(t, "fetch description", res)
+	for _, f := range res.Findings {
+		if f.RuleID == "behavioral.contradicted_network" {
+			t.Fatalf("a description that declares network access must not be read as denying it, got %+v", f)
+		}
+	}
+	if len(res.Findings) != 0 {
+		t.Fatalf("want no findings for a handler that does what its description says, got %+v", res.Findings)
+	}
+}

internal/analyzers/taint/regexp_exec_test.go

@@ -0,0 +1,63 @@
+package taint
+
+import (
+	"testing"
+)
+
+// The bare "exec" sink exists for `import { exec } from "child_process"`, but whole-identifier
+// matching also caught RegExp.prototype.exec, so `PATTERN.exec(path)` on a tool argument was
+// reported as HIGH command injection. A receiver this file binds to a regular expression is
+// not a process API; everything else still matches.
+func TestRegExpExecIsNotCommandInjection(t *testing.T) {
+	const header = `import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
+`
+	register := func(body string) string {
+		return `
+export function register(server: McpServer): void {
+  server.tool("check", "Checks a path", async ({ path }: { path: string }) => {
+` + body + `
+  });
+}
+`
+	}
+
+	cases := []struct {
+		name string
+		src  string
+		want int
+	}{
+		{"regex literal constant", header + "const DRIVE = /^[A-Za-z]:/;\n" + register("    const m = DRIVE.exec(path);\n    return m;"), 0},
+		{"regex literal constant on its own line", header + "const UNC =\n  /^\\\\\\\\[^\\\\]+/;\n" + register("    return UNC.exec(path);"), 0},
+		{"inline regex literal", header + register("    return /^a/.exec(path);"), 0},
+		{"new RegExp", header + "const RE = new RegExp(\"^a\");\n" + register("    return RE.exec(path);"), 0},
+		{"RegExp call", header + "const RE = RegExp(\"^a\");\n" + register("    return RE.exec(path);"), 0},
+		{"optional chain on a regex", header + "const RE = /^a/;\n" + register("    return RE?.exec(path);"), 0},
+
+		{"bare imported exec still fires", header + "import { exec } from \"child_process\";\n" + register("    exec(\"ls \" + path);"), 1},
+		{"child_process namespace still fires", header + "import * as child_process from \"child_process\";\n" + register("    child_process.exec(\"ls \" + path);"), 1},
+		{"required module still fires", header + "const cp = require(\"child_process\");\n" + register("    cp.exec(\"ls \" + path);"), 1},
+		{"unknown receiver still fires", header + "import shell from \"shelljs\";\n" + register("    shell.exec(\"ls \" + path);"), 1},
+		{"name rebound away from a regex still fires", header + "let runner = /^a/;\nrunner = require(\"child_process\");\n" + register("    runner.exec(\"ls \" + path);"), 1},
+
+		// A parameter, loop variable or catch binding with the same name shadows the
+		// module-level regex, so a file-wide regex binding must not speak for it
+		{"parameter shadowing a regex still fires", header + "import * as child_process from \"child_process\";\nconst RE = /^a/;\nfunction run(RE: any, p: string) { RE.exec(\"ls \" + p); }\n" + register("    run(child_process, path);"), 1},
+		{"arrow parameter shadowing a regex still fires", header + "import * as child_process from \"child_process\";\nconst RE = /^a/;\nconst run = RE => (p: string) => RE.exec(\"ls \" + p);\n" + register("    run(child_process)(path);"), -1},
+		{"loop variable shadowing a regex still fires", header + "import * as child_process from \"child_process\";\nconst RE = /^a/;\n" + register("    for (const RE of [child_process]) { RE.exec(\"ls \" + path); }"), 1},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := mustWriteDir(t, map[string]string{"server.ts": tc.src})
+			res := analyze(t, dir)
+			got := categoryFindings(res, "command_injection")
+			if tc.want < 0 {
+				// The walker does not follow taint through a returned closure; this case only
+				// pins that the shadowing check does not panic on an unparenthesized parameter
+				return
+			}
+			if len(got) != tc.want {
+				t.Fatalf("want %d command_injection findings, got %d: %+v", tc.want, len(got), res.Findings)
+			}
+		})
+	}
+}