From e500071daa49903c2237c40e155c1d5d58f77683 Mon Sep 17 00:00:00 2001 From: beichen Date: Thu, 8 Oct 2026 15:20:16 +0800 Subject: [PATCH] fix(web-remote): secure file previews and refresh changed directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use descriptor-relative no-follow access and bounded reads for file previews. Bind requests to a cwd snapshot and unique ID, then revalidate after I/O. Add filesystem race, browser response ordering and encrypted socket regressions. Prepare bilingual in-app release notes for v0.1.28-beta.6. CN: 修复网页文件预览的符号链接竞态越界与切换目录后内容错位，并准备 beta.6 更新说明 --- .github/workflows/ci.yml | 2 +- Glint/App/ReleaseNotes.swift | 19 ++ Glint/Resources/WebRemote/web-remote.js | 71 ++++-- Glint/WebRemote/WebRemoteProtocol.swift | 220 +++++++++++++----- Glint/WebRemote/WebRemoteServer.swift | 85 +++---- GlintTests/WebRemoteProtocolTests.swift | 101 +++++++- .../WebRemoteServerIntegrationTests.swift | 65 ++++++ scripts/test-web-remote-files.mjs | 153 ++++++++++++ 8 files changed, 583 insertions(+), 133 deletions(-) create mode 100644 scripts/test-web-remote-files.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e6d599c..1441fac4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,7 +61,7 @@ jobs: scripts/verify-ghostty-resources.sh build/Build/Products/Debug/Glint.app - name: Test Web Remote browser input - run: node --test scripts/test-web-remote-ime.mjs + run: node --test scripts/test-web-remote-*.mjs - name: Test Glint run: | diff --git a/Glint/App/ReleaseNotes.swift b/Glint/App/ReleaseNotes.swift index 6b42700b..583662ef 100644 --- a/Glint/App/ReleaseNotes.swift +++ b/Glint/App/ReleaseNotes.swift @@ -51,6 +51,25 @@ enum ReleaseNotes { /// to that placeholder later got mis-attributed). See CLAUDE.md /// "发版「更新内容」" for the release-time workflow. static let all: [ReleaseNote] = [ + ReleaseNote( + version: "0.1.28-beta.6", + en: [ + "Browse the current terminal folder from Web Remote and preview text, images, and static HTML. Image previews support pinch zoom and dragging.", + "File previews stay within the selected folder even while files are being replaced. Changing the terminal directory refreshes the browser, and late responses can no longer show content from the wrong folder.", + "Web Remote automatically resumes listening when the selected network interface recovers after a Wi-Fi change or sleep, without widening access to other interfaces.", + "Shared Codex daemon events now update the matching terminal instead of leaving another pane stuck thinking or overwriting its resume session.", + "Managed shell configuration blocks no longer swallow the next line into a comment, and inline suggestions respect an existing zsh-autosuggestions setup.", + "Switching workspaces more reliably restores split terminals. Accessibility tools can identify the current workspace and selected sidebar card in both sidebar layouts." + ], + zh: [ + "网页远程端支持浏览当前终端目录，预览文本、图片和静态 HTML；图片支持双指缩放和拖动。", + "文件在并发替换时也不会绕过所选目录的访问限制。终端切换目录后，网页会刷新文件列表，迟到的响应不再显示其他目录的内容。", + "Wi-Fi 切换或睡眠唤醒后，所选网卡恢复可用时，网页远程服务会自动恢复监听，不会扩大到其他网卡。", + "共享 Codex daemon 的事件会更新对应终端，不再让其他窗格卡在思考中或覆盖其会话恢复信息。", + "托管 shell 配置块不再把下一行粘成注释，命令提示也会尊重已有的 zsh-autosuggestions 配置。", + "切换工作区时，分屏终端的恢复更加可靠；侧栏展开和折叠时，辅助功能工具都能识别当前工作区和选中的侧栏卡片。" + ] + ), ReleaseNote( version: "0.1.28-beta.4", en: [ diff --git a/Glint/Resources/WebRemote/web-remote.js b/Glint/Resources/WebRemote/web-remote.js index 6e7fb5a6..c0bf1cb5 100644 --- a/Glint/Resources/WebRemote/web-remote.js +++ b/Glint/Resources/WebRemote/web-remote.js @@ -91,6 +91,7 @@ const translations = { image_unavailable: "无法解码此图片", files_limited: "仅显示前 200 项", files_unavailable: "当前终端没有可访问的本地目录", + file_root_changed: "终端目录已变化，正在刷新…", invalid_file_path: "文件路径无效或超出项目目录", file_unavailable: "文件或目录无法读取", file_too_large: "文件超过 128 KB，无法预览", @@ -161,6 +162,7 @@ const translations = { image_unavailable: "Unable to decode this image", files_limited: "Showing the first 200 items", files_unavailable: "This terminal has no accessible local directory", + file_root_changed: "The terminal directory changed. Refreshing…", invalid_file_path: "File path is invalid or outside the workspace", file_unavailable: "File or folder cannot be read", file_too_large: "Files over 128 KB cannot be previewed", @@ -343,6 +345,10 @@ let appliedThemeSignature = ""; let fileWorkspace = ""; let filePane = ""; let fileRoot = ""; +let filePaneCwd = ""; +let fileRequestSequence = 0; +let fileListRequest = ""; +let fileContentRequest = ""; let fileDirectory = ""; let filePath = ""; let htmlSource = ""; @@ -541,20 +547,30 @@ function handleMessage(raw) { renderState(message); chooseInitialPane(message); updateFilesButton(); - if (filePane && !message.workspaces.some(item => - item.id === fileWorkspace && item.panes?.some(pane => pane.id === filePane))) { - closeFiles(); + if (filePane) { + const pane = message.workspaces.find(item => item.id === fileWorkspace) + ?.panes?.find(pane => pane.id === filePane); + if (!pane) { + closeFiles(); + } else if ((pane.cwd || "") !== filePaneCwd) { + filePaneCwd = pane.cwd || ""; + fileRoot = ""; + browseFiles(""); + } } break; case "fileList": - if (message.workspace === fileWorkspace && message.pane === filePane && message.path === fileDirectory) { + if (matchesFileResponse(message, fileListRequest, fileDirectory) + && (!fileRoot || message.root === fileRoot)) { + fileListRequest = ""; fileRoot = message.root || ""; updateFileLocation(); renderFiles(message.entries || [], message.limit); } break; case "fileContent": - if (message.workspace === fileWorkspace && message.pane === filePane && message.path === filePath) { + if (matchesFileResponse(message, fileContentRequest, filePath) && message.root === fileRoot) { + fileContentRequest = ""; if (message.format === "html") { htmlSource = message.content; htmlSourceShown = false; @@ -571,7 +587,8 @@ function handleMessage(raw) { } break; case "fileImage": - if (message.workspace === fileWorkspace && message.pane === filePane && message.path === filePath) { + if (matchesFileResponse(message, fileContentRequest, filePath) && message.root === fileRoot) { + fileContentRequest = ""; elements.filesImage.src = `data:image/png;base64,${message.data}`; elements.filesImage.alt = elements.filesPreviewName.textContent; imageZoom.reset(); @@ -580,16 +597,25 @@ function handleMessage(raw) { elements.filesMessage.textContent = ""; } break; - case "fileError": - if (message.workspace !== fileWorkspace || message.pane !== filePane) break; - if (message.path === filePath && filePath) { - elements.filesContent.textContent = ""; - elements.filesMessage.textContent = fileErrorLabel(message.code); - } else if (message.path === fileDirectory) { + case "fileError": { + const reading = matchesFileResponse(message, fileContentRequest, filePath); + const listing = matchesFileResponse(message, fileListRequest, fileDirectory); + if (!reading && !listing) break; + if (message.code === "file-root-changed") { + fileRoot = ""; + browseFiles(""); + break; + } + if (reading) { + fileContentRequest = ""; + clearFilePreview(); + } else { + fileListRequest = ""; elements.filesList.replaceChildren(); - elements.filesMessage.textContent = fileErrorLabel(message.code); } + elements.filesMessage.textContent = fileErrorLabel(message.code); break; + } case "snapshot": if (message.pane !== selectedPane) return; clearTimeout(paneRetryTimer); @@ -774,6 +800,7 @@ function errorLabel(code) { function fileErrorLabel(code) { return t({ "files-unavailable": "files_unavailable", + "file-root-changed": "file_root_changed", "invalid-file-path": "invalid_file_path", "file-unavailable": "file_unavailable", "file-too-large": "file_too_large", @@ -922,6 +949,11 @@ function clearFilePreview() { elements.filesSourceToggle.hidden = true; } +function matchesFileResponse(message, request, path) { + return !!request && message.request === request && message.workspace === fileWorkspace + && message.pane === filePane && message.path === path; +} + function updateFilesButton() { elements.openFiles.hidden = !authenticated || !selectedPane; } @@ -939,7 +971,8 @@ function openFiles() { if (!workspace) return; fileWorkspace = workspace.id; filePane = selectedPane; - fileRoot = workspace.panes.find(pane => pane.id === selectedPane)?.cwd || ""; + filePaneCwd = workspace.panes.find(pane => pane.id === selectedPane)?.cwd || ""; + fileRoot = ""; fileDirectory = ""; filePath = ""; elements.filesPanel.hidden = false; @@ -955,6 +988,9 @@ function closeFiles() { fileWorkspace = ""; filePane = ""; fileRoot = ""; + filePaneCwd = ""; + fileListRequest = ""; + fileContentRequest = ""; filePath = ""; clearFilePreview(); elements.filesPanel.hidden = true; @@ -965,13 +1001,15 @@ function closeFiles() { function browseFiles(path) { fileDirectory = path; filePath = ""; + fileContentRequest = ""; + fileListRequest = String(++fileRequestSequence); updateFileLocation(); elements.filesUp.disabled = !path; elements.filesList.replaceChildren(); elements.filesPreviewName.textContent = ""; clearFilePreview(); elements.filesMessage.textContent = t("files_loading"); - send({ type: "listFiles", workspace: fileWorkspace, pane: filePane, path }); + send({ type: "listFiles", workspace: fileWorkspace, pane: filePane, path, root: fileRoot, request: fileListRequest }); } function renderFiles(entries, limit) { @@ -991,7 +1029,8 @@ function renderFiles(entries, limit) { elements.filesPreviewName.textContent = entry.name; clearFilePreview(); elements.filesMessage.textContent = t("files_loading"); - send({ type: "readFile", workspace: fileWorkspace, pane: filePane, path }); + fileContentRequest = String(++fileRequestSequence); + send({ type: "readFile", workspace: fileWorkspace, pane: filePane, path, root: fileRoot, request: fileContentRequest }); } }); elements.filesList.append(button); diff --git a/Glint/WebRemote/WebRemoteProtocol.swift b/Glint/WebRemote/WebRemoteProtocol.swift index 772f1e72..6c8f0fe1 100644 --- a/Glint/WebRemote/WebRemoteProtocol.swift +++ b/Glint/WebRemote/WebRemoteProtocol.swift @@ -177,84 +177,134 @@ enum WebRemoteFiles { ["html", "htm", "xhtml"].contains((path as NSString).pathExtension.lowercased()) } - static func resolve(root: String, relativePath: String) -> URL? { - guard relativePath.utf8.count <= 4096 else { return nil } - let components = relativePath.isEmpty ? [] : relativePath.split(separator: "/", omittingEmptySubsequences: false) - guard components.allSatisfy({ !$0.isEmpty && $0 != "." && $0 != ".." && !$0.contains("\0") }) else { - return nil + /// Keep traversal and I/O on descriptors. Re-opening a validated pathname + /// would let a concurrent rename/symlink swap escape the selected root. + final class Directory { + private let descriptor: Int32 + + init(root: String) throws { + guard root.hasPrefix("/"), !root.contains("\0") else { throw ErrorCode.invalidPath } + // Foundation can shorten /private/var back to the /var symlink. + // realpath gives the physical components needed by O_NOFOLLOW. + guard let resolved = realpath(root, nil) else { throw ErrorCode.unavailable } + defer { free(resolved) } + let canonical = String(cString: resolved) + var fd = Darwin.open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC) + guard fd >= 0 else { throw ErrorCode.unavailable } + for component in canonical.split(separator: "/") { + let next = openat(fd, String(component), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC) + Darwin.close(fd) + guard next >= 0 else { throw ErrorCode.invalidPath } + fd = next + } + descriptor = fd } - let rootURL = URL(fileURLWithPath: root, isDirectory: true).resolvingSymlinksInPath() - var target = rootURL - for component in components { - target.appendPathComponent(String(component)) - guard let values = try? target.resourceValues(forKeys: [.isSymbolicLinkKey]), - values.isSymbolicLink != true else { return nil } + + deinit { Darwin.close(descriptor) } + + private func openPath(_ path: String, directory: Bool) throws -> Int32 { + guard path.utf8.count <= 4096 else { throw ErrorCode.invalidPath } + let components = path.isEmpty ? [] : path.split(separator: "/", omittingEmptySubsequences: false) + guard components.allSatisfy({ !$0.isEmpty && $0 != "." && $0 != ".." && !$0.contains("\0") }) else { + throw ErrorCode.invalidPath + } + var fd = openat(descriptor, ".", O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC) + guard fd >= 0 else { throw ErrorCode.unavailable } + for (index, component) in components.enumerated() { + // O_NONBLOCK prevents a replaced FIFO/device from blocking before fstat. + let flags = O_RDONLY | O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK + | (directory || index < components.count - 1 ? O_DIRECTORY : 0) + let next = openat(fd, String(component), flags) + Darwin.close(fd) + guard next >= 0 else { throw ErrorCode.invalidPath } + fd = next + } + return fd } - let resolved = target.resolvingSymlinksInPath().standardizedFileURL.path - let base = rootURL.standardizedFileURL.path - guard resolved == base || resolved.hasPrefix(base == "/" ? "/" : base + "/") else { return nil } - return target - } - static func list(root: String, path: String) -> Result<[[String: Any]], ErrorCode> { - guard let url = resolve(root: root, relativePath: path) else { return .failure(.invalidPath) } - guard (try? url.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) == true else { - return .failure(.unavailable) + func list(path: String) throws -> [[String: Any]] { + let fd = try openPath(path, directory: true) + guard let directory = fdopendir(fd) else { + Darwin.close(fd) + throw ErrorCode.unavailable + } + defer { closedir(directory) } + var entries: [(name: String, kind: String)] = [] + while true { + errno = 0 + guard let entry = readdir(directory) else { + guard errno == 0 else { throw ErrorCode.unavailable } + break + } + let name = withUnsafePointer(to: &entry.pointee.d_name) { + $0.withMemoryRebound(to: CChar.self, capacity: Int(MAXPATHLEN)) { String(cString: $0) } + } + guard name != ".", name != ".." else { continue } + var info = stat() + guard fstatat(fd, name, &info, AT_SYMLINK_NOFOLLOW) == 0 else { continue } + let type = info.st_mode & S_IFMT + let kind = type == S_IFLNK ? "link" : type == S_IFDIR ? "directory" + : (isImage(name) ? "image" : "file") + entries.append((name, kind)) + } + entries.sort { + if ($0.kind == "directory") != ($1.kind == "directory") { return $0.kind == "directory" } + return $0.name.localizedStandardCompare($1.name) == .orderedAscending + } + return entries.prefix(maxEntries).map { ["name": $0.name, "kind": $0.kind] } } - do { - let children = try FileManager.default.contentsOfDirectory( - at: url, includingPropertiesForKeys: [.isDirectoryKey, .isSymbolicLinkKey], - options: [] - ) - let sorted = children.sorted { - let left = (try? $0.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) == true - let right = (try? $1.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) == true - return left == right - ? $0.lastPathComponent.localizedStandardCompare($1.lastPathComponent) == .orderedAscending - : left + + func read(path: String, limit: Int, tooLarge: ErrorCode) throws -> Data { + guard !path.isEmpty else { throw ErrorCode.invalidPath } + let fd = try openPath(path, directory: false) + defer { Darwin.close(fd) } + var info = stat() + guard fstat(fd, &info) == 0, info.st_mode & S_IFMT == S_IFREG else { + throw ErrorCode.unavailable } - return .success(sorted.prefix(maxEntries).map { child in - let values = try? child.resourceValues(forKeys: [.isDirectoryKey, .isSymbolicLinkKey]) - return [ - "name": child.lastPathComponent, - "kind": values?.isSymbolicLink == true ? "link" - : (values?.isDirectory == true ? "directory" - : (isImage(child.lastPathComponent) ? "image" : "file")), - ] - }) - } catch { return .failure(.unavailable) } + guard info.st_size <= limit else { throw tooLarge } + var data = Data() + var buffer = [UInt8](repeating: 0, count: 16 * 1024) + while true { + let count = buffer.withUnsafeMutableBytes { + Darwin.read(fd, $0.baseAddress, min($0.count, limit + 1 - data.count)) + } + if count < 0 { + if errno == EINTR { continue } + throw ErrorCode.unavailable + } + if count == 0 { return data } + data.append(contentsOf: buffer.prefix(count)) + // Also bound a file that grows after fstat; never read it unbounded. + guard data.count <= limit else { throw tooLarge } + } + } + } + + static func list(root: String, path: String) -> Result<[[String: Any]], ErrorCode> { + do { return .success(try Directory(root: root).list(path: path)) } + catch { return .failure(error as? ErrorCode ?? .unavailable) } } static func read(root: String, path: String) -> Result { - guard !path.isEmpty, let url = resolve(root: root, relativePath: path) else { - return .failure(.invalidPath) - } - guard let values = try? url.resourceValues(forKeys: [.isRegularFileKey, .fileSizeKey]), - values.isRegularFile == true else { return .failure(.unavailable) } - guard let size = values.fileSize, size <= maxPreviewBytes else { return .failure(.tooLarge) } - guard let data = try? Data(contentsOf: url), data.count <= maxPreviewBytes else { - return .failure(.unavailable) - } - guard !data.contains(where: { $0 < 32 && $0 != 9 && $0 != 10 && $0 != 13 }), - let text = String(data: data, encoding: .utf8) else { - return .failure(.notText) - } - return .success(text) + do { + let data = try Directory(root: root).read(path: path, limit: maxPreviewBytes, tooLarge: .tooLarge) + guard !data.contains(where: { $0 < 32 && $0 != 9 && $0 != 10 && $0 != 13 }), + let text = String(data: data, encoding: .utf8) else { return .failure(.notText) } + return .success(text) + } catch { return .failure(error as? ErrorCode ?? .unavailable) } } /// ImageIO decodes only the first frame and bounds the image sent over the /// encrypted socket. The browser receives PNG pixels, never source bytes. static func readImage(root: String, path: String) -> Result { - guard !path.isEmpty, isImage(path), - let url = resolve(root: root, relativePath: path) else { - return .failure(.invalidPath) - } - guard let values = try? url.resourceValues(forKeys: [.isRegularFileKey, .fileSizeKey]), - values.isRegularFile == true else { return .failure(.unavailable) } - guard let size = values.fileSize, size <= maxImageBytes else { return .failure(.imageTooLarge) } - + guard isImage(path) else { return .failure(.invalidPath) } + let input: Data + do { + input = try Directory(root: root).read(path: path, limit: maxImageBytes, tooLarge: .imageTooLarge) + } catch { return .failure(error as? ErrorCode ?? .unavailable) } let sourceOptions = [kCGImageSourceShouldCache: false] as CFDictionary - guard let source = CGImageSourceCreateWithURL(url as CFURL, sourceOptions), + guard let source = CGImageSourceCreateWithData(input as CFData, sourceOptions), CGImageSourceGetCount(source) > 0 else { return .failure(.imageUnavailable) } let thumbnailOptions = [ kCGImageSourceCreateThumbnailFromImageAlways: true, @@ -276,6 +326,8 @@ enum WebRemoteFiles { } enum ErrorCode: String, Error { + case rootChanged = "file-root-changed" + case rootUnavailable = "files-unavailable" case invalidPath = "invalid-file-path" case unavailable = "file-unavailable" case tooLarge = "file-too-large" @@ -285,6 +337,46 @@ enum WebRemoteFiles { } } +/// A file response belongs to one browser operation and one cwd snapshot. +/// Only an initial root listing may discover the cwd; all later operations +/// must echo it, so a local `cd` cannot silently rebase relative filenames. +struct WebRemoteFileRequest { + let workspace: UUID + let pane: String + let path: String + let root: String + let requestID: String + let read: Bool + + init?(_ object: [String: Any]) { + guard let type = object["type"] as? String, ["listFiles", "readFile"].contains(type), + let workspace = object["workspace"] as? String, workspace.count <= 36, + let workspaceID = UUID(uuidString: workspace), + let pane = object["pane"] as? String, pane.count <= 128, + let path = object["path"] as? String, path.utf8.count <= 4096, + let root = object["root"] as? String, root.utf8.count <= 4096, + let request = object["request"] as? String, !request.isEmpty, request.count <= 64, + !root.isEmpty || (type == "listFiles" && path.isEmpty) else { return nil } + self.workspace = workspaceID + self.pane = pane + self.path = path + self.root = root + self.requestID = request + self.read = type == "readFile" + } + + func validatedRoot(current: String?) -> Result { + guard let current else { return .failure(.rootUnavailable) } + guard root.isEmpty || root == current else { return .failure(.rootChanged) } + return .success(current) + } + + func error(_ code: WebRemoteFiles.ErrorCode) -> [String: Any] { + ["type": "fileError", "workspace": workspace.uuidString, "pane": pane, + "path": path, "root": root, "request": requestID, "code": code.rawValue] + } +} + struct WebRemoteAccessToken { static func generate() -> String { var bytes = [UInt8](repeating: 0, count: 32) diff --git a/Glint/WebRemote/WebRemoteServer.swift b/Glint/WebRemote/WebRemoteServer.swift index 09a7faa5..c048fd1f 100644 --- a/Glint/WebRemote/WebRemoteServer.swift +++ b/Glint/WebRemote/WebRemoteServer.swift @@ -922,18 +922,11 @@ final class WebRemoteServer: @unchecked Sendable { clientID: clientID ) case "listFiles", "readFile": - guard let value = object["workspace"] as? String, - value.count <= 36, - let workspaceID = UUID(uuidString: value), - let pane = object["pane"] as? String, - pane.count <= 128, - let path = object["path"] as? String, - path.utf8.count <= 4096 else { + guard let request = WebRemoteFileRequest(object) else { sendError("bad-request", to: clientID) return } - accessFiles(workspace: workspaceID, pane: pane, path: path, - read: type == "readFile", clientID: clientID) + accessFiles(request, clientID: clientID) default: sendError("unknown-command", to: clientID) } @@ -1104,59 +1097,57 @@ final class WebRemoteServer: @unchecked Sendable { } } - private func accessFiles(workspace: UUID, pane: String, path: String, - read: Bool, clientID: UUID) { + private func accessFiles(_ request: WebRemoteFileRequest, clientID: UUID) { DispatchQueue.main.async { [weak self] in guard let self, let store = WorkspaceStore.current else { return } - guard let root = store.webRemoteFileRoot(workspace: workspace, pane: pane) else { - self.queue.async { [weak self] in - self?.sendJSON(["type": "fileError", "workspace": workspace.uuidString, - "pane": pane, "path": path, - "code": "files-unavailable"], to: clientID) - } + let currentRoot = store.webRemoteFileRoot(workspace: request.workspace, pane: request.pane) + guard case let .success(root) = request.validatedRoot(current: currentRoot) else { + let error: WebRemoteFiles.ErrorCode = currentRoot == nil ? .rootUnavailable : .rootChanged + self.queue.async { [weak self] in self?.sendJSON(request.error(error), to: clientID) } return } DispatchQueue.global(qos: .userInitiated).async { [weak self] in - let response: [String: Any] - if read { - if WebRemoteFiles.isImage(path) { - switch WebRemoteFiles.readImage(root: root, path: path) { + var response: [String: Any] = [ + "workspace": request.workspace.uuidString, "pane": request.pane, + "path": request.path, "root": root, "request": request.requestID, + ] + if request.read { + if WebRemoteFiles.isImage(request.path) { + switch WebRemoteFiles.readImage(root: root, path: request.path) { case let .success(png): - response = ["type": "fileImage", "workspace": workspace.uuidString, - "pane": pane, - "path": path, "data": png.base64EncodedString()] - case let .failure(error): - response = ["type": "fileError", "workspace": workspace.uuidString, - "pane": pane, - "path": path, "code": error.rawValue] + response["type"] = "fileImage" + response["data"] = png.base64EncodedString() + case let .failure(error): response = request.error(error) } } else { - switch WebRemoteFiles.read(root: root, path: path) { + switch WebRemoteFiles.read(root: root, path: request.path) { case let .success(content): - response = ["type": "fileContent", "workspace": workspace.uuidString, - "pane": pane, - "path": path, "content": content, - "format": WebRemoteFiles.isHTML(path) ? "html" : "text"] - case let .failure(error): - response = ["type": "fileError", "workspace": workspace.uuidString, - "pane": pane, - "path": path, "code": error.rawValue] + response["type"] = "fileContent" + response["content"] = content + response["format"] = WebRemoteFiles.isHTML(request.path) ? "html" : "text" + case let .failure(error): response = request.error(error) } } } else { - switch WebRemoteFiles.list(root: root, path: path) { + switch WebRemoteFiles.list(root: root, path: request.path) { case let .success(entries): - response = ["type": "fileList", "workspace": workspace.uuidString, - "pane": pane, "root": root, - "path": path, "entries": entries, - "limit": WebRemoteFiles.maxEntries] - case let .failure(error): - response = ["type": "fileError", "workspace": workspace.uuidString, - "pane": pane, - "path": path, "code": error.rawValue] + response["type"] = "fileList" + response["entries"] = entries + response["limit"] = WebRemoteFiles.maxEntries + case let .failure(error): response = request.error(error) } } - self?.queue.async { [weak self] in self?.sendJSON(response, to: clientID) } + // I/O can outlive a cd, pane removal, or transition to SSH. + // Revalidate before handing the result back to the browser. + let completedResponse = response + DispatchQueue.main.async { [weak self] in + let latest = WorkspaceStore.current?.webRemoteFileRoot( + workspace: request.workspace, pane: request.pane + ) + let checkedResponse = latest == root ? completedResponse + : request.error(latest == nil ? .rootUnavailable : .rootChanged) + self?.queue.async { [weak self] in self?.sendJSON(checkedResponse, to: clientID) } + } } } } diff --git a/GlintTests/WebRemoteProtocolTests.swift b/GlintTests/WebRemoteProtocolTests.swift index 38d12e06..1c7c0a5f 100644 --- a/GlintTests/WebRemoteProtocolTests.swift +++ b/GlintTests/WebRemoteProtocolTests.swift @@ -501,9 +501,9 @@ final class WebRemoteProtocolTests: XCTestCase { XCTAssertEqual(entries.first?["name"] as? String, "main.swift") XCTAssertEqual(entries.first?["kind"] as? String, "file") XCTAssertEqual(try WebRemoteFiles.read(root: root.path, path: "src/main.swift").get(), "hello\n") - XCTAssertNil(WebRemoteFiles.resolve(root: root.path, relativePath: "../outside")) - XCTAssertNil(WebRemoteFiles.resolve(root: root.path, relativePath: "/etc/passwd")) - XCTAssertNil(WebRemoteFiles.resolve(root: root.path, relativePath: "src/../main.swift")) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "../outside").get()) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "/etc/passwd").get()) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "src/../main.swift").get()) } func testWebRemoteFilesRejectLinksBinaryAndOversize() throws { @@ -513,7 +513,7 @@ final class WebRemoteProtocolTests: XCTestCase { try FileManager.default.createSymbolicLink( at: root.appendingPathComponent("outside"), withDestinationURL: URL(fileURLWithPath: "/etc") ) - XCTAssertNil(WebRemoteFiles.resolve(root: root.path, relativePath: "outside/passwd")) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "outside/passwd").get()) try Data([0, 1, 2]).write(to: root.appendingPathComponent("binary")) try Data(repeating: 65, count: WebRemoteFiles.maxPreviewBytes + 1) .write(to: root.appendingPathComponent("large")) @@ -523,6 +523,97 @@ final class WebRemoteProtocolTests: XCTestCase { } else { XCTFail("Large content must be rejected") } } + func testFileDirectoryStaysAnchoredWhenRootIsReplaced() throws { + let fm = FileManager.default + let base = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + let root = base.appendingPathComponent("root") + let outside = base.appendingPathComponent("outside") + try fm.createDirectory(at: root, withIntermediateDirectories: true) + try fm.createDirectory(at: outside, withIntermediateDirectories: true) + defer { try? fm.removeItem(at: base) } + try Data("inside".utf8).write(to: root.appendingPathComponent("inside.txt")) + try Data("outside".utf8).write(to: outside.appendingPathComponent("outside.txt")) + let directory = try WebRemoteFiles.Directory(root: root.path) + try fm.moveItem(at: root, to: base.appendingPathComponent("moved")) + try fm.createSymbolicLink(at: root, withDestinationURL: outside) + for _ in 0..<2 { + XCTAssertEqual(try directory.list(path: "").compactMap { $0["name"] as? String }, ["inside.txt"]) + } + XCTAssertEqual(try directory.read(path: "inside.txt", limit: 128, tooLarge: .tooLarge), Data("inside".utf8)) + XCTAssertThrowsError(try directory.read(path: "outside.txt", limit: 128, tooLarge: .tooLarge)) + } + + func testFileReadsAndListingsCannotFollowConcurrentSymlinkSwaps() throws { + let fm = FileManager.default + let base = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + let root = base.appendingPathComponent("root") + let item = root.appendingPathComponent("item") + let alternate = root.appendingPathComponent("alternate") + let outside = base.appendingPathComponent("outside") + try fm.createDirectory(at: item, withIntermediateDirectories: true) + try fm.createDirectory(at: outside, withIntermediateDirectories: true) + defer { try? fm.removeItem(at: base) } + try Data("inside".utf8).write(to: item.appendingPathComponent("test.txt")) + try Data("outside".utf8).write(to: outside.appendingPathComponent("test.txt")) + try Data().write(to: outside.appendingPathComponent("outside-only.txt")) + try fm.createSymbolicLink(at: alternate, withDestinationURL: outside) + let group = DispatchGroup() + group.enter() + DispatchQueue.global().async { + defer { group.leave() } + for _ in 0..<4000 { + _ = renameatx_np(AT_FDCWD, item.path, AT_FDCWD, alternate.path, UInt32(RENAME_SWAP)) + } + } + defer { group.wait() } + for _ in 0..<500 { + if case let .success(text) = WebRemoteFiles.read(root: root.path, path: "item/test.txt") { + XCTAssertEqual(text, "inside") + } + if case let .success(entries) = WebRemoteFiles.list(root: root.path, path: "item") { + XCTAssertFalse(entries.contains { $0["name"] as? String == "outside-only.txt" }) + } + } + } + + func testFileReadersRejectLeafLinksAndSpecialFiles() throws { + let fm = FileManager.default + let root = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? fm.removeItem(at: root) } + try Data("inside".utf8).write(to: root.appendingPathComponent("text")) + try fm.createSymbolicLink(at: root.appendingPathComponent("link.png"), withDestinationURL: root.appendingPathComponent("text")) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "link.png").get()) + XCTAssertThrowsError(try WebRemoteFiles.readImage(root: root.path, path: "link.png").get()) + XCTAssertEqual(mkfifo(root.appendingPathComponent("pipe").path, 0o600), 0) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "pipe").get()) + XCTAssertThrowsError(try WebRemoteFiles.read(root: root.path, path: "").get()) + } + + func testFileRequestsRejectChangedRootsAndRequireCorrelation() throws { + var payload: [String: Any] = [ + "type": "readFile", "workspace": UUID().uuidString, "pane": "pane", + "root": "/project-A", "path": "README.md", "request": "17", + ] + let request = try XCTUnwrap(WebRemoteFileRequest(payload)) + XCTAssertEqual(try request.validatedRoot(current: "/project-A").get(), "/project-A") + XCTAssertThrowsError(try request.validatedRoot(current: "/project-B").get()) { + XCTAssertEqual($0 as? WebRemoteFiles.ErrorCode, .rootChanged) + } + XCTAssertThrowsError(try request.validatedRoot(current: nil).get()) + XCTAssertEqual(request.error(.rootChanged)["request"] as? String, "17") + payload.removeValue(forKey: "request") + XCTAssertNil(WebRemoteFileRequest(payload)) + payload["request"] = "18" + payload["root"] = "" + XCTAssertNil(WebRemoteFileRequest(payload), "Reads may not discover/rebase the root") + payload["type"] = "listFiles" + XCTAssertNil(WebRemoteFileRequest(payload), "Subdirectory listings must bind a root too") + payload["path"] = "" + let discovery = try XCTUnwrap(WebRemoteFileRequest(payload)) + XCTAssertEqual(try discovery.validatedRoot(current: "/project-B").get(), "/project-B") + } + @MainActor func testWebRemoteFileRootRejectsSSHWithoutRemoteTitleMetadata() throws { let project = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) @@ -562,7 +653,7 @@ final class WebRemoteProtocolTests: XCTestCase { XCTAssertTrue(WebRemoteFiles.isHTML("page.HTM")) let preview = try WebRemoteFiles.readImage(root: root.path, path: "photo.PNG").get() XCTAssertEqual(Array(preview.prefix(8)), [137, 80, 78, 71, 13, 10, 26, 10]) - XCTAssertNil(WebRemoteFiles.resolve(root: root.path, relativePath: "../photo.PNG")) + XCTAssertThrowsError(try WebRemoteFiles.readImage(root: root.path, path: "../photo.PNG").get()) } @MainActor diff --git a/GlintTests/WebRemoteServerIntegrationTests.swift b/GlintTests/WebRemoteServerIntegrationTests.swift index 3c568ac8..d88a6d48 100644 --- a/GlintTests/WebRemoteServerIntegrationTests.swift +++ b/GlintTests/WebRemoteServerIntegrationTests.swift @@ -247,6 +247,71 @@ final class WebRemoteServerIntegrationTests: XCTestCase { Data(repeating: 0, count: WebRemoteCrypto.challengeLength).base64EncodedString() } + @MainActor + func testEncryptedFileRequestsRejectCwdChangesAndReadRefreshedRoot() async throws { + let fm = FileManager.default + let base = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + let first = base.appendingPathComponent("A") + let second = base.appendingPathComponent("B") + try fm.createDirectory(at: first, withIntermediateDirectories: true) + try fm.createDirectory(at: second, withIntermediateDirectories: true) + defer { try? fm.removeItem(at: base) } + try Data("contents A".utf8).write(to: first.appendingPathComponent("same.txt")) + try Data("contents B".utf8).write(to: second.appendingPathComponent("same.txt")) + try fm.createSymbolicLink(at: second.appendingPathComponent("link.txt"), + withDestinationURL: first.appendingPathComponent("same.txt")) + var workspace = Workspace.fresh(name: "File test", accentHex: "5E5CE6", symbol: "F") + let pane = try XCTUnwrap(workspace.selectedTab?.focusedPane) + workspace.panes[pane]?.workingDirectory = first.path + let store = WorkspaceStore(activity: PaneActivityStore()) + store.workspaces = [workspace] + let handle = "\(workspace.id.uuidString):\(pane.value)" + let tokenKey = try XCTUnwrap(WebRemoteCrypto.tokenKey(from: XCTUnwrap(readyToken))) + let task = makeWebSocket() + defer { task.cancel(with: .goingAway, reason: nil) } + let challenge = try await receiveAuthChallenge(task) + let proof = WebRemoteCrypto.proof(tokenKey: tokenKey, challenge: challenge) + try await send(task, ["type": "authenticate", "proof": proof.base64EncodedString()]) + let keys = WebRemoteCrypto.sessionKeys(tokenKey: tokenKey, challenge: challenge) + let authenticated = try await receiveEncryptedJSON(task, key: keys.s2c) + XCTAssertEqual(authenticated["type"] as? String, "authenticated") + var counter: UInt64 = 0 + func request(_ type: String, root: String, path: String) async throws -> [String: Any] { + let requestID = String(counter) + let payload: [String: Any] = [ + "type": type, "workspace": workspace.id.uuidString, "pane": handle, + "root": root, "path": path, "request": requestID, + ] + let data = try JSONSerialization.data(withJSONObject: payload) + let frame = try XCTUnwrap(WebRemoteCrypto.sealFrame(plaintext: data, key: keys.c2s, counter: counter)) + counter += 1 + try await task.send(.data(frame)) + for _ in 0..<10 { + let reply = try await receiveEncryptedJSON(task, key: keys.s2c) + if reply["request"] as? String == requestID { return reply } + } + throw URLError(.badServerResponse) + } + let listing = try await request("listFiles", root: "", path: "") + let originalRoot = try XCTUnwrap(listing["root"] as? String) + XCTAssertEqual(listing["type"] as? String, "fileList") + let original = try await request("readFile", root: originalRoot, path: "same.txt") + XCTAssertEqual(original["content"] as? String, "contents A") + store.workspaces[0].panes[pane]?.workingDirectory = second.path + let stale = try await request("readFile", root: originalRoot, path: "same.txt") + XCTAssertEqual(stale["code"] as? String, "file-root-changed") + XCTAssertNil(stale["content"]) + let refreshed = try await request("listFiles", root: "", path: "") + let refreshedRoot = try XCTUnwrap(refreshed["root"] as? String) + XCTAssertNotEqual(refreshedRoot, originalRoot) + let current = try await request("readFile", root: refreshedRoot, path: "same.txt") + XCTAssertEqual(current["content"] as? String, "contents B") + let link = try await request("readFile", root: refreshedRoot, path: "link.txt") + XCTAssertEqual(link["type"] as? String, "fileError") + XCTAssertNil(link["content"]) + withExtendedLifetime(store) {} + } + // MARK: - Pure-function behaviour func testAuthBackoffCurveIsMonotonicAndCapped() { diff --git a/scripts/test-web-remote-files.mjs b/scripts/test-web-remote-files.mjs new file mode 100644 index 00000000..59a8b62c --- /dev/null +++ b/scripts/test-web-remote-files.mjs @@ -0,0 +1,153 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; +import vm from "node:vm"; + +const source = readFileSync(new URL("../Glint/Resources/WebRemote/web-remote.js", import.meta.url), "utf8"); +// Execute the shipped handlers with a small DOM/transport fixture. No copy of +// their request matching or state transitions lives in this test harness. +function shippedFunction(name) { + const start = source.indexOf(`function ${name}(`); + assert.notEqual(start, -1); + const end = source.indexOf("\nfunction ", start + 1); + return source.slice(start, end < 0 ? undefined : end); +} +function harness() { + const sent = []; + function element() { + return { + hidden: false, textContent: "", children: [], handlers: {}, srcdoc: "", + classList: { add() {}, remove() {} }, + addEventListener(type, handler) { this.handlers[type] = handler; }, + removeAttribute(name) { delete this[name]; }, + replaceChildren() { this.children = []; }, + append(child) { this.children.push(child); }, + }; + } + const elements = Object.fromEntries([ + "filesPanel", "filesUp", "filesLocation", "filesList", "filesPreviewName", "filesContent", + "filesImageViewport", "filesImage", "filesHTML", "filesSourceToggle", "filesMessage", "openFiles", + ].map(name => [name, element()])); + const context = vm.createContext({ + elements, sent, + document: { createElement: element, querySelector: element }, + imageZoom: { reset() {} }, + mobileSidebarLayout: { matches: false }, + send: message => sent.push(message), t: key => key, + applyBrand() {}, applyTheme() {}, renderState() {}, chooseInitialPane() {}, fitTerminal() {}, + }); + vm.runInContext(` + let fileWorkspace = "", filePane = "", fileRoot = "", filePaneCwd = ""; + let fileRequestSequence = 0, fileListRequest = "", fileContentRequest = ""; + let fileDirectory = "", filePath = "", htmlSource = "", htmlSourceShown = false; + let authenticated = true, selectedPane = "pane"; + let lastState = { workspaces: [{ id: "ws", panes: [{ id: "pane", cwd: "/A" }] }] }; + `, context); + for (const name of [ + "handleMessage", "fileErrorLabel", "sandboxedHTML", "clearFilePreview", "matchesFileResponse", + "updateFilesButton", "updateFileLocation", "openFiles", "closeFiles", "browseFiles", "renderFiles", + ]) vm.runInContext(shippedFunction(name), context); + const run = code => vm.runInContext(code, context); + const deliver = message => context.handleMessage(JSON.stringify(message)); + const listReply = (request, root = "/A", name = "README.md") => deliver({ + ...request, type: "fileList", root, entries: [{ name, kind: "file" }], limit: 200, + }); + const click = () => { + elements.filesList.children[0].handlers.click(); + return sent.at(-1); + }; + run("openFiles()"); + listReply(sent.at(-1)); + return { sent, elements, run, deliver, listReply, click }; +} + +test("directory listings and reads bind the root supplied by the server", () => { + const h = harness(); + assert.equal(h.sent[0].root, ""); + const read = h.click(); + assert.equal(read.root, "/A"); + assert.ok(read.request); + h.run('browseFiles("src")'); + assert.equal(h.sent.at(-1).root, "/A"); + assert.notEqual(h.sent.at(-1).request, read.request); +}); + +test("a cwd update refreshes the root and discards in-flight old contents", () => { + const h = harness(); + const read = h.click(); + h.deliver({ type: "state", workspaces: [{ id: "ws", panes: [{ id: "pane", cwd: "/B" }] }] }); + assert.equal(h.sent.at(-1).root, ""); + assert.equal(h.sent.at(-1).path, ""); + h.deliver({ ...read, type: "fileContent", content: "old A", format: "text" }); + assert.equal(h.elements.filesContent.textContent, ""); + h.listReply(h.sent.at(-1), "/B"); + const fresh = h.click(); + assert.equal(fresh.root, "/B"); + h.deliver({ ...fresh, type: "fileContent", content: "new B", format: "text" }); + assert.equal(h.elements.filesLocation.textContent, "/B"); + assert.equal(h.elements.filesContent.textContent, "new B"); +}); + +test("a server root-change error invalidates the old preview and discovers the new cwd", () => { + const h = harness(); + const read = h.click(); + h.deliver({ ...read, type: "fileError", code: "file-root-changed" }); + const discovery = h.sent.at(-1); + assert.equal(discovery.type, "listFiles"); + assert.equal(discovery.root, ""); + assert.equal(discovery.path, ""); + assert.equal(h.elements.filesList.children.length, 0); + h.deliver({ ...read, type: "fileContent", content: "stale", format: "text" }); + assert.equal(h.elements.filesContent.textContent, ""); +}); + +test("late listings cannot replace a refreshed listing at the same path", () => { + const h = harness(); + h.run('browseFiles("")'); + const old = h.sent.at(-1); + h.run('browseFiles("")'); + const fresh = h.sent.at(-1); + h.listReply(fresh, "/A", "new.txt"); + h.listReply(old, "/A", "old.txt"); + assert.match(h.elements.filesList.children[0].textContent, /new.txt/); +}); + +test("late reads of the same filename cannot overwrite a newer preview", () => { + const h = harness(); + const old = h.click(); + const fresh = h.click(); + h.deliver({ ...fresh, type: "fileContent", content: "new", format: "text" }); + h.deliver({ ...old, type: "fileContent", content: "old", format: "text" }); + assert.equal(h.elements.filesContent.textContent, "new"); +}); + +test("closing and reopening the same pane invalidates earlier responses", () => { + const h = harness(); + const read = h.click(); + h.run("closeFiles(); openFiles()"); + h.listReply(h.sent.at(-1)); + h.click(); + h.deliver({ ...read, type: "fileContent", content: "old", format: "text" }); + assert.equal(h.elements.filesContent.textContent, ""); +}); + +test("text, HTML and image contents from another root are never rendered", () => { + for (const [type, format] of [["fileContent", "text"], ["fileContent", "html"], ["fileImage", null]]) { + const h = harness(); + const read = h.click(); + h.deliver({ ...read, root: "/B", type, format, content: "wrong root", data: "cG5n" }); + assert.equal(h.elements.filesContent.textContent, ""); + assert.equal(h.elements.filesHTML.srcdoc, ""); + assert.equal(h.elements.filesImage.src, undefined); + } +}); + +test("errors for an obsolete read cannot clear the current preview", () => { + const h = harness(); + const old = h.click(); + const fresh = h.click(); + h.deliver({ ...fresh, type: "fileContent", content: "new", format: "text" }); + h.deliver({ ...old, type: "fileError", code: "file-root-changed" }); + assert.equal(h.elements.filesContent.textContent, "new"); + assert.equal(h.sent.at(-1).request, fresh.request); +});