Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ This Turborepo includes the following packages/apps:

### Apps and Packages
Comment thread
greptile-apps[bot] marked this conversation as resolved.

- `docs`: a [Next.js](https://nextjs.org/) app
- `web`: another [Next.js](https://nextjs.org/) app
- `@repo/ui`: a stub React component library shared by both `web` and `docs` applications
- `@repo/eslint-config`: `eslint` configurations (includes `eslint-config-next` and `eslint-config-prettier`)
- `@repo/typescript-config`: `tsconfig.json`s used throughout the monorepo
- `apps/api`: Express API server managing MCP server registrations, tool executions, logging, and policy evaluations.
- `apps/web`: Next.js web application interface.
- `apps/file-manager-mcp`: Custom Model Context Protocol (MCP) server providing secure sandboxed filesystem operations.
- `packages/db` ([@repo/db](./packages/db/README.md)): Prisma database library managing schemas, migrations, and SQLite instance. (See the [Schema Architecture Documentation](./packages/db/README.md)).
- `packages/shared` ([@repo/shared](./packages/shared)): Shared utility functions and formatting helpers.
- `packages/ui` ([@repo/ui](./packages/ui)): React UI components shared by the applications.
- `packages/eslint-config` ([@repo/eslint-config](./packages/eslint-config)): Monorepo ESLint configurations.
- `packages/typescript-config` ([@repo/typescript-config](./packages/typescript-config)): TSConfig setups.

Each package/app is 100% [TypeScript](https://www.typescriptlang.org/).

Expand Down
18 changes: 12 additions & 6 deletions apps/api/mcp/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,40 @@ This document explains the architecture and implementation of error handling, st
The gateway implements a custom error class, `AppError` (defined in [types.ts](../types.ts)), which extends the native `Error` class and introduces a `statusCode` field.

This model enables:

1. **Decoupled Error Classification**: The core logic (e.g., input validation, tool execution, and registry lookup) determines the semantic meaning of a failure and assigns the appropriate HTTP status code at the throw site.
2. **Safe Route Mapping**: The Express routing layer does not parse message strings or substrings. Instead, it inspects whether the caught error is an instance of `AppError` and maps it directly using `error.statusCode`.

### Status Code Mapping Table

| Error Scenario | HTTP Status Code | Thrown From | Exception Class |
|---|---|---|---|
| Invalid toolName type | 400 Bad Request | `ToolExecutor.execute` | `AppError` |
| Empty toolName value | 400 Bad Request | `ToolExecutor.execute` | `AppError` |
| Policy decision is not ALLOW | 403 Forbidden | `ToolExecutor.execute` | `AppError` |
| Requested tool is not registered | 404 Not Found | `ToolExecutor.execute` | `AppError` |
| Error Scenario | HTTP Status Code | Thrown From | Exception Class |
| -------------------------------- | ------------------------- | ---------------------------- | ---------------- |
| Invalid toolName type | 400 Bad Request | `ToolExecutor.execute` | `AppError` |
| Empty toolName value | 400 Bad Request | `ToolExecutor.execute` | `AppError` |
| Policy decision is not ALLOW | 403 Forbidden | `ToolExecutor.execute` | `AppError` |
| Requested tool is not registered | 404 Not Found | `ToolExecutor.execute` | `AppError` |
| Internal service crash / timeout | 500 Internal Server Error | Subprocess spawn / transport | Standard `Error` |

---

## Security Mitigations

### 1. Substring Collision Prevention

Using `instanceof AppError` and `error.statusCode` prevents routing bugs where user-supplied parameters (like a tool name consisting of the substring `"must be a"` or a decision value containing `"Tool not found"`) would accidentally match Express error handling filters.

### 2. Information Leakage Prevention (CWE-209)

Any exception that is not a subclass of `AppError` (such as a database connection pool timeout, subprocess exit code error, or network failure) is treated as an internal error. The Express route handler intercepts these, logs the raw error to `stderr` for internal auditing, and returns a generic response payload to the client:

```json
{
"error": "Failed to execute tool"
}
```

This masks server internals and prevents stack trace details from leaking to external clients.

### 3. Execution Timeout and Resource Cleanups

If a tool execution takes longer than the configured timeout, the request is aborted using `Promise.race()` and a standard Timeout `Error` is thrown, which naturally maps to a `500` response. The pending timer is cleaned up in a `finally` block to prevent timer leakages.
5 changes: 4 additions & 1 deletion apps/api/mcp/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ export class ToolExecutor {
}

if (decision !== "ALLOW") {
const error = new AppError(403, `Tool execution rejected with decision: ${decision}`);
const error = new AppError(
403,
`Tool execution rejected with decision: ${decision}`,
);
logger.error("Tool execution failed: Denied by policy", {
tool_name: toolName,
decision,
Expand Down
27 changes: 19 additions & 8 deletions apps/api/mcp/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { fileURLToPath } from "url";
import { StdioMCPServer } from "./stdio-server.js";
import { logger } from "./logger.js";


const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

Expand Down Expand Up @@ -277,7 +276,6 @@ describe("MCP Production-Ready Module", () => {
});
});


describe("ToolExecutor Execution & Safeness", () => {
it("should execute a registered tool successfully (Happy Path)", async () => {
const mockTool: Tool = {
Expand Down Expand Up @@ -318,10 +316,12 @@ describe("MCP Production-Ready Module", () => {
"mathAdd",
{ a: 2, b: 3 },
{ decision: "DENY", conversationId: "convDenied" },
)
),
).rejects.toThrow("Tool execution rejected with decision: DENY");

const errLog = loggedItems.find((log) => log.level === "error" && log.conversation_id === "convDenied");
const errLog = loggedItems.find(
(log) => log.level === "error" && log.conversation_id === "convDenied",
);
expect(errLog).toBeDefined();
expect(errLog.message).toBe("Tool execution failed: Denied by policy");
expect(errLog.decision).toBe("DENY");
Expand Down Expand Up @@ -506,9 +506,21 @@ describe("MCP Production-Ready Module", () => {

describe("Logger Metadata Protection", () => {
it("should prevent overriding core properties via meta argument", () => {
logger.info("Main message", { level: "hacked", message: "spoofed message", extra: "valid" });
logger.warn("Main message", { level: "hacked", message: "spoofed message", extra: "valid" });
logger.error("Main message", { level: "hacked", message: "spoofed message", extra: "valid" });
logger.info("Main message", {
level: "hacked",
message: "spoofed message",
extra: "valid",
});
logger.warn("Main message", {
level: "hacked",
message: "spoofed message",
extra: "valid",
});
logger.error("Main message", {
level: "hacked",
message: "spoofed message",
extra: "valid",
});

const infoLogs = loggedItems.filter((log) => log.extra === "valid");
expect(infoLogs.length).toBe(3);
Expand All @@ -524,4 +536,3 @@ describe("MCP Production-Ready Module", () => {
});
});
});

2 changes: 2 additions & 0 deletions apps/api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import dotenv from "dotenv";
import { formatDate } from "@repo/shared";
import { mcpDiscovery, mcpExecutor } from "../mcp/bootstrap.js";
import { AppError } from "../types.js";
import policiesRouter from "./policy/router.js";

dotenv.config();

Expand All @@ -12,6 +13,7 @@ const port = process.env.PORT || 3001;

app.use(cors());
app.use(express.json({ limit: "1mb" }));
app.use(policiesRouter);

@cubic-dev-ai cubic-dev-ai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Policy management routes are exposed without authentication/authorization. This allows unauthorized clients to create/update/delete enforcement policies.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/index.ts, line 16:

<comment>Policy management routes are exposed without authentication/authorization. This allows unauthorized clients to create/update/delete enforcement policies.</comment>

<file context>
@@ -12,6 +13,7 @@ const port = process.env.PORT || 3001;
 
 app.use(cors());
 app.use(express.json({ limit: "1mb" }));
+app.use(policiesRouter);
 
 // Health check endpoint
</file context>
Fix with cubic


// Health check endpoint
app.get("/health", (req, res) => {
Expand Down
108 changes: 108 additions & 0 deletions apps/api/src/policy/decision.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { db, ApprovalStatus } from "@repo/db";
import PolicyEngine from "./engine.js";
import type { ApprovalRequest, ConversationRequest } from "../../types.js";

export type Decision = "ALLOW" | "DENY" | "PENDING";

export interface DecisionResult {
decision: Decision;
reason?: string;
}

export async function decide(
context: ApprovalRequest,
conversation: ConversationRequest,
): Promise<DecisionResult> {
try {
// Step 1: Call PolicyEngine
const policy = await PolicyEngine(context, conversation);

// Step 2: Policy denied and does not require approval
if (!policy.allowed && !policy.requiresApproval) {
return {
decision: "DENY",
reason: policy.reason,
};
}

// Step 3: Policy requires human approval
if (policy.requiresApproval) {
if (!context.approvalId) {
const created = await db.approval.create({
data: {
tool_name: context.tool_name,
arguments: context.arguments as any,
status: ApprovalStatus.PENDING,
},
});

return {
decision: "PENDING",
reason: created.id,
};
}

const approval = await db.approval.findUnique({
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
where: {
id: context.approvalId,
},
});

if (!approval) {
return {
decision: "DENY",
reason: "Approval not found",
};
}

if (approval.tool_name !== context.tool_name) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
return {
decision: "DENY",
reason: "Approval tool name mismatch",
};
}

switch (approval.status) {
case ApprovalStatus.APPROVED:
await db.approval.delete({
where: { id: approval.id },
});
return {
decision: "ALLOW",
};
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Comment thread
greptile-apps[bot] marked this conversation as resolved.
case ApprovalStatus.PENDING:
return {
decision: "PENDING",
};
case ApprovalStatus.REJECTED:
return {
decision: "DENY",
reason: "Approval rejected",
};
default:
return {
decision: "DENY",
reason: "Unrecognized approval status",
};
}
}

// Step 4: Policy allowed
if (policy.allowed) {
return {
decision: "ALLOW",
};
}

// Fallback/Safety return
return {
decision: "DENY",
reason: "Unrecognized policy state",
};
} catch (error) {
return {
decision: "DENY",
reason: "Decision engine failure",
};
}
}
107 changes: 107 additions & 0 deletions apps/api/src/policy/engine.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import needsApproval from "./rules/approval.js";
import isblocked from "./rules/block.js";
import budgetExceeded from "./rules/budget.js";
import type { ApprovalRequest, ConversationRequest } from "../../types.js";
import { db } from "@repo/db";
import { logger } from "../../mcp/logger.js";

export interface PolicyEngineResult {
allowed: boolean;
requiresApproval: boolean;
reason?: string;
}

export default async function PolicyEngine(
context: ApprovalRequest,
conversation: ConversationRequest,
): Promise<PolicyEngineResult> {
let policy;
try {
const tool_name = context.tool_name;

// Fetch the policy record once to prevent double lookup and TOCTOU race conditions
policy = await db.policy.findUnique({
where: { tool_name },
});
} catch (error: any) {
logger.error("Failed to query policy table in PolicyEngine pre-fetch", {
tool_name: context.tool_name,
error_message: error.message || String(error),
});
return {
allowed: false,
requiresApproval: false,
reason: "Failed to query policy table",
};
}

try {
const tool_name = context.tool_name;

// 1. Block Check
const blockedResult = await isblocked(tool_name, policy);
if (!blockedResult.success) {
return {
allowed: false,
requiresApproval: false,
reason: blockedResult.reason,
};
}
if (blockedResult.result) {
return {
allowed: false,
requiresApproval: false,
reason: blockedResult.reason,
};
}

// 2. Budget Check
const budgetResult = await budgetExceeded(
conversation.conversationId,
conversation.token,
);
if (!budgetResult.success) {
return {
allowed: false,
requiresApproval: false,
reason: budgetResult.reason,
};
}
if (budgetResult.result) {
return {
allowed: false,
requiresApproval: false,
reason: budgetResult.reason,
};
}

// 3. Approval Check
const approvalResult = await needsApproval(tool_name);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
if (!approvalResult.success) {
return {
allowed: false,
requiresApproval: false,
reason: approvalResult.reason,
};
}
if (approvalResult.result) {
return {
allowed: false,
requiresApproval: true,
reason: approvalResult.reason,
};
}

// 4. Default Success (Allowed)
return {
allowed: true,
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
requiresApproval: false,
};
} catch (error: any) {
return {
allowed: false,
requiresApproval: false,
reason: error instanceof Error ? error.message : String(error),
};
}
}
Loading
Loading