@kaavee315checks…fix/metering-mcp-validation-bugs → master3 files · +13 −9updated 3d agoFixes two correctness bugs discovered while writing unit tests for these modules. Source-only change (3 files); independent of the test PR (#10521).
1. Prototype-chain validation bypass (metering)
isMeteredEntityType() (registry.ts) and assertBreakdownEntityType() (usage_breakdown/validation.ts) validated input with the in operator on a plain object, which walks the prototype chain. As a result, inherited Object.prototype keys (toString, constructor, hasOwnProperty, __proto__, valueOf) passed as valid entity types. These guards run on request input reaching POST /api/v3.1/{org,project}/usage/{entity_type}. Both now use Object.hasOwn().
2. MCP duplicate-slug miscount (mcp/v2/registration.ts)
On a repeated tool slug, registerToolsOnMcpServer() overwrote the existing { registered: true } status-map entry with a DUPLICATE error. This made registeredCount under-report tools that were in fact registered, and the returned status map misreported a successfully-registered slug, directly contradicting the in-code comment ("don't overwrite successful registration in map"). Duplicates are now counted separately into skippedCount without clobbering the successful entry. Actual registration behaviour (server.tool calls) is unchanged; only the bookkeeping is corrected.
isMeteredEntityType / assertBreakdownEntityType now reject toString, constructor, hasOwnProperty, __proto__, valueOf and still accept a real entity (tool_calls).registerToolsOnMcpServer([A, A, B]) → registeredCount === 2, skippedCount === 1, tools.A === { registered: true } (not clobbered), server.tool called twice.SKIP_VALIDATION=true corepack pnpm exec vitest run --config vitest.unit.config.ts (no-DB unit tier).git diff --cached confirms the change set is exactly the 3 source files, additions/edits only.PR #10521 currently contains generated tests for these same 3 files that assert the pre-fix (buggy) behaviour (rubric was "assert actual behaviour, never change source"). Those 3 assertions must be flipped to the corrected behaviour, and this fix should merge before that PR, otherwise those tests will fail against the fixed source. (Will update those assertions in #10521.)