From 9a6b7846d339cea53eae5e1e77e4ab9ca332f564 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Tue, 19 May 2026 16:34:48 -0500 Subject: [PATCH 1/2] =?UTF-8?q?PDX-492:=20feat(mcp)=20=E2=80=94=20add=20pr?= =?UTF-8?q?ovar=5Forg=5Fdescribe=20tool=20(reads=20workspace=20.metadata?= =?UTF-8?q?=20cache)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces provar_org_describe, a read-only MCP tool that surfaces cached Salesforce describe data from the Provar IDE workspace .metadata directory so authoring tools (provar_testcase_generate) can produce field-correct data steps without a live SF API call. Workspace discovery walks three candidates in order: 1. /workspace-/ (sibling pattern) 2. /Provar_Workspaces/workspace-/ 3. ~/Provar/workspace-/ (user-home fallback) Returns a structured cache-miss response with details.suggestion when the connection cache is absent, so the agent can either prompt the user to load the connection in Provar IDE or fall back to inline field hints. Registered under the existing 'inspect' tool group. H2b (sibling thread) consumes this tool's output to populate generator hints. RCA: provar_testcase_generate had no source of truth for which fields on a Salesforce object are required and what their types are. Agents either guessed (producing brittle tests), hard-coded names, or called the live SF API (slow + auth-dependent). The Provar IDE already caches describe data on disk after a connection is loaded — this PR exposes that cache as a read-only MCP tool so the cache becomes useful outside the IDE. Fix: New tool src/mcp/tools/orgDescribeTools.ts with strict path-policy checks on both project_path and connection_name (separator/traversal rejected). Cache schema is one file per object (.json preferred, .xml / .object accepted) so the existing IDE writer needs no change. Cache miss returns a stable shape with suggestion rather than an isError response, so callers do not need a try/catch path. 14 unit tests cover all 7 plan scenarios (workspace discovery, fallback, cache miss, path policy, happy path, field_filter, objects filter). Two smoke entries cover happy + miss. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/mcp-pilot-guide.md | 31 ++ docs/mcp.md | 108 +++++++ scripts/mcp-smoke.cjs | 30 ++ src/mcp/server.ts | 3 +- src/mcp/tools/orgDescribeTools.ts | 430 +++++++++++++++++++++++++ test/unit/mcp/orgDescribeTools.test.ts | 405 +++++++++++++++++++++++ 6 files changed, 1006 insertions(+), 1 deletion(-) create mode 100644 src/mcp/tools/orgDescribeTools.ts create mode 100644 test/unit/mcp/orgDescribeTools.test.ts diff --git a/docs/mcp-pilot-guide.md b/docs/mcp-pilot-guide.md index bf7e0073..27381cfd 100644 --- a/docs/mcp-pilot-guide.md +++ b/docs/mcp-pilot-guide.md @@ -483,6 +483,37 @@ If any FAIL indicator appears, report it to the Provar team with the prompt and --- +### Scenario 13: Org-Aware Test Case Generation + +The `provar_org_describe` tool surfaces cached Salesforce describe data from the Provar IDE workspace so the agent knows which fields on each object are required and what their types are — without making a live Salesforce API call. Use it as a hint source before generating data-heavy steps. + +**Prerequisite:** the project must have been opened in Provar IDE at least once with the named connection loaded, so the `.metadata//` directory is populated. + +**Try this prompt:** + +> "Before generating a test case that creates an Account, call `provar_org_describe` against my project at `/path/to/MyProject` for connection `MyOrg` and the `Account` object only. Use the required-field list to populate the create form." + +The tool returns the discovered workspace path, a cache age, and per-object required-field metadata. Example call: + +```jsonc +{ + "project_path": "/Users/you/git/MyProject", + "connection_name": "MyOrg", + "objects": ["Account"], + "field_filter": "required" +} +``` + +**What to look for (PASS):** + +- Response includes `workspace_path` resolved to a real `workspace-*` directory. +- `objects[0].required_fields` contains at least one field with `nillable: false`. +- The follow-up `provar_testcase_generate` call uses field names from the response. + +**Cache-miss behaviour (also PASS):** if the cache directory does not exist the tool returns `details.suggestion` telling the agent how to recover — either open the project in Provar IDE to populate the cache, or pass field-type hints inline. + +--- + ## Security Model ### What the server does diff --git a/docs/mcp.md b/docs/mcp.md index 33d23421..00405b29 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -50,6 +50,8 @@ The Provar DX CLI ships with a built-in **Model Context Protocol (MCP) server** - [provar_testplan_add-instance](#provar_testplan_add-instance) - [provar_testplan_create-suite](#provar_testplan_create-suite) - [provar_testplan_remove-instance](#provar_testplan_remove-instance) + - [Org metadata access](#org-metadata-access) + - [provar_org_describe](#provar_org_describe) - [NitroX — Hybrid Model page objects](#nitrox--hybrid-model-page-objects) - [provar_nitrox_discover](#provar_nitrox_discover) - [provar_nitrox_read](#provar_nitrox_read) @@ -1686,6 +1688,112 @@ Remove a `.testinstance` file from a plan suite. Path is validated to stay withi --- +## Org metadata access + +Tools that surface Salesforce org metadata to authoring tools without making a live API call. These read from data that has already been written to disk by the Provar IDE — they do **not** trigger metadata downloads themselves and they do **not** require an authenticated session. + +> **Distinct from `.provarCaches`:** the runtime cache used by `provar_automation_testrun` lives at `/.provarCaches/` and is regenerated per run. The cache read by `provar_org_describe` lives in the Provar IDE **workspace** (`/.metadata//`) and is updated when a user opens the project and loads a connection in the IDE. + +### `provar_org_describe` + +Read cached Salesforce describe data for one connection from the Provar workspace `.metadata` cache. Returns the object list, required-field schema, and a cache age. Use this before calling `provar_testcase_generate` so the generator can produce steps with correctly-typed field values. + +**Prerequisite:** the project must have been opened in Provar IDE at least once with the named connection loaded. If the cache is missing, the tool returns a structured response with `details.suggestion` rather than an error. + +**Workspace discovery heuristic** — the tool walks candidate directories in this order and uses the first one that exists: + +1. `/workspace-/` — sibling workspace pattern (default for Provar IDE in this workspace layout). +2. `/Provar_Workspaces/workspace-/` — shared `Provar_Workspaces` directory. +3. `~/Provar/workspace-/` — user-home fallback. + +`` is the project's basename with whitespace collapsed to single dashes and lowercased: `"My Project"` → `"my-project"`. + +| Input | Type | Required | Default | Description | +| ----------------- | ----------------------- | -------- | ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `project_path` | string | yes | — | Absolute path to the Provar test project root (the directory containing `.testproject`). Must be within `--allowed-paths`. | +| `connection_name` | string | yes | — | Connection name as defined in `.testproject` (e.g. `"MyOrg"`). Must match the `.metadata` subdirectory exactly. Path separators in this value are rejected (`PATH_TRAVERSAL`). | +| `objects` | string[] | no | all | Filter — only return data for these object API names. When omitted, lists every object cached under the connection directory. | +| `field_filter` | `'required'` \| `'all'` | no | `'required'` | Which fields to return. `'required'` includes only fields with `nillable=false`; `'all'` returns every cached field. | + +| Output field | Description | +| -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | +| `workspace_path` | Absolute resolved path to the discovered workspace, or `null` when none of the three candidate directories exists. | +| `cache_age_ms` | `mtime` delta in milliseconds of the connection cache directory, or `null` when the cache is missing. | +| `objects[]` | Array of `{ name, exists, required_fields, field_count }`. `exists` is `true` (cached), `false` (requested but not cached), or `null` (cache miss). | +| `details.suggestion` | Present **only** on cache miss. Tells the agent how to populate the cache (open Provar IDE) or how to proceed without it (inline hints). | + +**Example — happy path:** + +```jsonc +// Request +{ + "project_path": "/Users/me/git/MyProject", + "connection_name": "MyOrg", + "objects": ["Account", "Contact"], + "field_filter": "required" +} + +// Response +{ + "workspace_path": "/Users/me/git/workspace-MyProject", + "cache_age_ms": 1839200, + "objects": [ + { + "name": "Account", + "exists": true, + "required_fields": [ + { "name": "Name", "type": "string", "default_value": null, "nillable": false } + ], + "field_count": 24 + }, + { + "name": "Contact", + "exists": true, + "required_fields": [ + { "name": "LastName", "type": "string", "default_value": null, "nillable": false } + ], + "field_count": 31 + } + ] +} +``` + +**Example — cache miss:** + +```jsonc +// Response when the .metadata/ directory does not exist +{ + "workspace_path": "/Users/me/git/workspace-MyProject", + "cache_age_ms": null, + "objects": [{ "name": "Account", "exists": null, "required_fields": [], "field_count": 0 }], + "details": { + "suggestion": "Open this project in Provar IDE and load the 'MyOrg' connection, or pass field-type hints inline to provar_testcase_generate." + } +} +``` + +**On-disk cache schema (one file per object).** The tool reads `.json` first, then `.xml`, then `.object` as a fallback: + +```jsonc +// /.metadata//Account.json +{ + "name": "Account", + "fields": [ + { "name": "Name", "type": "string", "defaultValue": null, "nillable": false }, + { "name": "Phone", "type": "phone", "defaultValue": null, "nillable": true } + ] +} +``` + +**Error codes:** + +| Code | Meaning | +| ------------------ | ---------------------------------------------------------------------------------------------- | +| `PATH_NOT_ALLOWED` | `project_path` or the resolved workspace path is outside `--allowed-paths`. | +| `PATH_TRAVERSAL` | `project_path` contains `..` segments, or `connection_name` contains a path separator or `..`. | + +--- + ## NitroX — Hybrid Model page objects NitroX is Provar's **Hybrid Model** for locators. Instead of hand-written Java Page Objects it uses component-based `.po.json` files that map UI elements for any Salesforce component type: LWC, Screen Flow, Industry / OmniStudio, Experience Cloud, and standard HTML5. These files live in `nitroX/` directories inside your Provar project. diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 6cea444b..ae54a7f4 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -461,6 +461,36 @@ async function runTests() { test_item_id: '1', }); + // ── 54. provar_org_describe — cache miss ───────────────────────────────── + // TMP has no workspace at all → cache-miss response with details.suggestion + if (inGroup('inspect')) + await callTool('provar_org_describe', { + project_path: TMP, + connection_name: 'SmokeOrg', + objects: ['Account'], + }); + + // ── 55. provar_org_describe — happy path ───────────────────────────────── + // Set up a sibling workspace + .metadata/ with one fake object. + if (inGroup('inspect')) { + const fs = require('fs'); + const orgProject = path.join(TMP, 'org-describe-smoke-project'); + fs.mkdirSync(orgProject, { recursive: true }); + const cxnDir = path.join(TMP, 'workspace-org-describe-smoke-project', '.metadata', 'SmokeOrg'); + fs.mkdirSync(cxnDir, { recursive: true }); + fs.writeFileSync( + path.join(cxnDir, 'Account.json'), + JSON.stringify({ + name: 'Account', + fields: [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }], + }) + ); + await callTool('provar_org_describe', { + project_path: orgProject, + connection_name: 'SmokeOrg', + }); + } + server.stdin.end(); } diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 20429dba..1f9de722 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -34,6 +34,7 @@ import { registerAllTestPlanTools } from './tools/testPlanTools.js'; import { registerAllNitroXTools } from './tools/nitroXTools.js'; import { registerAllTestCaseStepTools } from './tools/testCaseStepTools.js'; import { registerAllConnectionTools } from './tools/connectionTools.js'; +import { registerAllOrgDescribeTools } from './tools/orgDescribeTools.js'; import { registerAllPrompts } from './prompts/index.js'; import { createDepthGuardState, @@ -64,7 +65,7 @@ const TOOL_GROUPS: Record/.metadata//.json + * + * Each JSON file contains: { name: "Account", fields: [ { name, type, defaultValue, nillable }, ... ] } + * + * As a fallback, .xml / .object files (CustomObject metadata) are also accepted + * to ease migration from the legacy Provar IDE Eclipse cache layout. + */ +interface CachedField { + name: string; + type?: string; + defaultValue?: string | null; + nillable?: boolean; +} + +interface CachedObject { + name?: string; + fields?: CachedField[]; +} + +// ── Workspace discovery ─────────────────────────────────────────────────────── + +/** + * Normalise a project basename for use in fallback workspace dir names: + * "My Project Path " → "my-project-path". + */ +export function projectNameDashes(projectPath: string): string { + return path.basename(projectPath).trim().replace(/\s+/g, '-').toLowerCase(); +} + +/** + * Build the ordered list of candidate workspace directories. + * First existing wins. + * 1. /workspace-/ — sibling workspace pattern. + * 2. /Provar_Workspaces/workspace-/ + * 3. ~/Provar/workspace-/ — user-home fallback. + */ +export function workspaceCandidates(projectPath: string): string[] { + const resolved = path.resolve(projectPath); + const parent = path.dirname(resolved); + const basename = path.basename(resolved); + const dashes = projectNameDashes(resolved); + return [ + path.join(parent, `workspace-${basename}`), + path.join(parent, 'Provar_Workspaces', `workspace-${dashes}`), + path.join(os.homedir(), 'Provar', `workspace-${dashes}`), + ]; +} + +/** Returns the first candidate workspace that exists, or null. */ +export function discoverWorkspace(projectPath: string): string | null { + for (const candidate of workspaceCandidates(projectPath)) { + try { + if (fs.existsSync(candidate) && fs.statSync(candidate).isDirectory()) { + return candidate; + } + } catch { + // Permission errors etc. — skip and try next candidate + } + } + return null; +} + +// ── Cache reading ───────────────────────────────────────────────────────────── + +const XML_PARSER = new XMLParser({ + ignoreAttributes: false, + attributeNamePrefix: '@_', + parseAttributeValue: false, + isArray: (name): boolean => name === 'fields', +}); + +/** Parse a JSON cache file into the canonical CachedObject shape. */ +function readJsonCacheFile(filePath: string): CachedObject { + const raw = fs.readFileSync(filePath, 'utf-8'); + return JSON.parse(raw) as CachedObject; +} + +/** + * Parse a legacy .object XML file (CustomObject metadata) into the canonical shape. + * Only extracts the bare minimum the tool needs: field name, type, nillable. + */ +function readXmlCacheFile(filePath: string): CachedObject { + const raw = fs.readFileSync(filePath, 'utf-8'); + const parsed = XML_PARSER.parse(raw) as Record; + const root = (parsed['CustomObject'] ?? parsed['toolingObjectInfo'] ?? {}) as Record; + const fieldsRaw = root['fields']; + if (!Array.isArray(fieldsRaw)) return { name: path.basename(filePath, path.extname(filePath)), fields: [] }; + + const fields: CachedField[] = []; + for (const f of fieldsRaw as Array>) { + const name = (f['fullName'] ?? f['name']) as string | undefined; + if (!name) continue; + fields.push({ + name, + type: (f['type'] as string | undefined) ?? 'unknown', + defaultValue: (f['defaultValue'] as string | undefined) ?? null, + // XML defaults: required = !nillable. In the .object format, "required" is rare, + // so we default to nillable=true (optional) unless explicitly required. + nillable: f['required'] === 'true' ? false : true, + }); + } + return { name: path.basename(filePath, path.extname(filePath)), fields }; +} + +/** Look up the cache file for one object, trying .json then .xml. */ +function findObjectCacheFile(connectionDir: string, objectName: string): string | null { + const jsonPath = path.join(connectionDir, `${objectName}.json`); + if (fs.existsSync(jsonPath)) return jsonPath; + const xmlPath = path.join(connectionDir, `${objectName}.xml`); + if (fs.existsSync(xmlPath)) return xmlPath; + // Legacy CustomObject layout (.object extension) + const objPath = path.join(connectionDir, `${objectName}.object`); + if (fs.existsSync(objPath)) return objPath; + return null; +} + +/** List all cached object names (stripped of extension) in a connection directory. */ +function listCachedObjectNames(connectionDir: string): string[] { + let entries: string[]; + try { + entries = fs.readdirSync(connectionDir); + } catch { + return []; + } + const names = new Set(); + for (const entry of entries) { + const ext = path.extname(entry); + if (ext === '.json' || ext === '.xml' || ext === '.object') { + names.add(path.basename(entry, ext)); + } + } + return [...names].sort(); +} + +/** Read one object's cache file and convert it to an OrgDescribeObject. */ +function readObject(connectionDir: string, objectName: string, fieldFilter: 'required' | 'all'): OrgDescribeObject { + const file = findObjectCacheFile(connectionDir, objectName); + if (!file) { + return { name: objectName, exists: false, required_fields: [], field_count: 0 }; + } + let cached: CachedObject; + try { + cached = path.extname(file) === '.json' ? readJsonCacheFile(file) : readXmlCacheFile(file); + } catch (e) { + log('warn', 'org_describe: failed to parse cache file', { file, error: (e as Error).message }); + return { name: objectName, exists: false, required_fields: [], field_count: 0 }; + } + + const allFields = cached.fields ?? []; + const filtered = fieldFilter === 'required' ? allFields.filter((f) => f.nillable === false) : allFields; + + const fields: OrgDescribeField[] = filtered.map((f) => ({ + name: f.name, + type: f.type ?? 'unknown', + default_value: f.defaultValue ?? null, + nillable: f.nillable ?? true, + })); + + return { + name: cached.name ?? objectName, + exists: true, + required_fields: fields, + field_count: allFields.length, + }; +} + +/** Compute the mtime delta (ms) of a directory. Returns null on error. */ +function cacheAgeMs(dir: string): number | null { + try { + const stat = fs.statSync(dir); + return Math.max(0, Date.now() - stat.mtimeMs); + } catch { + return null; + } +} + +// ── Suggestion strings ──────────────────────────────────────────────────────── + +function cacheMissSuggestion(connectionName: string): string { + return ( + `Open this project in Provar IDE and load the '${connectionName}' connection, ` + + 'or pass field-type hints inline to provar_testcase_generate.' + ); +} + +// ── Core logic ──────────────────────────────────────────────────────────────── + +interface DescribeArgs { + project_path: string; + connection_name: string; + objects?: string[]; + field_filter?: 'required' | 'all'; +} + +/** + * Resolve & policy-check the workspace + connection directory. + * Returns the connection directory if it exists and is allowed, otherwise null. + */ +function resolveConnectionDir( + workspacePath: string | null, + connectionName: string, + allowedPaths: string[] +): { connectionDir: string | null; resolvedWorkspace: string | null } { + if (!workspacePath) return { connectionDir: null, resolvedWorkspace: null }; + + // Reject path-shaped connection names outright. A real connection name from a + // .testproject is an identifier (e.g. "MyOrg"); any separator or traversal + // segment is almost certainly a misuse or injection attempt. + if (connectionName.includes('/') || connectionName.includes('\\') || connectionName.split(/[/\\]+/).includes('..')) { + throw new PathPolicyError( + 'PATH_TRAVERSAL', + `Invalid connection_name (contains path separators): ${connectionName}` + ); + } + + // Path policy: workspace MUST be inside allowed paths before any fs call against it. + const resolvedWorkspace = path.resolve(workspacePath); + assertPathAllowed(resolvedWorkspace, allowedPaths); + + const connectionDir = path.resolve(resolvedWorkspace, '.metadata', connectionName); + // Belt-and-braces check after composition. + assertPathAllowed(connectionDir, allowedPaths); + + if (!fs.existsSync(connectionDir) || !fs.statSync(connectionDir).isDirectory()) { + return { connectionDir: null, resolvedWorkspace }; + } + return { connectionDir, resolvedWorkspace }; +} + +function buildCacheMissResponse( + resolvedWorkspace: string | null, + args: DescribeArgs, + requestId: string +): Record { + const objects: OrgDescribeObject[] = (args.objects ?? []).map((name) => ({ + name, + exists: null, + required_fields: [], + field_count: 0, + })); + return { + requestId, + workspace_path: resolvedWorkspace, + cache_age_ms: null, + objects, + details: { suggestion: cacheMissSuggestion(args.connection_name) }, + }; +} + +function buildHappyResponse( + resolvedWorkspace: string, + connectionDir: string, + args: DescribeArgs, + requestId: string +): Record { + const fieldFilter = args.field_filter ?? 'required'; + const requestedNames = args.objects?.length ? args.objects : listCachedObjectNames(connectionDir); + const objects = requestedNames.map((name) => readObject(connectionDir, name, fieldFilter)); + return { + requestId, + workspace_path: resolvedWorkspace, + cache_age_ms: cacheAgeMs(connectionDir), + objects, + }; +} + +// ── Tool registration ───────────────────────────────────────────────────────── + +export function registerOrgDescribe(server: McpServer, config: ServerConfig): void { + server.registerTool( + 'provar_org_describe', + { + title: 'Describe Org Objects From Workspace Cache', + description: desc( + [ + 'Read cached Salesforce describe data for one connection from the Provar workspace .metadata cache.', + 'Prerequisite: the project must have been opened in Provar IDE at least once with the named connection loaded', + '— this tool is read-only and does NOT trigger a metadata download.', + 'Workspace discovery tries in order: /workspace-, ', + '/Provar_Workspaces/workspace-, then ~/Provar/workspace-.', + 'Returns an empty result with details.suggestion when the cache is missing.', + 'Distinct from the runtime .provarCaches cache used by test execution.', + ].join(' '), + 'Read cached describe data for one connection from the Provar workspace .metadata cache.' + ), + inputSchema: { + project_path: z + .string() + .describe( + desc( + 'Absolute path to the Provar test project root (the directory containing .testproject). Must be within --allowed-paths.', + 'string, absolute path to Provar test project root' + ) + ), + connection_name: z + .string() + .describe( + desc( + 'Connection name as defined in the .testproject file (e.g. "MyOrg"). The .metadata cache subdirectory must match this exactly.', + 'string, connection name as defined in .testproject' + ) + ), + objects: z + .array(z.string()) + .optional() + .describe( + desc( + 'Optional filter — only return data for these object API names (e.g. ["Account","Contact"]). When omitted, lists all cached objects for the connection.', + 'string[], optional; restrict to these object API names' + ) + ), + field_filter: z + .enum(['required', 'all']) + .optional() + .default('required') + .describe( + desc( + 'Which fields to include. "required" (default) returns only fields with nillable=false; "all" returns every cached field.', + "'required' | 'all'; default 'required'" + ) + ), + }, + }, + (args: DescribeArgs) => { + const requestId = makeRequestId(); + log('info', 'provar_org_describe', { + requestId, + connection_name: args.connection_name, + object_count: args.objects?.length ?? null, + }); + + try { + assertPathAllowed(args.project_path, config.allowedPaths); + const workspacePath = discoverWorkspace(args.project_path); + const { connectionDir, resolvedWorkspace } = resolveConnectionDir( + workspacePath, + args.connection_name, + config.allowedPaths + ); + + if (!connectionDir) { + const response = buildCacheMissResponse(resolvedWorkspace, args, requestId); + return { + content: [{ type: 'text' as const, text: JSON.stringify(response) }], + structuredContent: response, + }; + } + + const response = buildHappyResponse(resolvedWorkspace!, connectionDir, args, requestId); + return { + content: [{ type: 'text' as const, text: JSON.stringify(response) }], + structuredContent: response, + }; + } catch (err: unknown) { + const error = err as Error & { code?: string }; + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + error instanceof PathPolicyError ? error.code : error.code ?? 'ORG_DESCRIBE_ERROR', + error.message, + requestId + ) + ), + }, + ], + }; + } + } + ); +} + +export function registerAllOrgDescribeTools(server: McpServer, config: ServerConfig): void { + registerOrgDescribe(server, config); +} diff --git a/test/unit/mcp/orgDescribeTools.test.ts b/test/unit/mcp/orgDescribeTools.test.ts new file mode 100644 index 00000000..a8820f27 --- /dev/null +++ b/test/unit/mcp/orgDescribeTools.test.ts @@ -0,0 +1,405 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import { strict as assert } from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { describe, it, beforeEach, afterEach } from 'mocha'; +import { + registerOrgDescribe, + discoverWorkspace, + projectNameDashes, + workspaceCandidates, +} from '../../../src/mcp/tools/orgDescribeTools.js'; +import type { ServerConfig } from '../../../src/mcp/server.js'; + +// ── Minimal McpServer mock ───────────────────────────────────────────────────── + +type ToolHandler = (args: Record) => unknown; + +class MockMcpServer { + private handlers = new Map(); + + public registerTool(name: string, _config: unknown, handler: ToolHandler): void { + this.handlers.set(name, handler); + } + + public call(name: string, args: Record): ReturnType { + const h = this.handlers.get(name); + if (!h) throw new Error(`Tool not registered: ${name}`); + return h(args); + } +} + +// ── Helpers ──────────────────────────────────────────────────────────────────── + +function parseText(result: unknown): Record { + const r = result as { content: Array<{ type: string; text: string }> }; + return JSON.parse(r.content[0].text) as Record; +} + +function isError(result: unknown): boolean { + return (result as { isError?: boolean }).isError === true; +} + +interface CachedField { + name: string; + type: string; + defaultValue: string | null; + nillable: boolean; +} + +/** Write a JSON cache file for one object. */ +function writeJsonCache(connectionDir: string, objectName: string, fields: CachedField[]): void { + fs.mkdirSync(connectionDir, { recursive: true }); + fs.writeFileSync( + path.join(connectionDir, `${objectName}.json`), + JSON.stringify({ name: objectName, fields }), + 'utf-8' + ); +} + +// ── Test setup ───────────────────────────────────────────────────────────────── + +let tmpRoot: string; +let projectPath: string; +let server: MockMcpServer; +let config: ServerConfig; + +beforeEach(() => { + // Use realpathSync to canonicalise the path on macOS (/var → /private/var) so + // assertPathAllowed comparisons match the realpath the policy resolves to. + tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'org-describe-test-'))); + projectPath = path.join(tmpRoot, 'MyProject'); + fs.mkdirSync(projectPath, { recursive: true }); + + server = new MockMcpServer(); + // tmpRoot must be allowed so both the project path and any sibling workspace + // candidate (also under tmpRoot) pass the path policy check. + config = { allowedPaths: [tmpRoot] }; + registerOrgDescribe(server as never, config); +}); + +afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }); +}); + +// ── projectNameDashes / workspaceCandidates ─────────────────────────────────── + +describe('projectNameDashes', () => { + it('lowercases and replaces whitespace with single dashes', () => { + assert.equal(projectNameDashes('/x/My Project Path'), 'my-project-path'); + assert.equal(projectNameDashes('/x/ Spaced Name '), 'spaced-name'); + }); +}); + +describe('workspaceCandidates', () => { + it('returns three candidates in expected order', () => { + const cands = workspaceCandidates('/Users/alice/projects/My Project'); + assert.equal(cands.length, 3); + assert.ok( + cands[0].endsWith(`${path.sep}workspace-My Project`), + `Expected sibling workspace first, got: ${cands[0]}` + ); + assert.ok( + cands[1].includes('Provar_Workspaces') && cands[1].endsWith('workspace-my-project'), + `Expected Provar_Workspaces second, got: ${cands[1]}` + ); + assert.ok(cands[2].endsWith(`${path.sep}Provar${path.sep}workspace-my-project`)); + }); +}); + +// ── (a) Workspace discovery — sibling pattern ───────────────────────────────── + +describe('provar_org_describe — workspace discovery', () => { + it('(a) finds the sibling workspace at /workspace-', () => { + // /workspace-MyProject is the sibling pattern + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], siblingWorkspace, 'should discover sibling workspace'); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null; field_count: number }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].name, 'Account'); + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 1); + }); + + it('(b) falls back to user-home workspace when sibling missing (via override)', () => { + // Stand in for ~/Provar by using a HOME override. The tool uses os.homedir(), + // and we override $HOME for this test only. Set the home to a tmp dir so the + // path is inside allowed paths. + const fakeHome = path.join(tmpRoot, 'fakehome'); + fs.mkdirSync(fakeHome, { recursive: true }); + + const homeWorkspace = path.join(fakeHome, 'Provar', 'workspace-myproject'); + const connectionDir = path.join(homeWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Contact', [ + { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, + ]); + + // Override HOME and USERPROFILE so os.homedir() returns fakeHome + const oldHome = process.env['HOME']; + const oldUserProfile = process.env['USERPROFILE']; + process.env['HOME'] = fakeHome; + process.env['USERPROFILE'] = fakeHome; + try { + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], homeWorkspace, 'should discover user-home workspace'); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null }>; + assert.ok( + objects.some((o) => o.name === 'Contact' && o.exists === true), + 'should list Contact from home workspace cache' + ); + } finally { + if (oldHome === undefined) delete process.env['HOME']; + else process.env['HOME'] = oldHome; + if (oldUserProfile === undefined) delete process.env['USERPROFILE']; + else process.env['USERPROFILE'] = oldUserProfile; + } + }); + + it('discoverWorkspace returns null when no candidate exists', () => { + assert.equal(discoverWorkspace(projectPath), null); + }); +}); + +// ── (c) Cache miss ──────────────────────────────────────────────────────────── + +describe('provar_org_describe — cache miss', () => { + it('(c) returns suggestion when workspace exists but .metadata/ absent', () => { + // Create the sibling workspace dir but NOT the connection subdir + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true }); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MissingOrg', + objects: ['Account'], + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], siblingWorkspace); + assert.equal(body['cache_age_ms'], null); + + const details = body['details'] as { suggestion: string }; + assert.ok(details, 'details should be present on cache miss'); + assert.ok( + details.suggestion.includes('Provar IDE') && details.suggestion.includes('MissingOrg'), + `suggestion should mention IDE and connection name; got: ${details.suggestion}` + ); + + const objects = body['objects'] as Array<{ name: string; exists: boolean | null; required_fields: unknown[] }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].name, 'Account'); + assert.equal(objects[0].exists, null, 'exists must be null when cache missing entirely'); + }); + + it('returns suggestion when no workspace at all is discoverable', () => { + // No HOME override + no sibling workspace ⇒ workspace_path null. But os.homedir() + // will still produce a path; set HOME to a non-existent dir so the candidate doesn't exist. + const fakeHome = path.join(tmpRoot, 'nope'); + const oldHome = process.env['HOME']; + const oldUserProfile = process.env['USERPROFILE']; + process.env['HOME'] = fakeHome; + process.env['USERPROFILE'] = fakeHome; + try { + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'AnyOrg', + objects: ['Account', 'Contact'], + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], null); + assert.ok(body['details'], 'suggestion should be present'); + } finally { + if (oldHome === undefined) delete process.env['HOME']; + else process.env['HOME'] = oldHome; + if (oldUserProfile === undefined) delete process.env['USERPROFILE']; + else process.env['USERPROFILE'] = oldUserProfile; + } + }); +}); + +// ── (d) Path policy ─────────────────────────────────────────────────────────── + +describe('provar_org_describe — path policy', () => { + it('(d) rejects project_path outside allowed paths with PATH_NOT_ALLOWED', () => { + const strictServer = new MockMcpServer(); + registerOrgDescribe(strictServer as never, { allowedPaths: [tmpRoot] }); + + const result = strictServer.call('provar_org_describe', { + project_path: path.join(os.tmpdir(), 'definitely-outside'), + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected error_code: ${code}`); + }); + + it('rejects connection_name that would escape workspace dir via traversal', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true }); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: '../../escape', + }); + + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_TRAVERSAL' || code === 'PATH_NOT_ALLOWED', `Unexpected error_code: ${code}`); + }); +}); + +// ── (e) Happy path ──────────────────────────────────────────────────────────── + +describe('provar_org_describe — happy path', () => { + it('(e) returns the expected shape for two cached objects', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [ + { name: 'Name', type: 'string', defaultValue: null, nillable: false }, + { name: 'AccountNumber', type: 'string', defaultValue: null, nillable: true }, + ]); + writeJsonCache(connectionDir, 'Contact', [ + { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, + { name: 'Email', type: 'email', defaultValue: null, nillable: true }, + ]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], siblingWorkspace); + assert.ok(typeof body['cache_age_ms'] === 'number' && body['cache_age_ms'] >= 0); + + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + required_fields: Array<{ name: string; nillable: boolean }>; + field_count: number; + }>; + assert.equal(objects.length, 2); + + const account = objects.find((o) => o.name === 'Account'); + assert.ok(account); + assert.equal(account.exists, true); + assert.equal(account.field_count, 2, 'field_count reports total cached fields, not filtered'); + // default field_filter is "required" → only nillable=false fields included + assert.equal(account.required_fields.length, 1); + assert.equal(account.required_fields[0].name, 'Name'); + }); +}); + +// ── (f) field_filter ────────────────────────────────────────────────────────── + +describe('provar_org_describe — field_filter', () => { + it('(f) field_filter=required excludes nillable fields', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [ + { name: 'Name', type: 'string', defaultValue: null, nillable: false }, + { name: 'Phone', type: 'phone', defaultValue: null, nillable: true }, + ]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + field_filter: 'required', + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ required_fields: Array<{ name: string }> }>; + const fields = objects[0].required_fields.map((f) => f.name); + assert.deepEqual(fields, ['Name'], 'only nillable=false fields should appear'); + }); + + it('(f) field_filter=all includes nillable fields', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [ + { name: 'Name', type: 'string', defaultValue: null, nillable: false }, + { name: 'Phone', type: 'phone', defaultValue: null, nillable: true }, + ]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + field_filter: 'all', + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ required_fields: Array<{ name: string }> }>; + const names = objects[0].required_fields.map((f) => f.name).sort(); + assert.deepEqual(names, ['Name', 'Phone']); + }); +}); + +// ── (g) objects filter ──────────────────────────────────────────────────────── + +describe('provar_org_describe — objects filter', () => { + it('(g) restricts response to requested object names only', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + writeJsonCache(connectionDir, 'Contact', [ + { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, + ]); + writeJsonCache(connectionDir, 'Lead', [{ name: 'Company', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account', 'Lead'], + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ name: string }>; + const names = objects.map((o) => o.name).sort(); + assert.deepEqual(names, ['Account', 'Lead'], 'Contact should be excluded'); + }); + + it('reports exists=false for a requested object not present in cache', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account', 'Ghost'], + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null }>; + const ghost = objects.find((o) => o.name === 'Ghost'); + assert.ok(ghost); + assert.equal(ghost.exists, false, 'object not in cache → exists=false'); + }); +}); From f04eeff971f7d3ba96678951481512c3608ea462 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Wed, 20 May 2026 07:29:55 -0500 Subject: [PATCH 2/2] =?UTF-8?q?PDX-492:=20fix(mcp)=20=E2=80=94=20orgDescri?= =?UTF-8?q?be=20path-policy=20hardening,=20XML=20required-flag=20bug,=20ex?= =?UTF-8?q?ists-true=20on=20parse=20error,=20docs/tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RCA: Copilot review of PR #188 flagged six issues across correctness, security, and contract: (1) the .xml/.object fallback compared required==='true' as a string, but fast-xml-parser with parseTagValue=true (its default) coerces the value to boolean true, silently misclassifying required fields as nillable; (2) discoverWorkspace probed fs.existsSync/statSync against candidate dirs (including the ~/Provar home fallback) BEFORE any path-policy check, contradicting the project's --allowed-paths contract and potentially touching paths outside the policy; (3) when a cache file existed but failed to parse, readObject returned exists=false — indistinguishable from "object not cached", so callers could not detect corrupt/unsupported cache files; (4) docs/examples omitted the requestId field that the tool actually returns, making the documented shape drift from runtime; (5) unit tests covered only the .json cache path, leaving the legacy .xml and .object parsers (where the required-flag bug lived) untested; (6) the PATH_TRAVERSAL message read "contains path separators" but the validator also rejects bare ".." with no separators, so the message was inaccurate for that branch. Fix: (1) readXmlCacheFile now treats both boolean true and string "true" as required, so nillable is computed correctly regardless of parser config; (2) discoverWorkspace accepts allowedPaths and runs assertPathAllowed per candidate BEFORE fs.existsSync/statSync — a candidate outside policy is silently skipped (not a hard error) so discovery falls through to the next candidate naturally; (3) readObject parse failures now return exists=true with field_count=0 and a per-object error_message describing the parse failure, letting callers distinguish corrupt from missing; (4) docs/mcp.md adds requestId to the output table, adds it to both example responses, documents the new error_message field shape, and adds a third example showing the parse-error response; (5) added (h.1) .xml format test (regression guard for the required-flag bug), (h.2) .object format test, (i) parse-error test asserting exists=true + error_message, and (j) bare ".." connection_name test asserting the broadened message; (6) the PATH_TRAVERSAL message now reads "must not contain path separators or directory-traversal segments ('..')", covering both rejection conditions. 19/19 orgDescribe tests pass, full mocha 1174/1174, yarn lint clean, yarn compile clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/mcp.md | 36 ++++- src/mcp/tools/orgDescribeTools.ts | 53 +++++-- test/unit/mcp/orgDescribeTools.test.ts | 191 ++++++++++++++++++++++++- 3 files changed, 265 insertions(+), 15 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 00405b29..a4da48ab 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1715,12 +1715,14 @@ Read cached Salesforce describe data for one connection from the Provar workspac | `objects` | string[] | no | all | Filter — only return data for these object API names. When omitted, lists every object cached under the connection directory. | | `field_filter` | `'required'` \| `'all'` | no | `'required'` | Which fields to return. `'required'` includes only fields with `nillable=false`; `'all'` returns every cached field. | -| Output field | Description | -| -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `workspace_path` | Absolute resolved path to the discovered workspace, or `null` when none of the three candidate directories exists. | -| `cache_age_ms` | `mtime` delta in milliseconds of the connection cache directory, or `null` when the cache is missing. | -| `objects[]` | Array of `{ name, exists, required_fields, field_count }`. `exists` is `true` (cached), `false` (requested but not cached), or `null` (cache miss). | -| `details.suggestion` | Present **only** on cache miss. Tells the agent how to populate the cache (open Provar IDE) or how to proceed without it (inline hints). | +| Output field | Description | +| ------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `requestId` | UUID for this invocation. Echoed in MCP server logs for cross-correlation. Consistent with the rest of the MCP tool surface. | +| `workspace_path` | Absolute resolved path to the discovered workspace, or `null` when none of the three candidate directories exists (or all candidates were outside `--allowed-paths`). | +| `cache_age_ms` | `mtime` delta in milliseconds of the connection cache directory, or `null` when the cache is missing. | +| `objects[]` | Array of `{ name, exists, required_fields, field_count, error_message? }`. `exists` is `true` (cache file present), `false` (requested but not cached), or `null` (cache miss — the whole `.metadata/` directory is absent). | +| `objects[].error_message` | Present **only** when a cache file existed but failed to parse (`exists: true, field_count: 0`). Lets the agent distinguish a corrupt / unsupported cache file from a missing one. | +| `details.suggestion` | Present **only** on cache miss. Tells the agent how to populate the cache (open Provar IDE) or how to proceed without it (inline hints). | **Example — happy path:** @@ -1735,6 +1737,7 @@ Read cached Salesforce describe data for one connection from the Provar workspac // Response { + "requestId": "01HEXX...K7P", "workspace_path": "/Users/me/git/workspace-MyProject", "cache_age_ms": 1839200, "objects": [ @@ -1763,6 +1766,7 @@ Read cached Salesforce describe data for one connection from the Provar workspac ```jsonc // Response when the .metadata/ directory does not exist { + "requestId": "01HEXX...K7P", "workspace_path": "/Users/me/git/workspace-MyProject", "cache_age_ms": null, "objects": [{ "name": "Account", "exists": null, "required_fields": [], "field_count": 0 }], @@ -1772,6 +1776,26 @@ Read cached Salesforce describe data for one connection from the Provar workspac } ``` +**Example — parse error on a cached file:** + +```jsonc +// Response when Account.json exists but is corrupt / unparseable +{ + "requestId": "01HEXX...K7P", + "workspace_path": "/Users/me/git/workspace-MyProject", + "cache_age_ms": 1839200, + "objects": [ + { + "name": "Account", + "exists": true, + "required_fields": [], + "field_count": 0, + "error_message": "Failed to parse cache file (Account.json): Unexpected token } in JSON at position 42" + } + ] +} +``` + **On-disk cache schema (one file per object).** The tool reads `.json` first, then `.xml`, then `.object` as a fallback: ```jsonc diff --git a/src/mcp/tools/orgDescribeTools.ts b/src/mcp/tools/orgDescribeTools.ts index b5b273bf..26d52649 100644 --- a/src/mcp/tools/orgDescribeTools.ts +++ b/src/mcp/tools/orgDescribeTools.ts @@ -37,6 +37,8 @@ export interface OrgDescribeObject { exists: boolean | null; required_fields: OrgDescribeField[]; field_count: number; + /** Present only when the cache file existed but failed to parse. */ + error_message?: string; } export interface OrgDescribeResult { @@ -98,9 +100,26 @@ export function workspaceCandidates(projectPath: string): string[] { ]; } -/** Returns the first candidate workspace that exists, or null. */ -export function discoverWorkspace(projectPath: string): string | null { +/** + * Returns the first candidate workspace that exists AND is within allowedPaths, or null. + * + * Path policy is enforced PER CANDIDATE before any filesystem call: a candidate that + * sits outside `--allowed-paths` is silently skipped (it is not an error — discovery + * just moves on to the next). This means we never call fs.existsSync / fs.statSync + * against directories that the operator has explicitly placed off-limits, including + * the user-home fallback (~/Provar/...) when home sits outside the policy. + * + * When allowedPaths is empty (unrestricted mode), assertPathAllowed is a no-op and + * all candidates are probed exactly as before. + */ +export function discoverWorkspace(projectPath: string, allowedPaths: string[] = []): string | null { for (const candidate of workspaceCandidates(projectPath)) { + try { + assertPathAllowed(candidate, allowedPaths); + } catch { + // Candidate outside policy — skip without touching the filesystem. + continue; + } try { if (fs.existsSync(candidate) && fs.statSync(candidate).isDirectory()) { return candidate; @@ -142,13 +161,18 @@ function readXmlCacheFile(filePath: string): CachedObject { for (const f of fieldsRaw as Array>) { const name = (f['fullName'] ?? f['name']) as string | undefined; if (!name) continue; + // fast-xml-parser with parseTagValue=true (the default) coerces `true` + // into the boolean true; with parseTagValue=false it would stay as the string "true". + // Accept BOTH forms so we don't misclassify required fields as nillable on either path. + const requiredRaw = f['required']; + const isRequired = requiredRaw === true || requiredRaw === 'true'; fields.push({ name, type: (f['type'] as string | undefined) ?? 'unknown', defaultValue: (f['defaultValue'] as string | undefined) ?? null, // XML defaults: required = !nillable. In the .object format, "required" is rare, // so we default to nillable=true (optional) unless explicitly required. - nillable: f['required'] === 'true' ? false : true, + nillable: !isRequired, }); } return { name: path.basename(filePath, path.extname(filePath)), fields }; @@ -194,8 +218,19 @@ function readObject(connectionDir: string, objectName: string, fieldFilter: 'req try { cached = path.extname(file) === '.json' ? readJsonCacheFile(file) : readXmlCacheFile(file); } catch (e) { - log('warn', 'org_describe: failed to parse cache file', { file, error: (e as Error).message }); - return { name: objectName, exists: false, required_fields: [], field_count: 0 }; + const errorMessage = (e as Error).message; + log('warn', 'org_describe: failed to parse cache file', { file, error: errorMessage }); + // The cache file is present but unreadable. Report exists=true so the caller + // can distinguish "cache corrupt / unsupported format" from "object not cached" + // (exists=false). field_count=0 since we have no parsed fields, and error_message + // carries the underlying parse failure for diagnostics. + return { + name: objectName, + exists: true, + required_fields: [], + field_count: 0, + error_message: `Failed to parse cache file (${path.basename(file)}): ${errorMessage}`, + }; } const allFields = cached.fields ?? []; @@ -258,10 +293,12 @@ function resolveConnectionDir( // Reject path-shaped connection names outright. A real connection name from a // .testproject is an identifier (e.g. "MyOrg"); any separator or traversal // segment is almost certainly a misuse or injection attempt. - if (connectionName.includes('/') || connectionName.includes('\\') || connectionName.split(/[/\\]+/).includes('..')) { + const hasSeparator = connectionName.includes('/') || connectionName.includes('\\'); + const hasTraversal = connectionName === '..' || connectionName.split(/[/\\]+/).includes('..'); + if (hasSeparator || hasTraversal) { throw new PathPolicyError( 'PATH_TRAVERSAL', - `Invalid connection_name (contains path separators): ${connectionName}` + `Invalid connection_name (must not contain path separators or directory-traversal segments ('..')): ${connectionName}` ); } @@ -383,7 +420,7 @@ export function registerOrgDescribe(server: McpServer, config: ServerConfig): vo try { assertPathAllowed(args.project_path, config.allowedPaths); - const workspacePath = discoverWorkspace(args.project_path); + const workspacePath = discoverWorkspace(args.project_path, config.allowedPaths); const { connectionDir, resolvedWorkspace } = resolveConnectionDir( workspacePath, args.connection_name, diff --git a/test/unit/mcp/orgDescribeTools.test.ts b/test/unit/mcp/orgDescribeTools.test.ts index a8820f27..e0b851d6 100644 --- a/test/unit/mcp/orgDescribeTools.test.ts +++ b/test/unit/mcp/orgDescribeTools.test.ts @@ -65,6 +65,31 @@ function writeJsonCache(connectionDir: string, objectName: string, fields: Cache ); } +/** + * Write a legacy .object / .xml cache file (CustomObject metadata) with the given fields. + * `required` is emitted as the string "true" / "false" — but fast-xml-parser with the + * default parseTagValue=true will coerce it to a boolean before reaching the reader. + * The implementation must accept BOTH forms, which is what the legacy-format tests assert. + */ +function writeXmlCache( + connectionDir: string, + objectName: string, + fields: Array<{ name: string; type: string; required: boolean }>, + ext: '.xml' | '.object' = '.xml' +): void { + fs.mkdirSync(connectionDir, { recursive: true }); + const fieldsXml = fields + .map( + (f) => + ` \n ${f.name}\n ${f.type}\n ${String( + f.required + )}\n ` + ) + .join('\n'); + const xml = `\n\n${fieldsXml}\n\n`; + fs.writeFileSync(path.join(connectionDir, `${objectName}${ext}`), xml, 'utf-8'); +} + // ── Test setup ───────────────────────────────────────────────────────────────── let tmpRoot: string; @@ -179,7 +204,29 @@ describe('provar_org_describe — workspace discovery', () => { }); it('discoverWorkspace returns null when no candidate exists', () => { - assert.equal(discoverWorkspace(projectPath), null); + assert.equal(discoverWorkspace(projectPath, [tmpRoot]), null); + }); + + it('discoverWorkspace skips candidates outside allowedPaths without touching the filesystem', () => { + // Create a sibling workspace that DOES exist on disk but lies outside the allowed root. + // We force this by creating a parallel tmp tree and only allowing tmpRoot. + // The sibling pattern resolves to /workspace-; the parent of + // projectPath is tmpRoot, so the sibling itself is inside the allowed set — + // which is what we want for the happy case. To exercise the policy skip we use + // a separate "outside" project path whose sibling candidate lives outside. + const outsideRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'org-describe-outside-'))); + try { + const outsideProject = path.join(outsideRoot, 'OtherProject'); + fs.mkdirSync(outsideProject, { recursive: true }); + const outsideSibling = path.join(outsideRoot, 'workspace-OtherProject'); + fs.mkdirSync(outsideSibling, { recursive: true }); + + // With only tmpRoot allowed, discoverWorkspace MUST NOT return the outside sibling + // even though it exists on disk. + assert.equal(discoverWorkspace(outsideProject, [tmpRoot]), null); + } finally { + fs.rmSync(outsideRoot, { recursive: true, force: true }); + } }); }); @@ -403,3 +450,145 @@ describe('provar_org_describe — objects filter', () => { assert.equal(ghost.exists, false, 'object not in cache → exists=false'); }); }); + +// ── (h) Legacy cache formats — .xml / .object ──────────────────────────────── + +describe('provar_org_describe — legacy cache formats', () => { + it('(h.1) parses .xml CustomObject metadata and classifies required vs nillable correctly', () => { + // Regression guard for the required-flag bug: fast-xml-parser's default + // parseTagValue=true coerces "true" to the boolean true. + // The reader must treat boolean and string forms identically. + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeXmlCache( + connectionDir, + 'Account', + [ + { name: 'Name', type: 'string', required: true }, + { name: 'Phone', type: 'phone', required: false }, + ], + '.xml' + ); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account'], + field_filter: 'all', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + required_fields: Array<{ name: string; nillable: boolean }>; + field_count: number; + }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 2); + const byName = new Map(objects[0].required_fields.map((f) => [f.name, f.nillable])); + assert.equal( + byName.get('Name'), + false, + 'required field should have nillable=false (NOT misclassified as nillable)' + ); + assert.equal(byName.get('Phone'), true, 'non-required field should have nillable=true'); + }); + + it('(h.2) parses .object CustomObject metadata (legacy Eclipse layout)', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeXmlCache( + connectionDir, + 'Contact', + [ + { name: 'LastName', type: 'string', required: true }, + { name: 'Email', type: 'email', required: false }, + ], + '.object' + ); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Contact'], + field_filter: 'required', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + required_fields: Array<{ name: string }>; + field_count: number; + }>; + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 2, 'field_count counts all parsed fields, regardless of filter'); + // field_filter='required' → only nillable=false survives + const names = objects[0].required_fields.map((f) => f.name); + assert.deepEqual(names, ['LastName'], 'only the required field should pass the filter'); + }); +}); + +// ── (i) Parse-error reporting ───────────────────────────────────────────────── + +describe('provar_org_describe — parse errors', () => { + it('(i) returns exists=true with error_message when a cache file is corrupt', () => { + // A cache file that EXISTS but does not parse must NOT be reported as exists=false + // (that would conflate "not cached" with "corrupt") — the contract is exists=true, + // field_count=0, error_message set. + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + fs.mkdirSync(connectionDir, { recursive: true }); + fs.writeFileSync(path.join(connectionDir, 'Account.json'), '{ not valid json', 'utf-8'); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account'], + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + field_count: number; + error_message?: string; + }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].exists, true, 'corrupt cache file is "present" — not missing'); + assert.equal(objects[0].field_count, 0); + const errMsg = objects[0].error_message; + assert.ok(errMsg, 'error_message should describe the parse failure'); + assert.ok(errMsg.includes('Account.json'), `error_message should reference the file name; got: ${errMsg}`); + }); +}); + +// ── (j) Connection-name traversal — bare `..` ───────────────────────────────── + +describe('provar_org_describe — connection_name validation', () => { + it('(j) rejects bare ".." connection_name with PATH_TRAVERSAL and a clear message', () => { + // Regression guard for the broadened error message: the validator rejects `..` + // even when no separator is present. The message must mention BOTH conditions. + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true }); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: '..', + }); + + assert.equal(isError(result), true); + const body = parseText(result); + assert.equal(body['error_code'], 'PATH_TRAVERSAL'); + const msg = body['message'] as string; + assert.ok( + /path separators/i.test(msg) && msg.includes('..'), + `error message should mention both path separators and '..'; got: ${msg}` + ); + }); +});