Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
47 changes: 47 additions & 0 deletions app/client/packages/mcp/src/gates.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {
ELICITATION_TIMEOUT_CEILING_MS,
apiBaseUrlFromEnv,
elicitationTimeoutFromEnv,
gateEnabled,
gateEnabledUnlessFalse,
parsePositiveInt,
publicOriginFromEnv,
sessionLimitsFromEnv,
Expand Down Expand Up @@ -65,6 +67,26 @@ describe("gateEnabled — opt-in gate parsing (data/JS layers)", () => {
});
});

describe("gateEnabledUnlessFalse — default-on until false", () => {
it.each(["false", "FALSE", "False", " false "])(
"disables for %j",
(value) => {
expect(gateEnabledUnlessFalse(value)).toBe(false);
},
);

it.each(["0", "off", "no", "disabled", "true", "1", "", " "])(
"stays on for %j",
(value) => {
expect(gateEnabledUnlessFalse(value)).toBe(true);
},
);

it("stays on when the variable is unset", () => {
expect(gateEnabledUnlessFalse(undefined)).toBe(true);
});
});

describe("parsePositiveInt — session cap/TTL env overrides", () => {
it.each([
["25", 25],
Expand All @@ -84,6 +106,31 @@ describe("parsePositiveInt — session cap/TTL env overrides", () => {
);
});

describe("apiBaseUrlFromEnv — APPSMITH_API_BASE_URL origin for /api/v1 paths", () => {
it.each([
["http://127.0.0.1:8080", "http://127.0.0.1:8080"],
["http://127.0.0.1:8080/", "http://127.0.0.1:8080"],
["http://127.0.0.1:8080///", "http://127.0.0.1:8080"],
[" http://127.0.0.1:8080/ ", "http://127.0.0.1:8080"],
["http://127.0.0.1:8080/api/v1", "http://127.0.0.1:8080"],
["http://127.0.0.1:8080/api/v1/", "http://127.0.0.1:8080"],
["https://apps.example.com/api/v1/", "https://apps.example.com"],
[
"https://apps.example.com/appsmith/api/v1/",
"https://apps.example.com/appsmith",
],
["HTTPS://apps.example.com/API/V1", "HTTPS://apps.example.com"],
])("normalizes %j to %j", (value, expected) => {
expect(apiBaseUrlFromEnv(value)).toBe(expected);
});

it("leaves a non-api/v1 path prefix intact", () => {
expect(apiBaseUrlFromEnv("http://127.0.0.1:8080/appsmith")).toBe(
"http://127.0.0.1:8080/appsmith",
);
});
});

describe("publicOriginFromEnv — APPSMITH_MCP_PUBLIC_ORIGIN parsing (fail-closed)", () => {
it.each([
["https://apps.example.com", "https://apps.example.com"],
Expand Down
16 changes: 16 additions & 0 deletions app/client/packages/mcp/src/gates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ export function gateEnabled(value: string | undefined): boolean {
return value !== undefined && /^(1|true|yes|on)$/i.test(value.trim());
}

// Default-ON counterpart for capabilities whose server-side opt-in flags were removed. Enabled unless the env
// value is "false" (trim + case-insensitive); "0", "off", blank, and unset all stay enabled.
export function gateEnabledUnlessFalse(value: string | undefined): boolean {
return value === undefined || value.trim().toLowerCase() !== "false";
}

// Positive-integer env override (session caps, TTLs). Unset, non-numeric, fractional, zero, or negative values fall
// back to the built-in default rather than failing startup or silently disabling a limit.
export function parsePositiveInt(
Expand All @@ -21,6 +27,16 @@ export function parsePositiveInt(
return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback;
}

// APPSMITH_API_BASE_URL is concatenated with paths that already start with /api/v1. Operators often paste the
// Appsmith origin including that prefix (and a trailing slash). Strip both so we do not request /api/v1/api/v1/...
export function apiBaseUrlFromEnv(value: string): string {
return value
.trim()
.replace(/\/+$/, "")
.replace(/\/api\/v1$/i, "")
.replace(/\/+$/, "");
}

// Public origin override for the URLs build_application returns (APPSMITH_MCP_PUBLIC_ORIGIN). Accepts ONLY an
// absolute http(s) origin — scheme + host + optional port, with no path, query, fragment, or credentials (a bare
// trailing slash is normalized away). Anything else fails CLOSED to undefined, so URLs degrade to root-relative
Expand Down
17 changes: 11 additions & 6 deletions app/client/packages/mcp/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import {
MCP_SESSION_TTL_MS,
} from "./app.js";
import {
apiBaseUrlFromEnv,
elicitationTimeoutFromEnv,
gateEnabled,
gateEnabledUnlessFalse,
publicOriginFromEnv,
sessionLimitsFromEnv,
} from "./gates.js";
Expand All @@ -18,13 +20,16 @@ import {
} from "./governance/store.js";

const port = Number(process.env.APPSMITH_MCP_PORT ?? 8092);
const apiBaseUrl = process.env.APPSMITH_API_BASE_URL ?? "http://127.0.0.1:8080";
const apiBaseUrl = apiBaseUrlFromEnv(
process.env.APPSMITH_API_BASE_URL ?? "http://127.0.0.1:8080",
);

// The data layer and restricted JS objects are OFF unless explicitly enabled, matching the parent
// APPSMITH_MCP_ENABLED gate: an admin opts into each capability. Governed/destructive tools additionally require
// Mongo+Redis, so they only register when that infra is present.
const dataEnabled = gateEnabled(process.env.APPSMITH_MCP_DATA_ENABLED);
const jsEnabled = gateEnabled(process.env.APPSMITH_MCP_JS_ENABLED);
// Data and JS stay on unless an operator explicitly sets the env var to "false". Governed/destructive tools
// still require Mongo+Redis and register only when that infrastructure is present.
const dataEnabled = gateEnabledUnlessFalse(
process.env.APPSMITH_MCP_DATA_ENABLED,
);
const jsEnabled = gateEnabledUnlessFalse(process.env.APPSMITH_MCP_JS_ENABLED);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Optional Host-header allowlist (comma-separated hostnames) enforced on /mcp. Unset by default: this service is
// fronted by Caddy which preserves the original Host, so a default loopback list would reject the proxied public
Expand Down
Loading