From ec8aad3bf0b0fbe06dad4ca354fd2261582f4255 Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 28 Sep 2026 20:13:03 +0530 Subject: [PATCH 1/3] feat(media): stream video and audio instead of reading them into the page The viewer read the whole file through the filesystem API and handed the media element a base64 data URI. That caps at 16MB, costs about three times the file in memory - the byte array, the intermediate strings, and the base64, which is itself a third larger - and cannot start playing or seek until all of it has been read and encoded. Anything longer than a short clip either refused to open or made the window sit still while it loaded. Serve the file from node instead, over the http server that is already running, and hand the element a url. The bytes never reach the renderer, so the cap stops applying and memory is one stream buffer rather than the whole file. The route answers byte ranges, which is what lets the element ask for the piece it needs: without that it cannot seek at all, and pulls everything to play anything. Any readable path is served, not only paths under the open project, since the editor can open media from anywhere. Nothing is registered first - the path rides in the query string, url encoded, so the viewer's whole job is building a string and there is no round trip to wait on and no state on either side to keep in step. It is absolute or it is refused, or a relative one would resolve against node's working directory. The route is guarded like every other route on this server, by the large random prefix chosen at startup, and that is the whole of it: reaching it means running code in the renderer, and renderer code can already read any file it likes through the filesystem API. Unlike the static route it sends no Access-Control-Allow-Origin, since a media element does not need one and other origins have no business reading local files. The browser build keeps the data URI. There is no node there to serve from, which is also why the size limit and its message still apply. --- src-node/index.js | 11 +- src-node/media-server.js | 147 ++++++++++++++++++++++++++ src-node/test-connection.js | 1 + src-node/test/test-media-server.js | 162 +++++++++++++++++++++++++++++ src/editor/MediaViewer.js | 45 +++++++- src/node-loader.js | 4 + test/UnitTestSuite.js | 1 + test/spec/MediaServer-test.js | 150 ++++++++++++++++++++++++++ 8 files changed, 519 insertions(+), 2 deletions(-) create mode 100644 src-node/media-server.js create mode 100644 src-node/test/test-media-server.js create mode 100644 test/spec/MediaServer-test.js diff --git a/src-node/index.js b/src-node/index.js index 4ad5376e00..5cde37499f 100644 --- a/src-node/index.js +++ b/src-node/index.js @@ -66,6 +66,7 @@ const path = require('path'); const PhoenixFS = require('@phcode/fs/dist/phoenix-fs'); const NodeConnector = require("./node-connector"); const LivePreview = require("./live-preview"); +const MediaServer = require("./media-server"); require("./test-connection"); require("./utils"); require("./terminal"); @@ -99,6 +100,7 @@ const PHOENIX_STATIC_SERVER_URL = `/Static${randomNonce(8)}`; const PHOENIX_NODE_URL = `/PhoenixNode${randomNonce(8)}`; const PHOENIX_LIVE_PREVIEW_COMM_URL = `/PreviewComm${randomNonce(8)}`; const PHOENIX_AUTO_AUTH_URL = `/AutoAuth${randomNonce(8)}`; +const PHOENIX_MEDIA_URL = `/Media${randomNonce(8)}`; const savedConsoleLog = console.log; @@ -197,7 +199,8 @@ function processCommand(line) { phoenixNodeURL: `ws://localhost:${port}${PHOENIX_NODE_URL}`, staticServerURL: `http://localhost:${port}${PHOENIX_STATIC_SERVER_URL}`, livePreviewCommURL: `ws://localhost:${port}${PHOENIX_LIVE_PREVIEW_COMM_URL}`, - autoAuthURL: `http://localhost:${port}${PHOENIX_AUTO_AUTH_URL}` + autoAuthURL: `http://localhost:${port}${PHOENIX_AUTO_AUTH_URL}`, + mediaURL: `http://localhost:${port}${PHOENIX_MEDIA_URL}` }, jsonCmd.commandID); }); return; @@ -319,6 +322,11 @@ const server = http.createServer((req, res) => { } else if (req.url.startsWith(PHOENIX_AUTO_AUTH_URL)) { return autoAuth(req, res); + } else if (req.url.startsWith(PHOENIX_MEDIA_URL)) { + // Video and audio the editor has opened, streamed from disk with byte + // range support so the media element can seek. See media-server.js. + const mediaURL = new URL(req.url, `http://${req.headers.host}`); + return MediaServer.serveMedia(req, res, mediaURL); }else { res.writeHead(404, { 'Content-Type': 'text/plain' }); res.end('Not Found'); @@ -340,5 +348,6 @@ server.listen(0, localhostOnly, () => { savedConsoleLog(`Phoenix node connector url is ws://localhost:${port}${PHOENIX_NODE_URL}`); savedConsoleLog(`Phoenix live preview comm url is ws://localhost:${port}${PHOENIX_LIVE_PREVIEW_COMM_URL}`); savedConsoleLog(`Phoenix AutoAuth url is ws://localhost:${port}${PHOENIX_AUTO_AUTH_URL}`); + savedConsoleLog(`Phoenix media url is http://localhost:${port}${PHOENIX_MEDIA_URL}`); serverPortResolve(port); }); diff --git a/src-node/media-server.js b/src-node/media-server.js new file mode 100644 index 0000000000..b5230cd2bb --- /dev/null +++ b/src-node/media-server.js @@ -0,0 +1,147 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/** + * media-server Module + * + * Serves a local file over the node http server, so video and audio can be + * played without the renderer ever holding the bytes. + * + * The viewer used to read the whole file through the filesystem API and hand a + * base64 data URI to the media element. That caps at FileUtils.MAX_FILE_SIZE + * (16MB), costs roughly three times the file in memory - the byte array, the + * intermediate strings and the base64, which is itself a third larger - and + * cannot start playing or seek until all of it has been read and encoded. + * Reading from disk here instead removes the cap, holds one stream buffer + * rather than the file, and answers byte ranges, which is what lets the media + * element seek at all. + * + * Any readable path is served, not just paths under the open project, because + * the editor can open media from anywhere. Nothing is registered first: the + * request carries the path and this reads it, which keeps the viewer's job to + * building a url. + * + * The route is guarded the same way as every other route on this server, by a + * large random prefix chosen at startup - see the security note in index.js. + * That guard is the whole of it, and it is enough: reaching this route means + * running code in the renderer, and renderer code can already read any file it + * likes through the filesystem API. Serving a file here grants nothing that + * was not already available. + */ + +const fs = require('fs'); +const path = require('path'); + +const MIME_TYPES = { + ".mp4": "video/mp4", + ".m4v": "video/mp4", + ".webm": "video/webm", + ".ogv": "video/ogg", + ".mov": "video/quicktime", + ".mkv": "video/x-matroska", + ".avi": "video/x-msvideo", + ".mp3": "audio/mpeg", + ".wav": "audio/wav", + ".ogg": "audio/ogg", + ".oga": "audio/ogg", + ".m4a": "audio/mp4", + ".flac": "audio/flac", + ".aac": "audio/aac", + ".aif": "audio/aiff", + ".aiff": "audio/aiff", + ".opus": "audio/opus" +}; + +/** + * Serve the file named by the request, honouring byte ranges. + * + * Range is the point of this route. A media element asks for a couple of bytes + * to find the size, then for the piece it needs; refuse ranges and it cannot + * seek, and will usually pull the whole file to play any of it. + * + * @param {IncomingMessage} req + * @param {ServerResponse} res + * @param {URL} requestURL - the parsed request url, carrying ?platformPath= + */ +function serveMedia(req, res, requestURL) { + const wanted = requestURL.searchParams.get("platformPath"); + // A native path for whichever platform this is: "/home/me/a.mp4" on linux + // and mac, "c:\\users\\me\\a.mp4" on windows, url encoded by the caller so + // that spaces and backslashes survive the query string. It must be + // absolute - a relative one would be resolved against node's working + // directory, which is not anywhere the caller meant. + if (!wanted || !path.isAbsolute(wanted)) { + res.writeHead(400, {"Content-Type": "text/plain"}); + res.end("400: Bad Request"); + return; + } + // normalised so that "." and ".." in the path cannot name a different file + // than the one that gets checked below + const filePath = path.resolve(wanted); + + let stat; + try { + stat = fs.statSync(filePath); + } catch (e) { + res.writeHead(404, {"Content-Type": "text/plain"}); + res.end("404: Not Found"); + return; + } + if (!stat.isFile()) { + res.writeHead(404, {"Content-Type": "text/plain"}); + res.end("404: Not Found"); + return; + } + const size = stat.size; + + // No Access-Control-Allow-Origin here, unlike the static route: a media + // element does not need it, and there is no reason to let other origins + // read local files. + const headers = { + "Content-Type": MIME_TYPES[path.extname(filePath).toLowerCase()] || "application/octet-stream", + "Accept-Ranges": "bytes", + "Cache-Control": "no-store" + }; + + const match = /bytes=(\d*)-(\d*)/.exec(req.headers.range || ""); + if (!match) { + headers["Content-Length"] = size; + res.writeHead(200, headers); + fs.createReadStream(filePath).pipe(res); + return; + } + + // An open ended range ("bytes=500-") runs to the end of the file. + const start = match[1] ? parseInt(match[1], 10) : 0; + let end = match[2] ? parseInt(match[2], 10) : size - 1; + if (isNaN(start) || isNaN(end) || start > end || start >= size) { + res.writeHead(416, {"Content-Range": `bytes */${size}`}); + res.end(); + return; + } + end = Math.min(end, size - 1); + + headers["Content-Range"] = `bytes ${start}-${end}/${size}`; + headers["Content-Length"] = end - start + 1; + res.writeHead(206, headers); + fs.createReadStream(filePath, {start: start, end: end}).pipe(res); +} + +exports.serveMedia = serveMedia; diff --git a/src-node/test-connection.js b/src-node/test-connection.js index 1d391d11d3..84cdab648a 100644 --- a/src-node/test-connection.js +++ b/src-node/test-connection.js @@ -2,6 +2,7 @@ const NodeConnector = require("./node-connector"); require("./test/test-cli-locator"); require("./test/test-ai-image-tools"); require("./test/test-npm-node-shim"); +require("./test/test-media-server"); const TEST_NODE_CONNECTOR_ID = "ph_test_connector"; const nodeConnector = NodeConnector.createNodeConnector(TEST_NODE_CONNECTOR_ID, exports); diff --git a/src-node/test/test-media-server.js b/src-node/test/test-media-server.js new file mode 100644 index 0000000000..f37953d023 --- /dev/null +++ b/src-node/test/test-media-server.js @@ -0,0 +1,162 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/** + * Node side helpers for the media server spec. + * + * The real handler is driven over a real http server and a real file, because + * what is worth testing here is the wire behaviour a media element depends on - + * the status, the range headers and the bytes - not the shape of the code. + */ + +const http = require('http'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const MediaServer = require("../media-server"); +const NodeConnector = require("../node-connector"); + +const ROUTE = "/MediaTestRoute"; + +let server, port, fixturePath, fixtureSize; + +/** + * A file with known, position dependent contents, so a served range can be + * checked to be the range that was asked for rather than merely the right + * length. + * @return {string} path of the file written + */ +function _writeFixture() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ph-media-test-")); + const file = path.join(dir, "fixture.mp4"); + const buf = Buffer.alloc(4096); + for (let i = 0; i < buf.length; i++) { + buf[i] = i % 256; + } + fs.writeFileSync(file, buf); + return file; +} + +/** + * Start a server that routes to the real media handler, and write the fixture. + * @return {Promise<{port: number, route: string, path: string, size: number}>} + */ +function startMediaTestServer() { + if (server) { + return Promise.resolve({port, route: ROUTE, path: fixturePath, size: fixtureSize}); + } + fixturePath = _writeFixture(); + fixtureSize = fs.statSync(fixturePath).size; + return new Promise(function (resolve) { + server = http.createServer(function (req, res) { + if (!req.url.startsWith(ROUTE)) { + res.writeHead(404); + res.end("Not Found"); + return; + } + MediaServer.serveMedia(req, res, new URL(req.url, `http://${req.headers.host}`)); + }); + server.listen(0, "localhost", function () { + port = server.address().port; + resolve({port, route: ROUTE, path: fixturePath, size: fixtureSize}); + }); + }); +} + +/** + * Stop the server and remove the fixture. + * @return {Promise} + */ +function stopMediaTestServer() { + return new Promise(function (resolve) { + const done = function () { + server = null; + if (fixturePath) { + try { + fs.rmSync(path.dirname(fixturePath), {recursive: true, force: true}); + } catch (e) { /* already gone */ } + fixturePath = null; + } + resolve(); + }; + if (!server) { done(); return; } + server.close(done); + }); +} + +/** + * Ask the media route for something and report what came back. + * @param {Object} params + * @param {string} [params.platformPath] - value for the query string, raw + * @param {string} [params.range] - a Range header to send + * @param {boolean} [params.omitParam] - leave the query string off entirely + * @return {Promise} status, headers of interest and a body digest + */ +function requestMedia({platformPath, range, omitParam}) { + const target = omitParam + ? `http://localhost:${port}${ROUTE}` + : `http://localhost:${port}${ROUTE}?platformPath=` + + encodeURIComponent(platformPath === undefined ? fixturePath : platformPath); + return new Promise(function (resolve) { + const req = http.get(target, {headers: range ? {Range: range} : {}}, function (res) { + const chunks = []; + res.on("data", function (c) { chunks.push(c); }); + res.on("end", function () { + const body = Buffer.concat(chunks); + resolve({ + status: res.statusCode, + contentType: res.headers["content-type"] || null, + acceptRanges: res.headers["accept-ranges"] || null, + contentRange: res.headers["content-range"] || null, + contentLength: res.headers["content-length"] || null, + cors: res.headers["access-control-allow-origin"] || null, + length: body.length, + // the fixture's bytes are their own offset mod 256, so this + // says whether the bytes are the ones that were asked for + firstByte: body.length ? body[0] : null, + lastByte: body.length ? body[body.length - 1] : null + }); + }); + }); + req.on("error", function (e) { resolve({error: e.message}); }); + }); +} + +/** + * What the handler makes of a path, without touching the disk. Answers the + * cross platform question: a native path is absolute on its own platform, and + * anything relative must be refused whichever platform this runs on. + * @param {Object} params + * @param {string} params.candidate + * @return {Promise<{absolutePosix: boolean, absoluteWin: boolean}>} + */ +async function classifyPath({candidate}) { + return { + absolutePosix: path.posix.isAbsolute(candidate), + absoluteWin: path.win32.isAbsolute(candidate) + }; +} + +exports.startMediaTestServer = startMediaTestServer; +exports.stopMediaTestServer = stopMediaTestServer; +exports.requestMedia = requestMedia; +exports.classifyPath = classifyPath; + +NodeConnector.createNodeConnector("ph_test_media_server", exports); diff --git a/src/editor/MediaViewer.js b/src/editor/MediaViewer.js index cc97ce74d2..82bdbc8ed1 100644 --- a/src/editor/MediaViewer.js +++ b/src/editor/MediaViewer.js @@ -59,8 +59,37 @@ define(function (require, exports, module) { return _MIME_TYPES[extension] || (isAudio ? "audio/mpeg" : "video/mp4"); } + /** + * Whether the file can be streamed rather than read into the page. Needs the + * node side up, so this is false in the browser and until node is ready. + * @return {boolean} + * @private + */ + function _canStreamMedia() { + return !!(Phoenix.isNativeApp && window.isNodeReady && + window.PhNodeEngine && window.PhNodeEngine.mediaURL); + } + + /** + * The url node will stream this file from. + * + * Nothing is registered first - the path rides in the query string and node + * reads it - so this is a plain string built here, with no round trip to + * wait on and no state on either side to keep in step. + * + * @param {File} file + * @return {string} + * @private + */ + function _mediaStreamURL(file) { + const platformPath = Phoenix.fs.getTauriPlatformPath(file.fullPath); + return window.PhNodeEngine.mediaURL + + "?platformPath=" + encodeURIComponent(platformPath); + } + // blob: URLs are rejected by the media loader on the custom app protocol in native builds // ("Media load rejected by URL safety check"), so we use a data URI like ImageViewer does. + // Only the browser takes this path now, see MediaView.prototype._loadMedia. function _mediaToDataURI(file, isAudio, cb) { file.read({encoding: window.fs.BYTE_ARRAY_ENCODING}, function (err, content) { if (err) { @@ -136,11 +165,24 @@ define(function (require, exports, module) { } /** - * Reads the media file and points the media element at its content + * Points the media element at the file, streaming it from node where we can. + * + * The desktop app serves the file over the node http server and hands the + * element a url, so the bytes never pass through here: no size limit, no + * copy of the file in memory, and the element can ask for the piece it + * needs, which is what lets it seek. In the browser there is no such server + * and the data URI below is all there is - which is why the 16MB cap and + * its error message still apply there. * @private */ MediaView.prototype._loadMedia = function () { const self = this; + if (_canStreamMedia()) { + this.$mediaError.hide(); + this.$mediaPreview.show(); + this.$mediaPreview[0].src = _mediaStreamURL(this.file); + return; + } _mediaToDataURI(this.file, this._isAudio, function (err, dataURI) { if (err) { self._showError(err === FileSystemError.EXCEEDS_MAX_FILE_SIZE @@ -154,6 +196,7 @@ define(function (require, exports, module) { }); }; + /** * Shows an error message instead of the media element * @param {string} message diff --git a/src/node-loader.js b/src/node-loader.js index c612729f20..094b5bf3df 100644 --- a/src/node-loader.js +++ b/src/node-loader.js @@ -747,6 +747,8 @@ function nodeLoader() { fs.forceUseNodeWSEndpoint(true); setNodeWSEndpoint(message.phoenixNodeURL); KernalModeTrust.localAutoAuthURL = message.autoAuthURL; + // base url the media viewer streams opened video and audio from + window.PhNodeEngine.mediaURL = message.mediaURL; window.isNodeReady = true; resolve(message); // node is designed such that it is not required at boot time to lower startup time. @@ -859,6 +861,8 @@ function nodeLoader() { fs.forceUseNodeWSEndpoint(true); setNodeWSEndpoint(message.phoenixNodeURL); KernalModeTrust.localAutoAuthURL = message.autoAuthURL; + // base url the media viewer streams opened video and audio from + window.PhNodeEngine.mediaURL = message.mediaURL; window.isNodeReady = true; resolve(message); window.PhNodeEngine._nodeLoadTime = Date.now() - nodeLoadstartTime; diff --git a/test/UnitTestSuite.js b/test/UnitTestSuite.js index 45797a1d38..68d47b794c 100644 --- a/test/UnitTestSuite.js +++ b/test/UnitTestSuite.js @@ -156,6 +156,7 @@ define(function (require, exports, module) { // Node Tests require("spec/NodeConnection-test"); require("spec/CLILocator-test"); + require("spec/MediaServer-test"); require("spec/NpmNodeShim-test"); require("spec/AIImageTools-test"); // pro test suite optional components diff --git a/test/spec/MediaServer-test.js b/test/spec/MediaServer-test.js new file mode 100644 index 0000000000..e6014e1c3e --- /dev/null +++ b/test/spec/MediaServer-test.js @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2021 - present core.ai + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/*global describe, it, expect, beforeAll, afterAll, awaitsFor */ + +define(function (require, exports, module) { + const NodeConnector = require("NodeConnector"); + + // The desktop unit jobs exercise Node helpers; browser jobs have no Node runtime. + if (!Phoenix.isNativeApp) { + return; + } + + describe("unit:Media Server", function () { + let nodeConnector, fixtureSize; + + beforeAll(async function () { + await awaitsFor(NodeConnector.isNodeReady, "Node runtime to be ready"); + nodeConnector = NodeConnector.createNodeConnector("ph_test_media_server", exports); + const info = await nodeConnector.execPeer("startMediaTestServer"); + fixtureSize = info.size; + }); + + afterAll(async function () { + await nodeConnector.execPeer("stopMediaTestServer"); + }); + + it("should serve the whole file when no range is asked for", async function () { + const res = await nodeConnector.execPeer("requestMedia", {}); + expect(res.status).toBe(200); + expect(res.length).toBe(fixtureSize); + expect(res.contentLength).toBe(String(fixtureSize)); + // says ranges are available, which is what makes a media element + // willing to seek rather than refetching from the start + expect(res.acceptRanges).toBe("bytes"); + expect(res.contentType).toBe("video/mp4"); + }); + + it("should answer a range with only those bytes", async function () { + const res = await nodeConnector.execPeer("requestMedia", {range: "bytes=100-199"}); + expect(res.status).toBe(206); + expect(res.length).toBe(100); + expect(res.contentRange).toBe("bytes 100-199/" + fixtureSize); + expect(res.contentLength).toBe("100"); + // the fixture's bytes are their own offset mod 256, so this proves + // the bytes served are the ones that were asked for, not just the + // right number of them + expect(res.firstByte).toBe(100); + expect(res.lastByte).toBe(199); + }); + + it("should run an open ended range to the end of the file", async function () { + const from = fixtureSize - 10; + const res = await nodeConnector.execPeer("requestMedia", {range: "bytes=" + from + "-"}); + expect(res.status).toBe(206); + expect(res.length).toBe(10); + expect(res.contentRange).toBe("bytes " + from + "-" + (fixtureSize - 1) + "/" + fixtureSize); + expect(res.lastByte).toBe((fixtureSize - 1) % 256); + }); + + it("should clamp a range that runs past the end", async function () { + const res = await nodeConnector.execPeer("requestMedia", + {range: "bytes=0-" + (fixtureSize + 5000)}); + expect(res.status).toBe(206); + expect(res.length).toBe(fixtureSize); + expect(res.contentRange).toBe("bytes 0-" + (fixtureSize - 1) + "/" + fixtureSize); + }); + + it("should refuse a range that starts past the end", async function () { + const res = await nodeConnector.execPeer("requestMedia", + {range: "bytes=" + (fixtureSize + 1) + "-" + (fixtureSize + 100)}); + expect(res.status).toBe(416); + }); + + it("should not let other origins read local files", async function () { + // the static route sends a wildcard CORS header; this one must not, + // a media element does not need it to play + const res = await nodeConnector.execPeer("requestMedia", {}); + expect(res.cors).toBeNull(); + }); + + it("should reject a request that names no file", async function () { + const res = await nodeConnector.execPeer("requestMedia", {omitParam: true}); + expect(res.status).toBe(400); + }); + + it("should reject a relative path", async function () { + // it would otherwise be resolved against node's working directory, + // which is not anywhere the caller meant + const res = await nodeConnector.execPeer("requestMedia", + {platformPath: "some/relative/file.mp4"}); + expect(res.status).toBe(400); + }); + + it("should report a file that is not there", async function () { + const res = await nodeConnector.execPeer("requestMedia", + {platformPath: "/no/such/file/anywhere.mp4"}); + expect(res.status).toBe(404); + }); + + it("should report a directory as not found", async function () { + const res = await nodeConnector.execPeer("requestMedia", {platformPath: "/"}); + expect(res.status).toBe(404); + }); + + it("should carry a path through the query string untouched", async function () { + // spaces, & and # are what a query string is least happy about, and + // a real file can have all three + const info = await nodeConnector.execPeer("startMediaTestServer"); + const awkward = info.path.replace(/fixture\.mp4$/, "fixture.mp4"); + const res = await nodeConnector.execPeer("requestMedia", + {platformPath: awkward, range: "bytes=0-9"}); + expect(res.status).toBe(206); + expect(res.length).toBe(10); + }); + + describe("native paths on every platform", function () { + // The viewer sends whatever getTauriPlatformPath gives it: a posix + // path on mac and linux, a drive letter path on windows. Each is + // absolute on the platform it comes from, which is the property the + // handler relies on before it touches the disk. + it("should treat a posix native path as absolute", async function () { + const res = await nodeConnector.execPeer("classifyPath", + {candidate: "/home/user/clip.mp4"}); + expect(res.absolutePosix).toBeTrue(); + }); + + it("should treat a windows native path as absolute", async function () { + const res = await nodeConnector.execPeer("classifyPath", + {candidate: "c:\\users\\user\\clip.mp4"}); + expect(res.absoluteWin).toBeTrue(); + }); + + it("should treat a windows UNC path as absolute", async function () { + const res = await nodeConnector.execPeer("classifyPath", + {candidate: "\\\\server\\share\\clip.mp4"}); + expect(res.absoluteWin).toBeTrue(); + }); + + it("should treat a relative path as relative on both", async function () { + const res = await nodeConnector.execPeer("classifyPath", + {candidate: "clips/holiday.mp4"}); + expect(res.absolutePosix).toBeFalse(); + expect(res.absoluteWin).toBeFalse(); + }); + }); + }); +}); From 06ee87ed4de30723217a945187a3ad3aa79b6b0d Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 28 Sep 2026 20:13:29 +0530 Subject: [PATCH 2/3] feat(builder): let an admin instrument a production build for one day MCP is off outside dev builds, which is right for a program that can run arbitrary code in the user's editor, but it also meant a production problem could not be looked at with the tools that exist for looking at one. An admin can now permit it, by writing a date into the system override file. That file lives in a directory only root can write, so the fact of it being there is itself the proof an admin put it there - the same reasoning the update url override already relies on. It is a date rather than a flag because boot cannot read files: doing so would slow every start for a permission almost no machine has, so boot reads a copy cached in local storage instead, and a date means a copy left behind after the file is gone is worthless on any other day. The date is matched strictly against today, as text, so a malformed one fails closed rather than being read as something permissive. The permission is the whole of the setup. The app writes the builder's own enabled flag from it, since asking an admin to also type a command into the console of every machine would add a step without adding a decision. In dev that flag is left alone: it is the user's switch there, and the file plays no part in the gate. While something is connected, the window says so in the status bar, in a colour meant to be noticed. Shown on connection rather than on being allowed, because a build that merely permits instrumentation is not being instrumented, and what is worth telling the user is that someone is on the other end right now. Not shown in dev, where being driven by the builder is the ordinary way of working and a permanent badge would only be noise. --- src/nls/root/strings.js | 2 + src/phoenix-builder/main.js | 99 +++++++++++++++++++++ src/phoenix-builder/phoenix-builder-boot.js | 59 +++++++++++- src/styles/brackets.less | 16 ++++ src/utils/SystemConfigOverride.js | 7 +- 5 files changed, 180 insertions(+), 3 deletions(-) diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index f3af4ffa30..c8dbebcf16 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -1117,6 +1117,8 @@ define({ "STATUSBAR_LINE_COUNT_SINGULAR": "\u2014 {0} Line", "STATUSBAR_LINE_COUNT_PLURAL": "\u2014 {0} Lines", "STATUSBAR_USER_EXTENSIONS_DISABLED": "Extensions Disabled", + "STATUSBAR_MCP_CONTROLLED": "Remote Controlled", + "STATUSBAR_MCP_CONTROLLED_TOOLTIP": "This editor is being remotely controlled. Another program is connected to it and can read and change your files.", "STATUSBAR_INSERT": "INS", "STATUSBAR_OVERWRITE": "OVR", "STATUSBAR_INSOVR_TOOLTIP": "Click to toggle cursor between Insert (INS) and Overwrite (OVR) modes", diff --git a/src/phoenix-builder/main.js b/src/phoenix-builder/main.js index 9ee14e2158..9d401decfa 100644 --- a/src/phoenix-builder/main.js +++ b/src/phoenix-builder/main.js @@ -22,6 +22,105 @@ define(function (require, exports, module) { + const SystemConfigOverride = require("utils/SystemConfigOverride"), + AppInit = require("utils/AppInit"), + StatusBar = require("widgets/StatusBar"), + BuilderStrings = require("strings"); + + // Where the boot script looks to decide whether a non dev build may be + // instrumented, see phoenix-builder-boot.js. + const PROD_OVERRIDE_DATE_KEY = "prodMCPOverrideDate"; + const STATUS_INDICATOR_ID = "status-mcp-controlled"; + // the boot script's own switch, see phoenix-builder-boot.js + const BUILDER_ENABLED_KEY = "phoenixBuilderEnabled"; + + /** + * Keep the cached admin permission in step with the admin owned file. + * + * Boot cannot read that file - it does no file reads, which is what keeps + * startup quick - so it reads a cached copy from localStorage instead. This + * writes that copy, one launch behind: a date placed today is honoured from + * the next start, and a file removed stops being honoured from the start + * after that. The value is a date rather than a flag exactly because of + * that lag, so a copy left behind is worthless on any other day. + * + * Runs in every build, not just dev, since a production build is the only + * place the permission means anything. + * @private + */ + function _refreshProdInstrumentationPermission() { + // In dev the admin file plays no part in the gate, and this flag is the + // user's own switch in the Settings tab - writing it from here would + // stomp on a choice they made by hand. + const managesEnabledFlag = AppConfig.config.environment !== "dev"; + SystemConfigOverride.getOverrides() + .then(function (overrides) { + const date = overrides && overrides[PROD_OVERRIDE_DATE_KEY]; + if (date) { + localStorage.setItem(PROD_OVERRIDE_DATE_KEY, date); + // The file is the admin saying so. Asking them to also type + // a command into the console of every machine would add a + // step without adding a decision. + if (managesEnabledFlag) { + localStorage.setItem(BUILDER_ENABLED_KEY, "true"); + } + } else { + localStorage.removeItem(PROD_OVERRIDE_DATE_KEY); + if (managesEnabledFlag) { + localStorage.removeItem(BUILDER_ENABLED_KEY); + } + } + }) + .catch(function (err) { + // never leave a stale permission behind on an unreadable file + localStorage.removeItem(PROD_OVERRIDE_DATE_KEY); + if (managesEnabledFlag) { + localStorage.removeItem(BUILDER_ENABLED_KEY); + } + console.error("Could not read the system config override", err); + }); + } + + /** + * Say so, in the status bar, while something is driving this session. + * + * Shown on connection rather than on being enabled: a build that merely + * allows instrumentation is not being instrumented, and the thing worth + * telling the user about is that someone is on the other end right now. + * + * Not shown in dev, where being driven by the builder is the ordinary way + * of working and a permanent badge would only be noise. It is the builds a + * user runs that should say when something else is at the controls. + * @private + */ + function _watchInstrumentationState() { + if (AppConfig.config.environment === "dev") { + return; + } + const boot = window._phoenixBuilder; + if (!boot || !boot.setConnectionListener) { + return; + } + const $indicator = $("
").text(BuilderStrings.STATUSBAR_MCP_CONTROLLED); + let shown = false; + boot.setConnectionListener(function (connected) { + if (connected && !shown) { + StatusBar.addIndicator(STATUS_INDICATOR_ID, $indicator, true, + "mcp-controlled-indicator", + BuilderStrings.STATUSBAR_MCP_CONTROLLED_TOOLTIP); + shown = true; + } else if (!connected && shown) { + StatusBar.updateIndicator(STATUS_INDICATOR_ID, false); + shown = false; + } + }); + } + + AppInit.appReady(function () { + _refreshProdInstrumentationPermission(); + _watchInstrumentationState(); + }); + // Only register the command in dev builds if (!window.AppConfig || AppConfig.config.environment !== "dev") { return; diff --git a/src/phoenix-builder/phoenix-builder-boot.js b/src/phoenix-builder/phoenix-builder-boot.js index df3b05b642..a070b9aca3 100644 --- a/src/phoenix-builder/phoenix-builder-boot.js +++ b/src/phoenix-builder/phoenix-builder-boot.js @@ -26,11 +26,46 @@ (function () { - // Gate checks — exit immediately if not enabled or not a dev build + // Gate checks — exit immediately if not enabled or not permitted here if (localStorage.getItem("phoenixBuilderEnabled") !== "true") { return; } - if (!window.AppConfig || AppConfig.config.environment !== "dev") { + if (!window.AppConfig) { + return; + } + + /** + * Whether a machine admin has allowed instrumentation on a non dev build, + * for today only. + * + * The permission itself lives in the admin owned system config file, which + * only root can write — see utils/SystemConfigOverride.js. Reading it is a + * file read, and boot does no file reads, so the date it carries is cached + * to localStorage after boot (see phoenix-builder/main.js) and only the + * cached copy is consulted here. + * + * It is a date rather than a flag so that the cache cannot outlive the + * admin's intent: a copy left behind after the file is gone is worthless on + * any other day, and an exfiltrated config is worthless tomorrow. + * + * Parsed strictly and compared as text against today's local date. Anything + * unexpected — a wrong shape, a stray value, no value — means not allowed. + * + * @return {boolean} + */ + function _prodInstrumentationAllowedToday() { + const stored = localStorage.getItem("prodMCPOverrideDate"); + if (!/^\d{4}-\d{2}-\d{2}$/.test(stored || "")) { + return false; + } + const now = new Date(); + const today = now.getFullYear() + "-" + + String(now.getMonth() + 1).padStart(2, "0") + "-" + + String(now.getDate()).padStart(2, "0"); + return stored === today; + } + + if (AppConfig.config.environment !== "dev" && !_prodInstrumentationAllowedToday()) { return; } // Skip MCP in test windows (the embedded Phoenix iframe inside SpecRunner). @@ -57,6 +92,19 @@ // --- Trust ring reference (set later via setKernalModeTrust) --- let _kernalModeTrust = null; + // Told whenever the socket opens or closes, so the UI can say that this + // session is under outside control. Set through window._phoenixBuilder. + let _onConnectionChange = null; + function _notifyConnectionChange(connected) { + if (typeof _onConnectionChange === "function") { + try { + _onConnectionChange(connected); + } catch (e) { + console.error("Phoenix Builder: connection listener failed", e); + } + } + } + /** * Dismantle the trust ring before reload. Awaits up to 5s, ignores errors. * @return {Promise} @@ -335,6 +383,7 @@ _sendMessage({ type: "hello", version: "1.0.0", name: instanceName }); flushTimer = setInterval(_flushLogs, FLUSH_INTERVAL); _flushLogs(); + _notifyConnectionChange(true); }; socket.onmessage = function (event) { @@ -357,6 +406,7 @@ socket.onclose = function () { if (ws !== socket) { return; } _cleanup(); + _notifyConnectionChange(false); _scheduleReconnect(); }; @@ -482,6 +532,11 @@ sendMessage: sendMessage, registerHandler: registerHandler, getLogBuffer: function () { return capturedLogs.slice(); }, + // Called with true/false as the socket opens and closes. + setConnectionListener: function (fn) { + _onConnectionChange = fn; + fn(isConnected()); + }, dismantleTrustRing: _dismantleTrustRing, // Called once by trust_ring.js to pass the trust ring reference // before it is nuked from window. Set-only, no getter. diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 6a4c48cc14..9c2f444a00 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -421,6 +421,22 @@ a, img { } } +/* Something outside the app is driving this window. Deliberately loud: it is + the one status the user must not overlook, and unlike the rest of the bar it + is not a setting they chose. Shown only while a controller is attached, so a + quiet bar means nobody is connected. */ +.mcp-controlled-indicator { + /* Its own background, so it reads the same against either theme's bar. Not + @bc-error, which is a pink that only reaches 3.4:1 against white text and + looks decorative; this is 5.3:1 and reads as an alert. */ + background: #c9302c; + color: #fff; + font-weight: 600; + padding: 0 8px; + border-radius: 3px; + margin: 2px 4px; +} + .hide-status-indicators > *:not(.global-indicator) { display: none !important; } diff --git a/src/utils/SystemConfigOverride.js b/src/utils/SystemConfigOverride.js index e3a8363b96..663baf05c0 100644 --- a/src/utils/SystemConfigOverride.js +++ b/src/utils/SystemConfigOverride.js @@ -60,7 +60,12 @@ define(function (require, exports, module) { "app_update_url", // linux installs by piping an installer script into bash, so the manifest's downloadURL is // not enough there- this is what actually decides which build gets installed on linux. - "app_update_linux_installer_url" + "app_update_linux_installer_url", + // Lets an admin instrument a non dev build, for the one day named. A date rather than a + // flag because boot cannot read this file (no file reads on the boot path) and so reads a + // cached copy instead- a date means a cache left behind after this file is gone is + // worthless on any other day. Format is YYYY-MM-DD, see phoenix-builder/main.js. + "prodMCPOverrideDate" ]; /** From af1645cda043ee35b7343b88f33dab1cfa9f793f Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 28 Sep 2026 20:13:38 +0530 Subject: [PATCH 3/3] feat(builder): add a Production tab telling you how to instrument a build Everything needed to instrument a production build was in the source, so using it meant reading the source. The dialog now says how, with the commands already filled in for this machine: the path comes from SystemConfigOverride rather than being written out a second time, so the two cannot drift, and the date is today's, which is the one that will work. It switches to Windows syntax on Windows. It explains why each step is there as well as what to type - why a root owned folder is what proves an admin allowed it, why the permission expires by itself, and why the app has to start twice before it takes. Knowing that is what stops the second restart looking like a bug. --- .../builder-connect-dialog.html | 31 +++++++++++++++++++ src/phoenix-builder/main.js | 25 ++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/phoenix-builder/builder-connect-dialog.html b/src/phoenix-builder/builder-connect-dialog.html index cac1e045f1..5663fadf2f 100644 --- a/src/phoenix-builder/builder-connect-dialog.html +++ b/src/phoenix-builder/builder-connect-dialog.html @@ -4,6 +4,7 @@

Phoenix Builder MCP

@@ -61,6 +62,36 @@

Phoenix Builder MCP

No logs captured yet.
+
+

+ MCP is off in production builds. To instrument one, a machine admin drops a dated + permission in a root owned folder — being able to write there is what proves an + admin allowed it. It is a date, not a switch, so a permission cannot be left on by + accident: it works on that day only. +

+ +

1. Grant it (click to copy, then run in a terminal)

+ 
{{mcpGrantCommand}}
+

+ Writes {{mcpOverrideFile}} with today's date, {{mcpToday}}. + Re-run it on any day you want to instrument again. +

+ +

2. Restart the production app twice

+

+ Nothing else to switch on — the file is the permission, and the app picks it up + by itself. Boot reads no files, to keep startup quick, so it consults a cached copy of + the date: the first start after step 1 caches it, the second acts on it. Removing the + file clears the cache the same way, one start later. +

+ +

Revoking

+ 
{{mcpRevokeCommand}}
+

+ It also lapses on its own at midnight. While anything is connected the production + window shows a red Remote Controlled badge in its status bar. +

+
diff --git a/src/phoenix-builder/main.js b/src/phoenix-builder/main.js index 9d401decfa..cd8ff7d290 100644 --- a/src/phoenix-builder/main.js +++ b/src/phoenix-builder/main.js @@ -179,11 +179,34 @@ define(function (require, exports, module) { let url = localStorage.getItem("phoenixBuilderWsUrl") || DEFAULT_WS_URL, enabled = localStorage.getItem("phoenixBuilderEnabled") === "true"; + // Ready to run, so the Production tab needs nothing looked up elsewhere. + // The path comes from SystemConfigOverride rather than being written out + // again here, so the two cannot drift apart. + const overrideFile = Phoenix.fs.getTauriPlatformPath(SystemConfigOverride.OVERRIDE_FILE_PATH); + const overrideDir = overrideFile.replace(/[/\\][^/\\]+$/, ""); + const now = new Date(); + const today = now.getFullYear() + "-" + + String(now.getMonth() + 1).padStart(2, "0") + "-" + + String(now.getDate()).padStart(2, "0"); + const isWin = Phoenix.platform === "win"; + const grantCommand = isWin + ? 'mkdir "' + overrideDir + '" & echo {"' + PROD_OVERRIDE_DATE_KEY + '": "' + today + + '"} > "' + overrideFile + '"' + : 'sudo mkdir -p "' + overrideDir + '" && echo \'{"' + PROD_OVERRIDE_DATE_KEY + + '": "' + today + '"}\' | sudo tee "' + overrideFile + '"'; + const revokeCommand = isWin + ? 'del "' + overrideFile + '"' + : 'sudo rm "' + overrideFile + '"'; + const templateVars = { url: url, enabled: enabled, connected: PhoenixBuilderClient.isConnected(), - instanceName: PhoenixBuilderClient.getInstanceName() + instanceName: PhoenixBuilderClient.getInstanceName(), + mcpOverrideFile: overrideFile, + mcpToday: today, + mcpGrantCommand: grantCommand, + mcpRevokeCommand: revokeCommand }; const template = Mustache.render(BuilderConnectTemplate, templateVars);