diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1aedd493f..0876eb74c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -807,7 +807,7 @@ Widgets is a separate page because extension UI is an execution surface, not cat Declarative widgets support forms and actions, status/data/text/code/markdown, tables, tabs, charts, polls, jobs, streams, subscriptions, progress, media, files, maps, calendars, kanban, and composition through `group`, `metric`, and `callout`. One recursive validator limits the tree to depth 8 and 256 nodes and reports the exact failing path. Nested interactive components use an explicit id or stable tree path as identity; `subscription.render` remains transitively passive so an incoming event cannot smuggle a new active control tree past validation. Text, attributes, links, media routes, and field values are escaped or constrained for their actual sink. -Module widgets receive a narrow parent-mediated fetch bridge. The iframe's policy denies ambient network and origin authority; the parent accepts requests only to the exact owning extension prefix under `/api/extensions//...` and returns the response through a nonce-bound message exchange. The host-generated module bootstrap observes the content-sized `#root` edge with `ResizeObserver` plus a load measurement, integer-deduplicates and clamps resize messages, and receives a nonce-bound dispose message that rejects pending child fetch promises and disconnects the observer. Module source loading is also bounded and aborted when a mount becomes stale. The module source endpoint (`/api/extensions/{skill}/module/{path}`) authorizes purely against the live loader registration — no skill discovery on the read path — and serves any reviewed `.js`/`.mjs` file of the skill directory, the declared entry or a sibling such as `lib/x.js`, from the texts captured when the bundle's module tab registered rather than re-reading the skill directory (the bytes served are the bytes the reviewed bundle loaded from; an edit after load is not served until the skill reloads; dependency, cache, and dot directories are never captured, and traversal-shaped paths are refused before any lookup). It answers with `Access-Control-Allow-Origin: *`, because the requesting `srcdoc` frame has an opaque origin and its script fetches are cross-origin and anonymous; the declared entry still executes as a classic script, and sibling files load through ` diff --git a/ouroboros/contracts/plugin_api.py b/ouroboros/contracts/plugin_api.py index 93b3d4235..cb65e5326 100644 --- a/ouroboros/contracts/plugin_api.py +++ b/ouroboros/contracts/plugin_api.py @@ -124,7 +124,13 @@ class PluginAPI(Protocol): *, methods: Sequence[str] = ("GET",), ) -> None: - """Register ``/api/extensions//`` for allowed methods.""" + """Register ``/api/extensions//`` for allowed methods. + + The host owns GET/HEAD under three prefixes of that namespace — + ``manifest``, ``module/...`` (any depth) and ``settings_section`` — so + a skill route registered there is shadowed for GET/HEAD (POST and the + other methods still reach the skill); register under another path. + """ ... def register_ws_handler( diff --git a/ouroboros/gateway/extensions.py b/ouroboros/gateway/extensions.py index 08f0b3673..a53ec04b2 100644 --- a/ouroboros/gateway/extensions.py +++ b/ouroboros/gateway/extensions.py @@ -528,16 +528,22 @@ async def api_extension_module(request: Request) -> Response: (``lib/x.js``). Authorization and content are one loader read under one lock: 409 when the skill has no live bundle; 404 when the path is not among the files captured when its module tab registered (dependency, cache, and - dot directories are never captured); 400 for a path with a backslash or + dot-prefixed paths are never captured); 400 for a path with a backslash or NUL, an empty/``.``/``..`` segment (the ASGI server already decoded ``%2e%2e`` and ``%2F``), or a non-``.js``/``.mjs`` suffix. The body is the text captured at load — no per-request disk read, so an edit after load is not served until the skill reloads (DEVELOPMENT "Passive GET"). The requesting ``srcdoc`` frame has an opaque origin and fetches anonymously - cross-origin, hence ``Access-Control-Allow-Origin: *`` (no credentials). + cross-origin, hence ``Access-Control-Allow-Origin: *`` (no credentials) on + every response, refusals included — else ``import()`` sees a CORS failure. """ from ouroboros.extension_loader import live_module_sources + headers = {"Cache-Control": "no-store", "Access-Control-Allow-Origin": "*"} + + def refuse(message: str, status: int) -> Response: + return JSONResponse({"error": message}, status_code=status, headers=headers) + skill_name = str(request.path_params.get("skill") or "").strip() path = str(request.path_params.get("entry") or "") if ( @@ -545,18 +551,14 @@ async def api_extension_module(request: Request) -> Response: or any(part in {"", ".", ".."} for part in path.split("/")) or not path.endswith((".js", ".mjs")) ): - return json_error("invalid module path", 400) + return refuse("invalid module path", 400) sources = live_module_sources(skill_name) if sources is None: - return json_error(f"extension {skill_name!r} not live", 409) + return refuse(f"extension {skill_name!r} not live", 409) source = sources.get(path) if source is None: - return json_error("module path is not a reviewed JavaScript file of a live widget", 404) - return Response( - source, - media_type="application/javascript; charset=utf-8", - headers={"Cache-Control": "no-store", "Access-Control-Allow-Origin": "*"}, - ) + return refuse("module path is not a reviewed JavaScript file of a live widget", 404) + return Response(source, media_type="application/javascript; charset=utf-8", headers=headers) async def api_extension_settings_section(request: Request) -> JSONResponse: diff --git a/ouroboros/skill_review.py b/ouroboros/skill_review.py index 0ad8ee911..c393ad50a 100644 --- a/ouroboros/skill_review.py +++ b/ouroboros/skill_review.py @@ -16,6 +16,7 @@ from typing import Any, Dict, List, Optional from ouroboros.config import adaptive_quorum, get_auto_grant_enabled from ouroboros.reviewer_slot_config import commit_triad_delivery, reviewer_slot_config_error from ouroboros.skill_review_passes import ( + WASM_MAGIC, SkillBinaryPayload as _SkillBinaryPayload, binary_file_descriptor, executable_magic_kind, @@ -67,12 +68,11 @@ from ouroboros.utils import ( ) log = logging.getLogger(__name__) -# The reviewable skill payload is bound by ONE pack-level token budget (reusing the -# review stack's SSOT REVIEW_PROMPT_TOKEN_BUDGET) instead of arbitrary per-file / -# file-count BYTE caps: a 76 KB data file or a 41-file skill is fully reviewable when -# the whole pack fits a 1M-context reviewer. Loadable executables / unreadable files -# are still refused (safety, not size). Headroom reserves the rest of the reviewer -# prompt (governance docs + checklist + framing) so the SKILL pack alone is bounded. +# The reviewable skill payload is bound by ONE pack-level token budget (the review +# stack's SSOT REVIEW_PROMPT_TOKEN_BUDGET), not per-file / file-count BYTE caps: a 76 KB +# data file or a 41-file skill is fully reviewable when the whole pack fits a 1M-context +# reviewer. Loadable executables / unreadable files are still refused (safety, not size). +# Headroom reserves the rest of the reviewer prompt (governance docs + checklist + framing). _SKILL_PACK_TOKEN_HEADROOM = 120_000 def _skill_pack_token_budget() -> int: @@ -82,11 +82,10 @@ def _skill_pack_token_budget() -> int: _SKILL_CHECKLIST_SECTION = "Skill Review Checklist" -# Lexical download filter retained ONLY for the marketplace fetcher's coarse -# pre-gate (ouroboros/marketplace/fetcher.py). Skill REVIEW itself judges file -# CONTENT — loader magic bytes, see ``skill_review_passes.executable_magic_kind`` -# — never filenames (X4/В21): a renamed ELF is still blocked, while a text file -# with a scary extension stays reviewable. +# Lexical download filter retained ONLY for the marketplace fetcher's coarse pre-gate +# (ouroboros/marketplace/fetcher.py). Skill REVIEW itself judges file CONTENT — loader +# magic bytes, see ``skill_review_passes.executable_magic_kind`` — never filenames +# (X4/В21): a renamed ELF is still blocked; a text file with a scary extension stays reviewable. _LOADABLE_BINARY_EXTENSIONS = frozenset( {".so", ".dylib", ".dll", ".pyc", ".pyo", ".node", ".exe", ".bin"} ) @@ -204,7 +203,8 @@ def _read_skill_file( ) -> tuple[Optional[str], bytes, Optional[Dict[str, Any]]]: """Read one skill file: ``(text, sha256_digest, descriptor)`` — exactly one set. Loadable executables (CONTENT magic bytes, never filename) hard-block review; - other non-UTF-8 files yield a typed descriptor instead of raw bytes.""" + WebAssembly (``WASM_MAGIC``, even when its bytes decode as UTF-8) and other + non-UTF-8 files yield a typed descriptor instead of raw bytes.""" try: data = path.read_bytes() except OSError as exc: @@ -219,7 +219,7 @@ def _read_skill_file( if kind: raise _SkillBinaryPayload(rel, len(data), kind) digest = hashlib.sha256(data).digest() - if text is not None: + if text is not None and not data.startswith(WASM_MAGIC): return text, digest, None return None, digest, binary_file_descriptor(rel, data, filename=path.name) @@ -266,7 +266,7 @@ def _build_skill_file_packs( file_digests.append((rel, file_digest)) if descriptor is not None: # typed descriptor, never raw non-UTF-8 bytes body = json.dumps(descriptor, indent=2, sort_keys=True) - rel_head = f"{rel} (non-UTF-8 file — descriptor only, content not inlined)" + rel_head = f"{rel} (binary file — descriptor only, content not inlined)" block = f"### {rel_head}\n\n```json\n{body}\n```" else: block = f"### {rel}\n\n```\n{body}\n```" diff --git a/ouroboros/skill_review_passes.py b/ouroboros/skill_review_passes.py index d9571287f..0f7850a86 100644 --- a/ouroboros/skill_review_passes.py +++ b/ouroboros/skill_review_passes.py @@ -19,10 +19,12 @@ from typing import Any, Callable, Dict, List, Tuple # LLMs: files whose CONTENT starts with a loader magic number are hard-blocked # from the skill payload surface — judged by magic bytes, never by filename. # These prefixes are unambiguous (never legitimate text), so they block -# regardless of UTF-8 validity. WebAssembly (``\x00asm``) is deliberately NOT +# regardless of UTF-8 validity. WebAssembly (``WASM_MAGIC``) is deliberately NOT # here: the host never loads it natively — it runs only inside the browser's -# sandboxed widget frame — so a ``.wasm`` file is admitted like any other -# non-UTF-8 asset, as a content-hash-bound descriptor the reviewer sees. +# sandboxed widget frame — so a module is admitted as a content-hash-bound +# descriptor the reviewer sees, routed there by its magic even when its bytes +# happen to decode as UTF-8 (the canonical 8-byte module does). +WASM_MAGIC = b"\x00asm" _EXECUTABLE_MAGICS: Tuple[Tuple[bytes, str], ...] = ( (b"\x7fELF", "ELF executable / shared object"), (b"\xfe\xed\xfa\xce", "Mach-O executable (32-bit)"), diff --git a/tests/test_gateway_widgets.py b/tests/test_gateway_widgets.py index 73201e5a4..38b207fbf 100644 --- a/tests/test_gateway_widgets.py +++ b/tests/test_gateway_widgets.py @@ -129,6 +129,7 @@ def test_api_extension_module_serves_reviewed_js_files_without_discovery(tmp_pat "lib/y.mjs": "export const y = 2;\n", "node_modules/dep/index.js": "module.exports = 1;\n", # review-opaque: never captured ".hidden/h.js": "window.__hidden = 1;\n", # dot directory: never captured + ".hidden.js": "window.__dotfile = 1;\n", # dot-prefixed file: never captured "notes.txt": "not javascript\n", } for rel, body in files.items(): @@ -172,21 +173,28 @@ def test_api_extension_module_serves_reviewed_js_files_without_discovery(tmp_pat # The sources live on the loader bundle, never in a browser-facing projection. assert "window.__" not in json.dumps(extension_loader.snapshot()) assert "window.__" not in json.dumps(client.get("/api/widgets").json()) - # Not captured: review-opaque and dot directories, files the payload lacks. - for rel in ("node_modules/dep/index.js", ".hidden/h.js", "missing.js", "lib/missing.mjs"): - assert get(rel).status_code == 404, rel + # A refusal carries the same no-store/ACAO headers as a 200: the opaque-origin + # frame's ``import()`` then reads the 4xx instead of an unreadable CORS failure. + def refused(response, status: int, label: str) -> None: + assert response.status_code == status, (label, response.status_code, response.text) + assert response.headers["cache-control"] == "no-store", label + assert response.headers["access-control-allow-origin"] == "*", label + + # Not captured: review-opaque and dot-prefixed paths, files the payload lacks. + for rel in ("node_modules/dep/index.js", ".hidden/h.js", ".hidden.js", "missing.js", "lib/missing.mjs"): + refused(get(rel), 404, rel) # Shape-rejected before any lookup; percent-escapes arrive decoded, so an # encoded traversal is the same ``..`` segment as a literal one. for rel in ( "%2e%2e/widget.js", "..%2Fwidget.js", "lib%2F..%2Fwidget.js", "%2e/widget.js", "lib%5Cx.js", "%2Flib/x.js", "lib//x.js", "%00.js", "widget.js%00", "notes.txt", "plugin.py", "lib/", "", ): - assert get(rel).status_code == 400, rel + refused(get(rel), 400, rel) # Cross-skill and unloaded: a live skill never serves another's files. - assert client.get("/api/extensions/ext_other/module/widget.js").status_code == 404 - assert client.get("/api/extensions/nope/module/widget.js").status_code == 409 + refused(client.get("/api/extensions/ext_other/module/widget.js"), 404, "ext_other") + refused(client.get("/api/extensions/nope/module/widget.js"), 409, "nope") extension_loader.unload_extension("ext_module") - assert get("widget.js").status_code == 409 + refused(get("widget.js"), 409, "unloaded") assert all(count == 0 for count in calls.values()), calls diff --git a/tests/test_skill_review.py b/tests/test_skill_review.py index 6723031d3..98a36a6eb 100644 --- a/tests/test_skill_review.py +++ b/tests/test_skill_review.py @@ -1243,7 +1243,7 @@ def test_skill_review_admits_wasm_as_content_hash_bound_descriptor(tmp_path): assert descriptor is not None and descriptor.pop("mime_from_name") assert descriptor == {"path": "core.wasm", "size": len(wasm), "sha256": _hashlib.sha256(wasm).hexdigest()} joined = "\n".join(_build_skill_file_packs(skill_dir)) - assert "core.wasm (non-UTF-8 file — descriptor only" in joined + assert "core.wasm (binary file — descriptor only" in joined assert _hashlib.sha256(wasm).hexdigest() in joined # Content-hash-bound: one changed byte is a different payload (the stored # review goes stale), exactly like every other payload file. @@ -1252,6 +1252,46 @@ def test_skill_review_admits_wasm_as_content_hash_bound_descriptor(tmp_path): assert compute_content_hash(skill_dir) != before +def test_skill_review_routes_utf8_decodable_wasm_to_descriptor(tmp_path): + """W5-7/W5-11: the descriptor route is chosen by the WebAssembly magic, not by a + failed UTF-8 decode. The canonical 8-byte module and a functional module made + only of ASCII bytes both decode as UTF-8, yet neither may be inlined as text — + "the reviewer does not read the WebAssembly code" must hold for every module. + A native loader magic renamed ``.wasm`` still hard-blocks by content.""" + import hashlib as _hashlib + + from ouroboros.skill_review import _build_skill_file_packs, _read_skill_file, _SkillBinaryPayload + from ouroboros.skill_review_passes import WASM_MAGIC + + skill_dir = tmp_path / "skills" / "wasmtext" + skill_dir.mkdir(parents=True) + (skill_dir / "skill.json").write_text('{"name": "wasmtext"}', encoding="utf-8") + canonical = WASM_MAGIC + b"\x01\x00\x00\x00" + # (func (export "f") (result i32) i32.const 42) — type, function, export and code + # sections; every byte < 0x80 (validated with WebAssembly.validate, f() == 42). + functional = ( + canonical + + b"\x01\x05\x01\x60\x00\x01\x7f" + + b"\x03\x02\x01\x00" + + b"\x07\x05\x01\x01f\x00\x00" + + b"\x0a\x06\x01\x04\x00\x41\x2a\x0b" + ) + for name, module in (("empty.wasm", canonical), ("answer.wasm", functional)): + module.decode("utf-8") # the precondition the decode-first branch mistook for text + (skill_dir / name).write_bytes(module) + text, digest, descriptor = _read_skill_file(skill_dir / name, relpath=name) + assert text is None and digest == _hashlib.sha256(module).digest(), name + assert descriptor is not None and descriptor["sha256"] == _hashlib.sha256(module).hexdigest() + assert descriptor["path"] == name and descriptor["size"] == len(module) + joined = "\n".join(_build_skill_file_packs(skill_dir)) + assert "empty.wasm (binary file — descriptor only" in joined + assert "answer.wasm (binary file — descriptor only" in joined + assert "\x00asm" not in joined # module bytes never inlined, not even as "text" + (skill_dir / "core.wasm").write_bytes(b"\x7fELF" + b"\x00" * 32) # ELF disguised as wasm + with pytest.raises(_SkillBinaryPayload): + _read_skill_file(skill_dir / "core.wasm", relpath="core.wasm") + + def test_skill_review_does_not_block_text_by_scary_filename(tmp_path): """Capability preservation: the block is content-judged, so a valid UTF-8 text file survives review even with a formerly-blocking extension or an