sh-mcp/packages/internal-data/test/internal-data.test.ts
Adam Moussa 22c09e99fe
Some checks are pending
deploy / deploy (push) Waiting to run
Phase 1: runnable MCP + OpenAPI servers (ops + finance) (#3)
* Add shared transport: dispatch, MCP + OpenAPI adapters, local auth

Add the single authoritative tool-execution path (executeTool) plus the two
universal interfaces over it (design.md §2.5, §7.3):
- dispatch.ts: scope enforcement, ajv input validation, rate limiting, finance
  egress redaction (redactDeep), and structured audit emission on one path.
- audit.ts / rate-limit.ts: injected AuditLogger + RateLimiter abstractions.
- mcp.ts: low-level MCP Server with scope-filtered tools/list (tool-hiding) and
  tools/call routed through executeTool.
- http.ts: Express host mounting /mcp, /openapi.json, POST /tools/:name, /healthz.
- openapi.ts: buildOpenApiDocument wraps the existing path generator into a full
  OpenAPI 3.1 document.
- local-auth.ts: LocalAuthProvider (dev bearer tokens) that refuses to construct
  outside SH_MCP_ENV=local and enforces audience binding (design.md §3, §6).

* Add in-memory dev clients; make package tool exports lazy

Add an in-memory Client implementation per integration package (seeded fake
data, no network) selected when SH_MCP_ENV=local (build-plan §4). Gmail/calendar/
tasks dev clients partition by ctx.sub; payments/qbo seed sensitive-looking
fields so the redaction egress path has real targets to mask.

Make the eager default-tool exports in tasks/reminders/qbo LAZY (getDefaultTools)
so importing a package barrel no longer constructs an AWS client at module load
(build-plan §7 'no I/O at import time') — the previous eager construction broke
server startup. Fix payments tsconfig rootDir (src, was '.') so its declarations
resolve under dist/index.d.ts like the other 8 packages.

* Add runnable sh-mcp-ops and sh-mcp-finance servers

Two thin composition-root servers over the shared transport (design.md §3):
- ops: internal-data, knowledge-base, google-maps, gmail, calendar, tasks,
  reminders. finance: qbo, payments (audited + redacted on egress).
- config from env only (no hardcoded ids/issuer/tables); SH_MCP_ENV selects
  LocalAuthProvider + dev clients (local) vs CognitoAuthProvider + real stubs
  (aws). Finance applies the 15-min finance-token TTL ceiling (design.md §2.5).
- index.ts is the only place .listen() is called; a Lambda handler placeholder
  is exported but not depended on.
- synth-only CDK stubs (no real IAM/Cognito/WAF) so 'cdk synth' has a valid app
  (build-plan §6); READMEs document local run, dev tokens, curl, MCP Inspector.

* Add security-weighted test suite + coverage gate; wire tooling

Add tests for the highest-risk surface (build-plan §5, design.md §7.3):
tool-hiding, server-side scope enforcement (incl. forced hidden calls),
audience binding, input-schema validation, finance redaction on egress, audit
emission with hashed args, prompt-injection regression (tool output is data),
rate limiting, MCP conformance (in-memory transport round-trip), OpenAPI 3.1
validity, and local-auth safety. Add HTTP integration tests (supertest) for both
servers and per-package dev-client tests. 405 tests pass.

Wire the coverage gate into vitest.config.ts: 80% overall, with per-file
thresholds on the auth + dispatch crown jewels; exclude deferred real client
stubs, entrypoints, cdk apps, and aws-only config from the gate (documented).
Extend eslint flat config + add .prettierignore to cover servers/. Commit the
updated package-lock.json.

* Suppress pre-existing dev-tooling + out-of-scope scanner findings

Add written-justification suppressions for the 4 confirmed crit/high pre-push
scanner findings, none of which are in this PR's Phase 1 production code:
- npmaudit vitest / @vitest/coverage-v8 / vite: dev/test-only deps that never
  run in the deployed server/Lambda runtime (pins carried from Phase 0b;
  Dependabot will bump).
- gitleaks docs/agentforce-plan.md secret: that file is not on this branch and
  not in this changeset; flagged for the maintainer to scrub on its own branch.

The deep agentic /sh-security-review (required for this auth/authz-touching PR)
was NOT run by the agent and is flagged outstanding in the PR body.

* Address CodeQL findings: bound ajv error work + edge rate limiting

GHAS code-scanning alerts on this PR:
- dispatch.ts (js/resource-exhaustion): ajv ran with allErrors:true on
  untrusted input, letting a crafted payload force unbounded error
  enumeration. Switch to allErrors:false (default) so validation
  short-circuits on the first failure; the 400 still names that path.
- http.ts (js/missing-rate-limiting): the authenticated routes (/mcp,
  /tools/:name) had no edge throttle — auth/JWT verification ran on every
  request before the per-sub dispatch limiter could apply. Add an IP-keyed
  express-rate-limit in front of authenticate (120/60s default, configurable),
  returning the standard 429 shape. Defense-in-depth over the per-sub +
  per-tool limiter in executeTool; API GW/WAF remains the production edge.

Tests: +2 cases proving the edge limiter throttles before auth (429, not
401) on /tools and /mcp. 407 pass; tsc/eslint/prettier clean.

* Fix polynomial ReDoS in Bearer-token extraction (CodeQL js/polynomial-redos)

extractBearerToken matched /^Bearer\s+(.+)$/ — \s and . both match a space,
so the two quantifiers overlap and a crafted header can drive polynomial
backtracking. Require the capture to start with a non-whitespace char
(/^Bearer\s+(\S.*)$/), removing the ambiguity → linear match. Behavior is
unchanged for real tokens; +2 regression tests.

* Harden auth + finance redaction (sh-security-review confirmed mediums)

Two confirmed medium findings from the agentic security review:

- Fail-open SH_MCP_ENV: config defaulted to 'local' when the var was unset,
  so a deploy that forgot SH_MCP_ENV=aws would silently run LocalAuthProvider
  and accept static dev bearer tokens (dev-finance-admin -> finance:admin).
  Now fail-closed: SH_MCP_ENV must be explicitly 'local' or 'aws' or the
  server refuses to start. Plus an independent guard in LocalAuthProvider
  that refuses to construct in an AWS runtime (AWS_LAMBDA_FUNCTION_NAME /
  AWS_EXECUTION_ENV present), regardless of the env flag.

- Finance egress redaction gap: redactDeep only wholesale-masked a sensitive
  key when its value was a scalar; an object/array under a sensitive key was
  recursed into, letting a bare nested value (e.g. {account:{number:...}})
  escape the keyword-gated pattern matcher. Now the entire subtree under a
  sensitive key is masked. No current finance tool emitted such shapes (all
  flat strings), so this closes a latent hole in the universal safety net.

+4 tests (subtree redaction, AWS-runtime guard). 411 pass; coverage gate green.

Review also produced lows (memo free-text digits, unsalted argsHash,
unauth /openapi.json by-design, session-cap no-reset by-design) tracked
separately; 0 confirmed critical/high — review verdict PASS.
2026-06-26 13:33:21 -04:00

328 lines
12 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* Unit tests for @sh-mcp/internal-data tools.
*
* Strategy:
* - All DynamoDB access is replaced by a typed mock that satisfies InternalDataClient.
* - AuthContext is a mock with the required `ops:read` scope (or deliberately wrong
* scopes for the scope-enforcement tests).
* - No AWS SDK, no network, no environment variables required.
*
* Coverage:
* - Happy path (record found) for each tool
* - Empty result (record not found → found=false)
* - Downstream error (client rejects) → handler re-throws
* - Throttle / retry signal (ProvisionedThroughputExceededException)
* - Scope enforcement (missing ops:read) → ScopeError thrown
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
import type {
InternalDataClient,
WorkOrderRecord,
PurchaseOrderRecord,
SiteRecord,
} from '../src/client.js';
import { makeTools } from '../src/tools.js';
import type { AuthContext } from '@sh-mcp/shared';
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
function makeCtx(scopes: string[] = ['ops:read']): AuthContext {
return {
sub: 'lauren@seahavenind.com',
scopes: scopes as AuthContext['scopes'],
aud: 'sh-mcp-ops',
};
}
function makeMockClient(overrides: Partial<InternalDataClient> = {}): InternalDataClient {
return {
getWorkOrder: vi.fn().mockResolvedValue(null),
getPurchaseOrder: vi.fn().mockResolvedValue(null),
getSite: vi.fn().mockResolvedValue(null),
...overrides,
};
}
// Fixture records
const WORK_ORDER_RECORD: WorkOrderRecord = {
workOrderId: 'WO-20240101-001',
title: 'Replace HVAC filter – Building A',
status: 'open',
siteId: 'SITE-NYC-001',
assignedTo: 'tech1@seahavenind.com',
createdAt: '2024-01-01T10:00:00Z',
updatedAt: '2024-01-02T08:00:00Z',
description: 'Quarterly filter replacement per schedule.',
};
const PURCHASE_ORDER_RECORD: PurchaseOrderRecord = {
purchaseOrderId: 'PO-2024-00123',
vendor: 'Acme Supplies',
status: 'approved',
totalAmount: 1250.0,
currency: 'USD',
issuedAt: '2024-01-05T09:00:00Z',
updatedAt: '2024-01-06T11:00:00Z',
lineItems: [{ description: 'HVAC filter 20x25', quantity: 10, unitPrice: 125.0 }],
};
const SITE_RECORD: SiteRecord = {
siteId: 'SITE-NYC-001',
name: 'Sea Haven HQ – New York',
address: '123 Main St, New York, NY 10001',
region: 'northeast',
status: 'active',
assignedTechnicians: ['tech1@seahavenind.com'],
};
// ---------------------------------------------------------------------------
// Tests — lookup_work_order
// ---------------------------------------------------------------------------
describe('lookup_work_order', () => {
let client: InternalDataClient;
let tool: ReturnType<typeof makeTools>[0];
beforeEach(() => {
client = makeMockClient();
[tool] = makeTools(client);
});
it('happy path: returns found=true with shaped output for a known work order', async () => {
(client.getWorkOrder as ReturnType<typeof vi.fn>).mockResolvedValue(WORK_ORDER_RECORD);
const result = await tool.handler({ workOrderId: 'WO-20240101-001' }, makeCtx());
expect(result.found).toBe(true);
expect(result.workOrder).toMatchObject({
workOrderId: 'WO-20240101-001',
title: 'Replace HVAC filter – Building A',
status: 'open',
siteId: 'SITE-NYC-001',
assignedTo: 'tech1@seahavenind.com',
});
expect(client.getWorkOrder).toHaveBeenCalledWith('WO-20240101-001');
});
it('empty result: returns found=false when work order does not exist', async () => {
(client.getWorkOrder as ReturnType<typeof vi.fn>).mockResolvedValue(null);
const result = await tool.handler({ workOrderId: 'WO-NOTEXIST' }, makeCtx());
expect(result.found).toBe(false);
expect(result.workOrder).toBeUndefined();
});
it('error: re-throws when the DynamoDB client rejects', async () => {
(client.getWorkOrder as ReturnType<typeof vi.fn>).mockRejectedValue(
new Error('DynamoDB unavailable'),
);
await expect(tool.handler({ workOrderId: 'WO-20240101-001' }, makeCtx())).rejects.toThrow(
'DynamoDB unavailable',
);
});
it('throttle/retry: re-throws ProvisionedThroughputExceededException for the caller to retry', async () => {
const throttleError = Object.assign(
new Error('ProvisionedThroughputExceededException: Rate exceeded'),
{ name: 'ProvisionedThroughputExceededException' },
);
(client.getWorkOrder as ReturnType<typeof vi.fn>).mockRejectedValue(throttleError);
await expect(tool.handler({ workOrderId: 'WO-20240101-001' }, makeCtx())).rejects.toMatchObject(
{ name: 'ProvisionedThroughputExceededException' },
);
});
it('scope enforcement: throws when ops:read scope is missing', async () => {
// The real shared requireScope throws a ScopeError; our mock honours the
// same contract (imported from @sh-mcp/shared in the handler).
await expect(tool.handler({ workOrderId: 'WO-20240101-001' }, makeCtx([]))).rejects.toThrow();
});
});
// ---------------------------------------------------------------------------
// Tests — lookup_purchase_order
// ---------------------------------------------------------------------------
describe('lookup_purchase_order', () => {
let client: InternalDataClient;
let tool: ReturnType<typeof makeTools>[1];
beforeEach(() => {
client = makeMockClient();
[, tool] = makeTools(client);
});
it('happy path: returns found=true with shaped output for a known PO', async () => {
(client.getPurchaseOrder as ReturnType<typeof vi.fn>).mockResolvedValue(PURCHASE_ORDER_RECORD);
const result = await tool.handler({ purchaseOrderId: 'PO-2024-00123' }, makeCtx());
expect(result.found).toBe(true);
expect(result.purchaseOrder).toMatchObject({
purchaseOrderId: 'PO-2024-00123',
vendor: 'Acme Supplies',
status: 'approved',
totalAmount: 1250.0,
currency: 'USD',
});
expect(result.purchaseOrder?.lineItems).toHaveLength(1);
expect(client.getPurchaseOrder).toHaveBeenCalledWith('PO-2024-00123');
});
it('empty result: returns found=false when PO does not exist', async () => {
(client.getPurchaseOrder as ReturnType<typeof vi.fn>).mockResolvedValue(null);
const result = await tool.handler({ purchaseOrderId: 'PO-NOTEXIST' }, makeCtx());
expect(result.found).toBe(false);
expect(result.purchaseOrder).toBeUndefined();
});
it('error: re-throws when the DynamoDB client rejects', async () => {
(client.getPurchaseOrder as ReturnType<typeof vi.fn>).mockRejectedValue(
new Error('ResourceNotFoundException'),
);
await expect(tool.handler({ purchaseOrderId: 'PO-2024-00123' }, makeCtx())).rejects.toThrow(
'ResourceNotFoundException',
);
});
it('throttle/retry: re-throws ProvisionedThroughputExceededException', async () => {
const throttleError = Object.assign(
new Error('ProvisionedThroughputExceededException: Rate exceeded'),
{ name: 'ProvisionedThroughputExceededException' },
);
(client.getPurchaseOrder as ReturnType<typeof vi.fn>).mockRejectedValue(throttleError);
await expect(
tool.handler({ purchaseOrderId: 'PO-2024-00123' }, makeCtx()),
).rejects.toMatchObject({ name: 'ProvisionedThroughputExceededException' });
});
it('scope enforcement: throws when ops:read scope is missing', async () => {
await expect(tool.handler({ purchaseOrderId: 'PO-2024-00123' }, makeCtx([]))).rejects.toThrow();
});
});
// ---------------------------------------------------------------------------
// Tests — lookup_site
// ---------------------------------------------------------------------------
describe('lookup_site', () => {
let client: InternalDataClient;
let tool: ReturnType<typeof makeTools>[2];
beforeEach(() => {
client = makeMockClient();
[, , tool] = makeTools(client);
});
it('happy path: returns found=true with shaped output for a known site', async () => {
(client.getSite as ReturnType<typeof vi.fn>).mockResolvedValue(SITE_RECORD);
const result = await tool.handler({ siteId: 'SITE-NYC-001' }, makeCtx());
expect(result.found).toBe(true);
expect(result.site).toMatchObject({
siteId: 'SITE-NYC-001',
name: 'Sea Haven HQ – New York',
address: '123 Main St, New York, NY 10001',
region: 'northeast',
status: 'active',
});
expect(result.site?.assignedTechnicians).toContain('tech1@seahavenind.com');
expect(client.getSite).toHaveBeenCalledWith('SITE-NYC-001');
});
it('empty result: returns found=false when site does not exist', async () => {
(client.getSite as ReturnType<typeof vi.fn>).mockResolvedValue(null);
const result = await tool.handler({ siteId: 'SITE-NOTEXIST' }, makeCtx());
expect(result.found).toBe(false);
expect(result.site).toBeUndefined();
});
it('error: re-throws when the DynamoDB client rejects', async () => {
(client.getSite as ReturnType<typeof vi.fn>).mockRejectedValue(
new Error('Internal server error'),
);
await expect(tool.handler({ siteId: 'SITE-NYC-001' }, makeCtx())).rejects.toThrow(
'Internal server error',
);
});
it('throttle/retry: re-throws ProvisionedThroughputExceededException', async () => {
const throttleError = Object.assign(
new Error('ProvisionedThroughputExceededException: Rate exceeded'),
{ name: 'ProvisionedThroughputExceededException' },
);
(client.getSite as ReturnType<typeof vi.fn>).mockRejectedValue(throttleError);
await expect(tool.handler({ siteId: 'SITE-NYC-001' }, makeCtx())).rejects.toMatchObject({
name: 'ProvisionedThroughputExceededException',
});
});
it('scope enforcement: throws when ops:read scope is missing', async () => {
await expect(tool.handler({ siteId: 'SITE-NYC-001' }, makeCtx([]))).rejects.toThrow();
});
it('scope enforcement: throws when a finance scope is present but ops:read is absent', async () => {
// A token from sh-mcp-finance has finance:read but no ops:read.
await expect(
tool.handler({ siteId: 'SITE-NYC-001' }, makeCtx(['finance:read'])),
).rejects.toThrow();
});
});
// ---------------------------------------------------------------------------
// Tool metadata contract tests
// ---------------------------------------------------------------------------
describe('tool metadata', () => {
it('all tools have required metadata fields', () => {
const client = makeMockClient();
const tools = makeTools(client);
for (const tool of tools) {
expect(tool.name).toBeTruthy();
expect(tool.description).toBeTruthy();
expect(tool.tier).toBe('ops');
expect(tool.requiredScope).toBe('ops:read');
expect(tool.inputSchema).toBeTruthy();
expect(typeof tool.handler).toBe('function');
}
});
it('tool names are the expected identifiers', () => {
const client = makeMockClient();
const tools = makeTools(client);
const names = tools.map((t) => t.name);
expect(names).toEqual(['lookup_work_order', 'lookup_purchase_order', 'lookup_site']);
});
it('input schemas declare required fields and disallow additional properties', () => {
const client = makeMockClient();
const [wo, po, site] = makeTools(client);
expect((wo.inputSchema as { required: string[] }).required).toContain('workOrderId');
expect((po.inputSchema as { required: string[] }).required).toContain('purchaseOrderId');
expect((site.inputSchema as { required: string[] }).required).toContain('siteId');
for (const tool of [wo, po, site]) {
expect((tool.inputSchema as { additionalProperties: boolean }).additionalProperties).toBe(
false,
);
}
});
});