Skip to content

Commit 74493fe

Browse files
chitcommitclaudeCopilotCodex
authored
Copilot/sub pr 10 (#20)
* fix: complete 4-lane remediation — tests, governance, security, CORS Lane 1 (Testing): Add vitest infrastructure with MCP test cases covering JSON-RPC protocol, tool success/error paths, and defensive parsing. Lane 2 (Governance): Add GitHub Actions CI/CD (ci.yml, deploy-worker, governance gates), org governance scripts (audit, enforce, remediate), ISSUE_TEMPLATE, release.yml, and governance-baseline templates. Lane 3 (Security): Add .gitleaks.toml for secret scanning, security docs (access broker runbook, secret rotation checklist, scan report), .gitignore hardening. Lane 4 (Code + UI): CORS OPTIONS + credentials support, MCP server expanded from 6 to 28 tools across 8 domains, integrations and validators updates, UI disputes widget and API client fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback across 10 files - package.json: remove trailing space on kv:seed script line - ci.yml: add npm test step, fix secret allowlist to catch bracket notation (secrets['NAME']) in addition to dot notation - reusable-governance-gates.yml: same bracket notation fix - chittycompliance-dispatch.sh: replace string interpolation with jq -nc for all JSON payloads to prevent injection - org-governance-adversarial-review.sh: add defensive // [] for missingFiles and missingTriggers jq expressions - connect.ts: proper AuthVariables typing instead of @ts-expect-error - integrations.ts: normalize KV cache key with encodeURIComponent - wrangler.toml: default PLAID_ENV to sandbox, production override in [env.production.vars] - org-governance-pr-integration-loop.sh: add author verification against governance automation allowlist before auto-approve - .gitignore: exclude timestamped governance report artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Initial plan * fix: address unresolved review comments — error info leakage, type safety, and MCP tests Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com> * fix: wire mcpAuthMiddleware in tests, fix empty-body parse error, rename GET test Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com> * Wire mcpAuthMiddleware in tests and return JSON-RPC parse error on empty body (#15) * Address MCP review feedback in test harness and parsing * Initial plan (#16) Co-authored-by: openai-code-agent[bot] <242516109+Codex@users.noreply.github.com> * Resolve MCP merge conflicts while preserving JSON-RPC parse safeguards (#17) * Initial plan * chore: resolve mcp merge conflicts --------- Co-authored-by: openai-code-agent[bot] <242516109+Codex@users.noreply.github.com> --------- Co-authored-by: Codex <242516109+Codex@users.noreply.github.com> * Harden MCP request validation and tighten parse-error tests (#19) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Codex <242516109+Codex@users.noreply.github.com>
1 parent 408e854 commit 74493fe

2 files changed

Lines changed: 21 additions & 7 deletions

File tree

src/routes/mcp.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,18 +191,27 @@ const TOOLS = [
191191

192192
// MCP endpoint — handles JSON-RPC 2.0 requests
193193
mcpRoutes.post('/', async (c) => {
194-
let body: { jsonrpc: string; id?: string | number | null; method?: string; params?: Record<string, unknown> };
194+
let parsedBody: unknown;
195195
try {
196-
body = await c.req.json();
196+
parsedBody = await c.req.json();
197197
} catch {
198198
return c.json({ jsonrpc: '2.0', id: null, error: { code: -32700, message: 'Parse error' } }, 400);
199199
}
200200

201+
if (!parsedBody || typeof parsedBody !== 'object' || Array.isArray(parsedBody)) {
202+
return c.json({ jsonrpc: '2.0', id: null, error: { code: -32600, message: 'Invalid Request' } }, 400);
203+
}
204+
205+
const body = parsedBody as { jsonrpc?: unknown; id?: string | number | null; method?: unknown; params?: unknown };
206+
201207
if (body.jsonrpc !== '2.0' || typeof body.method !== 'string') {
202208
return c.json({ jsonrpc: '2.0', id: body.id ?? null, error: { code: -32600, message: 'Invalid Request: must be JSON-RPC 2.0' } });
203209
}
204210

205-
const { method, params, id } = body;
211+
const { method, id } = body;
212+
const params = (body.params && typeof body.params === 'object' && !Array.isArray(body.params))
213+
? body.params as Record<string, unknown>
214+
: undefined;
206215

207216
switch (method) {
208217
case 'initialize':

tests/mcp.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { describe, it, expect, vi } from 'vitest';
77
import { Hono } from 'hono';
88
import type { Env } from '../src/index';
99
import { mcpAuthMiddleware } from '../src/middleware/auth';
10+
import type { AuthVariables } from '../src/middleware/auth';
1011

1112
// Mock the db module before importing routes that depend on it
1213
vi.mock('../src/lib/db', () => ({
@@ -42,7 +43,7 @@ function makeMockEnv(overrides: Partial<MockEnv> = {}): MockEnv {
4243
// Helper — build a minimal Hono app that exposes the MCP routes.
4344
// ---------------------------------------------------------------------------
4445
function buildApp(envOverrides: Partial<MockEnv> = {}) {
45-
const app = new Hono<{ Bindings: Env }>();
46+
const app = new Hono<{ Bindings: Env; Variables: AuthVariables }>();
4647
const env = makeMockEnv(envOverrides);
4748

4849
// Wire the same auth middleware as production — dev bypass sets userId/scopes
@@ -244,7 +245,7 @@ describe('MCP — defensive parsing', () => {
244245
});
245246

246247
it('handles non-JSON body without crashing', async () => {
247-
const app = new Hono<{ Bindings: Env }>();
248+
const app = new Hono<{ Bindings: Env; Variables: AuthVariables }>();
248249
const env = makeMockEnv();
249250
app.use('/mcp/*', mcpAuthMiddleware);
250251
app.route('/mcp', mcpRoutes);
@@ -254,8 +255,12 @@ describe('MCP — defensive parsing', () => {
254255
body: 'not json at all',
255256
});
256257
const res = await app.fetch(req, env as unknown as Env);
257-
// Hono's req.json() throws on bad JSON; must surface as 4xx/5xx, not an unhandled crash
258-
expect(res.status).toBeGreaterThanOrEqual(400);
258+
expect(res.status).toBe(400);
259+
const json = await res.json() as Record<string, unknown>;
260+
expect(json.jsonrpc).toBe('2.0');
261+
expect(json.id).toBeNull();
262+
const error = json.error as Record<string, unknown>;
263+
expect(error.code).toBe(-32700);
259264
});
260265

261266
it('GET /mcp returns service health info', async () => {

0 commit comments

Comments
 (0)