Skip to content

fix(ea1): reject footnote-legend '*', bound pre-colon gap, detect YAML block-list and JSON wildcard grants - #447

Open
yashrajp22 wants to merge 3 commits into
mainfrom
yashrajp22/ea1-wildcard-remaining-gaps
Open

fix(ea1): reject footnote-legend '*', bound pre-colon gap, detect YAML block-list and JSON wildcard grants#447
yashrajp22 wants to merge 3 commits into
mainfrom
yashrajp22/ea1-wildcard-remaining-gaps

Conversation

@yashrajp22

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #405 / #417 (and duplicate #438). #417 correctly bounded the EA1 wildcard-grant regex to a single line and a standalone *, but an execution-verified edge-case audit of the merged pattern found two remaining false-positive paths and three detection gaps. This PR fixes all five.

Fixes #444. Fixes #445. (Systemic paragraph-crossing across all 15 pattern files is tracked separately in #446 — out of scope here.)

Cases and how each is solved

False positives removed (#444)

Case 1 — footnote/legend text after a Tools: label.

Tools: * = requires authentication
Tools: * marks optional parameters

The (?!\*|\w) lookahead rejects ** and *word, but a standalone * followed by a space passed — both lines fired EA1/MEDIUM on benign docs.
Solution: a bare (unquoted) * now only counts when it ends the line, optionally closed by ] and/or a # comment. A real bare-scalar grant (tools: *, tools: [*], tools: * # allow all) has nothing else after the value; a footnote legend always does. Quoted forms ("*", '*') are unambiguous and keep matching anywhere on the line.

Case 2 — blank-line gap before the colon.

several tools

: * item

#417 bounded the whitespace after the colon, but \s*: before it still crossed newlines, so a markdown definition-list line two paragraphs later still bridged (matched text tools\n\n: * ).
Solution: [ \t]*: — the key and colon must share a line. No real YAML/JSON/TOML syntax breaks a line between key and colon.

Detection gaps closed (#445)

Case 3 — JSON quoted keys never matched.

"tools": ["*"]
"permissions": "*"

The old prefix required the colon directly after the key word, so the closing quote of a JSON key broke the match — despite JSON being the most common encoding for MCP/agent configs.
Solution: the key may be wrapped in optional quotes: ['\"]?(?:tools?|permissions?)['\"]?.

Case 4 — the idiomatic YAML block-list form never matched.

tools:
  - "*"

The block-sequence dash was never part of the pattern.
Solution: a second pattern matches a key followed by exactly one newline and a first list item that is a standalone wildcard. A blank line still breaks the match (the #405 cross-heading bridge cannot return), and the standalone-* lookahead keeps markdown lists (- **Read**, - *note*) out. Later items are intentionally out of scope until seen in practice — matching arbitrary positions across lines widens the false-positive surface.

Case 5 — wildcard not in first position of an inline list.

tools: ["search", "*"]

The old pattern required * immediately after [.
Solution: a quoted * anywhere inside same-line brackets now matches (\[[^\]\r\n]*['\"]\*['\"]). Quoted-only on purpose: markdown link/bold text inside [...] cannot satisfy it, and an unquoted * mid-list in YAML is an alias, not a wildcard.

Validation

Risk

  • Confidence stays 0.85 for both patterns; severity mapping unchanged.
  • Known accepted non-detections (unchanged from current main): multi-line flow lists (tools: [\n "*"\n]) and wildcards in later block-list items.

@yashrajp22
yashrajp22 requested a review from rng1995 August 27, 2026 11:02

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Requesting changes: the new same-line list branch introduces an EA1 false positive when an explicit tool object has a nested argument value of "*"; the wildcard must be a top-level tool-list element. The focused suite passes, but this head has no hosted checks, and the feature commit also lacks the required DCO sign-off. Please add the nested-object regression, narrow the match, and rerun signed CI.

# stays on one line so the match can never bridge paragraphs (#405, #444).
(
r"['\"]?(?:tools?|permissions?)['\"]?[ \t]*:[ \t]*"
r"(?:\[[^\]\r\n]*['\"]\*['\"]"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Restrict the wildcard to a top-level tool-list element. The unrestricted [^\]\r\n]* scan treats any nested quoted star as the wildcard tool itself: valid JSON such as "tools": [{"name": "search", "arguments": {"glob": "*"}}] now emits EA1 even though the tool list explicitly contains only search. Match scalar list elements without descending into nested objects/arrays, and add this JSON case as a negative regression.

…ck-list and JSON wildcard grants

Fixes #444. Fixes #445. Follow-up to #405/#417.

Remaining false positives (#444):
- A bare '*' now counts only when it ends the line (optionally ']' and/or
  a '#' comment), so footnote legends like 'Tools: * = requires auth' no
  longer fire. Quoted '*' stays unambiguous as a scalar or top-level list
  element.
- The gap before the colon is bounded to the same line ([ \t]*:), so a
  blank line followed by a markdown definition-list ': *' can no longer
  bridge paragraphs.

Detection gaps (#445):
- The key may be quoted, catching JSON forms: "tools": ["*"] and
  "permissions": "*".
- New block-list branch catches the idiomatic YAML form (tools: newline
  '- "*"'), bounded to a single newline with a standalone-star item so
  markdown lists of bold/italic names cannot collide.
- A quoted '*' as a later top-level element of a same-line list now
  matches (tools: ["search", "*"]). The list branch excludes braces so
  a '*' nested inside an explicit tool object
  (tools: [{name: grep, pattern: "*"}]) is treated as an argument value
  for a named tool, not a wildcard grant.

Validated against a 49-case matrix (20 genuine grant forms, 11 new
detections, 18 false-positive classes); full suite 2966 passed, ruff
check/format clean.

Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
@yashrajp22
yashrajp22 force-pushed the yashrajp22/ea1-wildcard-remaining-gaps branch from 6a2a7d4 to a6b1d91 Compare August 31, 2026 15:52
@yashrajp22

Copy link
Copy Markdown
Collaborator Author

All three points addressed in the rewritten head (a6b1d91):

1. Nested-object false positive — fixed and regression-tested. The same-line list branch now excludes {/} (\[[^\]{}\r\n]*['\"]\*['\"]), so the match can never cross into an explicit tool object: a quoted * only counts as a top-level list element. Verified:

  • tools: [{"name": "grep", "pattern": "*"}] → no longer flagged (new test)
  • tools: [{name: search, glob: "*"}] → no longer flagged (new test)
  • tools: ["search", "*"] → still flagged (regression guard)

One documented non-detection this implies: a top-level "*" after an object element (tools: [{...}, "*"]) is not matched — crossing braces correctly would need balanced-brace matching, which a regex can't do reliably, and the FP cost of skipping braces outweighs that rare form.

2. DCO — the branch was rewritten with Signed-off-by on the (single) commit.

3. CI — the branch is rebased onto current main (d3a849a) and force-pushed, so hosted checks run against a signed, up-to-date head. Local run on the rebased head: 3,969 passed / 0 failed (-m "not integration and not provider"), ruff check and ruff format --check clean.

@yashrajp22
yashrajp22 requested a review from rng1995 August 31, 2026 15:53

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Re-review: the rewritten head is signed, rebased directly onto current main, and all hosted checks are green. It fixes the reported nested-object case, and the focused and relevant static-pattern suites pass. However, the same list regex still descends into nested arrays because [ remains allowed in its scan: tools: ["search", ["grep", "*"]] still emits EA1/MEDIUM even though * is not a top-level tool-list element. Please prevent traversal into nested arrays and add this negative regression; the existing review thread remains open.

github-actions Bot and others added 2 commits August 31, 2026 16:49
The same-line list branch excluded '{'/'}' but still allowed '[', so a
quoted '*' inside a nested array (tools: ["search", ["grep", "*"]])
was treated as a top-level tool-list element. Exclude '[' from the scan
as well and document the quoted-delimiter ceiling. Adds the nested-array
and nested-arguments-object negative regressions from review.

Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
@yashrajp22

Copy link
Copy Markdown
Collaborator Author

Fixed in d44a3f4 (signed, stacked on the updated branch — no history rewrite this time):

  • The list scan now excludes [ as well as {/} (\[[^\][{}\r\n]*['\"]\*['\"]), so it cannot descend into nested arrays or objects — a quoted * only matches as a top-level tool-list element.
  • Negative regressions added for both reported shapes:
    • tools: ["search", ["grep", "*"]] → not flagged
    • "tools": [{"name": "search", "arguments": {"glob": "*"}}] → not flagged
  • Guard kept: tools: ["search", "*"] → still flagged.
  • Documented ceiling in the code comment: a quoted string element containing one of those delimiters ahead of a genuine top-level "*" (e.g. tools: ["search {x}", "*"]) also stops the scan — walking quoted strings correctly needs a parser, not a pattern, and the miss is strictly safer than the traversal FPs.

Local on the rebased head: 3,995 passed / 0 failed (-m "not integration and not provider"), ruff check + format --check clean.

@yashrajp22
yashrajp22 requested a review from rng1995 August 31, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants