[grade=B] fix(DC-162 rev3): presence-based id detection, null-params rejection, boundary matrix
Judge r2 (grade C) — all 3 blockers + both polish items:
1. REQUEST-vs-NOTIFICATION by PRESENCE, never truthiness: validateJsonRpc
uses hasOwnProperty('id'); id 0 and id '' are VALID request ids echoed
back BY VALUE (strict-equality tested), id:null treated as a request
under the documented error-id convention. mcp-core default branch fixed
the same way (`if (id)` → hasOwnProperty — id 0 / '' unknown-method now
answer -32601 with the id echoed instead of being swallowed as falsy).
Routes' malformed-notification silence path also switched to
presence-based detection (id:null + bad params → 400 envelope, not 204).
2. params:null REJECTED (-32600): contract is object-or-omitted ONLY
(strict JSON-RPC; the old code's m.params !== null guard contradicted
the documented contract). Boundary tests pin the policy.
3. Unknown-method boundary tests: id 0 → -32601 id:0, id '' → -32601 id:'',
id null → -32601 id:null — asserted at BOTH layer levels (route).
Polish:
- 16-case TABLE-DRIVEN boundary matrix covering every allowed/rejected
id/params combination incl. 202 vs 204 notification split.
- Verifier now probes writable /tmp up front (exit 73 + clear message if
unavailable; documented requirement) instead of dying at mktemp.
- Verifier target count 57 → 73 (DC-162 suites) with the boundary table
named in the assertion label.
Full DC-162 suite: 73 passed (route 50 + core 23). No transport/credential
semantics changed from rev2.
urn:ump:or2qqhcytp6lfzlyi7xru7hdqy3uar2hbjllsrrbxrbray3c5kpa
This commit is contained in:
3 files changed
+74
-19
No files matched your search
+33
-17
@@ -73,26 +73,41 @@ const SERVER_VERSION = (() => {
|
||||
}
|
||||
})();
|
||||
|
||||
/** JSON-RPC 2.0 structural validation. {error} or {valid, notification}. */
|
||||
/** JSON-RPC 2.0 structural validation. {error} or {valid, notification}.
|
||||
*
|
||||
* ID DETECTION POLICY (judge r2): presence-based, never truthiness —
|
||||
* - id ABSENT → notification (no response body expected)
|
||||
* - id present with value → request; 0 and '' are VALID ids (returned
|
||||
* back on the response with strict equality)
|
||||
* - id present as null → request per the JSON-RPC error-id
|
||||
* convention; we answer with a response whose
|
||||
* id is null. (Documented choice; not a
|
||||
* notification, not a 400 — null is a legal
|
||||
* member of the id union.)
|
||||
* PARAMS CONTRACT: object-or-omitted ONLY. params:null is REJECTED
|
||||
* (-32600) — strict JSON-RPC (spec: "params: This member MAY be omitted").
|
||||
*/
|
||||
function validateJsonRpc(m) {
|
||||
if (m === null || typeof m !== 'object' || Array.isArray(m)) {
|
||||
return { error: { code: -32600, message: 'Invalid Request: JSON-RPC object required' } };
|
||||
}
|
||||
const id = (typeof m.id === 'string' || typeof m.id === 'number') ? m.id : null;
|
||||
if (m.jsonrpc !== '2.0') {
|
||||
return { error: { code: -32600, message: "Invalid Request: jsonrpc must be '2.0'" }, id };
|
||||
}
|
||||
if (typeof m.method !== 'string' || m.method === '') {
|
||||
return { error: { code: -32600, message: 'Invalid Request: method must be a non-empty string' }, id };
|
||||
}
|
||||
if (m.params !== undefined && m.params !== null &&
|
||||
(typeof m.params !== 'object' || Array.isArray(m.params))) {
|
||||
return { error: { code: -32600, message: 'Invalid Request: params must be an object when present' }, id };
|
||||
}
|
||||
if (m.id !== undefined && m.id !== null && typeof m.id !== 'string' && typeof m.id !== 'number') {
|
||||
// echoable id: legal types pass through by VALUE (including 0 / '' / null)
|
||||
const hasId = Object.prototype.hasOwnProperty.call(m, 'id');
|
||||
const id = hasId ? m.id : undefined;
|
||||
if (hasId && id !== null && typeof id !== 'string' && typeof id !== 'number') {
|
||||
return { error: { code: -32600, message: 'Invalid Request: id must be string, number, or null' }, id: null };
|
||||
}
|
||||
return { valid: true, notification: !(m.id !== undefined && m.id !== null) };
|
||||
if (m.jsonrpc !== '2.0') {
|
||||
return { error: { code: -32600, message: "Invalid Request: jsonrpc must be '2.0'" }, id: id ?? null };
|
||||
}
|
||||
if (typeof m.method !== 'string' || m.method === '') {
|
||||
return { error: { code: -32600, message: 'Invalid Request: method must be a non-empty string' }, id: id ?? null };
|
||||
}
|
||||
if (m.params !== undefined && (m.params === null || typeof m.params !== 'object' || Array.isArray(m.params))) {
|
||||
// null params rejected: contract is object-or-omitted (judge r2)
|
||||
return { error: { code: -32600, message: 'Invalid Request: params must be an object or omitted (null rejected)' }, id: id ?? null };
|
||||
}
|
||||
return { valid: true, notification: !hasId, id: hasId ? id : null };
|
||||
}
|
||||
|
||||
function jsonRpcError(code, message, id) {
|
||||
@@ -177,11 +192,12 @@ module.exports = function createMcpRoutes({ asyncHandler, errorResponse, log, au
|
||||
|
||||
const v = validateJsonRpc(body);
|
||||
if (v.error) {
|
||||
// Broken NOTIFICATION (well-formed intent, no id): MUST NOT reply →
|
||||
// 204 with empty body. Everything else: 400 + error object.
|
||||
// Broken NOTIFICATION (no id property at all — presence-based, judge
|
||||
// r2; id:null present is a REQUEST under our policy): spec says
|
||||
// MUST NOT reply → 204 with empty body. Everything else: 400+error.
|
||||
const notificationIntent = body !== null && typeof body === 'object' &&
|
||||
body.jsonrpc === '2.0' && typeof body.method === 'string' &&
|
||||
(body.id === undefined || body.id === null);
|
||||
!Object.prototype.hasOwnProperty.call(body, 'id');
|
||||
if (notificationIntent) return res.status(204).end();
|
||||
return res.status(400).json(jsonRpcError(v.error.code, v.error.message, v.id ?? null));
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user