mirror of
https://github.com/ruvnet/ruflo.git
synced 2026-10-23 03:47:00 +09:00
* dream(security): #3138 opt-in MCP governance policy enforcement (evaluated, ACCEPT) .harness/mcp-policy.json declared auditLog/maxToolCallsPerTurn/dangerousPatterns but was only ever read by the offline metaharness mcp-scan linter -- the actual running claude-flow MCP server (mcp-server.ts) never consulted it, so any connected client could call every tool with no audit trail and no call budget. Adds an opt-in PolicyEnforcer (RUFLO_MCP_ENFORCE_POLICY=1, default off) wiring auditLog + maxToolCallsPerTurn into the real tools/call dispatch path. Default behavior is unconditionally unchanged. 16/16 new tests, 39/43 broader MCP regression sweep (4 failures traced to pre-existing unbuilt sibling packages, unrelated to this change). Independent adversarial critique found one real, disclosed limitation (maxToolCallsPerTurn is session-lifetime cumulative, not true per-turn) -- documented in the module, gist, and issue; not a blocker for landing opt-in/default-off. Full research, hypothesis, evaluation receipt, and witness: issue #3138 and docs/dream-cycle/dream-gist-2026-08-31.md. Co-Authored-By: RuFlo <ruv@ruv.net> Claude-Session: https://claude.ai/code/session_01QqhXjtDofNNJPX3CCVQyrk * dream(security): fail-closed on policy load/audit-log failure (review round 1) Addresses ruvnet's PR review on #3139: an enforcement feature that silently falls back to unrestricted execution when its own config is missing/malformed, or when a mandatory audit-log write fails, isn't real enforcement. - loadMcpPolicy() returning null (missing/malformed .harness/mcp-policy.json) now denies the call instead of falling through unconstrained. - A failed appendAuditLog() write, when policy.auditLog is true, now denies the call instead of silently proceeding unaudited. - New evaluateToolCall() is the single enforcement entry point combining fail-closed policy loading + budget + fail-closed audit logging; mcp-server.ts's tools/call wiring simplified to call it directly. - 8 new tests: fail-closed on null policy, fail-closed on malformed JSON on disk (server-integration level), fail-closed on unwritable audit log (server-integration level + unit level), concurrent-call budget accounting. 24/24 passing. Two of the review's five findings remain open by design, not silently dropped: the maxToolCallsPerTurn session-lifetime-vs-per-turn semantic gap (needs a real TTL/reset design) and the stdio-only enforcement scope (HTTP/ WS transport lives in a separate @claude-flow/mcp package) — both already documented in this module's own doc comment and flagged as follow-up in the PR/issue, not attempted here to keep this round's diff reviewable. Getting the repo's other currently-broken workflows (CI/CD Pipeline, CVE Audit Gate, Cross-Agent Integration Tests, V3 CI, Verification Pipeline) green is out of scope for this PR — confirmed pre-existing and base-branch- wide (see PR comment), unrelated to this diff. Co-Authored-By: RuFlo <ruv@ruv.net> Claude-Session: https://claude.ai/code/session_01QqhXjtDofNNJPX3CCVQyrk --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
rUvandClaude authored and GitHub committed
1 parent
0db381f7f7
commit
e7fb8edc19
5 files changed
+583
-1
No files matched your search
@@ -170,7 +170,7 @@ as a first-class, high-priority candidate, not a background note.
|
||||
| 2026-08-28 | memory | `HybridBackend.queryHybrid()`'s dead `weights` field (computed, never consumed by combineUnion) replaced with real weighted-RRF fusion (`combineWeighted`), matching Qdrant/Weaviate/Azure AI Search's weighted-RRF pattern | #3118 | #3119 | yes | ACCEPT | 461/462 suite green (1 pre-existing unrelated env failure, confirmed identical on baseline); weight-sensitivity test fails on baseline, passes on candidate (git-stash isolated); post-review: added finite/non-negative weight validation + deterministic id-based tie-break | 47e2f669... | MERGED 2026-09-14 (verified via main's own commit fde5b3b) |
|
||||
| 2026-08-29 | swarm | `TopologyManager.rebalanceHybrid()`'s worker-mesh loop added edges one-directionally (worker→target only, never reciprocated) — same function's coordinator loop and all 3 sibling rebalance*() methods do this correctly; flagged live by 08-24's own night, fixed tonight | #3122 | #3123 | yes | ACCEPT | deterministic (Math.random mocked) regression test: baseline missing reciprocal edge, candidate symmetric; full package 221/221 tests, 0 regressions; independent adversarial critic CONFIRMED | 9c1e9cd4... | rebased onto main (post #3086/#3094/#3103/#3110/#3119 merges), awaiting fresh CI as of 2026-09-14 |
|
||||
| 2026-08-30 | performance | `diskann-backend.ts` (375 lines, zero call sites anywhere in the monorepo, `@ruvector/diskann` not a dependency) removed; `claudemd-generator.ts`'s generated CLAUDE.md corrected from the unreproduced "150x-12,500x"/DiskANN claim to point at the reproducible `benchmark-intelligence.mjs --only=hnsw` command instead of a hardcoded multiplier | #3129 | #3130 | yes | ACCEPT | baseline-fails/candidate-passes test confirmed via git-stash; full suite byte-identical 106-failure list baseline vs candidate (pre-existing env gap); live re-run of benchmark-intelligence.mjs found recall@10 0.9867(N=5k)/0.9233(N=20k) — itself disagreeing with the "~0.99" doc figure, reported not papered over; reviewer REJECTed v1 (CI red, 11-row ledger backfill, ambiguous cited number) — all 3 addressed same night | 6f56440a... | rebased onto main (post #3086/#3094/#3103/#3110/#3119/#3123 merges), awaiting fresh CI as of 2026-09-14 |
|
||||
| 2026-08-31 | security | MCP governance policy (.harness/mcp-policy.json) was never enforced at runtime — opt-in enforcement wired in | #3138 | #3139 | yes | ACCEPT | see issue #3138 | n/a (backfill) | unmerged draft as of 09-05 |
|
||||
| 2026-08-31 | security | MCP `tools/call` dispatch never read `.harness/mcp-policy.json` (existed only for offline `metaharness mcp-scan` linting) — opt-in `PolicyEnforcer` (env `RUFLO_MCP_ENFORCE_POLICY`) wires `auditLog` + `maxToolCallsPerTurn` into the live stdio dispatch, fail-closed on policy load/audit-log failure (post-review) | #3138 | #3139 | yes | ACCEPT | 16/16 new tests, 39/43 broader MCP sweep (4 pre-existing unbuilt-sibling-package env failures, unrelated) | 670cbbad... | rebased onto main (post #3086/#3094/#3103/#3110/#3119/#3123/#3130 merges), awaiting fresh CI as of 2026-09-14 |
|
||||
| 2026-09-01 | security | maxToolCallsPerTurn was a session-lifetime cumulative cap that never reset — sliding-window reset implemented | #3151 | #3152 | yes | ACCEPT | see issue #3151 | n/a (backfill) | unmerged draft as of 09-05 |
|
||||
| 2026-09-02 | intelligence | LearningBridge.consolidate() was reward-blind — fixed to weight consolidation by trajectory reward | #3159 | #3160 | yes | ACCEPT | see issue #3159 | n/a (backfill) | unmerged draft as of 09-05 |
|
||||
| 2026-09-03 | memory | SmartRetrieval's MMR diversity step used token-Jaccard similarity instead of the available embedding-cosine | #3168 | #3169 | yes | ACCEPT | see issue #3168 | n/a (backfill) | unmerged draft as of 09-05 |
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
# Security SOTA Report — 2026-08-31
|
||||
|
||||
TL;DR: In 2026, every major agent framework (LangGraph, OpenAI Agents SDK, Google ADK) and the MCP spec itself (2026-07-28) converged on the same shape — a synchronous pre-execution interception point that checks a declarative tool-call policy before dispatch, plus a separate audit-emission step. Ruflo already authored exactly this policy schema (`.harness/mcp-policy.json`, ADR-150) months ago, but tonight's code audit confirmed the running `claude-flow` MCP server (`mcp-server.ts`) never read it — any connected client could call any tool with zero audit trail and zero call budget. Tonight's candidate wires the two policy fields that are in this server's actual jurisdiction (`auditLog`, `maxToolCallsPerTurn`) into the real `tools/call` dispatch path, fully opt-in, evaluated with a real deterministic $0 test suite (16/16 passing) plus a broader 43-test MCP regression sweep (39/43 passing, 4 failures traced to pre-existing unbuilt sibling packages, not this candidate).
|
||||
|
||||
## What's New in 2026
|
||||
|
||||
| Finding | Source | Confidence |
|
||||
|---|---|---|
|
||||
| MCP spec requires servers to rate-limit tool calls and log usage; no wire-level allow/deny primitive — left to implementers | modelcontextprotocol.io/specification/2026-07-28/server/tools | A |
|
||||
| OWASP Agentic Skills Top 10 (2026) recommends default-deny allowlists + structured audit logging as baseline platform guidance; no spend-cap guidance anywhere | owasp.org/www-project-agentic-skills-top-10 | A |
|
||||
| "Before the Tool Call" (OAP spec): synchronous pre-action interception vs. declarative policy cuts empirical attack success from 74.6% (permissive) to 0% (restrictive), 879 attempts | Uchibeke, arXiv:2603.20953 (2026-03-21) | B |
|
||||
| Google ADK `before_tool_callback` intercepts every dispatch pre-execution, can block/substitute; org policy via Runner Plugins, per-agent via local callbacks (two-tier split) | google/adk-docs (official) | A |
|
||||
| LangGraph `interrupt_on`: declarative per-tool-name → interrupt-config map, requires a checkpointer | docs.langchain.com (official) | A |
|
||||
| CrewAI OSS core has **no** standardized pre-tool-call authorization provider contract — open, unresolved GitHub issue, not shipped | github.com/crewAIInc/crewAI issue #4877 | A |
|
||||
| OpenAI Agents SDK ships tool-call guardrails (block/replace/tripwire) but no native spend/budget concept, confirmed by two independent write-ups | dev.to/pat9000; runcycles.io | B |
|
||||
| mcp-firewall (OSS MCP gateway): default-deny RBAC + Ed25519-signed hash-chained audit trail + fail-closed approval — the closest complete prior-art implementation of what Ruflo's policy file already declares | github.com/ressl/mcp-firewall (fetched directly) | A |
|
||||
|
||||
## Ruflo Current Capability
|
||||
|
||||
`.harness/mcp-policy.json` (ADR-150 iter 30) declares `defaultDeny: true`, `auditLog: true`, `requireApprovalForDangerous: true`, `maxToolCallsPerTurn: 200`, and a `dangerousPatterns` list — but it existed solely to satisfy `metaharness harness mcp-scan`'s offline linting. Direct grep of `v3/@claude-flow/cli/src/mcp-server.ts` (the code `npx ruflo mcp start` actually runs) confirmed zero references to `policy`/`allow`/`deny`/`approval`/`dangerous`/`auditLog`/`toolTimeout`/`maxToolCalls` anywhere in the file, before tonight. The policy file's own rationale comment scopes `dangerousPatterns`/`allowShell`/`allowNetwork`/`allowFileWrite` to a *different* layer (native Claude Code tools — Bash/Write/Edit/WebFetch — not this MCP server's `memory_*`/`hooks_*`/`agentdb_*` tool surface), so tonight's fix targets only the two fields genuinely in this server's jurisdiction: `auditLog` and `maxToolCallsPerTurn`.
|
||||
|
||||
## Competitor Comparison
|
||||
|
||||
| Competitor | Runtime tool-call enforcement? | Audit logging? | Approval gate? | Grade |
|
||||
|---|---|---|---|---|
|
||||
| LangGraph | Yes — `interrupt()` in the executor itself | No native log | Yes (HITL pause) | B |
|
||||
| OpenAI Agents SDK | Yes — guardrails run on every call, can raise a tripwire | No native log | Yes, developer-configured | A |
|
||||
| Google ADK | Yes — `before_tool_callback` real interception point | Not built-in | Developer-built | B |
|
||||
| CrewAI (OSS core) | No — confirmed absent; RBAC/audit/HITL exist only in paid Enterprise AMP | Enterprise-only | Enterprise-only | B |
|
||||
| Anthropic MCP reference SDK | No — spec calls for consent, reference SDKs ship permissive defaults by design | No | Elicitation exists as a capability, not enforced | B |
|
||||
| mcp-firewall (OSS gateway) | Yes, exactly this spec — default-deny, fail-closed | Yes, signed & hash-chained | Yes | A |
|
||||
|
||||
Synthesis: **not a first-mover gap.** Ruflo is catching up to a pattern that's split between partial native hooks in the major frameworks and a dedicated "MCP gateway" OSS category (mcp-firewall, Enkrypt AI, MCPGuard, protect-mcp, agentgateway) that already ships the full spec. The honest framing is "adopting a proven pattern," not inventing one — and even the frameworks with native interception (LangGraph/OpenAI SDK/ADK) still lack built-in audit logging, which stays a real differentiator if Ruflo ships it by default once wired.
|
||||
|
||||
## Hypothesis
|
||||
|
||||
> Given the claude-flow MCP server's `tools/call` dispatch handler, when an opt-in `PolicyEnforcer` (env `RUFLO_MCP_ENFORCE_POLICY=1`) reads `auditLog` and `maxToolCallsPerTurn` from `.harness/mcp-policy.json` and enforces them at the point of dispatch, then every tool call should be audit-logged and per-session call volume should be bounded, relative to today's completely unenforced baseline, subject to: (1) default behavior (flag unset) is byte-for-byte unchanged; (2) existing MCP-related tests remain green; (3) zero LLM/API cost.
|
||||
|
||||
Frozen before evaluation began; not modified after seeing results.
|
||||
|
||||
## Benchmarks / Evaluation
|
||||
|
||||
**evaluated: accepted.** Real evaluator: `vitest run`, deterministic, $0, zero LLM calls.
|
||||
|
||||
- New suite `__tests__/mcp-policy-enforcer.test.ts`: **16/16 passing** — unit coverage of the policy loader (missing file, malformed JSON → both fail open, never throw), the per-session budget counter (allow-up-to-limit, deny-beyond, independent per session), the audit-log writer (no-op when `auditLog` false, one JSONL line per call, never throws on an unwritable path), and 3 integration tests that instantiate the real `MCPServerManager` and call its actual `handleMCPMessage('tools/call', ...)` (with `mcp-client.js`'s ~300-tool registry mocked out for isolation/speed): default-flag-unset path (5/5 calls succeed, identical to pre-existing behavior), enabled-but-no-policy-file (fails open), enabled-with-budget=1 (1st call allowed, 2nd denied with a policy error).
|
||||
- Broader regression sweep, all 11 MCP-related test files in the package: **39/43 passing**. The 4 failures (`mcp-tools-deep.test.ts`, `mcp-client.test.ts`, `mcp-client-guardrail.test.ts`, `issue-2612-mcp-rename.test.ts`, plus the 2 e2e HTTP tests) all trace to unbuilt sibling packages in this fresh checkout (`@claude-flow/cli-core/dist`, `@claude-flow/neural/dist`, `@claude-flow/cli/dist` — all confirmed missing via direct `test -d` before touching any code), none of which import `policy-enforcer.ts` or the modified `tools/call` branch. Same class of environmental gap disclosed by multiple prior dream-cycle nights (e.g. 2026-08-17's "unbuilt sibling package @claude-flow/cli-core").
|
||||
- `@claude-flow/mcp` (a different sibling, imported by an unrelated pre-existing method `startHttpServer()` in the same file) was also unbuilt and broke Vite's static transform of the whole module for every test importing it; built with a plain `tsc` (clean, no errors) before running tests — a legitimate one-time environment fix, not a candidate change.
|
||||
|
||||
## Darwin Results
|
||||
|
||||
Skipped — scope mismatch, same class as 5 of the last 6 dream-cycle nights. This is a binary/config-wiring correctness fix (either the two policy fields are consulted at dispatch or they aren't), not a continuous/categorical parameter; `@metaharness/darwin`'s real interface (`ruvector harness darwin <config> --execute`) evolves scoped tunable parameters against a benchmark corpus.
|
||||
|
||||
## SOTA Proof & Witness
|
||||
|
||||
| Field | Value |
|
||||
|---|---|
|
||||
| Session commit | `29f048fc3b556f857cf2b126d2a84c19d2daa0d0` |
|
||||
| Report SHA-256 (pre-witness content) | `329d372953c27d8e6ff4471d11ebeb44fd7434b137a9e70f37c4741310e1bf88` |
|
||||
| Witness stamp | `670cbbad1a7ecddc562f0c4abb5aebb5d367834ea10d3ce350118eeacc7ab8ea` |
|
||||
| Evaluation receipt | `__tests__/mcp-policy-enforcer.test.ts`, 16/16 passing (committed on this branch) |
|
||||
| Flywheel evidence | None — no `.claude-flow/flywheel/` state in this repo; no signed `@metaharness/flywheel` bundle (bespoke deterministic vitest suite, not an LLM-task corpus the replay tooling targets) |
|
||||
| Darwin lineage | None — skipped, scope mismatch (see Darwin Results) |
|
||||
|
||||
Verifier procedure (same as every prior dream-cycle night, e.g. #3034/#3044): the SHA-256 above was computed over this report's content *before* this Witness section held its final values (unavoidably self-referential otherwise — hashing a file that contains its own hash). To reproduce: take this file's content with the Witness table's three value cells blanked back out, SHA-256 it, concatenate with the session commit, SHA-256 again — must equal the witness stamp.
|
||||
|
||||
## Recommended Next Steps
|
||||
|
||||
1. **Land tonight's opt-in wiring**, then flip `RUFLO_MCP_ENFORCE_POLICY` on by default in a follow-up night once the audit-log volume/rotation story and the `sessionId`-forgery question (flagged by the adversarial critic — see PR) are separately resolved; shipping audit-by-default is the one dimension where competitors (bar mcp-firewall) are still weak.
|
||||
2. **A future security night should scope `dangerousPatterns`/`requireApprovalForDangerous` at the correct layer** — Ruflo's native-Claude-Code-tool surface (Bash/Write/Edit/WebFetch), not this MCP server, per the policy file's own rationale; that is a materially different, larger piece of work deliberately left out of tonight's patch.
|
||||
3. **Do not re-open the "AgentDB memory poisoning" security direction** (ADR-381/ADR-382, researched 2026-07-31/08-01/08-06/08-11, code never implemented) as a 5th consecutive rehash without new evidence — tonight intentionally picked an orthogonal, previously-flagged-but-unactioned direction (MCP governance, surfaced by 2026-08-19's swarm-night scan) instead, per STEP 1.1's duplicate-direction rule.
|
||||
@@ -0,0 +1,302 @@
|
||||
/**
|
||||
* Opt-in MCP governance policy enforcement (dream-cycle 2026-08-31, security).
|
||||
*
|
||||
* `.harness/mcp-policy.json` declared `auditLog` / `maxToolCallsPerTurn` but
|
||||
* nothing in the running MCP server (`mcp-server.ts`) ever read it before
|
||||
* this change — every connected client could call every tool with no audit
|
||||
* trail and no call budget. These tests cover the new opt-in enforcement
|
||||
* module directly, plus the wiring into `MCPServerManager`'s `tools/call`
|
||||
* dispatch via a mocked `mcp-client.js` (avoids loading the real 300+ tool
|
||||
* registry).
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import {
|
||||
isPolicyEnforcementEnabled,
|
||||
loadMcpPolicy,
|
||||
checkAndRecordCall,
|
||||
appendAuditLog,
|
||||
evaluateToolCall,
|
||||
resetPolicyEnforcerState,
|
||||
setAuditLogPathForTesting,
|
||||
getAuditLogPath,
|
||||
type McpPolicy,
|
||||
} from '../src/mcp-tools/policy-enforcer.js';
|
||||
|
||||
describe('isPolicyEnforcementEnabled', () => {
|
||||
it('is disabled by default (unset env)', () => {
|
||||
expect(isPolicyEnforcementEnabled({})).toBe(false);
|
||||
});
|
||||
it('is disabled for any value other than "1"/"true"', () => {
|
||||
expect(isPolicyEnforcementEnabled({ RUFLO_MCP_ENFORCE_POLICY: '0' })).toBe(false);
|
||||
expect(isPolicyEnforcementEnabled({ RUFLO_MCP_ENFORCE_POLICY: 'no' })).toBe(false);
|
||||
});
|
||||
it('is enabled for "1" or "true" (case-insensitive)', () => {
|
||||
expect(isPolicyEnforcementEnabled({ RUFLO_MCP_ENFORCE_POLICY: '1' })).toBe(true);
|
||||
expect(isPolicyEnforcementEnabled({ RUFLO_MCP_ENFORCE_POLICY: 'true' })).toBe(true);
|
||||
expect(isPolicyEnforcementEnabled({ RUFLO_MCP_ENFORCE_POLICY: 'TRUE' })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadMcpPolicy', () => {
|
||||
it('loads the repo policy file successfully', () => {
|
||||
const policy = loadMcpPolicy(path.join(process.cwd(), '..', '..', '..', '.harness', 'mcp-policy.json'));
|
||||
// repo root is 3 levels up from v3/@claude-flow/cli when tests run from the package dir
|
||||
expect(policy === null || typeof policy === 'object').toBe(true);
|
||||
});
|
||||
it('returns null for a missing file (never throws)', () => {
|
||||
expect(loadMcpPolicy('/nonexistent/path/mcp-policy.json')).toBeNull();
|
||||
});
|
||||
it('returns null for malformed JSON (never throws)', () => {
|
||||
const tmp = path.join(os.tmpdir(), `bad-policy-${Date.now()}.json`);
|
||||
fs.writeFileSync(tmp, '{ not valid json', 'utf-8');
|
||||
try {
|
||||
expect(loadMcpPolicy(tmp)).toBeNull();
|
||||
} finally {
|
||||
fs.unlinkSync(tmp);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkAndRecordCall — per-session budget', () => {
|
||||
beforeEach(() => resetPolicyEnforcerState());
|
||||
|
||||
it('allows unlimited calls when maxToolCallsPerTurn is absent/invalid', () => {
|
||||
const policy: McpPolicy = {};
|
||||
for (let i = 0; i < 5; i++) {
|
||||
expect(checkAndRecordCall(policy, 's1').allowed).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('allows calls up to the limit, denies the (limit+1)th', () => {
|
||||
const policy: McpPolicy = { maxToolCallsPerTurn: 3 };
|
||||
expect(checkAndRecordCall(policy, 's1').allowed).toBe(true);
|
||||
expect(checkAndRecordCall(policy, 's1').allowed).toBe(true);
|
||||
expect(checkAndRecordCall(policy, 's1').allowed).toBe(true);
|
||||
const denied = checkAndRecordCall(policy, 's1');
|
||||
expect(denied.allowed).toBe(false);
|
||||
expect(denied.reason).toMatch(/maxToolCallsPerTurn/);
|
||||
});
|
||||
|
||||
it('tracks sessions independently', () => {
|
||||
const policy: McpPolicy = { maxToolCallsPerTurn: 1 };
|
||||
expect(checkAndRecordCall(policy, 'a').allowed).toBe(true);
|
||||
expect(checkAndRecordCall(policy, 'a').allowed).toBe(false);
|
||||
expect(checkAndRecordCall(policy, 'b').allowed).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('appendAuditLog', () => {
|
||||
let logFile: string;
|
||||
beforeEach(() => {
|
||||
logFile = path.join(os.tmpdir(), `mcp-audit-test-${Date.now()}-${Math.random()}.jsonl`);
|
||||
setAuditLogPathForTesting(logFile);
|
||||
});
|
||||
afterEach(() => {
|
||||
setAuditLogPathForTesting(null);
|
||||
if (fs.existsSync(logFile)) fs.unlinkSync(logFile);
|
||||
});
|
||||
|
||||
it('does nothing and reports success (nothing was required) when policy.auditLog is not true', () => {
|
||||
expect(appendAuditLog({ auditLog: false }, {
|
||||
timestamp: new Date().toISOString(), sessionId: 's', toolName: 't', allowed: true,
|
||||
})).toBe(true);
|
||||
expect(fs.existsSync(logFile)).toBe(false);
|
||||
});
|
||||
|
||||
it('appends one JSONL record per call when auditLog is true, and reports success', () => {
|
||||
const policy: McpPolicy = { auditLog: true };
|
||||
expect(appendAuditLog(policy, { timestamp: 't1', sessionId: 's1', toolName: 'memory_store', allowed: true })).toBe(true);
|
||||
expect(appendAuditLog(policy, { timestamp: 't2', sessionId: 's1', toolName: 'memory_store', allowed: false, reason: 'denied' })).toBe(true);
|
||||
const lines = fs.readFileSync(logFile, 'utf-8').trim().split('\n');
|
||||
expect(lines).toHaveLength(2);
|
||||
expect(JSON.parse(lines[0])).toMatchObject({ toolName: 'memory_store', allowed: true });
|
||||
expect(JSON.parse(lines[1])).toMatchObject({ allowed: false, reason: 'denied' });
|
||||
});
|
||||
|
||||
it('never throws, and reports failure, when the log path is unwritable', () => {
|
||||
setAuditLogPathForTesting('/nonexistent-dir-xyz/audit.jsonl');
|
||||
let result: boolean | undefined;
|
||||
expect(() => {
|
||||
result = appendAuditLog({ auditLog: true }, {
|
||||
timestamp: 't', sessionId: 's', toolName: 'x', allowed: true,
|
||||
});
|
||||
}).not.toThrow();
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('evaluateToolCall — single enforcement entry point (fail-closed)', () => {
|
||||
let logFile: string;
|
||||
beforeEach(() => {
|
||||
resetPolicyEnforcerState();
|
||||
logFile = path.join(os.tmpdir(), `mcp-audit-eval-${Date.now()}-${Math.random()}.jsonl`);
|
||||
setAuditLogPathForTesting(logFile);
|
||||
});
|
||||
afterEach(() => {
|
||||
setAuditLogPathForTesting(null);
|
||||
if (fs.existsSync(logFile)) fs.unlinkSync(logFile);
|
||||
});
|
||||
|
||||
it('denies with a clear reason when policy is null (missing/malformed file) — fail-closed', () => {
|
||||
const result = evaluateToolCall(null, 's1', 'memory_store');
|
||||
expect(result.allowed).toBe(false);
|
||||
expect(result.reason).toMatch(/missing or invalid.*failing closed/i);
|
||||
});
|
||||
|
||||
it('allows a call within budget with a working audit log', () => {
|
||||
const result = evaluateToolCall({ auditLog: true, maxToolCallsPerTurn: 5 }, 's1', 'memory_store');
|
||||
expect(result.allowed).toBe(true);
|
||||
expect(fs.readFileSync(logFile, 'utf-8').trim().split('\n')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('denies once the session budget is exhausted, and still logs the denial', () => {
|
||||
const policy: McpPolicy = { auditLog: true, maxToolCallsPerTurn: 1 };
|
||||
expect(evaluateToolCall(policy, 's1', 'memory_store').allowed).toBe(true);
|
||||
const second = evaluateToolCall(policy, 's1', 'memory_store');
|
||||
expect(second.allowed).toBe(false);
|
||||
expect(second.reason).toMatch(/maxToolCallsPerTurn/);
|
||||
const lines = fs.readFileSync(logFile, 'utf-8').trim().split('\n');
|
||||
expect(lines).toHaveLength(2);
|
||||
expect(JSON.parse(lines[1]).allowed).toBe(false);
|
||||
});
|
||||
|
||||
it('denies — fail-closed — when auditLog is required but the write fails, even though budget allows the call', () => {
|
||||
setAuditLogPathForTesting('/nonexistent-dir-xyz/audit.jsonl');
|
||||
const result = evaluateToolCall({ auditLog: true, maxToolCallsPerTurn: 5 }, 's1', 'memory_store');
|
||||
expect(result.allowed).toBe(false);
|
||||
expect(result.reason).toMatch(/audit log write failed.*failing closed/i);
|
||||
});
|
||||
|
||||
it('does not require a working audit log when policy.auditLog is not set', () => {
|
||||
setAuditLogPathForTesting('/nonexistent-dir-xyz/audit.jsonl');
|
||||
const result = evaluateToolCall({ maxToolCallsPerTurn: 5 }, 's1', 'memory_store');
|
||||
expect(result.allowed).toBe(true);
|
||||
});
|
||||
|
||||
it('handles concurrent calls for the same session without double-allowing past the budget', async () => {
|
||||
const policy: McpPolicy = { maxToolCallsPerTurn: 3 };
|
||||
// evaluateToolCall is synchronous end-to-end (no await inside), so
|
||||
// Promise.all over synchronous calls cannot interleave — this pins
|
||||
// that invariant rather than exercising real concurrency.
|
||||
const results = await Promise.all(
|
||||
Array.from({ length: 5 }, () => Promise.resolve(evaluateToolCall(policy, 'concurrent-session', 'memory_store'))),
|
||||
);
|
||||
expect(results.filter(r => r.allowed)).toHaveLength(3);
|
||||
expect(results.filter(r => !r.allowed)).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAuditLogPath', () => {
|
||||
afterEach(() => setAuditLogPathForTesting(null));
|
||||
it('defaults to a path under the OS tmpdir', () => {
|
||||
expect(getAuditLogPath().startsWith(os.tmpdir())).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// --- Integration: MCPServerManager.tools/call dispatch wiring ---
|
||||
// mcp-client.js pulls in the full ~300-tool registry; mock it so this test
|
||||
// stays fast, deterministic, and isolated to the new dispatch-path wiring.
|
||||
vi.mock('../src/mcp-client.js', () => ({
|
||||
listMCPTools: () => [{ name: 'demo_tool', description: 'demo', inputSchema: {} }],
|
||||
hasTool: (name: string) => name === 'demo_tool',
|
||||
callMCPTool: vi.fn(async () => ({ ok: true })),
|
||||
}));
|
||||
|
||||
describe('MCPServerManager tools/call — policy wiring', () => {
|
||||
const ORIGINAL_ENV = process.env.RUFLO_MCP_ENFORCE_POLICY;
|
||||
let policyPath: string;
|
||||
|
||||
beforeEach(() => {
|
||||
resetPolicyEnforcerState();
|
||||
setAuditLogPathForTesting(path.join(os.tmpdir(), `mcp-audit-integration-${Date.now()}.jsonl`));
|
||||
policyPath = path.join(os.tmpdir(), `mcp-policy-integration-${Date.now()}.json`);
|
||||
});
|
||||
afterEach(() => {
|
||||
if (ORIGINAL_ENV === undefined) delete process.env.RUFLO_MCP_ENFORCE_POLICY;
|
||||
else process.env.RUFLO_MCP_ENFORCE_POLICY = ORIGINAL_ENV;
|
||||
setAuditLogPathForTesting(null);
|
||||
if (fs.existsSync(policyPath)) fs.unlinkSync(policyPath);
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
async function dispatch(server: any, toolName: string, sessionId: string) {
|
||||
return server.handleMCPMessage(
|
||||
{ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: toolName, arguments: {} } },
|
||||
sessionId,
|
||||
);
|
||||
}
|
||||
|
||||
it('default (flag unset): every call succeeds, no budget enforced — unchanged from pre-existing behavior', async () => {
|
||||
delete process.env.RUFLO_MCP_ENFORCE_POLICY;
|
||||
const { MCPServerManager } = await import('../src/mcp-server.js');
|
||||
const server = new (MCPServerManager as any)();
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const res = await dispatch(server, 'demo_tool', 'sess-default');
|
||||
expect(res.error).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it('enabled + no policy file found: denies — fail-closed on missing config (PR #3139 review round 1)', async () => {
|
||||
process.env.RUFLO_MCP_ENFORCE_POLICY = '1';
|
||||
const mod = await import('../src/mcp-tools/policy-enforcer.js');
|
||||
const spy = vi.spyOn(mod, 'loadMcpPolicy').mockReturnValue(null);
|
||||
const { MCPServerManager } = await import('../src/mcp-server.js');
|
||||
const server = new (MCPServerManager as any)();
|
||||
const res = await dispatch(server, 'demo_tool', 'sess-nopolicy');
|
||||
expect(res.error).toBeDefined();
|
||||
expect(res.error.message).toMatch(/Policy denied.*missing or invalid.*failing closed/i);
|
||||
spy.mockRestore();
|
||||
});
|
||||
|
||||
it('enabled + malformed policy JSON on disk: denies — fail-closed', async () => {
|
||||
process.env.RUFLO_MCP_ENFORCE_POLICY = '1';
|
||||
fs.writeFileSync(policyPath, '{ not valid json', 'utf-8');
|
||||
// Confirm the disk content really is malformed via the real loader
|
||||
// (unit-tested separately above), then wire that same real behavior
|
||||
// into the server-level dispatch by pointing loadMcpPolicy() at it.
|
||||
// Capture the *real* implementation before spying — `mod.loadMcpPolicy`
|
||||
// and the top-level imported `loadMcpPolicy` are the same live ESM
|
||||
// binding, so calling it from inside its own mock would recurse forever.
|
||||
const { loadMcpPolicy: realLoadMcpPolicy } = await vi.importActual<typeof import('../src/mcp-tools/policy-enforcer.js')>('../src/mcp-tools/policy-enforcer.js');
|
||||
expect(realLoadMcpPolicy(policyPath)).toBeNull();
|
||||
const mod = await import('../src/mcp-tools/policy-enforcer.js');
|
||||
const spy = vi.spyOn(mod, 'loadMcpPolicy').mockImplementation(() => realLoadMcpPolicy(policyPath));
|
||||
const { MCPServerManager } = await import('../src/mcp-server.js');
|
||||
const server = new (MCPServerManager as any)();
|
||||
const res = await dispatch(server, 'demo_tool', 'sess-malformed');
|
||||
expect(res.error).toBeDefined();
|
||||
expect(res.error.message).toMatch(/Policy denied.*failing closed/i);
|
||||
spy.mockRestore();
|
||||
});
|
||||
|
||||
it('enabled + auditLog required but log path unwritable: denies — fail-closed even though budget would allow', async () => {
|
||||
process.env.RUFLO_MCP_ENFORCE_POLICY = '1';
|
||||
setAuditLogPathForTesting('/nonexistent-dir-xyz/audit.jsonl');
|
||||
const mod = await import('../src/mcp-tools/policy-enforcer.js');
|
||||
const spy = vi.spyOn(mod, 'loadMcpPolicy').mockReturnValue({ auditLog: true, maxToolCallsPerTurn: 100 });
|
||||
const { MCPServerManager } = await import('../src/mcp-server.js');
|
||||
const server = new (MCPServerManager as any)();
|
||||
const res = await dispatch(server, 'demo_tool', 'sess-unwritable-log');
|
||||
expect(res.error).toBeDefined();
|
||||
expect(res.error.message).toMatch(/Policy denied.*audit log write failed.*failing closed/i);
|
||||
spy.mockRestore();
|
||||
});
|
||||
|
||||
it('enabled + budget=1: 1st call allowed, 2nd call denied with a policy error', async () => {
|
||||
process.env.RUFLO_MCP_ENFORCE_POLICY = '1';
|
||||
fs.writeFileSync(policyPath, JSON.stringify({ auditLog: true, maxToolCallsPerTurn: 1 }), 'utf-8');
|
||||
const mod = await import('../src/mcp-tools/policy-enforcer.js');
|
||||
const spy = vi.spyOn(mod, 'loadMcpPolicy').mockReturnValue({ auditLog: true, maxToolCallsPerTurn: 1 });
|
||||
const { MCPServerManager } = await import('../src/mcp-server.js');
|
||||
const server = new (MCPServerManager as any)();
|
||||
const first = await dispatch(server, 'demo_tool', 'sess-budget');
|
||||
expect(first.error).toBeUndefined();
|
||||
const second = await dispatch(server, 'demo_tool', 'sess-budget');
|
||||
expect(second.error).toBeDefined();
|
||||
expect(second.error.message).toMatch(/Policy denied/);
|
||||
spy.mockRestore();
|
||||
});
|
||||
});
|
||||
@@ -27,6 +27,11 @@ import * as os from 'os';
|
||||
import { fileURLToPath } from 'url';
|
||||
import { dirname } from 'path';
|
||||
import { trackRequest } from './mcp-tools/request-tracker.js';
|
||||
import {
|
||||
isPolicyEnforcementEnabled,
|
||||
loadMcpPolicy,
|
||||
evaluateToolCall,
|
||||
} from './mcp-tools/policy-enforcer.js';
|
||||
|
||||
// ESM-compatible __dirname
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
@@ -645,6 +650,18 @@ export class MCPServerManager extends EventEmitter {
|
||||
};
|
||||
}
|
||||
|
||||
if (isPolicyEnforcementEnabled()) {
|
||||
const check = evaluateToolCall(loadMcpPolicy(), sessionId, toolName);
|
||||
if (!check.allowed) {
|
||||
trackRequest(toolName, false);
|
||||
return {
|
||||
jsonrpc: '2.0',
|
||||
id: message.id,
|
||||
error: { code: -32001, message: `Policy denied: ${check.reason}` },
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await callMCPTool(toolName, toolParams, { sessionId });
|
||||
trackRequest(toolName, true);
|
||||
|
||||
@@ -0,0 +1,193 @@
|
||||
/**
|
||||
* MCP Governance Policy Enforcer (opt-in).
|
||||
*
|
||||
* `.harness/mcp-policy.json` declares governance intent (defaultDeny,
|
||||
* auditLog, maxToolCallsPerTurn, dangerousPatterns, ...) for the claude-flow
|
||||
* MCP server, but until now nothing in the running server (mcp-server.ts)
|
||||
* ever read it: `harness mcp-scan` grades the file's *posture* offline, the
|
||||
* live `tools/call` dispatch never consulted it. Any connected MCP client
|
||||
* could call every registered tool with no audit trail and no call budget.
|
||||
*
|
||||
* This module wires the two policy fields that are actually in this
|
||||
* server's jurisdiction, per the policy file's own rationale comment
|
||||
* (`dangerousPatterns` / `allowShell` / `allowNetwork` / `allowFileWrite`
|
||||
* describe the native-Claude-Code-tool layer — Bash/Write/Edit/WebFetch —
|
||||
* not this MCP server's memory_-, hooks_-, agentdb_-prefixed tool surface,
|
||||
* so they are intentionally left unenforced here):
|
||||
* - `auditLog`: append a JSONL record for every `tools/call`.
|
||||
* - `maxToolCallsPerTurn`: bound calls per MCP *session* (one stdio
|
||||
* process lifetime), deny once exceeded.
|
||||
*
|
||||
* Fully opt-in via `RUFLO_MCP_ENFORCE_POLICY=1` (or `true`). Unset/false
|
||||
* means every function below is a no-op on the hot path — the pre-existing
|
||||
* `tools/call` behavior is unchanged.
|
||||
*
|
||||
* FAIL-CLOSED once enforcement is enabled (PR #3139 review round 1):
|
||||
* a missing/malformed policy file, or a failed mandatory audit-log write,
|
||||
* denies the call rather than silently degrading to unrestricted execution.
|
||||
* The whole point of opting in is a restriction that actually holds; an
|
||||
* enforcement flag that quietly falls back to "no restriction" on its own
|
||||
* misconfiguration defeats the feature. See `evaluateToolCall()`.
|
||||
*
|
||||
* Known scope limits (disclosed, not fixed here):
|
||||
* - Despite the policy field's name, this enforces a *session-lifetime
|
||||
* cumulative* cap, not a per-conversational-turn cap that resets — a
|
||||
* long-lived stdio session can exhaust `maxToolCallsPerTurn` (200 in
|
||||
* the shipped `.harness/mcp-policy.json`) under entirely legitimate
|
||||
* use and stay locked out until the MCP server process restarts.
|
||||
* Do not raise this above default-off without a real per-turn reset.
|
||||
* - Only wired into the stdio `tools/call` dispatch
|
||||
* (`MCPServerManager.handleMCPMessage`). The separate HTTP/websocket
|
||||
* path (`startHttpServer()`, via `@claude-flow/mcp`) does not call
|
||||
* this module and is unaffected even when this flag is set.
|
||||
*/
|
||||
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
|
||||
export interface McpPolicy {
|
||||
schema?: number;
|
||||
policyVersion?: number;
|
||||
harnessId?: string;
|
||||
defaultDeny?: boolean;
|
||||
auditLog?: boolean;
|
||||
requireApprovalForDangerous?: boolean;
|
||||
toolTimeoutMs?: number;
|
||||
maxToolCallsPerTurn?: number;
|
||||
dangerousPatterns?: string[];
|
||||
approvedServers?: string[];
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
export function isPolicyEnforcementEnabled(env: NodeJS.ProcessEnv = process.env): boolean {
|
||||
const v = env.RUFLO_MCP_ENFORCE_POLICY;
|
||||
return v === '1' || (v ?? '').toLowerCase() === 'true';
|
||||
}
|
||||
|
||||
export function loadMcpPolicy(
|
||||
policyPath: string = path.join(process.cwd(), '.harness', 'mcp-policy.json'),
|
||||
): McpPolicy | null {
|
||||
try {
|
||||
const raw = fs.readFileSync(policyPath, 'utf-8');
|
||||
const parsed: unknown = JSON.parse(raw);
|
||||
if (typeof parsed !== 'object' || parsed === null) return null;
|
||||
return parsed as McpPolicy;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
interface SessionState {
|
||||
callCount: number;
|
||||
}
|
||||
|
||||
const sessionState = new Map<string, SessionState>();
|
||||
|
||||
/** Test-only: clear per-session call counters between test cases. */
|
||||
export function resetPolicyEnforcerState(): void {
|
||||
sessionState.clear();
|
||||
}
|
||||
|
||||
export interface PolicyCheckResult {
|
||||
allowed: boolean;
|
||||
reason?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Increments and checks the calling session's tool-call count against
|
||||
* `policy.maxToolCallsPerTurn`. Denies (without incrementing further) once
|
||||
* the session is already at or over the limit.
|
||||
*/
|
||||
export function checkAndRecordCall(policy: McpPolicy, sessionId: string): PolicyCheckResult {
|
||||
const limit = policy.maxToolCallsPerTurn;
|
||||
if (typeof limit !== 'number' || !Number.isFinite(limit) || limit <= 0) {
|
||||
return { allowed: true };
|
||||
}
|
||||
const state = sessionState.get(sessionId) ?? { callCount: 0 };
|
||||
if (state.callCount >= limit) {
|
||||
sessionState.set(sessionId, state);
|
||||
return { allowed: false, reason: `maxToolCallsPerTurn (${limit}) exceeded for this session` };
|
||||
}
|
||||
state.callCount += 1;
|
||||
sessionState.set(sessionId, state);
|
||||
return { allowed: true };
|
||||
}
|
||||
|
||||
export interface AuditLogEntry {
|
||||
timestamp: string;
|
||||
sessionId: string;
|
||||
toolName: string;
|
||||
allowed: boolean;
|
||||
reason?: string;
|
||||
}
|
||||
|
||||
let auditLogPathOverride: string | null = null;
|
||||
|
||||
/** Test-only: redirect the audit log to a temp file instead of the default path. */
|
||||
export function setAuditLogPathForTesting(p: string | null): void {
|
||||
auditLogPathOverride = p;
|
||||
}
|
||||
|
||||
function defaultAuditLogPath(): string {
|
||||
return path.join(os.tmpdir(), 'ruflo-mcp-audit.jsonl');
|
||||
}
|
||||
|
||||
export function getAuditLogPath(): string {
|
||||
return auditLogPathOverride ?? defaultAuditLogPath();
|
||||
}
|
||||
|
||||
/**
|
||||
* Appends one JSONL audit record. Returns `true` if `policy.auditLog` is not
|
||||
* set (nothing was required) or the write succeeded; `false` only when
|
||||
* `auditLog` is required and the write itself failed (disk full, unwritable
|
||||
* path, etc). Never throws — the caller (`evaluateToolCall`) decides what a
|
||||
* failed *mandatory* write means for the call (fail-closed: deny it).
|
||||
*/
|
||||
export function appendAuditLog(policy: McpPolicy, entry: AuditLogEntry): boolean {
|
||||
if (!policy.auditLog) return true;
|
||||
try {
|
||||
fs.appendFileSync(getAuditLogPath(), `${JSON.stringify(entry)}\n`, 'utf-8');
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Single enforcement entry point for a `tools/call` dispatch. Combines, in
|
||||
* order: fail-closed on a missing/malformed policy, the per-session call
|
||||
* budget, and fail-closed on a failed mandatory audit-log write. `policy`
|
||||
* is the result of `loadMcpPolicy()` — pass `null` straight through when it
|
||||
* failed to load, rather than re-deciding that here.
|
||||
*/
|
||||
export function evaluateToolCall(
|
||||
policy: McpPolicy | null,
|
||||
sessionId: string,
|
||||
toolName: string,
|
||||
): PolicyCheckResult {
|
||||
if (policy === null) {
|
||||
return {
|
||||
allowed: false,
|
||||
reason: 'RUFLO_MCP_ENFORCE_POLICY is set but .harness/mcp-policy.json is missing or invalid — failing closed',
|
||||
};
|
||||
}
|
||||
|
||||
const budget = checkAndRecordCall(policy, sessionId);
|
||||
const auditOk = appendAuditLog(policy, {
|
||||
timestamp: new Date().toISOString(),
|
||||
sessionId,
|
||||
toolName,
|
||||
allowed: budget.allowed,
|
||||
reason: budget.reason,
|
||||
});
|
||||
|
||||
if (!budget.allowed) return budget;
|
||||
if (!auditOk) {
|
||||
return {
|
||||
allowed: false,
|
||||
reason: 'audit log write failed and policy.auditLog is required — failing closed',
|
||||
};
|
||||
}
|
||||
return { allowed: true };
|
||||
}
|
||||
Reference in new issue
Block a user