From aa9418436d0b1f9bcfafb0ab9a23a247a8f511da Mon Sep 17 00:00:00 2001 From: styltsou Date: Sun, 28 Jun 2026 12:14:47 +0300 Subject: [PATCH] Tighten rule precision heuristics --- README.md | 14 +- RULE_ACCURACY_IMPROVEMENTS.md | 238 ++++++++++++++++ docs/CURRENT_STATE.md | 15 +- docs/RULES.md | 18 +- docs/RULES_AND_REPORTS.md | 21 +- src/cli/completion.ts | 2 +- src/core/schema.ts | 22 ++ ...ating-description-mentions-side-effects.ts | 15 +- src/rules/helpers.ts | 45 +++ .../safety/batch-operation-requires-limit.ts | 9 +- .../safety/destructive-requires-guard.ts | 12 +- .../external-communication-requires-guard.ts | 36 ++- .../safety/financial-requires-idempotency.ts | 13 +- src/rules/schema/list-requires-pagination.ts | 14 +- src/rules/schema/string-should-be-enum.ts | 3 +- tests/core/analyze.test.ts | 8 +- .../__snapshots__/evals.test.ts.snap | 42 --- .../__snapshots__/policy.test.ts.snap | 6 - .../__snapshots__/reporters.test.ts.snap | 110 +------ tests/rules/rules.test.ts | 268 +++++++++++++++++- 20 files changed, 700 insertions(+), 211 deletions(-) create mode 100644 RULE_ACCURACY_IMPROVEMENTS.md diff --git a/README.md b/README.md index 2f65f08..8f041a7 100644 --- a/README.md +++ b/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. diff --git a/RULE_ACCURACY_IMPROVEMENTS.md b/RULE_ACCURACY_IMPROVEMENTS.md new file mode 100644 index 0000000..cdbd4d2 --- /dev/null +++ b/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). diff --git a/docs/CURRENT_STATE.md b/docs/CURRENT_STATE.md index 506d2c8..87f8ede 100644 --- a/docs/CURRENT_STATE.md +++ b/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: diff --git a/docs/RULES.md b/docs/RULES.md index f333728..e84a4e0 100644 --- a/docs/RULES.md +++ b/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: diff --git a/docs/RULES_AND_REPORTS.md b/docs/RULES_AND_REPORTS.md index 9fa7209..fe72924 100644 --- a/docs/RULES_AND_REPORTS.md +++ b/docs/RULES_AND_REPORTS.md @@ -24,21 +24,32 @@ Rules should not: ## Current Rule Set -The current default rules cover 10 reliable checks: +The current default rules cover 15 reliable checks: -- Destructive operations should declare an explicit agent guard or confirmation signal. +- Destructive DELETE or mutating operations should declare an explicit agent guard or confirmation signal. - Financial mutations should expose idempotency or deduplication inputs. -- External communication operations should declare confirmation or guard signals. +- External-recipient operations should declare confirmation or guard signals. - Mutating operations should expose dry-run, preview, or validate-only behavior when possible. -- Likely list operations should expose pagination or limit parameters. +- Batch operations with collection-shaped inputs should expose limits. +- Likely collection GET operations should expose pagination or limit parameters. - Vague boolean inputs should be renamed or replaced with constrained values. -- Likely constrained string inputs should expose enum values. +- Likely constrained string inputs should expose enum values or another explicit pattern. - Sensitive response fields should be redacted or explicitly reviewed. +- File uploads should expose size or content constraints. +- Broad auth scopes should be narrowed or reviewed. - Operations should have summary or description text. +- Weak descriptions should be expanded. +- Mutating operation descriptions should mention side effects. - Operations should define structured 4xx or 5xx error response schemas. The rule registry in `src/rules/index.ts` defines which rules run by default. Tests assert stable default finding order for the risky example. +## Heuristic Precision + +Rules that infer operation intent should avoid matching arbitrary description prose. ToolSafe now provides an intent-text helper that searches operation ID, generated name, method, path, summary, and tags, plus tokenized keyword matching for camelCase and path segments. This keeps rules deterministic while reducing false positives from words that merely appear in long-form documentation. + +Some rules add extra structural evidence before reporting. Batch findings require collection-shaped inputs, ambiguous external communication verbs require recipient-shaped inputs, and list pagination skips GET paths that end in a path parameter because those usually represent single-resource lookups. + ## Finding Quality A finding should help an engineer decide what to change. The most useful findings include: diff --git a/src/cli/completion.ts b/src/cli/completion.ts index bb12edf..f75d39d 100644 --- a/src/cli/completion.ts +++ b/src/cli/completion.ts @@ -13,7 +13,7 @@ export function generateBashCompletion(): string { return fi - case ${words[1]} in + case \${words[1]} in init) case $prev in -a|--analyze) diff --git a/src/core/schema.ts b/src/core/schema.ts index 466ba1a..d12b23a 100644 --- a/src/core/schema.ts +++ b/src/core/schema.ts @@ -32,6 +32,16 @@ export function hasAnyQueryParameter(tool: NormalizedTool, names: readonly strin ); } +export function hasArrayInput(tool: NormalizedTool): boolean { + return ( + tool.parameters.some((parameter) => isArraySchema(parameter.schema)) || + isArraySchema(tool.requestBodySchema) || + getTopLevelSchemaProperties(tool.requestBodySchema).some((property) => + isArraySchema(property.schema), + ) + ); +} + export function getTopLevelSchemaPropertyNames(schema: unknown): string[] { return getTopLevelSchemaProperties(schema).map((property) => property.name); } @@ -93,6 +103,14 @@ export function hasSchemaConstraint(schema: unknown, constraintNames: readonly s ); } +export function hasSchemaKeyword(schema: unknown, keywordNames: readonly string[]): boolean { + if (!isObject(schema)) { + return false; + } + + return keywordNames.some((name) => (schema as Record)[name] !== undefined); +} + function getSchemaType(schema: unknown): string | undefined { if (!isObject(schema)) { return undefined; @@ -103,6 +121,10 @@ function getSchemaType(schema: unknown): string | undefined { return typeof type === 'string' ? type : undefined; } +function isArraySchema(schema: unknown): boolean { + return getSchemaType(schema) === 'array'; +} + function unwrapArraySchema(schema: unknown): unknown { if (!isObject(schema)) { return schema; diff --git a/src/rules/docs/mutating-description-mentions-side-effects.ts b/src/rules/docs/mutating-description-mentions-side-effects.ts index 3caa57d..3f2242b 100644 --- a/src/rules/docs/mutating-description-mentions-side-effects.ts +++ b/src/rules/docs/mutating-description-mentions-side-effects.ts @@ -1,20 +1,23 @@ import type { Rule } from '@/core/types'; import { includesAny } from '@/core/strings'; import { createFinding } from '@/rules/findings'; -import { getOperationSearchText } from '@/rules/helpers'; const MUTATING_METHODS = ['POST', 'PUT', 'PATCH', 'DELETE'] as const; const SIDE_EFFECT_VERBS = [ + 'create', 'creates', 'creates a', 'creates an', + 'update', 'updates', 'updates a', 'updates an', + 'delete', 'deletes', 'deletes a', 'deletes an', + 'remove', 'removes', 'removes a', 'removes an', @@ -32,8 +35,11 @@ const SIDE_EFFECT_VERBS = [ 'transfers', 'publishes', 'submits', + 'cancel', 'cancels', + 'revoke', 'revokes', + 'terminate', 'terminates', ]; @@ -53,9 +59,12 @@ export const mutatingDescriptionMentionsSideEffectsRule: Rule = { return []; } - const searchText = getOperationSearchText(tool); + const documentationText = [tool.summary, tool.description] + .filter((value): value is string => typeof value === 'string') + .join(' ') + .toLowerCase(); - if (includesAny(searchText, SIDE_EFFECT_VERBS)) { + if (includesAny(documentationText, SIDE_EFFECT_VERBS)) { return []; } diff --git a/src/rules/helpers.ts b/src/rules/helpers.ts index fc90c5a..f0ac87e 100644 --- a/src/rules/helpers.ts +++ b/src/rules/helpers.ts @@ -23,6 +23,28 @@ export function getOperationSearchText(tool: NormalizedTool): string { .toLowerCase(); } +/** + * Returns lowercased operation intent text without long-form description prose. + * + * Rules that infer operation intent should prefer this over + * `getOperationSearchText()` to avoid matching incidental words in docs. + */ +export function getOperationIntentText(tool: NormalizedTool): string { + return [tool.id, tool.operationId, tool.name, tool.method, tool.path, tool.summary, ...tool.tags] + .filter((value): value is string => typeof value === 'string') + .join(' ') + .toLowerCase(); +} + +export function hasOperationIntentKeyword( + tool: NormalizedTool, + keywords: readonly string[], +): boolean { + const tokens = getOperationIntentTokens(tool); + + return keywords.some((keyword) => tokens.has(keyword.toLowerCase())); +} + export function hasOperationExtension(tool: NormalizedTool, names: readonly string[]): boolean { if (!isObject(tool.rawOperation)) { return false; @@ -30,3 +52,26 @@ export function hasOperationExtension(tool: NormalizedTool, names: readonly stri return names.some((name) => Object.hasOwn(tool.rawOperation as Record, name)); } + +function getOperationIntentTokens(tool: NormalizedTool): Set { + return new Set( + [ + tool.id, + tool.operationId, + tool.name, + tool.method, + tool.path, + tool.summary, + ...tool.tags, + ].flatMap((value) => (typeof value === 'string' ? tokenizeIntent(value) : [])), + ); +} + +function tokenizeIntent(value: string): string[] { + return value + .replace(/([a-z0-9])([A-Z])/g, '$1 $2') + .replace(/[^a-zA-Z0-9]+/g, ' ') + .toLowerCase() + .split(/\s+/) + .filter(Boolean); +} diff --git a/src/rules/safety/batch-operation-requires-limit.ts b/src/rules/safety/batch-operation-requires-limit.ts index 684d8cf..247bbe9 100644 --- a/src/rules/safety/batch-operation-requires-limit.ts +++ b/src/rules/safety/batch-operation-requires-limit.ts @@ -1,10 +1,9 @@ import type { Rule } from '@/core/types'; -import { hasAnyInputField } from '@/core/schema'; -import { includesAny } from '@/core/strings'; +import { hasAnyInputField, hasArrayInput } from '@/core/schema'; import { createFinding } from '@/rules/findings'; -import { getOperationSearchText } from '@/rules/helpers'; +import { hasOperationIntentKeyword } from '@/rules/helpers'; -const BATCH_KEYWORDS = ['batch', 'bulk', 'mass', 'bulk-update', 'bulk-delete', 'batch-create']; +const BATCH_KEYWORDS = ['batch', 'bulk', 'mass']; const LIMIT_FIELDS = [ 'limit', @@ -28,7 +27,7 @@ export const batchOperationRequiresLimitRule: Rule = { category: 'safety', defaultSeverity: 'warning', check: ({ tool }) => { - const isBatch = includesAny(getOperationSearchText(tool), BATCH_KEYWORDS); + const isBatch = hasOperationIntentKeyword(tool, BATCH_KEYWORDS) && hasArrayInput(tool); if (!isBatch) { return []; diff --git a/src/rules/safety/destructive-requires-guard.ts b/src/rules/safety/destructive-requires-guard.ts index 881be3a..993ae03 100644 --- a/src/rules/safety/destructive-requires-guard.ts +++ b/src/rules/safety/destructive-requires-guard.ts @@ -1,8 +1,9 @@ -import type { Rule } from '@/core/types'; +import type { HttpMethod, Rule } from '@/core/types'; import { hasAnyInputField } from '@/core/schema'; -import { includesAny } from '@/core/strings'; import { createFinding } from '@/rules/findings'; -import { getOperationSearchText, hasOperationExtension } from '@/rules/helpers'; +import { hasOperationExtension, hasOperationIntentKeyword } from '@/rules/helpers'; + +const MUTATING_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); const DESTRUCTIVE_KEYWORDS = [ 'delete', @@ -48,8 +49,9 @@ export const destructiveRequiresGuardRule: Rule = { category: 'safety', defaultSeverity: 'error', check: ({ tool }) => { - const searchText = getOperationSearchText(tool); - const isDestructive = tool.method === 'DELETE' || includesAny(searchText, DESTRUCTIVE_KEYWORDS); + const isDestructive = + tool.method === 'DELETE' || + (MUTATING_METHODS.has(tool.method) && hasOperationIntentKeyword(tool, DESTRUCTIVE_KEYWORDS)); if (!isDestructive) { return []; diff --git a/src/rules/safety/external-communication-requires-guard.ts b/src/rules/safety/external-communication-requires-guard.ts index 70acd75..4829433 100644 --- a/src/rules/safety/external-communication-requires-guard.ts +++ b/src/rules/safety/external-communication-requires-guard.ts @@ -1,24 +1,25 @@ import type { HttpMethod, Rule } from '@/core/types'; import { hasAnyInputField } from '@/core/schema'; -import { includesAny } from '@/core/strings'; import { createFinding } from '@/rules/findings'; -import { getOperationSearchText, hasOperationExtension } from '@/rules/helpers'; +import { hasOperationExtension, hasOperationIntentKeyword } from '@/rules/helpers'; const MUTATING_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); -const EXTERNAL_COMMUNICATION_KEYWORDS = [ +const HIGH_CONFIDENCE_EXTERNAL_KEYWORDS = [ 'email', + 'emails', 'sms', - 'message', 'notify', 'notification', + 'notifications', 'invite', + 'invites', 'webhook', - 'publish', - 'send', - 'broadcast', + 'webhooks', ]; +const AMBIGUOUS_EXTERNAL_KEYWORDS = ['message', 'messages', 'publish', 'send', 'broadcast']; + const GUARD_FIELDS = [ 'confirm', 'confirmation', @@ -30,6 +31,21 @@ const GUARD_FIELDS = [ 'recipient_confirmed', ]; +const RECIPIENT_FIELDS = [ + 'to', + 'recipient', + 'recipients', + 'recipientEmail', + 'recipient_email', + 'email', + 'emailAddress', + 'email_address', + 'phone', + 'phoneNumber', + 'phone_number', + 'address', +]; + const GUARD_EXTENSIONS = [ 'x-agent-guard', 'x-toolsafe-guard', @@ -48,9 +64,11 @@ export const externalCommunicationRequiresGuardRule: Rule = { category: 'safety', defaultSeverity: 'warning', check: ({ tool }) => { - const searchText = getOperationSearchText(tool); const isExternalCommunication = - MUTATING_METHODS.has(tool.method) && includesAny(searchText, EXTERNAL_COMMUNICATION_KEYWORDS); + MUTATING_METHODS.has(tool.method) && + (hasOperationIntentKeyword(tool, HIGH_CONFIDENCE_EXTERNAL_KEYWORDS) || + (hasOperationIntentKeyword(tool, AMBIGUOUS_EXTERNAL_KEYWORDS) && + hasAnyInputField(tool, RECIPIENT_FIELDS))); if (!isExternalCommunication) { return []; diff --git a/src/rules/safety/financial-requires-idempotency.ts b/src/rules/safety/financial-requires-idempotency.ts index ffb97a8..27ca10b 100644 --- a/src/rules/safety/financial-requires-idempotency.ts +++ b/src/rules/safety/financial-requires-idempotency.ts @@ -1,23 +1,23 @@ import type { HttpMethod, Rule } from '@/core/types'; import { hasAnyInputField } from '@/core/schema'; -import { includesAny } from '@/core/strings'; import { createFinding } from '@/rules/findings'; -import { getOperationSearchText } from '@/rules/helpers'; +import { hasOperationIntentKeyword } from '@/rules/helpers'; const MUTATING_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); const FINANCIAL_KEYWORDS = [ 'payment', + 'payments', 'charge', 'refund', 'transfer', 'payout', + 'payouts', 'invoice', + 'invoices', 'billing', 'subscription', - 'credit', - 'debit', - 'bank', + 'subscriptions', ]; const IDEMPOTENCY_FIELDS = [ @@ -42,9 +42,8 @@ export const financialRequiresIdempotencyRule: Rule = { category: 'safety', defaultSeverity: 'warning', check: ({ tool }) => { - const searchText = getOperationSearchText(tool); const isFinancialMutation = - MUTATING_METHODS.has(tool.method) && includesAny(searchText, FINANCIAL_KEYWORDS); + MUTATING_METHODS.has(tool.method) && hasOperationIntentKeyword(tool, FINANCIAL_KEYWORDS); if (!isFinancialMutation || hasAnyInputField(tool, IDEMPOTENCY_FIELDS)) { return []; diff --git a/src/rules/schema/list-requires-pagination.ts b/src/rules/schema/list-requires-pagination.ts index b7023ff..f885a79 100644 --- a/src/rules/schema/list-requires-pagination.ts +++ b/src/rules/schema/list-requires-pagination.ts @@ -1,8 +1,7 @@ import type { Rule } from '@/core/types'; import { hasAnyQueryParameter } from '@/core/schema'; -import { includesAny } from '@/core/strings'; import { createFinding } from '@/rules/findings'; -import { getOperationSearchText } from '@/rules/helpers'; +import { hasOperationIntentKeyword } from '@/rules/helpers'; const LIST_KEYWORDS = [ 'list', @@ -43,7 +42,9 @@ export const listRequiresPaginationRule: Rule = { defaultSeverity: 'warning', check: ({ tool }) => { const isLikelyList = - tool.method === 'GET' && includesAny(getOperationSearchText(tool), LIST_KEYWORDS); + tool.method === 'GET' && + !pathEndsWithParameter(tool.path) && + hasOperationIntentKeyword(tool, LIST_KEYWORDS); if (!isLikelyList || hasAnyQueryParameter(tool, PAGINATION_PARAMETERS)) { return []; @@ -59,3 +60,10 @@ export const listRequiresPaginationRule: Rule = { ]; }, }; + +function pathEndsWithParameter(path: string): boolean { + const segments = path.split('/').filter(Boolean); + const finalSegment = segments.at(-1); + + return finalSegment !== undefined && /^\{[^}]+\}$/.test(finalSegment); +} diff --git a/src/rules/schema/string-should-be-enum.ts b/src/rules/schema/string-should-be-enum.ts index d21ea60..08b1103 100644 --- a/src/rules/schema/string-should-be-enum.ts +++ b/src/rules/schema/string-should-be-enum.ts @@ -1,5 +1,5 @@ import type { Rule } from '@/core/types'; -import { getInputSchemaProperties, hasEnum, isStringSchema } from '@/core/schema'; +import { getInputSchemaProperties, hasEnum, hasSchemaKeyword, isStringSchema } from '@/core/schema'; import { matchesNormalizedName } from '@/core/strings'; import { createFinding } from '@/rules/findings'; @@ -31,6 +31,7 @@ export const stringShouldBeEnumRule: Rule = { (property) => isStringSchema(property.schema) && !hasEnum(property.schema) && + !hasSchemaKeyword(property.schema, ['pattern']) && matchesNormalizedName(property.name, ENUM_LIKE_NAMES), ); diff --git a/tests/core/analyze.test.ts b/tests/core/analyze.test.ts index dfd949c..12602f6 100644 --- a/tests/core/analyze.test.ts +++ b/tests/core/analyze.test.ts @@ -18,15 +18,15 @@ describe('analyzeOpenApi', () => { highRiskTools: 3, findingCounts: { info: 5, - warning: 18, + warning: 16, error: 1, }, }); expect(result.scores).toEqual({ - overall: 13, + overall: 21, safety: 66, schema: 92, - docs: 79, + docs: 87, errors: 80, agentUsability: 96, auth: 100, @@ -38,7 +38,7 @@ describe('analyzeOpenApi', () => { ['createUser', 'medium'], ['deleteUser', 'high'], ]); - expect(result.findings).toHaveLength(24); + expect(result.findings).toHaveLength(22); expect(result.findings[0]).toMatchObject({ ruleId: 'safety/destructive-requires-guard', severity: 'error', diff --git a/tests/generators/__snapshots__/evals.test.ts.snap b/tests/generators/__snapshots__/evals.test.ts.snap index 8f37ad0..05884f7 100644 --- a/tests/generators/__snapshots__/evals.test.ts.snap +++ b/tests/generators/__snapshots__/evals.test.ts.snap @@ -153,16 +153,6 @@ exports[`eval idea generator generates stable advisory eval ideas for the risky "reason": "List/search operation has no pagination or limit query parameter.", "type": "list_enforces_result_limit", }, - { - "expectedBehavior": "Update the description to clearly state what side effects this operation has (e.g., "Creates a new user account" rather than just "Create user").", - "id": "createuser_docs_mutating_description_mentions_side_effects", - "input": {}, - "method": "POST", - "operationId": "createUser", - "path": "/users", - "reason": "Mutating operation description does not mention side effects or state changes.", - "type": "finding_requires_eval", - }, { "expectedBehavior": "The runtime should adapt API failures into a structured error with a generic code, message, and recoverability signal.", "id": "createuser_errors_missing_error_schema", @@ -220,16 +210,6 @@ exports[`eval idea generator generates stable advisory eval ideas for the risky "reason": "Boolean input 'force' is too vague for reliable agent use.", "type": "ambiguous_boolean_requires_review", }, - { - "expectedBehavior": "Update the description to clearly state what side effects this operation has (e.g., "Creates a new user account" rather than just "Create user").", - "id": "deleteuser_docs_mutating_description_mentions_side_effects", - "input": {}, - "method": "DELETE", - "operationId": "deleteUser", - "path": "/users/{id}", - "reason": "Mutating operation description does not mention side effects or state changes.", - "type": "finding_requires_eval", - }, { "expectedBehavior": "The runtime should adapt API failures into a structured error with a generic code, message, and recoverability signal.", "id": "deleteuser_errors_missing_error_schema", @@ -459,17 +439,6 @@ cases: expectedBehavior: The runtime should apply a safe default limit or the API should require explicit pagination parameters. reason: List/search operation has no pagination or limit query parameter. - - id: createuser_docs_mutating_description_mentions_side_effects - type: finding_requires_eval - operationId: createUser - method: POST - path: /users - input: {} - expectedBehavior: Update the description to clearly state what side effects this - operation has (e.g., "Creates a new user account" rather than just "Create - user"). - reason: Mutating operation description does not mention side effects or state - changes. - id: createuser_errors_missing_error_schema type: error_response_is_structured operationId: createUser @@ -515,17 +484,6 @@ cases: expectedBehavior: The eval should verify that the boolean meaning is documented clearly or requires review before autonomous use. reason: Boolean input 'force' is too vague for reliable agent use. - - id: deleteuser_docs_mutating_description_mentions_side_effects - type: finding_requires_eval - operationId: deleteUser - method: DELETE - path: /users/{id} - input: {} - expectedBehavior: Update the description to clearly state what side effects this - operation has (e.g., "Creates a new user account" rather than just "Create - user"). - reason: Mutating operation description does not mention side effects or state - changes. - id: deleteuser_errors_missing_error_schema type: error_response_is_structured operationId: deleteUser diff --git a/tests/generators/__snapshots__/policy.test.ts.snap b/tests/generators/__snapshots__/policy.test.ts.snap index b663001..42703f9 100644 --- a/tests/generators/__snapshots__/policy.test.ts.snap +++ b/tests/generators/__snapshots__/policy.test.ts.snap @@ -39,7 +39,6 @@ exports[`policy generator generates a stable advisory policy draft for the risky "Mutating operation does not expose dry-run, preview, or validate-only mode.", ], "recommendedControls": [ - "Update the description to clearly state what side effects this operation has (e.g., "Creates a new user account" rather than just "Create user").", "Normalize unstructured API errors before returning them to agents.", "Prefer dry-run, preview, or validate-only mode before mutation.", "Validate constrained string inputs against an allowed set.", @@ -61,7 +60,6 @@ exports[`policy generator generates a stable advisory policy draft for the risky "recommendedControls": [ "Require explicit user confirmation before execution.", "Require confirmation or guard metadata for destructive execution.", - "Update the description to clearly state what side effects this operation has (e.g., "Creates a new user account" rather than just "Create user").", "Normalize unstructured API errors before returning them to agents.", "Prefer dry-run, preview, or validate-only mode before mutation.", "Expand the description to clearly explain what the operation does and when to use it.", @@ -194,8 +192,6 @@ operations: - Mutating operation does not expose dry-run, preview, or validate-only mode. recommendedControls: - - Update the description to clearly state what side effects this operation - has (e.g., "Creates a new user account" rather than just "Create user"). - Normalize unstructured API errors before returning them to agents. - Prefer dry-run, preview, or validate-only mode before mutation. - Validate constrained string inputs against an allowed set. @@ -217,8 +213,6 @@ operations: recommendedControls: - Require explicit user confirmation before execution. - Require confirmation or guard metadata for destructive execution. - - Update the description to clearly state what side effects this operation - has (e.g., "Creates a new user account" rather than just "Create user"). - Normalize unstructured API errors before returning them to agents. - Prefer dry-run, preview, or validate-only mode before mutation. - Expand the description to clearly explain what the operation does and diff --git a/tests/reporters/__snapshots__/reporters.test.ts.snap b/tests/reporters/__snapshots__/reporters.test.ts.snap index bfc15ec..774683c 100644 --- a/tests/reporters/__snapshots__/reporters.test.ts.snap +++ b/tests/reporters/__snapshots__/reporters.test.ts.snap @@ -15,15 +15,15 @@ exports[`reporters renders the risky fixture as stable pretty JSON 1`] = ` "highRiskTools": 3, "findingCounts": { "info": 5, - "warning": 18, + "warning": 16, "error": 1 } }, "scores": { - "overall": 13, + "overall": 21, "safety": 66, "schema": 92, - "docs": 79, + "docs": 87, "errors": 80, "agentUsability": 96, "auth": 100 @@ -241,21 +241,6 @@ exports[`reporters renders the risky fixture as stable pretty JSON 1`] = ` "GET operation appears to return a collection" ] }, - { - "ruleId": "docs/mutating-description-mentions-side-effects", - "severity": "warning", - "category": "docs", - "toolId": "createUser", - "toolName": "createUser", - "method": "POST", - "path": "/users", - "message": "Mutating operation description does not mention side effects or state changes.", - "recommendation": "Update the description to clearly state what side effects this operation has (e.g., \\"Creates a new user account\\" rather than just \\"Create user\\").", - "evidence": [ - "Method: POST", - "No description provided" - ] - }, { "ruleId": "errors/missing-error-schema", "severity": "warning", @@ -312,21 +297,6 @@ exports[`reporters renders the risky fixture as stable pretty JSON 1`] = ` "Boolean input: force" ] }, - { - "ruleId": "docs/mutating-description-mentions-side-effects", - "severity": "warning", - "category": "docs", - "toolId": "deleteUser", - "toolName": "deleteUser", - "method": "DELETE", - "path": "/users/{id}", - "message": "Mutating operation description does not mention side effects or state changes.", - "recommendation": "Update the description to clearly state what side effects this operation has (e.g., \\"Creates a new user account\\" rather than just \\"Create user\\").", - "evidence": [ - "Method: DELETE", - "No description provided" - ] - }, { "ruleId": "errors/missing-error-schema", "severity": "warning", @@ -447,17 +417,17 @@ API: Risky Example API 1.0.0 | Destructive operations | 1 | | High-risk operations | 3 | | Errors | 1 | -| Warnings | 18 | +| Warnings | 16 | | Info | 5 | ## Scores | Category | Score | | --- | ---: | -| Overall | 13/100 | +| Overall | 21/100 | | Safety | 66/100 | | Schema | 92/100 | -| Docs | 79/100 | +| Docs | 87/100 | | Errors | 80/100 | | Agent usability | 96/100 | | Auth | 100/100 | @@ -531,11 +501,6 @@ API: Risky Example API 1.0.0 - Recommendation: Expose limit, page, pageSize, cursor, or offset parameters to prevent unbounded agent outputs. - Evidence: GET operation appears to return a collection -- **docs/mutating-description-mentions-side-effects**: \`POST /users\` - - Mutating operation description does not mention side effects or state changes. - - Recommendation: Update the description to clearly state what side effects this operation has (e.g., "Creates a new user account" rather than just "Create user"). - - Evidence: Method: POST; No description provided - - **errors/missing-error-schema**: \`POST /users\` - Operation does not define a structured 4xx or 5xx error response schema. - Recommendation: Define structured error responses with stable error codes and messages so agents can recover safely. @@ -556,11 +521,6 @@ API: Risky Example API 1.0.0 - Recommendation: Rename vague booleans to describe the exact behavior they enable, or replace them with a constrained enum. - Evidence: Boolean input: force -- **docs/mutating-description-mentions-side-effects**: \`DELETE /users/{id}\` - - Mutating operation description does not mention side effects or state changes. - - Recommendation: Update the description to clearly state what side effects this operation has (e.g., "Creates a new user account" rather than just "Create user"). - - Evidence: Method: DELETE; No description provided - - **errors/missing-error-schema**: \`DELETE /users/{id}\` - Operation does not define a structured 4xx or 5xx error response schema. - Recommendation: Define structured error responses with stable error codes and messages so agents can recover safely. @@ -1130,35 +1090,6 @@ exports[`reporters SARIF output matches snapshot 1`] = ` "path": "/users" } }, - { - "ruleId": "docs/mutating-description-mentions-side-effects", - "ruleIndex": 13, - "level": "warning", - "message": { - "text": "Mutating operation description does not mention side effects or state changes.\\n\\nRecommendation: Update the description to clearly state what side effects this operation has (e.g., \\"Creates a new user account\\" rather than just \\"Create user\\").\\n\\nEvidence:\\n - Method: POST\\n - No description provided" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "examples/risky-openapi.yaml" - }, - "region": { - "snippet": { - "text": "POST /users" - } - } - } - } - ], - "properties": { - "category": "docs", - "toolId": "createUser", - "toolName": "createUser", - "method": "POST", - "path": "/users" - } - }, { "ruleId": "errors/missing-error-schema", "ruleIndex": 14, @@ -1275,35 +1206,6 @@ exports[`reporters SARIF output matches snapshot 1`] = ` "path": "/users" } }, - { - "ruleId": "docs/mutating-description-mentions-side-effects", - "ruleIndex": 13, - "level": "warning", - "message": { - "text": "Mutating operation description does not mention side effects or state changes.\\n\\nRecommendation: Update the description to clearly state what side effects this operation has (e.g., \\"Creates a new user account\\" rather than just \\"Create user\\").\\n\\nEvidence:\\n - Method: DELETE\\n - No description provided" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "examples/risky-openapi.yaml" - }, - "region": { - "snippet": { - "text": "DELETE /users/{id}" - } - } - } - } - ], - "properties": { - "category": "docs", - "toolId": "deleteUser", - "toolName": "deleteUser", - "method": "DELETE", - "path": "/users/{id}" - } - }, { "ruleId": "errors/missing-error-schema", "ruleIndex": 14, diff --git a/tests/rules/rules.test.ts b/tests/rules/rules.test.ts index cfd0135..2805f42 100644 --- a/tests/rules/rules.test.ts +++ b/tests/rules/rules.test.ts @@ -39,12 +39,10 @@ describe('default rule engine', () => { 'safety/mutating-requires-dry-run', 'errors/missing-error-schema', 'schema/list-requires-pagination', - 'docs/mutating-description-mentions-side-effects', 'errors/missing-error-schema', 'safety/mutating-requires-dry-run', 'schema/string-should-be-enum', 'schema/vague-boolean', - 'docs/mutating-description-mentions-side-effects', 'errors/missing-error-schema', 'safety/mutating-requires-dry-run', 'docs/weak-description', @@ -94,6 +92,54 @@ describe('safety/financial-requires-idempotency', () => { expect(findings).toHaveLength(0); }); + + test('does not flag bank metadata updates as financial movement', () => { + const tool = makeTool({ + method: 'PATCH', + path: '/accounts/{id}/bank-name', + name: 'updateAccountBankName', + summary: 'Update account bank name', + }); + + const findings = financialRequiresIdempotencyRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('does not flag read-only loyalty credit endpoints', () => { + const tool = makeTool({ + method: 'GET', + path: '/loyalty/credits', + name: 'listLoyaltyCredits', + summary: 'List loyalty credits', + }); + + const findings = financialRequiresIdempotencyRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('flags refund operations without idempotency inputs', () => { + const tool = makeTool({ + method: 'POST', + path: '/payments/{id}/refund', + name: 'refundPayment', + summary: 'Refund payment', + }); + + const findings = financialRequiresIdempotencyRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(1); + }); }); describe('safety/external-communication-requires-guard', () => { @@ -132,6 +178,66 @@ describe('safety/external-communication-requires-guard', () => { expect(findings).toHaveLength(0); }); + + test('does not flag internal message stores without recipient fields', () => { + const tool = makeTool({ + method: 'POST', + path: '/chat/messages', + name: 'createChatMessage', + summary: 'Create chat message', + requestBodySchema: { + type: 'object', + properties: { + body: { type: 'string' }, + }, + }, + }); + + const findings = externalCommunicationRequiresGuardRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('does not flag CMS publish operations without recipient fields', () => { + const tool = makeTool({ + method: 'POST', + path: '/articles/{id}/publish', + name: 'publishArticle', + summary: 'Publish article', + }); + + const findings = externalCommunicationRequiresGuardRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('flags external notifications with recipient fields', () => { + const tool = makeTool({ + method: 'POST', + path: '/notifications/email', + name: 'sendNotificationEmail', + summary: 'Send notification email', + requestBodySchema: { + type: 'object', + properties: { + recipientEmail: { type: 'string' }, + }, + }, + }); + + const findings = externalCommunicationRequiresGuardRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(1); + }); }); describe('safety/destructive-requires-guard', () => { @@ -170,6 +276,59 @@ describe('safety/destructive-requires-guard', () => { expect(findings).toHaveLength(0); }); + + test('does not flag destructive-sounding read-only operations', () => { + const tool = makeTool({ + method: 'GET', + path: '/subscriptions/{id}/cancellation-history', + name: 'getCancellationHistory', + summary: 'Get cancellation history', + }); + + const findings = destructiveRequiresGuardRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('does not flag description-only destructive words', () => { + const tool = makeTool({ + method: 'POST', + path: '/subscriptions/{id}/resume', + name: 'resumeSubscription', + summary: 'Resume subscription', + description: 'Resumes a subscription that was previously cancelled.', + }); + + const findings = destructiveRequiresGuardRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('does not flag destructive operations with guard metadata', () => { + const tool = makeTool({ + method: 'POST', + path: '/users/{id}/deactivate', + name: 'deactivateUser', + rawOperation: { + 'x-agent-guard': { + mode: 'require_confirmation', + }, + }, + }); + + const findings = destructiveRequiresGuardRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); }); describe('safety/mutating-requires-dry-run', () => { @@ -252,6 +411,38 @@ describe('schema/list-requires-pagination', () => { expect(findings).toHaveLength(0); }); + + test('does not flag single-resource GET paths ending in parameters', () => { + const tool = makeTool({ + method: 'GET', + path: '/users/{id}', + name: 'getUser', + summary: 'Get user', + }); + + const findings = listRequiresPaginationRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + + test('does not flag nested single-resource GET paths ending in parameters', () => { + const tool = makeTool({ + method: 'GET', + path: '/users/{id}/events/{eventId}', + name: 'getUserEvent', + summary: 'Get user event', + }); + + const findings = listRequiresPaginationRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); }); describe('schema/vague-boolean', () => { @@ -349,6 +540,32 @@ describe('schema/string-should-be-enum', () => { expect(findings).toHaveLength(0); }); + + test('does not flag pattern-constrained strings', () => { + const tool = makeTool({ + method: 'GET', + path: '/users', + name: 'listUsers', + parameters: [ + { + name: 'sort', + in: 'query', + required: false, + schema: { + type: 'string', + pattern: '^-?[a-zA-Z]+(,-?[a-zA-Z]+)*$', + }, + }, + ], + }); + + const findings = stringShouldBeEnumRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); }); describe('schema/sensitive-response-fields', () => { @@ -573,6 +790,22 @@ describe('docs/mutating-description-mentions-side-effects', () => { expect(findings).toHaveLength(0); }); + test('does not flag mutating operations with bare side-effect verbs', () => { + const tool = makeTool({ + method: 'POST', + path: '/users', + name: 'createUser', + description: 'Create user', + }); + + const findings = mutatingDescriptionMentionsSideEffectsRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); + test('does not flag read-only operations', () => { const tool = makeTool({ method: 'GET', @@ -597,6 +830,15 @@ describe('safety/batch-operation-requires-limit', () => { path: '/users/batch', name: 'batchCreateUsers', summary: 'Batch create users', + requestBodySchema: { + type: 'object', + properties: { + users: { + type: 'array', + items: { type: 'object' }, + }, + }, + }, }); const findings = batchOperationRequiresLimitRule.check({ @@ -645,6 +887,28 @@ describe('safety/batch-operation-requires-limit', () => { expect(findings).toHaveLength(0); }); + + test('does not flag batch-sounding scalar operations', () => { + const tool = makeTool({ + method: 'POST', + path: '/jobs/bulk-status-check', + name: 'bulkStatusCheck', + summary: 'Bulk status check', + requestBodySchema: { + type: 'object', + properties: { + jobId: { type: 'string' }, + }, + }, + }); + + const findings = batchOperationRequiresLimitRule.check({ + tool, + allTools: [tool], + }); + + expect(findings).toHaveLength(0); + }); }); describe('auth/dangerous-auth-scope', () => {