diff --git a/sass/editor/_editor-mcp.scss b/sass/editor/_editor-mcp.scss index f8cdc5dcf..7e9ae7771 100644 --- a/sass/editor/_editor-mcp.scss +++ b/sass/editor/_editor-mcp.scss @@ -48,6 +48,11 @@ .mcp-field { flex: 1; min-width: 0; + + // invalid port: keep the error ring while typing, over pcui's focus ring + &.pcui-error { + box-shadow: $element-shadow-error; + } } .mcp-status-value { @@ -84,6 +89,11 @@ font-size: 11px; line-height: 1.4; white-space: normal; + + // invalid port + &.pcui-error { + color: $error-secondary; + } } .mcp-toggle { diff --git a/sass/editor/_editor-project-cms.scss b/sass/editor/_editor-project-cms.scss index 4211d4c18..36294f842 100644 --- a/sass/editor/_editor-project-cms.scss +++ b/sass/editor/_editor-project-cms.scss @@ -2108,11 +2108,20 @@ color: $text-dark; } } + + // invalid port; wins over the focus border + &.pcui-error { + border-color: $error; + } } .mcp-hint { padding: 4px 0; font-size: 12px; + + &.pcui-error { + color: $error-secondary; + } } // button(): primary to connect, secondary to cancel or disconnect diff --git a/src/editor/mcp/connection.ts b/src/editor/mcp/connection.ts index 0fa6e4a7f..e8c86bb2b 100644 --- a/src/editor/mcp/connection.ts +++ b/src/editor/mcp/connection.ts @@ -2,6 +2,7 @@ import { Events } from '@playcanvas/observer'; import { config } from '@/editor/config'; +import { parsePort } from './port'; import { handleRequest } from './request'; const DEFAULT_PORT = 52000; @@ -109,10 +110,16 @@ class MCPConnection extends Events { * unexpectedly (server restart, transient network, background-tab throttling) we retry * automatically until it reconnects or disconnect() is called. * - * @param port - The MCP server port to connect to. + * @param port - The MCP server port to connect to. An invalid port is refused. * @param role - The peer role announced to the server. */ connect(port: number = DEFAULT_PORT, role: Role = 'editor') { + // new WebSocket() throws on an out-of-range port, which would strand us in 'connecting' + if (parsePort(port) === null) { + error(`Invalid port ${port}: use a number from 1 to 65535`); + return; + } + this._port = port; this._role = role; this._forceClosed = false; diff --git a/src/editor/mcp/popover.ts b/src/editor/mcp/popover.ts index 6c40bd36a..81b162140 100644 --- a/src/editor/mcp/popover.ts +++ b/src/editor/mcp/popover.ts @@ -4,6 +4,7 @@ import { TooltipHandle } from '@/common/tooltips'; import { button as pmButton } from '@/editor/pickers/project-management/components/button'; import { DEFAULT_PORT } from './connection'; +import { parsePort } from './port'; // MCP mark from Hugeicons (mcp-server-solid-sharp), fills set to currentColor. const MCP_ICON = @@ -15,6 +16,8 @@ const BLOCKED_HINTS: Record = { prompt: 'Allow "Apps on device" for this site if the browser asks, or check the server is running' }; +const INVALID_PORT_HINT = 'Enter a port from 1 to 65535'; + /** * Create the MCP button and its connect popover, and reconnect if the page was connected * before it reloaded. Callers place the returned button. @@ -69,12 +72,30 @@ const createMcpButton = (placement: 'right' | 'account') => { popover.append(toggle); root.append(popover); + // the field's port, or null if it isn't valid (an empty field included) + const fieldPort = () => parsePort(portField.value); + const updateHint = () => { - const blocked = BLOCKED_HINTS[editor.call('mcp:blocked')]; - hint.text = blocked ?? ''; - hint.hidden = !blocked; + const invalid = fieldPort() === null; + const text = invalid ? INVALID_PORT_HINT : BLOCKED_HINTS[editor.call('mcp:blocked')]; + hint.text = text ?? ''; + hint.hidden = !text; + hint.error = invalid; + portField.error = invalid; + // only Connect needs a valid port; Cancel and Disconnect always work + toggle.enabled = !invalid || editor.call('mcp:status') !== 'disconnected'; }; + // validate as the user types; the DOM input event also catches paste and mobile keyboards + portField.input.addEventListener('input', updateHint); + portField.on('change', updateHint); + // Escape restores the pre-edit value without firing input or change; pcui emits keydown after + portField.on('keydown', (evt: KeyboardEvent) => { + if (evt.key === 'Escape') { + updateHint(); + } + }); + const render = (state: string) => { statusValue.text = state.charAt(0).toUpperCase() + state.slice(1); statusValue.class.remove('mcp-connecting', 'mcp-connected'); @@ -98,7 +119,12 @@ const createMcpButton = (placement: 'right' | 'account') => { toggle.on('click', () => { if (editor.call('mcp:status') === 'disconnected') { - const port = parseInt(portField.value, 10) || DEFAULT_PORT; + const port = fieldPort(); + if (port === null) { + // Connect is disabled while the port is invalid; never store a bad port either, + // or every reload would retry it + return; + } editor.call('localStorage:set', 'editor:mcp:connected', true); editor.call('localStorage:set', 'editor:mcp:port', port); editor.call('mcp:connect', port); @@ -149,7 +175,8 @@ const createMcpButton = (placement: 'right' | 'account') => { // auto-reconnect if connected before a reload (branch switch, checkpoint restore, project open) if (editor.call('localStorage:get', 'editor:mcp:connected')) { - const port = editor.call('localStorage:get', 'editor:mcp:port') || DEFAULT_PORT; + // a bad port stored before validation existed falls back to the default + const port = parsePort(editor.call('localStorage:get', 'editor:mcp:port')) ?? DEFAULT_PORT; portField.value = String(port); editor.call('mcp:connect', port); } diff --git a/src/editor/mcp/port.ts b/src/editor/mcp/port.ts new file mode 100644 index 000000000..3f419a533 --- /dev/null +++ b/src/editor/mcp/port.ts @@ -0,0 +1,20 @@ +const MIN_PORT = 1; +const MAX_PORT = 65535; + +/** + * Parse an MCP server port from the port field or local storage. new WebSocket() throws on a + * port outside 1-65535, so anything that isn't a whole number in that range is rejected. + * + * @param value - The raw value: the port field's text or a stored port. + * @returns The port, or null if the value isn't a valid port. + */ +const parsePort = (value: unknown): number | null => { + const text = String(value ?? '').trim(); + if (!/^\d{1,5}$/.test(text)) { + return null; + } + const port = Number(text); + return port >= MIN_PORT && port <= MAX_PORT ? port : null; +}; + +export { parsePort }; diff --git a/test/editor/mcp/port.test.ts b/test/editor/mcp/port.test.ts new file mode 100644 index 000000000..b05b007ef --- /dev/null +++ b/test/editor/mcp/port.test.ts @@ -0,0 +1,37 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { parsePort } from '../../../src/editor/mcp/port'; + +describe('parsePort', () => { + it('accepts ports from 1 to 65535', () => { + expect(parsePort('52000')).to.equal(52000); + expect(parsePort('1')).to.equal(1); + expect(parsePort('65535')).to.equal(65535); + expect(parsePort(' 8080 ')).to.equal(8080); + }); + + it('accepts a stored number', () => { + expect(parsePort(52000)).to.equal(52000); + }); + + it('rejects ports out of range, which new WebSocket() throws on', () => { + // values seen in Sentry: a 52000 typo and two other out-of-range ports + expect(parsePort('520000')).to.equal(null); + expect(parsePort('400000')).to.equal(null); + expect(parsePort('5475445')).to.equal(null); + expect(parsePort('65536')).to.equal(null); + expect(parsePort('0')).to.equal(null); + expect(parsePort(-1)).to.equal(null); + }); + + it('rejects anything that is not a whole number', () => { + expect(parsePort('')).to.equal(null); + expect(parsePort('52000abc')).to.equal(null); + expect(parsePort('5e4')).to.equal(null); + expect(parsePort('52000.5')).to.equal(null); + expect(parsePort(52000.5)).to.equal(null); + expect(parsePort(null)).to.equal(null); + expect(parsePort(undefined)).to.equal(null); + }); +});