Skip to content

v0.3.0 — SARIF output, inline suppression, and five false-positive fixes

Choose a tag to compare

@dheerajjha dheerajjha released this 08 Aug 10:21
· 21 commits to main since this release

Added

  • SARIF 2.1.0 output: check --format sarif. Findings land in GitHub
    code scanning as annotations on the diff and entries in the Security tab,
    tracked across commits instead of re-read from a CI log every time.
    (#182)

    --format {text,json,sarif} is the new surface; --json keeps working as
    an alias for --format json and its output is byte-identical. The format
    decides what is printed and never what is returned — exit codes are
    unchanged across all three.

    Two decisions worth stating rather than burying:

    • deprecated maps to warning, not error. Code scanning's default
      gate fails on error alone, so this mapping decides whether a
      deprecation blocks a merge. The spec gives deprecated features 12+
      months; making a server unmergeable today over something that breaks
      next year gets the integration switched off, and then it catches
      nothing. breakingerror and advisorynote.
    • The driver declares all 21 rules, not only those that fired. That is
      what lets code scanning tell "this rule ran and found nothing" from
      "this rule does not exist", and therefore close a resolved alert instead
      of leaving it open forever. A clean tree still emits a full run with
      results: [].

    Paths are repo-relative and forward-slashed on every platform, because
    code scanning matches results to the diff by path and an absolute path
    from the scanning machine matches nothing — the annotations would silently
    never appear. Project-level findings (R010 asks about the whole tree) are
    kept with an empty locations array rather than dropped.

    Validated in CI against a vendored copy of the official schema at
    schemas/sarif-2.1.0.schema.json; it is checked in rather than fetched so
    the suite does not depend on the network across four Python versions.

  • Inline suppression: # mcp-migrate: ignore[R001] -- reason. Silence a
    single wrong finding without switching a rule off for the whole project.
    Both comment syntaxes, so it reads naturally in Python and TypeScript.
    (#180)

    A suppressed finding does not count against the grade. A suppression that
    still costs you the grade is not a suppression — the only move left would
    be to stop running the tool. That makes the grade partly self-reported, so
    it is auditable by construction rather than by convention:

    • the rule id is required; a blanket ignore would also silence rules that
      do not exist yet, and nobody revisits it
    • a reason is expected, and directives without one are reported
    • the suppression count prints on every run, never behind a flag
    • --show-suppressions lists each one with its file, rule and reason
    • directives that matched nothing are reported, so stale ones cannot
      quietly accumulate

    Malformed directives are reported rather than dropped: someone wrote it
    believing it worked, and the finding it was meant to silence is about to
    appear anyway.

    R005, R015 and R016 report at most one finding per file, so
    suppressing their line silences those rules for that whole file. Documented
    in the README; every other rule is genuinely per-line.

  • A registry entry records what its grade was computed from.
    mcp-migrate entry emits an optional suppressed: count when the scan
    had any, registry/schema.yaml accepts it, validate_registry.py
    enforces a non-negative int, and the board marks those rows.
    (#220, @soltonigiri)

    Suppression deliberately does not cost the grade, and that decision was
    fine while the number stayed in a terminal. It stopped being fine the
    moment the same grade could be submitted to a public board with nothing
    anywhere recording that findings were silenced — the board's whole claim
    is that its grades are reproducible, and a grade you cannot reproduce
    without knowing what was suppressed does not meet that bar.

    Optional and omitted when zero, so the sixteen existing entries and the
    rendered board are byte-identical. Whether the badge should change is
    a separate and genuinely contested question, still open in
    discussion #210.

Changed

  • check --json gained two required keys, suppressed and
    unused_suppressions.
    Both are always present, empty array included. A
    consumer validating against the 0.2.0 schema will reject 0.3.0 output until
    it is updated; schemas/check-json.schema.json is the executable contract.
    Findings that were suppressed are absent from findings and from counts,
    so a consumer that sums counts still gets a total matching findings.

    unused_suppressions carries one entry per rule id (rule, path, line,
    reason). It is in the JSON and not only on the console because stale
    suppressions accumulate in CI, and CI is exactly the consumer that reads
    --json and never sees a console line.

Fixed

  • Fixers no longer edit inside string literals. A new
    string_lines() helper in fixers/_textedit.py marks every line that is
    part of a Python docstring or a TypeScript template literal, and the
    line-based fixers decline to touch them.
    (#105, @IronLad123)

    Two fixers were editing prose. ResourceNotFoundErrorCodeFixer rewrote
    -32002-32602 inside a docstring describing historical behaviour —
    at safe confidence, leaving the file parseable and the documentation
    wrong, so nothing caught it. TasksPollingFixer inserted
    # TODO(mcp-migrate): ... into a string literal, where # opens no
    comment and the line is simply corrupted. On failure the helper returns
    every line, so an unparseable file makes fixers decline rather than guess.

  • R018 and R019 now match the SDK names real code uses. They keyed on
    list_roots, create_message, ListTasksRequest and friends, missing the
    Params/Schema/Result variants that appear in actual servers — a file
    importing ListRootsRequestSchema or GetTaskPayloadResultSchema scanned
    clean for both rules.
    (#99, @IronLad123)

    Known consequence: R007 and R018 now both fire on the CreateMessage*
    family, two findings for one symbol at two severities. Both claims are
    true, so neither rule is wrong; the noise is tracked in
    #221.

  • Wire method names are bounded at their end. A new wire_method()
    helper builds \b<name>(?![\w/-]), so roots/listeners,
    notifications/initializedAt, logging/setLevelPolicy and
    tasks/listeners stop reading as the methods whose names they begin with.
    Five false positives on a four-line file, gone.
    (#88, @IronLad123)

    Applied at each pattern's definition rather than its call site, because
    WIRE_RX in R009 and R019 and the logging/setLevel literal in R012 each
    feed both the Python and the TypeScript path — bounding only the Python
    call sites left the same file still producing false positives when scanned
    as TypeScript.

  • Two superseded rule modules removed, and a guard so it cannot recur.
    r001_session_id.py and r006_sse_transport.py were the 0.1.0
    originals, superseded and never deleted. Both rule ids were declared
    twice, and which implementation went live was decided by filename sort
    order inside all_rules() rather than by intent.
    (#219, @ankitverma31)

    Nothing was wrong at runtime — the maintained implementations were the
    live ones and the rule count was 21 either way. But rename the surviving
    module to anything that sorts earlier and the tool silently reverts to a
    superseded rule, with the count unchanged and every test still green.
    test_rule_hygiene.py now reads the declarations statically — not
    through all_rules(), whose import is where the dedup happens — and
    fails if a rule id is declared twice, or if a rule declared on disk does
    not survive into all_rules().

    The README rule table linked R001 and R006 to the dead modules, so
    anyone following those links was reading a superseded implementation.
    Caught by test_docs.py the moment the files were removed.

  • R004 no longer fires on a wire method name that returns no tools.
    Naming tools/list is not handling it, and the rule could not tell the
    difference. Three shapes are now excluded: a per-method config map key
    ({"tools/list": {"ttl_ms": ...}}), a lookup back into that map
    (CACHE_POLICY["tools/list"]), and the method field of an outbound
    request (send({"jsonrpc": "2.0", "method": "tools/list"}) — only
    requests carry method, so that is always the client side of the wire).
    Both languages.
    (#218)

    Found by reading dealfluence/adeu rather than by trusting our own
    output: 4 of the 5 R004 findings we reported against it were wrong,
    and the config map exists there because they implement this revision's
    cache metadata — so the rule was penalising them for correctly
    implementing another part of the same spec. Their grade goes D (44) →
    C (72)
    together with #217.

    Handler shapes all still fire, including the one object-literal key whose
    value opens the handler body on the same line
    ({"tools/list": async () => ...}), where the sort look-ahead has
    something real to scan. A key whose value is a bare identifier is skipped
    and the miss accepted: that body lives in another function, so the
    look-ahead was never going to find its sort there anyway. R004 is
    advisory — three wrong points on a stranger's grade cost more than three
    missed ones.

    R015 and R016 were checked for the same trigger and are not affected;
    both anchor on a call-site shape (setRequestHandler("tools/list", ...))
    rather than a bare literal.


Install: pip install mcp-migrate==0.3.0

Full changelog: https://github.com/dheerajjha/mcp-migrate/blob/main/CHANGELOG.md