Skip to content

Commit e860630

Browse files
authored
fix: Mcp server baseUrl suffix omission (#42212)
## Description - When server url in the environment is set with suffixes, mcp server parses to remove the extra fields. - Hotwires JS and Data enabled as it's not controlled using env variables Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Testing > [!NOTE] > **How CI runs on fork PRs — no action needed from you.** > 1. **Workflow approval.** GitHub holds the first run on fork PRs until a maintainer approves it, so a pause before any check appears is expected. > 2. **Credential-free checks.** Once approved, format, lint, typecheck, unit tests, cyclic-dependency and compile-only build checks run without repository secrets. Only the checks relevant to what you changed (client / server / RTS) will run, and their logs are safe to debug against. > 3. **Maintainer-triggered checks.** Cypress, Playwright, Docker builds and deploy previews need secrets, so a maintainer starts them with `/approve-ci`, and `/build-deploy-preview` when hands-on testing is needed. Approval is pinned to one commit — pushing again requires fresh approval. > > The `awaiting-maintainer` / `awaiting-contributor` labels show whose turn it is. You do **not** need `ok-to-test` or any slash command. Full detail: [Pull request check states](https://github.com/appsmithorg/appsmith/blob/release/contributions/CodeContributionsGuidelines.md#pull-request-check-states). Select the validation relevant to this change: - [ ] Client unit tests - [ ] Server unit tests - [ ] Cypress - [ ] Playwright - [ ] Deploy preview - [x] Not applicable Suggested Cypress tags or specs: ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Data and JavaScript capabilities are now enabled by default. * API base URLs are handled more reliably, including trailing slashes, `/api/v1` suffixes, whitespace, and nested paths. * **Bug Fixes** * Prevented duplicated API path prefixes when configuring the application API URL. * Improved compatibility with API URLs using different capitalization or additional path segments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD 718fb12 yet > <hr>Tue, 08 Sep 2026 18:38:01 UTC <!-- end of auto-generated comment: Cypress test results -->
1 parent 7acb9d0 commit e860630

3 files changed

Lines changed: 74 additions & 6 deletions

File tree

app/client/packages/mcp/src/gates.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import {
22
ELICITATION_TIMEOUT_CEILING_MS,
3+
apiBaseUrlFromEnv,
34
elicitationTimeoutFromEnv,
45
gateEnabled,
6+
gateEnabledUnlessFalse,
57
parsePositiveInt,
68
publicOriginFromEnv,
79
sessionLimitsFromEnv,
@@ -65,6 +67,26 @@ describe("gateEnabled — opt-in gate parsing (data/JS layers)", () => {
6567
});
6668
});
6769

70+
describe("gateEnabledUnlessFalse — default-on until false", () => {
71+
it.each(["false", "FALSE", "False", " false "])(
72+
"disables for %j",
73+
(value) => {
74+
expect(gateEnabledUnlessFalse(value)).toBe(false);
75+
},
76+
);
77+
78+
it.each(["0", "off", "no", "disabled", "true", "1", "", " "])(
79+
"stays on for %j",
80+
(value) => {
81+
expect(gateEnabledUnlessFalse(value)).toBe(true);
82+
},
83+
);
84+
85+
it("stays on when the variable is unset", () => {
86+
expect(gateEnabledUnlessFalse(undefined)).toBe(true);
87+
});
88+
});
89+
6890
describe("parsePositiveInt — session cap/TTL env overrides", () => {
6991
it.each([
7092
["25", 25],
@@ -84,6 +106,31 @@ describe("parsePositiveInt — session cap/TTL env overrides", () => {
84106
);
85107
});
86108

109+
describe("apiBaseUrlFromEnv — APPSMITH_API_BASE_URL origin for /api/v1 paths", () => {
110+
it.each([
111+
["http://127.0.0.1:8080", "http://127.0.0.1:8080"],
112+
["http://127.0.0.1:8080/", "http://127.0.0.1:8080"],
113+
["http://127.0.0.1:8080///", "http://127.0.0.1:8080"],
114+
[" http://127.0.0.1:8080/ ", "http://127.0.0.1:8080"],
115+
["http://127.0.0.1:8080/api/v1", "http://127.0.0.1:8080"],
116+
["http://127.0.0.1:8080/api/v1/", "http://127.0.0.1:8080"],
117+
["https://apps.example.com/api/v1/", "https://apps.example.com"],
118+
[
119+
"https://apps.example.com/appsmith/api/v1/",
120+
"https://apps.example.com/appsmith",
121+
],
122+
["HTTPS://apps.example.com/API/V1", "HTTPS://apps.example.com"],
123+
])("normalizes %j to %j", (value, expected) => {
124+
expect(apiBaseUrlFromEnv(value)).toBe(expected);
125+
});
126+
127+
it("leaves a non-api/v1 path prefix intact", () => {
128+
expect(apiBaseUrlFromEnv("http://127.0.0.1:8080/appsmith")).toBe(
129+
"http://127.0.0.1:8080/appsmith",
130+
);
131+
});
132+
});
133+
87134
describe("publicOriginFromEnv — APPSMITH_MCP_PUBLIC_ORIGIN parsing (fail-closed)", () => {
88135
it.each([
89136
["https://apps.example.com", "https://apps.example.com"],

app/client/packages/mcp/src/gates.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ export function gateEnabled(value: string | undefined): boolean {
88
return value !== undefined && /^(1|true|yes|on)$/i.test(value.trim());
99
}
1010

11+
// Default-ON counterpart for capabilities whose server-side opt-in flags were removed. Enabled unless the env
12+
// value is "false" (trim + case-insensitive); "0", "off", blank, and unset all stay enabled.
13+
export function gateEnabledUnlessFalse(value: string | undefined): boolean {
14+
return value === undefined || value.trim().toLowerCase() !== "false";
15+
}
16+
1117
// Positive-integer env override (session caps, TTLs). Unset, non-numeric, fractional, zero, or negative values fall
1218
// back to the built-in default rather than failing startup or silently disabling a limit.
1319
export function parsePositiveInt(
@@ -21,6 +27,16 @@ export function parsePositiveInt(
2127
return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback;
2228
}
2329

30+
// APPSMITH_API_BASE_URL is concatenated with paths that already start with /api/v1. Operators often paste the
31+
// Appsmith origin including that prefix (and a trailing slash). Strip both so we do not request /api/v1/api/v1/...
32+
export function apiBaseUrlFromEnv(value: string): string {
33+
return value
34+
.trim()
35+
.replace(/\/+$/, "")
36+
.replace(/\/api\/v1$/i, "")
37+
.replace(/\/+$/, "");
38+
}
39+
2440
// Public origin override for the URLs build_application returns (APPSMITH_MCP_PUBLIC_ORIGIN). Accepts ONLY an
2541
// absolute http(s) origin — scheme + host + optional port, with no path, query, fragment, or credentials (a bare
2642
// trailing slash is normalized away). Anything else fails CLOSED to undefined, so URLs degrade to root-relative

app/client/packages/mcp/src/server.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ import {
66
MCP_SESSION_TTL_MS,
77
} from "./app.js";
88
import {
9+
apiBaseUrlFromEnv,
910
elicitationTimeoutFromEnv,
1011
gateEnabled,
12+
gateEnabledUnlessFalse,
1113
publicOriginFromEnv,
1214
sessionLimitsFromEnv,
1315
} from "./gates.js";
@@ -18,13 +20,16 @@ import {
1820
} from "./governance/store.js";
1921

2022
const port = Number(process.env.APPSMITH_MCP_PORT ?? 8092);
21-
const apiBaseUrl = process.env.APPSMITH_API_BASE_URL ?? "http://127.0.0.1:8080";
23+
const apiBaseUrl = apiBaseUrlFromEnv(
24+
process.env.APPSMITH_API_BASE_URL ?? "http://127.0.0.1:8080",
25+
);
2226

23-
// The data layer and restricted JS objects are OFF unless explicitly enabled, matching the parent
24-
// APPSMITH_MCP_ENABLED gate: an admin opts into each capability. Governed/destructive tools additionally require
25-
// Mongo+Redis, so they only register when that infra is present.
26-
const dataEnabled = gateEnabled(process.env.APPSMITH_MCP_DATA_ENABLED);
27-
const jsEnabled = gateEnabled(process.env.APPSMITH_MCP_JS_ENABLED);
27+
// Data and JS stay on unless an operator explicitly sets the env var to "false". Governed/destructive tools
28+
// still require Mongo+Redis and register only when that infrastructure is present.
29+
const dataEnabled = gateEnabledUnlessFalse(
30+
process.env.APPSMITH_MCP_DATA_ENABLED,
31+
);
32+
const jsEnabled = gateEnabledUnlessFalse(process.env.APPSMITH_MCP_JS_ENABLED);
2833

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

0 commit comments

Comments
 (0)