Tighten rule precision heuristics

README.md

@@ -261,13 +261,13 @@ jobs:
 
 Rules are grouped into categories:
 
-| Category          | Focus                                                 | Example rule                        |
-| ----------------- | ----------------------------------------------------- | ----------------------------------- |
-| `safety`          | Destructive, financial, external-communication guards | `safety/destructive-requires-guard` |
-| `schema`          | Boolean clarity, enums, sensitive fields              | `schema/vague-boolean`              |
-| `docs`            | Missing or weak descriptions                          | `docs/missing-description`          |
-| `errors`          | Structured error response schemas                     | `errors/missing-error-schema`       |
-| `agent_usability` | Pagination, limits                                    | `schema/list-requires-pagination`   |
+| Category          | Focus                                              | Example rule                        |
+| ----------------- | -------------------------------------------------- | ----------------------------------- |
+| `safety`          | Guard signals, idempotency, batch limits           | `safety/destructive-requires-guard` |
+| `schema`          | Boolean clarity, enums, sensitive fields           | `schema/vague-boolean`              |
+| `docs`            | Missing, weak, or unclear side-effect descriptions | `docs/missing-description`          |
+| `errors`          | Structured error response schemas                  | `errors/missing-error-schema`       |
+| `agent_usability` | Bounded list/search outputs                        | `schema/list-requires-pagination`   |
 
 Run `toolsafe rules` to see the full list.
 

RULE_ACCURACY_IMPROVEMENTS.md

@@ -0,0 +1,238 @@
+# ToolSafe Rule Accuracy Improvements
+
+## Context
+
+ToolSafe's rules currently rely on keyword matching against
+`getOperationSearchText()`, which concatenates `id`, `operationId`, `name`,
+`method`, `path`, `summary`, `description`, and `tags` into one lowercased
+string. Several rules then do a simple substring/word match against this
+blob. This produces a structural false-positive problem: a keyword
+appearing anywhere in free text (especially `description`, which can be a
+full paragraph) triggers a finding, even when that text is not describing
+the operation's own action.
+
+Goal of this pass: reduce false positives by (1) scoping keyword matching to
+the parts of the operation that actually signal intent (method + path +
+operationId + summary — NOT full description body), (2) adding negative/
+exclusion checks where a single keyword is not sufficient signal, and (3)
+adding "should NOT flag" regression tests for every rule below, sourced from
+realistic operation names.
+
+Do not change rule IDs, severities, or the `Rule` interface. Only change
+matching logic and add tests + a CHANGELOG entry per rule.
+
+---
+
+## 1. `safety/destructive-requires-guard.ts`
+
+**Problem:** Matches `DESTRUCTIVE_KEYWORDS` against the full search text,
+including descriptions. A `GET /cancellation-reasons` or a `POST
+/subscriptions/{id}/resume` whose description happens to mention "previously
+cancelled" will false-positive. Also: keyword match has no method
+constraint other than the OR with `DELETE`, so a `GET` whose summary says
+"Remove duplicate filters from the response" will incorrectly flag.
+
+**Fix:**
+
+- Require the operation to be mutating (`POST`/`PUT`/`PATCH`/`DELETE`) OR
+  method `DELETE`, before keyword matching applies at all. A `GET` should
+  never be flagged by this rule regardless of keyword content.
+- Scope keyword matching to `operationId`, `path` segments, and `summary`
+  only — drop `description` from the text searched by this rule
+  specifically (add a narrower `getOperationIntentText()` helper that
+  excludes `description`).
+- Match keywords as whole path/word segments, not raw substrings (e.g.
+  match `/cancel` as a path segment or `cancel` as a whole word in
+  operationId/summary, not as a substring of `cancellation`).
+
+**Add regression tests for:**
+
+- `GET /subscriptions/{id}/cancellation-history` → should NOT flag.
+- `POST /subscriptions/{id}/resume` with description mentioning "cancel" → should NOT flag.
+- `DELETE /users/{id}` with no guard field → SHOULD flag (existing case).
+- `POST /users/{id}/deactivate` with `x-agent-guard` extension → should NOT flag.
+
+---
+
+## 2. `safety/external-communication-requires-guard.ts`
+
+**Problem:** `EXTERNAL_COMMUNICATION_KEYWORDS` includes generic words like
+`message`, `send`, `publish`. A `POST /chat/messages` (internal chat
+storage, not external comms) or `POST /articles/{id}/publish` (CMS publish,
+not external comms) will false-positive.
+
+**Fix:**
+
+- Split the keyword list into high-confidence external-recipient signals
+  (`email`, `sms`, `webhook`, `notify`, `invite`) vs. ambiguous ones
+  (`message`, `send`, `publish`, `broadcast`). Require ambiguous keywords to
+  co-occur with a recipient-shaped field in the input schema (e.g. `to`,
+  `recipient`, `email`, `phoneNumber`, `address`) before flagging.
+- Drop `description` from the searched text for the same reason as Rule 1;
+  match against operationId/path/summary only.
+
+**Add regression tests for:**
+
+- `POST /chat/messages` (internal message store, no recipient field) → should NOT flag.
+- `POST /articles/{id}/publish` (no recipient field) → should NOT flag.
+- `POST /notifications/email` with `recipientEmail` field, no guard → SHOULD flag.
+
+---
+
+## 3. `safety/financial-requires-idempotency.ts`
+
+**Problem:** Keywords like `credit`, `debit`, `bank` are broad. A `GET
+/users/{id}/credits` (loyalty points) or `PATCH /accounts/{id}/bank-name`
+(metadata edit, not a money movement) could false-positive once mutating +
+keyword conditions are met.
+
+**Fix:**
+
+- Require the keyword match to be on `operationId` or last path segment
+  (the resource/action being acted on), not on substrings anywhere in the
+  path. E.g. `bank-name` should not match `bank` as a financial-action
+  keyword; require the segment to equal or start the action verb position.
+- Consider removing `credit`/`debit`/`bank` as standalone triggers and
+  instead require they appear alongside a clear money-movement verb
+  (`charge`, `transfer`, `payout`, `refund`, `payment`) in the same
+  operationId/summary.
+
+**Add regression tests for:**
+
+- `PATCH /accounts/{id}/bank-name` → should NOT flag.
+- `GET /loyalty/credits` → excluded already by method, but add explicit test.
+- `POST /payments/{id}/refund` with no idempotency field → SHOULD flag.
+
+---
+
+## 4. `safety/batch-operation-requires-limit.ts`
+
+**Problem:** `mass` and `all` (via reuse risk if list keywords get merged)
+are weak signals. `mass` rarely appears; lower priority, but the keyword
+match still spans descriptions, so a non-batch endpoint whose description
+says "supports bulk import via separate endpoint" would false-positive.
+
+**Fix:**
+
+- Scope matching to operationId/path/summary, exclude description body
+  (same fix pattern as above).
+- Require an actual array/list-shaped request body input in addition to the
+  keyword, since a genuinely "batch" endpoint should have a collection
+  input; a keyword match with a scalar-only body is likely a false positive
+  (e.g. `POST /jobs/bulk-status-check` with a single `jobId`).
+
+**Add regression tests for:**
+
+- `POST /jobs/bulk-status-check` with single scalar `jobId` field → should NOT flag (or document why it's intentionally still flagged, if you decide not to add the array-shape check).
+- `POST /users/bulk-delete` with `userIds: string[]` and no limit field → SHOULD flag.
+
+---
+
+## 5. `docs/mutating-description-mentions-side-effects.ts`
+
+**Problem:** This rule is actually reasonably scoped already (it only
+checks for presence of side-effect verbs, doesn't need a "should NOT flag"
+fix as urgently), but it currently treats `tool.description` absence and
+"no side-effect verb present" identically. A short, otherwise-good
+description like "Creates resource" already contains "creates" so it
+passes — verify this is intended. Lower priority than rules 1–4.
+
+**Fix (optional/lower priority):**
+
+- No change required to matching logic. Add a test confirming a one-word
+  summary like `"Create"` (no article, stem only) still matches due to
+  substring matching on `creates`. Decide if `create` (no -s) should also be
+  in `SIDE_EFFECT_VERBS` — currently only `creates`/`creates a`/`creates an`
+  are listed, so a summary of exactly `"Create user"` will NOT match and
+  WILL false-flag a perfectly fine description. Add `create`, `update`,
+  `delete`, `remove`, `cancel`, `revoke` (bare stems) to the verb list.
+
+**Add regression test for:**
+
+- `POST /users` with description `"Create user"` → currently false-flags; should NOT flag after stem fix.
+
+---
+
+## 6. `schema/list-requires-pagination.ts`
+
+**Problem:** `LIST_KEYWORDS` includes generic resource-name plurals like
+`customers`, `users`, `items`, `events`. Any `GET` whose path or summary
+contains these words is treated as a "list" operation even for singular
+lookups like `GET /users/{id}/events/{eventId}` (a single nested resource
+fetch, not a list).
+
+**Fix:**
+
+- Before keyword matching, check whether the operation path ends in a path
+  parameter (e.g. `/{id}`) at the final segment. If so, it's a single-item
+  fetch, not a collection — skip the rule regardless of keyword.
+- Keep keyword matching only for paths whose final segment is NOT a path
+  parameter.
+
+**Add regression tests for:**
+
+- `GET /users/{id}` → should NOT flag (singular resource, no plural-keyword issue here since `users` already implies a list keyword hit before the param-check fix).
+- `GET /users/{id}/events/{eventId}` → should NOT flag after the param fix.
+- `GET /users` with no pagination param → SHOULD flag (existing case).
+
+---
+
+## 7. `schema/string-should-be-enum.ts`, `schema/vague-boolean.ts`
+
+**Problem:** Both match on property _name_ only (`status`, `mode`, `force`,
+`flag`, etc.) regardless of context. These are lower false-positive risk
+than the safety rules since they're schema-local, but a field named
+`sortOrder` (string, free text like "name,-createdAt") would false-positive
+on `order`/`sort` even though enumerating all valid sort expressions isn't
+feasible.
+
+**Fix (lower priority):**
+
+- For `string-should-be-enum`, exclude properties whose schema has a
+  `pattern` constraint already (a regex-constrained string is already
+  explicit, just not via enum — don't double-flag).
+- For `vague-boolean`, no major change needed; this one is genuinely just a
+  naming-convention heuristic and is reasonably safe. Document this as
+  intentionally aggressive/informational rather than "fixing" it.
+
+**Add regression test for:**
+
+- `string-should-be-enum`: property `sortOrder` with `pattern` set → should NOT flag.
+
+---
+
+## 8. Cross-cutting: add a narrower search-text helper
+
+Add `getOperationIntentText()` in `src/rules/helpers.ts` alongside the
+existing `getOperationSearchText()`:
+
+```ts
+// Excludes `description` — used by rules where matching against free-text
+// prose (rather than the operation's own identifiers) causes false positives.
+export function getOperationIntentText(tool: NormalizedTool): string {
+  return [tool.id, tool.operationId, tool.name, tool.method, tool.path, tool.summary]
+    .filter((value): value is string => typeof value === 'string')
+    .join(' ')
+    .toLowerCase();
+}
+```
+
+Migrate rules 1–4 above to use this instead of `getOperationSearchText()`.
+Keep `getOperationSearchText()` as-is for any rule that intentionally wants
+full-text matching (if any remain after this pass — currently none should).
+
+---
+
+## 9. Validation step (do this last)
+
+After implementing the above:
+
+1. Run `toolsafe lint` against at least 2 large public OpenAPI specs (Stripe,
+   GitHub, Shopify, or Twilio's public spec — pick whichever are easiest to
+   fetch) before and after this change.
+2. Record finding counts per rule, before/after, in
+   `docs/CURRENT_STATE.md` under a new "Precision notes" section.
+3. Confirm the total finding count drops without losing the seeded findings
+   in `examples/risky-openapi.yaml` (run `bun run examples:check` — it
+   should still report the same risky operations as findings unless this
+   plan explicitly says otherwise above).

docs/CURRENT_STATE.md

@@ -72,7 +72,7 @@ The default rule set now has 10 rules. Milestone 7 added:
 - `schema/string-should-be-enum`
 - `schema/sensitive-response-fields`
 
-These rules keep the same deterministic, evidence-based shape as the initial rule set. They inspect operation text, explicit inputs, top-level request schemas, and top-level response schemas.
+These rules keep the same deterministic, evidence-based shape as the initial rule set. They inspect operation intent text, explicit inputs, top-level request schemas, and top-level response schemas.
 
 ### Milestone 8: Policy Draft Generator
 
@@ -171,6 +171,19 @@ ToolSafe now supports `toolsafe.config.json` for tuning without changing source:
 - Template files live in `src/cli/commands/init/` as plain JSON/YAML.
 - `--analyze` flag discovers OpenAPI specs by naming convention (`openapi.*`, `swagger.*`, `spec.*`, or in `openapi/`/`swagger/` dirs) and content sniffing (first 4KB checked for `openapi` key), then lints each and prints a summary.
 
+### Milestone 18: Rule Precision Pass
+
+Rule matching was tightened to reduce false positives while keeping deterministic output:
+
+- Added operation intent text and tokenized keyword matching so safety rules search operation IDs, generated names, methods, paths, summaries, and tags instead of arbitrary description prose.
+- `safety/destructive-requires-guard` now ignores destructive-sounding read-only operations and description-only keyword hits.
+- `safety/external-communication-requires-guard` separates high-confidence recipient channels from ambiguous verbs like `send`, `message`, and `publish`; ambiguous matches require recipient-shaped inputs.
+- `safety/financial-requires-idempotency` focuses on clearer financial action/resource keywords instead of broad `bank`, `credit`, or `debit` matches.
+- `safety/batch-operation-requires-limit` now requires collection-shaped input in addition to batch/bulk intent.
+- `schema/list-requires-pagination` skips GET paths ending in a path parameter, reducing single-resource lookup false positives.
+- `schema/string-should-be-enum` no longer flags pattern-constrained strings.
+- `docs/mutating-description-mentions-side-effects` checks summary/description prose and accepts bare side-effect verbs such as `Create user`.
+
 ## Not Implemented Yet
 
 The following are planned or mentioned in product docs but are not complete at this point:

docs/RULES.md

@@ -6,22 +6,28 @@ ToolSafe currently runs 15 default rules. Rules are deterministic and operate on
 
 | Rule ID                                           | Severity | Category        | Purpose                                                                                                              |
 | ------------------------------------------------- | -------- | --------------- | -------------------------------------------------------------------------------------------------------------------- |
-| `safety/destructive-requires-guard`               | error    | safety          | Flags destructive operations without explicit confirmation fields or guard metadata.                                 |
-| `safety/batch-operation-requires-limit`           | warning  | safety          | Flags batch or bulk operations without a limit or max items parameter.                                               |
+| `safety/destructive-requires-guard`               | error    | safety          | Flags DELETE or mutating destructive operations without explicit confirmation fields or guard metadata.              |
+| `safety/batch-operation-requires-limit`           | warning  | safety          | Flags batch or bulk operations with collection-shaped inputs but no limit or max items parameter.                    |
 | `auth/dangerous-auth-scope`                       | warning  | auth            | Flags security requirements with overly broad or dangerous scopes.                                                   |
-| `safety/financial-requires-idempotency`           | warning  | safety          | Flags financial mutations without an idempotency key or request ID.                                                  |
-| `safety/external-communication-requires-guard`    | warning  | safety          | Flags email, SMS, notification, invite, webhook, or broadcast operations without confirmation or guard metadata.     |
+| `safety/financial-requires-idempotency`           | warning  | safety          | Flags mutating payment, refund, transfer, payout, invoice, billing, or subscription operations without idempotency.  |
+| `safety/external-communication-requires-guard`    | warning  | safety          | Flags likely external-recipient operations without confirmation or guard metadata.                                   |
 | `safety/mutating-requires-dry-run`                | warning  | safety          | Flags mutating operations without dry-run, preview, validate-only, or plan-only inputs.                              |
-| `schema/list-requires-pagination`                 | warning  | agent usability | Flags likely list/search operations without pagination or limit parameters.                                          |
+| `schema/list-requires-pagination`                 | warning  | agent usability | Flags likely collection GET operations without pagination or limit parameters.                                       |
 | `schema/unconstrained-file-upload`                | warning  | schema          | Flags file upload inputs that lack size or content constraints.                                                      |
 | `schema/vague-boolean`                            | warning  | schema          | Flags vague boolean inputs such as `force` or `flag`.                                                                |
-| `schema/string-should-be-enum`                    | warning  | schema          | Flags likely constrained string inputs such as `role`, `status`, or `mode` without enum values.                      |
+| `schema/string-should-be-enum`                    | warning  | schema          | Flags likely constrained string inputs such as `role`, `status`, or `mode` without enum values or another pattern.   |
 | `schema/sensitive-response-fields`                | warning  | schema          | Flags response schemas with sensitive top-level fields such as tokens, secrets, credentials, or payment identifiers. |
 | `docs/missing-description`                        | warning  | docs            | Flags operations with neither summary nor description.                                                               |
 | `docs/weak-description`                           | info     | docs            | Flags descriptions that are too short or contain generic placeholder text.                                           |
 | `docs/mutating-description-mentions-side-effects` | warning  | docs            | Flags mutating operations whose descriptions lack mention of side effects.                                           |
 | `errors/missing-error-schema`                     | warning  | errors          | Flags operations without any structured 4xx or 5xx response schema.                                                  |
 
+## Matching Precision
+
+Safety and usability rules use operation intent text: operation ID, generated name, method, path, summary, and tags. They intentionally ignore long-form description prose so incidental words in documentation do not trigger findings.
+
+Keyword checks tokenize camelCase names and path segments before matching. For example, `GET /subscriptions/{id}/cancellation-history` does not satisfy the destructive rule just because it contains `cancellation`, and ambiguous external communication verbs such as `send`, `message`, or `publish` need recipient-shaped inputs before they are treated as external communication.
+
 ## Rule Output
 
 Each finding includes: