Skip to content

bug(input validation): invalid filters silently widen or empty the query #39

Description

@cyanheads

Server version

0.2.3

mcp-ts-core version

^0.11.0

Runtime

Bun

Runtime version

1.3.14

Transport

HTTP (Streamable HTTP)

Description

Explicitly invalid filter values are accepted as successful queries. A blank stateRoute / route is trimmed to undefined and the call silently widens to an unfiltered statewide query; an unsupported region value, a non-positive or fractional terminal ID, and a reversed milepost range all return an empty success. In every case the caller cannot distinguish "your value was rejected" from "there is genuinely nothing here" — and in the blank case the response reports success on a query it never ran.

An omitted optional filter must keep its current meaning (no filter). The defect is confined to values the caller supplied explicitly.

Steps to reproduce

{"name":"wsdot_search_alerts","arguments":{"stateRoute":"","limit":1}}
{"name":"wsdot_search_cameras","arguments":{"stateRoute":"","limit":1}}
{"name":"wsdot_get_travel_times","arguments":{"route":"","limit":1}}
{"name":"wsdot_search_cameras","arguments":{"region":"East"}}
{"name":"wsdot_search_alerts","arguments":{"region":"NW"}}
{"name":"wsdot_get_terminal_space","arguments":{"departingTerminalId":0}}
{"name":"wsdot_get_ferry_schedule","arguments":{"departingTerminalId":7.5,"arrivingTerminalId":3}}
{"name":"wsdot_search_alerts","arguments":{"startMilepost":100,"endMilepost":10}}
{"name":"wsdot_search_cameras","arguments":{"startMilepost":100,"endMilepost":10}}

Actual behavior

Blank filters widen the query. input.stateRoute?.trim() || undefined converts an explicit empty string to omission, and appliedFilters then echoes {} — so the response asserts that no filter was requested. Current unfiltered totals reached this way: 175 alerts, 1,700 cameras, 163 travel-time corridors.

Unsupported region values are accepted. region is a bare z.string() on both search tools, and the two tools use different vocabularies — wsdot_search_alerts matches WSDOT region names and wsdot_search_cameras matches region codes. Passing one tool's vocabulary to the other returns an empty success: region: "East" on cameras, region: "NW" on alerts. The live feeds carry exactly the sets the field descriptions already document — alerts: Eastern, North Central, Northwest, Olympic, South Central, Southwest; cameras: ER, NC, NW, OL, OS, SC, SW, WA.

Terminal IDs are unconstrained numbers. wsdot_get_terminal_space.departingTerminalId and both wsdot_get_ferry_schedule terminal fields are plain z.number(). departingTerminalId: 0 and -3 return an empty success whose notice suggests verifying IDs; 7.5 is interpolated straight into the upstream path, and the resulting upstream rejection is reported as invalid_terminal_pair rather than as bad input.

Reversed milepost ranges are evaluated rather than rejected, and the two tools do different things with them:

Tool Handling of startMilepost > endMilepost
wsdot_search_alerts Tested as extent overlap against an inverted interval, so it keeps only alerts whose extent covers both bounds — plus every alert that reports no milepost at all. Result is data-dependent: an arbitrary non-empty set is as possible as an empty one.
wsdot_search_cameras The bounds are applied as two independent filters (>= start, <= end), so no camera that reports a milepost can satisfy both.

Both currently return zero rows with the generic "try removing the filters" notice, because every alert and every camera in the live feeds reports a milepost.

Expected behavior

  • Reject an explicitly blank stateRoute, route, and region at the schema boundary instead of converting it to omission. An absent field keeps its current no-filter meaning.
  • Constrain region to a closed set per tool — the alert names and the camera codes are separate vocabularies and must not share one validator.
  • Require a positive integer terminal ID on wsdot_get_terminal_space and wsdot_get_ferry_schedule.
  • Refine both milepost schemas so a supplied pair satisfies startMilepost <= endMilepost; a single bound alone stays valid.
  • Route every rejection through the same schema/tool-boundary validation path that already rejects limit: 501, so the caller gets a structured input error rather than a successful empty page.

Additional context

limit: 501 is already rejected with an accessible validation error (-32602, Too big: expected number to be <=500 at limit); these domain constraints should fail the same way.

Two existing tests assert the behavior this issue reverses and must be updated rather than supplemented: tests/tools/traffic-tools.test.tsstrips whitespace-only stateRoute filter (once for searchAlerts, once for searchCameras). A whitespace-only value is an explicit value and should be rejected on the same terms as "".

Tradeoff on the closed region sets: an enum rejects a region WSDOT might add later. WSDOT regions are administrative divisions that change on a scale of decades, and the alternative — validating against values observed in the fetched feed — cannot report the rejection before the request is made. The enum is preferred; both field descriptions already publish the closed sets as fact.

#44 and #45 both specify blank-value rejection for new filters and inherit whichever policy lands here. #46 is the output-side counterpart — an empty string reaching a response surface rather than arriving as input — and the two should settle on one coherent stance for the empty string in each direction.

Touchpoints

A floor, not a ceiling — re-verify at implementation time.

  • src/mcp-server/tools/definitions/search-alerts.tool.tsinput (stateRoute, region, startMilepost, endMilepost); the handler's ?.trim() || undefined normalization and the appliedFilters echo built from it
  • src/mcp-server/tools/definitions/search-cameras.tool.ts — same fields, same normalization, same echo
  • src/mcp-server/tools/definitions/get-travel-times.tool.tsinput.route and the routeFilter enrichment
  • src/mcp-server/tools/definitions/get-terminal-space.tool.tsinput.departingTerminalId
  • src/mcp-server/tools/definitions/get-ferry-schedule.tool.tsinput.departingTerminalId / input.arrivingTerminalId; a rejected ID must not reach the invalid_terminal_pair path
  • src/services/traffic/traffic-service.tsAlertSearchParams / CameraSearchParams and the milepost filter bodies in searchAlerts / searchCameras; the no-milepost passthrough is deliberate and stays
  • tests/tools/traffic-tools.test.ts — update the two whitespace-stripping tests; add rejection cases per field and negative cases proving an omitted filter still returns the unfiltered set
  • tests/tools/ferry-tools.test.ts — terminal-ID rejection cases for both ferry tools
  • Schema descriptions on every field changed, plus the tool descriptions that advertise the accepted forms
  • README.md — the region lists under wsdot_search_alerts and wsdot_search_cameras
  • docs/design.md — the per-tool Input lines for alerts, travel times, cameras, and terminal space
  • No packaging surface: server.json and manifest.json track environment variables only

Acceptance criteria

  • Each of the nine reproduction calls returns a structured input-validation error naming the offending field, not a successful page.
  • wsdot_search_alerts with region: "Northwest" and wsdot_search_cameras with region: "NW" still succeed; each tool rejects the other's vocabulary.
  • Omitting stateRoute / route / region / both mileposts still returns the full unfiltered set with appliedFilters: {}.
  • A single milepost bound supplied alone is still accepted, and the alert extent-overlap semantics for a correctly ordered range are unchanged.
  • wsdot_get_terminal_space with a valid terminal ID and wsdot_get_ferry_schedule with a valid pair are unaffected.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions