diff --git a/services/comfyui_history.py b/services/comfyui_history.py index 352e762..32ba52e 100644 --- a/services/comfyui_history.py +++ b/services/comfyui_history.py @@ -6,6 +6,7 @@ Parses ComfyUI /history/{prompt_id} responses and extracts image output metadata import json import logging import os +import unicodedata from typing import Any, Dict, List, Optional from urllib.parse import urlencode @@ -24,7 +25,20 @@ COMFYUI_URL = ( ) HISTORY_TIMEOUT = 5 PREVIEWABLE_MEDIA_TYPES = ("images", "video", "audio", "3d", "text") -THREE_D_EXTENSIONS = (".obj", ".fbx", ".gltf", ".glb", ".usdz") +THREE_D_EXTENSIONS = ( + ".obj", + ".fbx", + ".gltf", + ".glb", + ".stl", + ".ply", + ".spz", + ".splat", + ".ksplat", + ".usdz", +) +ADVANCED_3D_RESULT_MAX_ENTRIES = 8 +ADVANCED_3D_RESULT_PATH_MAX_LENGTH = 1024 TEXT_PREVIEW_MAX_LENGTH = 1024 FILE_TEXT_EXTENSIONS = frozenset( {"txt", "md", "markdown", "json", "csv", "yaml", "yml", "xml", "log"} @@ -97,6 +111,58 @@ def _has_unsafe_file_characters(value: str) -> bool: return any(ord(char) < 32 or ord(char) == 127 for char in value) +def _has_unsafe_advanced_3d_characters(value: str) -> bool: + return any(unicodedata.category(char) in {"Cc", "Cf", "Cs"} for char in value) + + +def _normalize_advanced_3d_result(result: Any) -> dict[str, Any] | None: + if ( + not isinstance(result, list) + or not result + or len(result) > ADVANCED_3D_RESULT_MAX_ENTRIES + ): + return None + + raw_path = result[0] + if ( + not isinstance(raw_path, str) + or len(raw_path) > ADVANCED_3D_RESULT_PATH_MAX_LENGTH + ): + return None + + normalized_path = raw_path.replace("\\", "/") + if ( + not normalized_path + or len(normalized_path) > ADVANCED_3D_RESULT_PATH_MAX_LENGTH + or _has_unsafe_advanced_3d_characters(normalized_path) + or normalized_path.startswith("/") + or any(marker in normalized_path for marker in (":", "%", "?", "#")) + ): + return None + + segments = normalized_path.split("/") + if any( + not segment or segment in {".", ".."} or segment != segment.strip() + for segment in segments + ): + return None + + filename = segments[-1] + if not _has_3d_extension(filename): + return None + + # SECURITY: result metadata is untrusted and may contain private host state. + # Inspect only the validated path at index zero; never project later entries. + return normalize_history_output_ref( + { + "filename": filename, + "subfolder": "/".join(segments[:-1]), + "type": "output", + }, + "3d", + ) + + def _normalize_file_text_ref(output_ref: Any) -> Optional[Dict[str, Any]]: if not isinstance(output_ref, dict): return None @@ -275,6 +341,10 @@ def extract_output_refs(history_item: Dict[str, Any]) -> List[Dict[str, Any]]: if normalized: results.append(normalized) + advanced_3d_ref = _normalize_advanced_3d_result(node_output.get("result")) + if advanced_3d_ref: + results.append(advanced_3d_ref) + file_refs = node_output.get("files") if isinstance(file_refs, list) and len(file_refs) <= FILE_OUTPUT_MAX_REFS: for ref in file_refs: diff --git a/tests/e2e/specs/r107_live_backend.spec.js b/tests/e2e/specs/r107_live_backend.spec.js index 524f796..e4054cd 100644 --- a/tests/e2e/specs/r107_live_backend.spec.js +++ b/tests/e2e/specs/r107_live_backend.spec.js @@ -275,6 +275,99 @@ test.describe('R107 Live Backend Parity', () => { expect(assetApiCalls).toBe(0); }); + test('Job Monitor renders official advanced 3d results as one bounded view link', async ({ page }) => { + const jobId = "job-advanced-3d-result"; + const metadataCanary = "metadata-value-must-not-project"; + let assetApiCalls = 0; + + await page.evaluate(() => { + window.__openclawOpenedUrls = []; + window.open = (url, target) => { + window.__openclawOpenedUrls.push({ url, target }); + return null; + }; + }); + + await page.route(`**/history/${jobId}`, async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + [jobId]: { + status: { status_str: "success", completed: true }, + outputs: { + "9": { + result: [ + "models/scene one.splat", + { camera: metadataCanary }, + [{ model: metadataCanary }], + ], + }, + "10": { + result: ["../private.glb"], + }, + }, + }, + }), + }); + }); + + await page.route('**/openclaw/trace/**', async route => { + await route.fulfill({ + status: 404, + contentType: 'application/json', + body: JSON.stringify({ error: 'not_found' }), + }); + }); + + await page.route('**/api/assets**', async route => { + assetApiCalls += 1; + await route.fulfill({ + status: 500, + contentType: 'application/json', + body: JSON.stringify({ error: 'asset_api_should_not_be_called' }), + }); + }); + + await clickTab(page, 'Jobs'); + await page.locator('input[placeholder="prompt_id"]').fill(jobId); + await page.getByText('Add').click(); + + const jobRow = page.locator('.openclaw-job-row').first(); + await expect(page.locator('.openclaw-kv-val.ok')).toHaveText('completed', { timeout: 10000 }); + const fallbacks = jobRow.locator('.openclaw-job-output-media-fallback'); + await expect(fallbacks).toHaveCount(1); + await expect(fallbacks).toContainText('3d output available'); + await expect(jobRow).not.toContainText(metadataCanary); + await expect(jobRow.locator('img, canvas')).toHaveCount(0); + + await fallbacks.click(); + const opened = await page.evaluate(() => { + const entry = window.__openclawOpenedUrls[0]; + if (!entry) return null; + const parsed = new URL(entry.url, window.location.origin); + return { + origin: parsed.origin, + currentOrigin: window.location.origin, + pathname: parsed.pathname, + filename: parsed.searchParams.get("filename"), + subfolder: parsed.searchParams.get("subfolder"), + type: parsed.searchParams.get("type"), + target: entry.target, + }; + }); + expect(opened).toEqual({ + origin: opened.currentOrigin, + currentOrigin: opened.currentOrigin, + pathname: expect.stringMatching(/\/view$/), + filename: "scene one.splat", + subfolder: "models", + type: "output", + target: "_blank", + }); + expect(assetApiCalls).toBe(0); + }); + test('Job Monitor renders HDR image outputs as explicit fallbacks', async ({ page }) => { const jobId = "job-hdr-refs"; let hdrViewRequests = 0; diff --git a/tests/test_r236_advanced_3d_result.py b/tests/test_r236_advanced_3d_result.py new file mode 100644 index 0000000..74da532 --- /dev/null +++ b/tests/test_r236_advanced_3d_result.py @@ -0,0 +1,187 @@ +import json +import unittest +from urllib.parse import parse_qs, urlparse + +from services.comfyui_history import extract_output_refs + + +class TestR236Advanced3DResult(unittest.TestCase): + VALID_SUFFIXES = ( + "glb", + "gltf", + "obj", + "fbx", + "stl", + "ply", + "spz", + "splat", + "ksplat", + "usdz", + ) + + @staticmethod + def _extract(result): + return extract_output_refs({"outputs": {"9": {"result": result}}}) + + def test_official_result_tuple_projects_only_first_3d_path(self): + class GuardedResult(list): + def __getitem__(self, index): + if index != 0: + raise AssertionError("later result entries were inspected") + return super().__getitem__(index) + + class ExplosiveMetadata: + def __str__(self): + raise AssertionError("later result metadata was inspected") + + def __repr__(self): + raise AssertionError("later result metadata was inspected") + + metadata_canary = "metadata-value-must-not-project" + outputs = self._extract( + GuardedResult( + [ + "models/scene one.splat", + ExplosiveMetadata(), + [{"model": metadata_canary}], + ] + ) + ) + + self.assertEqual(len(outputs), 1) + output = outputs[0] + self.assertEqual( + { + "filename": output["filename"], + "subfolder": output["subfolder"], + "type": output["type"], + "media_type": output["media_type"], + "asset_hash": output["asset_hash"], + "asset_api_id": output["asset_api_id"], + "asset_api_required": output["asset_api_required"], + "resolution": output["resolution"], + }, + { + "filename": "scene one.splat", + "subfolder": "models", + "type": "output", + "media_type": "3d", + "asset_hash": "", + "asset_api_id": "", + "asset_api_required": False, + "resolution": "view", + }, + ) + params = parse_qs(urlparse(output["view_url"]).query) + self.assertEqual( + params, + { + "filename": ["scene one.splat"], + "subfolder": ["models"], + "type": ["output"], + }, + ) + self.assertNotIn(metadata_canary, json.dumps(output, sort_keys=True)) + + def test_accepts_reviewed_suffixes_and_normalizes_backslashes(self): + history = { + "outputs": { + str(index): { + "result": [ + ( + f"nested\\folder\\scene.{suffix.upper()}" + if index == 0 + else f"nested/scene.{suffix.upper()}" + ) + ] + } + for index, suffix in enumerate(self.VALID_SUFFIXES) + } + } + + outputs = extract_output_refs(history) + + self.assertEqual(len(outputs), len(self.VALID_SUFFIXES)) + self.assertTrue(all(output["media_type"] == "3d" for output in outputs)) + self.assertEqual(outputs[0]["subfolder"], "nested/folder") + self.assertEqual(outputs[0]["filename"], "scene.GLB") + self.assertEqual( + [output["filename"].rsplit(".", 1)[-1].lower() for output in outputs], + list(self.VALID_SUFFIXES), + ) + unicode_output = self._extract(["模型/場景😀.glb"]) + self.assertEqual( + (unicode_output[0]["subfolder"], unicode_output[0]["filename"]), + ("模型", "場景😀.glb"), + ) + + def test_enforces_container_and_unicode_path_bounds(self): + max_path = ("a" * (1024 - len(".glb"))) + ".glb" + self.assertEqual(len(self._extract([max_path] + [{}] * 7)), 1) + + rejected = ( + [], + ["scene.glb"] + [{}] * 8, + [("a" * (1025 - len(".glb"))) + ".glb"], + ) + for result in rejected: + with self.subTest(result_length=len(result)): + self.assertEqual(self._extract(result), []) + + def test_rejects_malformed_or_unsafe_first_entries(self): + rejected = ( + None, + "scene.glb", + {}, + [None], + [123], + [""], + [" "], + [" scene.glb"], + ["scene.glb "], + ["\u00a0scene.glb"], + ["/absolute/scene.glb"], + ["//evil.example/scene.glb"], + ["https://evil.example/scene.glb"], + ["file:scene.glb"], + ["C:\\private\\scene.glb"], + ["../scene.glb"], + ["safe/../scene.glb"], + ["safe/./scene.glb"], + ["safe//scene.glb"], + ["safe/\x00scene.glb"], + ["safe/\u0085scene.glb"], + ["safe/\u202escene.glb"], + ["safe/\ud800scene.glb"], + ["scene.glb?token=secret"], + ["scene.png"], + ["scene.glb.exe"], + ) + for result in rejected: + with self.subTest(result=result): + self.assertEqual(self._extract(result), []) + + def test_existing_output_families_remain_unchanged(self): + outputs = extract_output_refs( + { + "outputs": { + "1": { + "images": [{"filename": "image.png", "type": "output"}], + "video": [{"filename": "clip.webm", "type": "output"}], + "audio": [{"filename": "sound.wav", "type": "output"}], + "3d": ["classic.glb"], + "text": ["hello"], + "files": [{"filename": "report.txt", "type": "output"}], + } + } + } + ) + + self.assertEqual( + [output["media_type"] for output in outputs], + ["images", "video", "audio", "3d", "text", "text"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/web/openclaw_asset_refs.js b/web/openclaw_asset_refs.js index d9c8c0b..4f748f6 100644 --- a/web/openclaw_asset_refs.js +++ b/web/openclaw_asset_refs.js @@ -1,7 +1,21 @@ const PREVIEWABLE_MEDIA_TYPES = new Set(["images", "video", "audio", "3d", "text"]); -const THREE_D_EXTENSIONS = [".obj", ".fbx", ".gltf", ".glb", ".usdz"]; +const THREE_D_EXTENSIONS = [ + ".obj", + ".fbx", + ".gltf", + ".glb", + ".stl", + ".ply", + ".spz", + ".splat", + ".ksplat", + ".usdz", +]; const HDR_IMAGE_EXTENSIONS = [".exr", ".hdr"]; const TEXT_PREVIEW_MAX_LENGTH = 1024; +const ADVANCED_3D_RESULT_MAX_ENTRIES = 8; +const ADVANCED_3D_RESULT_PATH_MAX_LENGTH = 1024; +const UNSAFE_ADVANCED_3D_PATH_CHARACTERS = /[\p{Cc}\p{Cf}\p{Cs}]/u; const FILE_TEXT_EXTENSIONS = new Set(["txt", "md", "markdown", "json", "csv", "yaml", "yml", "xml", "log"]); const FILE_OUTPUT_TYPES = new Set(["input", "output", "temp"]); const FILE_OUTPUT_MAX_REFS = 64; @@ -153,6 +167,62 @@ function codePointLength(value = "") { return Array.from(String(value)).length; } +function hasUnsafeAdvanced3dPathCharacters(value = "") { + return UNSAFE_ADVANCED_3D_PATH_CHARACTERS.test(String(value)); +} + +function normalizeAdvanced3dResult(result) { + if ( + !Array.isArray(result) + || result.length === 0 + || result.length > ADVANCED_3D_RESULT_MAX_ENTRIES + ) { + return null; + } + + const rawPath = result[0]; + if ( + typeof rawPath !== "string" + || codePointLength(rawPath) > ADVANCED_3D_RESULT_PATH_MAX_LENGTH + ) { + return null; + } + + const normalizedPath = rawPath.replaceAll("\\", "/"); + if ( + !normalizedPath + || codePointLength(normalizedPath) > ADVANCED_3D_RESULT_PATH_MAX_LENGTH + || hasUnsafeAdvanced3dPathCharacters(normalizedPath) + || normalizedPath.startsWith("/") + || [":", "%", "?", "#"].some((marker) => normalizedPath.includes(marker)) + ) { + return null; + } + + const segments = normalizedPath.split("/"); + if (segments.some((segment) => ( + !segment + || segment === "." + || segment === ".." + || segment !== segment.trim() + ))) { + return null; + } + + const filename = segments.at(-1); + if (!has3dExtension(filename)) { + return null; + } + + // SECURITY: later result entries may contain private host metadata. Never + // inspect or project anything except the validated path at index zero. + return normalizeComfyOutputRef({ + filename, + subfolder: segments.slice(0, -1).join("/"), + type: "output", + }, "3d"); +} + function normalizeFileTextOutputRef(outputRef) { if (!outputRef || typeof outputRef !== "object" || Array.isArray(outputRef)) { return null; @@ -324,6 +394,11 @@ export function extractHistoryOutputRefs(historyItem = {}) { } } + const advanced3dRef = normalizeAdvanced3dResult(nodeOutput.result); + if (advanced3dRef) { + results.push(advanced3dRef); + } + const fileRefs = nodeOutput.files; if (Array.isArray(fileRefs) && fileRefs.length <= FILE_OUTPUT_MAX_REFS) { for (const fileRef of fileRefs) { diff --git a/web/tests/unit/openclaw_asset_refs.test.js b/web/tests/unit/openclaw_asset_refs.test.js index 2492a4f..99491f5 100644 --- a/web/tests/unit/openclaw_asset_refs.test.js +++ b/web/tests/unit/openclaw_asset_refs.test.js @@ -433,6 +433,140 @@ describe("openclaw asset refs", () => { } }); + it("normalizes only the bounded official advanced 3d result path", () => { + const metadataCanary = "metadata-value-must-not-project"; + const explosiveMetadata = new Proxy({}, { + get() { + throw new Error("later result metadata was inspected"); + }, + ownKeys() { + throw new Error("later result metadata was inspected"); + }, + }); + const guardedResult = new Proxy([ + "models/scene one.splat", + explosiveMetadata, + [{ model: metadataCanary }], + ], { + get(target, property, receiver) { + if (property === "1" || property === "2") { + throw new Error("later result entries were inspected"); + } + return Reflect.get(target, property, receiver); + }, + }); + const outputs = extractHistoryOutputRefs({ + outputs: { + "9": { + result: guardedResult, + }, + }, + }); + + expect(outputs).toHaveLength(1); + expect(outputs[0]).toEqual(expect.objectContaining({ + filename: "scene one.splat", + subfolder: "models", + type: "output", + media_type: "3d", + asset_hash: "", + asset_api_id: "", + asset_api_required: false, + resolution: "view", + viewParams: { + filename: "scene one.splat", + subfolder: "models", + type: "output", + }, + })); + expect(JSON.stringify(outputs[0])).not.toContain(metadataCanary); + }); + + it("keeps advanced 3d suffix and path parity with the backend", () => { + const suffixes = [ + "glb", + "gltf", + "obj", + "fbx", + "stl", + "ply", + "spz", + "splat", + "ksplat", + "usdz", + ]; + const outputs = extractHistoryOutputRefs({ + outputs: Object.fromEntries(suffixes.map((suffix, index) => [ + String(index), + { + result: [ + index === 0 + ? `nested\\folder\\scene.${suffix.toUpperCase()}` + : `nested/scene.${suffix.toUpperCase()}`, + ], + }, + ])), + }); + + expect(outputs).toHaveLength(suffixes.length); + expect(outputs.every((output) => output.media_type === "3d")).toBe(true); + expect(outputs[0]).toEqual(expect.objectContaining({ + filename: "scene.GLB", + subfolder: "nested/folder", + })); + expect(outputs.map((output) => output.filename.split(".").pop().toLowerCase())).toEqual(suffixes); + expect(extractHistoryOutputRefs({ + outputs: { "unicode": { result: ["模型/場景😀.glb"] } }, + })[0]).toEqual(expect.objectContaining({ + filename: "場景😀.glb", + subfolder: "模型", + })); + }); + + it("rejects unsafe advanced 3d result containers and first paths", () => { + const maxPath = `${"a".repeat(1024 - ".glb".length)}.glb`; + expect(extractHistoryOutputRefs({ + outputs: { "9": { result: [maxPath, {}, {}, {}, {}, {}, {}, {}] } }, + })).toHaveLength(1); + + const rejected = [ + null, + "scene.glb", + {}, + [], + [null], + [123], + [""], + [" "], + [" scene.glb"], + ["scene.glb "], + ["\u00a0scene.glb"], + ["scene.glb", {}, {}, {}, {}, {}, {}, {}, {}], + [`${"a".repeat(1025 - ".glb".length)}.glb`], + ["/absolute/scene.glb"], + ["//evil.example/scene.glb"], + ["https://evil.example/scene.glb"], + ["file:scene.glb"], + ["C:\\private\\scene.glb"], + ["../scene.glb"], + ["safe/../scene.glb"], + ["safe/./scene.glb"], + ["safe//scene.glb"], + ["safe/\u0000scene.glb"], + ["safe/\u0085scene.glb"], + ["safe/\u202escene.glb"], + ["safe/\ud800scene.glb"], + ["scene.glb?token=secret"], + ["scene.png"], + ["scene.glb.exe"], + ]; + for (const result of rejected) { + expect(extractHistoryOutputRefs({ + outputs: { "9": { result } }, + })).toEqual([]); + } + }); + it("detects HDR image refs by filename suffix without treating hashes as HDR", () => { expect(isHdrImageFilename("render.EXR")).toBe(true); expect(isHdrImageFilename("studio.hdr")).toBe(true);