diff --git a/app/ui/catalogue/fixtures.js b/app/ui/catalogue/fixtures.js index 95d5889d..af27ba20 100644 --- a/app/ui/catalogue/fixtures.js +++ b/app/ui/catalogue/fixtures.js @@ -1,12 +1,12 @@ // Hide, don't warn -- shown rather than asserted. // -// One good module and five broken ones, put through the SAME shell code the app +// One good module and four broken ones, put through the SAME shell code the app // uses. The good one's panel appears. The broken ones do not appear at all: no // placeholder, no greyed row, no warning chip. What you see on this page is the // whole of what a user would see. // // These mirror core/tests/fixtures/fixture_modules, so the browser and the -// Python suite are looking at the same five ways of being wrong. +// Python suite are looking at the same four ways of being wrong. export const GOOD = { id: "audio_clock", @@ -31,11 +31,6 @@ export const GOOD = { // Each of these is refused somewhere different, which is the point: they are // caught at the earliest layer that can catch them. export const BROKEN = [ - { - why: "raises on import", - caughtBy: "core: the scan imports it in a try", - spec: { id: "explodes", title: "Explodes", mount: "demo.panel", settings: {}, ui: "/nope/missing.js" }, - }, { why: "asks for a renderer its type does not have", caughtBy: "core: schema validation, before the manifest", diff --git a/app/ui/catalogue/hide.js b/app/ui/catalogue/hide.js index a9f37fc8..b80fdc32 100644 --- a/app/ui/catalogue/hide.js +++ b/app/ui/catalogue/hide.js @@ -22,7 +22,7 @@ async function render(theme) { const section = el("section", "cat-section"); section.append(el("h2", null, "What a user sees")); section.append(el("p", null, - "Six modules go in: one valid, five broken in different ways. Everything below " + + "Five modules go in: one valid, four broken in different ways. Everything below " + "is the whole of what reaches the screen.")); const pane = el("section", "cat-pane"); diff --git a/app/ui/composer/tests/lint_fixtures/clean/ui.js b/app/ui/composer/tests/lint_fixtures/clean/ui.js deleted file mode 100644 index c59a0bff..00000000 --- a/app/ui/composer/tests/lint_fixtures/clean/ui.js +++ /dev/null @@ -1,7 +0,0 @@ -export function setup({ composer, values }) { - const button = composer.button.md({ - label: "Re-key", - onClick: () => values.set("enabled", true), - }); - return button; -} diff --git a/app/ui/composer/tests/lint_fixtures/offenders/ui.js b/app/ui/composer/tests/lint_fixtures/offenders/ui.js deleted file mode 100644 index e6ff8691..00000000 --- a/app/ui/composer/tests/lint_fixtures/offenders/ui.js +++ /dev/null @@ -1,12 +0,0 @@ -export function setup({ composer }) { - const b = composer.button.md({ label: "x" }); - b.node.style.color = "#ff0000"; - b.node.classList.add("mine"); - b.node.innerHTML = "hi"; - const smuggled = '
still styling
'; - const raw = document.createElement("div"); - raw.setAttribute("style", "margin: 12px"); - document.head.appendChild(raw); - void smuggled; - return new Function("return 1")(); -} diff --git a/app/ui/composer/tests/rule_kit_never_interprets.test.js b/app/ui/composer/tests/rule_kit_never_interprets.test.js new file mode 100644 index 00000000..7d7d2856 --- /dev/null +++ b/app/ui/composer/tests/rule_kit_never_interprets.test.js @@ -0,0 +1,44 @@ +// The kit and the shell never interpret content. Modules ship no JavaScript, so +// there is nothing to scan in modules/. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; + +import { scan, scanSource, code } from "../../../../tools/kit_lint/scan.js"; +import { KIT_RULES } from "../../../../tools/kit_lint/rules.js"; + +const here = dirname(fileURLToPath(import.meta.url)); +const repo = join(here, "..", "..", "..", ".."); + +test("the kit never interprets content", () => { + // The kit may style and may build elements; that is its job. What it may not + // do is parse anything a module gave it. + const violations = scan(join(repo, "app", "ui", "composer"), KIT_RULES); + assert.deepEqual(violations, [], + violations.map((v) => `${v.file}:${v.line} ${v.rule}`).join("\n")); +}); + +test("the shell never interprets content either", () => { + assert.deepEqual(scan(join(repo, "app", "ui", "shell"), KIT_RULES), []); +}); + +// --- the scanner has to actually catch things ------------------------------ + +test("the scanner catches what it claims to", () => { + // Without this, "zero violations" could mean "the scanner is broken". + const caught = new Set(scanSource("el.innerHTML = x; eval(y);", KIT_RULES).map((v) => v.rule)); + for (const rule of ["markup", "eval"]) assert.ok(caught.has(rule), `not caught: "${rule}"`); +}); + +test("prose is not code", () => { + // Otherwise every rule becomes unmentionable in the comment explaining it. + const commented = ` + // never write innerHTML here + /* and eval( is fine to discuss */ + export const ok = 1; + `; + assert.deepEqual(scanSource(commented, KIT_RULES), []); + assert.ok(!code(commented).includes("innerHTML")); +}); diff --git a/app/ui/composer/tests/rule_no_module_styling.test.js b/app/ui/composer/tests/rule_no_module_styling.test.js deleted file mode 100644 index c64e3a60..00000000 --- a/app/ui/composer/tests/rule_no_module_styling.test.js +++ /dev/null @@ -1,72 +0,0 @@ -// Rule 1: a module never styles anything. -// -// The runtime half is stronger than this -- a module is handed { composer, -// values, on } and nothing that could style or build -- but a scanner catches -// the attempt in the file rather than at the moment it would have run. - -import test from "node:test"; -import assert from "node:assert/strict"; -import { fileURLToPath } from "node:url"; -import { dirname, join } from "node:path"; -import { readFileSync } from "node:fs"; - -import { scan, scanSource, code } from "../../../../tools/kit_lint/scan.js"; -import { MODULE_RULES, KIT_RULES } from "../../../../tools/kit_lint/rules.js"; - -const here = dirname(fileURLToPath(import.meta.url)); -const repo = join(here, "..", "..", "..", ".."); -const FIXTURES = join(here, "lint_fixtures"); - -test("every module in the tree is clean", () => { - // Empty today; the value is that it stays empty as modules arrive. - const violations = scan(join(repo, "modules"), MODULE_RULES); - assert.deepEqual(violations, [], - violations.map((v) => `${v.file}:${v.line} ${v.rule} -- ${v.why}`).join("\n")); -}); - -test("the kit never interprets content", () => { - // The kit may style and may build elements; that is its job. What it may not - // do is parse anything a module gave it. - const violations = scan(join(repo, "app", "ui", "composer"), KIT_RULES); - assert.deepEqual(violations, [], - violations.map((v) => `${v.file}:${v.line} ${v.rule}`).join("\n")); -}); - -test("the shell never interprets content either", () => { - assert.deepEqual(scan(join(repo, "app", "ui", "shell"), KIT_RULES), []); -}); - -// --- the scanner has to actually catch things ------------------------------ - -test("a clean module passes", () => { - const source = readFileSync(join(FIXTURES, "clean", "ui.js"), "utf8"); - assert.deepEqual(scanSource(source, MODULE_RULES), []); -}); - -test("the scanner catches every rule it claims to", () => { - // Without this, "zero violations" could mean "the scanner is broken" -- which - // is the failure mode of every lint nobody tests. - const source = readFileSync(join(FIXTURES, "offenders", "ui.js"), "utf8"); - const caught = new Set(scanSource(source, MODULE_RULES).map((v) => v.rule)); - for (const rule of ["inline-style", "style-attribute", "class-manipulation", - "raw-element", "markup", "stylesheet", "hardcoded-colour", - "hardcoded-size", "eval"]) { - assert.ok(caught.has(rule), `the offender fixture was not caught by "${rule}"`); - } -}); - -test("prose is not code", () => { - // Otherwise every rule becomes unmentionable in the comment explaining it. - const commented = ` - // never write innerHTML here - /* and #ff0000 and 12px are fine to discuss */ - export const ok = 1; - `; - assert.deepEqual(scanSource(commented, MODULE_RULES), []); - assert.ok(!code(commented).includes("innerHTML")); -}); - -test("a url containing a hash is not a colour", () => { - assert.deepEqual(scanSource('export const a = "#abc";', MODULE_RULES).map((v) => v.rule), - ["hardcoded-colour"], "a real colour is caught"); -}); diff --git a/app/ui/composer/tests/rule_unknown_element_hides.test.js b/app/ui/composer/tests/rule_unknown_element_hides.test.js index 54ec28fc..a5ac14bc 100644 --- a/app/ui/composer/tests/rule_unknown_element_hides.test.js +++ b/app/ui/composer/tests/rule_unknown_element_hides.test.js @@ -76,21 +76,6 @@ test("nothing about a hidden module reaches the screen", async () => { assert.equal(host.textContent, ""); }); -test("a module whose ui.js fails to load is hidden, not half-mounted", async () => { - const host = document.createElement("div"); - document.body.appendChild(host); - mounts.offer("test.region", host); - - const { mounted, hidden } = await mountAll( - { modules: [{ ...spec("has_ui", { on: { type: "bool", default: true, label: "On" } }), ui: "/missing.js" }] }, - { load: () => { throw new Error("404"); } }, - ); - - assert.deepEqual(mounted, []); - assert.equal(hidden[0].id, "has_ui"); - assert.equal(host.childElementCount, 0, "its panel did not stay behind"); -}); - test("a module naming a mount point nobody offers is absent", async () => { // Hide, don't warn, one level up: a typo'd mount point makes a module vanish // rather than landing somewhere arbitrary. @@ -100,44 +85,3 @@ test("a module naming a mount point nobody offers is absent", async () => { assert.deepEqual(mounted, []); assert.match(hidden[0].why, /no region offers/); }); - -test("a module is handed no way to touch the document", async () => { - const host = document.createElement("div"); - document.body.appendChild(host); - mounts.offer("test.region", host); - - let context = null; - await mountAll( - { modules: [{ ...spec("m", { on: { type: "bool", default: true, label: "On" } }), ui: "/ui.js" }] }, - { load: async () => ({ setup: (ctx) => { context = ctx; } }) }, - ); - - assert.deepEqual(Object.keys(context).sort(), ["on", "shell", "values"]); - - // Not "no value IS a node" -- that was the old assertion, and it passed while - // `composer` sat in this object handing out handles whose .node is a real - // element, reachable to the whole document through node.ownerDocument. The - // property that matters is that nothing here PRODUCES one either. - for (const [name, value] of Object.entries(context)) { - assert.equal(value instanceof globalThis.Node, false, `${name} is a DOM node`); - } - assert.equal("composer" in context, false, - "an element factory is a way to obtain a DOM node"); - - // `on` is callable, and must hand back an unsubscribe rather than anything - // that leads to the page. - const off = context.on(() => {}); - assert.equal(typeof off, "function"); - assert.equal(off instanceof globalThis.Node, false); - - // Nothing one level down is a node either -- the services are the other place - // a handle could have been tucked away. - for (const [name, value] of Object.entries(context.shell)) { - for (const [inner, member] of Object.entries(value)) { - assert.equal(member instanceof globalThis.Node, false, `shell.${name}.${inner}`); - } - } - - // The service surface is named one by one, so it can be reviewed. - assert.deepEqual(Object.keys(context.shell).sort(), ["density", "theme"]); -}); diff --git a/app/ui/shell/panels.js b/app/ui/shell/panels.js index e3e1108c..871b3de5 100644 --- a/app/ui/shell/panels.js +++ b/app/ui/shell/panels.js @@ -1,28 +1,12 @@ // Turning announcements into panels. // -// Everything a module can reach is decided here, and it is deliberately little: -// { values, on, shell }. No DOM node, no document, no element factory. -// -// `composer` used to be in that list, and it was the hole in the guarantee: a -// factory returns a handle whose `.node` is a real element, so a module could -// take one, style it, and reach the whole document through `node.ownerDocument` -// -- no lint to evade, just the intended API. No module ever used it. Panels are -// rendered from the DECLARATION, and a module that wants to draw something is a -// module that should be declaring it instead. -// -// What is true, stated exactly: the kit hands a module nothing that touches the -// DOM. What is NOT true, and must not be claimed: that a module CANNOT reach the -// DOM. A ui.js is an ES module in the page realm and `document` is a global. -// tools/kit_lint raises the cost of doing that accidentally; it is a discipline -// check and not a boundary, and only real isolation (a worker, another realm) -// would make it one. First-party modules in this repo are reviewed; a -// third-party module would need that isolation before it could be trusted. +// A module draws nothing. It declares its settings and this file has the kit +// render them: no DOM node, no document, no element factory ever reaches a module. +// A module with a broken declaration is hidden whole, never half-mounted. -import { composer } from "../composer/composer.js"; import { renderPanel, defaultsOf } from "../composer/panel.js"; import { hostFor, offered } from "./mounts.js"; import * as values from "./values.js"; -import { services } from "./services.js"; /** * mountAll(manifest) -> { mounted, hidden } @@ -32,7 +16,7 @@ import { services } from "./services.js"; * indistinguishable from one that was never installed, which is the point -- * the alternative is a UI full of things that cannot be used. */ -export async function mountAll(manifest, { load = (path) => import(path) } = {}) { +export async function mountAll(manifest) { const mounted = []; const hidden = []; @@ -40,8 +24,7 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {}) // Nothing to render. A module may exist purely to contribute ComfyUI nodes // (a loader, a model's compatibility module), and those have no panel and // no mount. Skipping is not hiding: there is no failure to report. - const renders = Object.keys(spec.settings || {}).length > 0 || spec.ui; - if (!renders) continue; + if (!Object.keys(spec.settings || {}).length) continue; const host = hostFor(spec.mount); if (!host) { @@ -50,58 +33,20 @@ export async function mountAll(manifest, { load = (path) => import(path) } = {}) } try { - // Values first: a module's ui.js may read them during setup, and a - // headless run has them without any of this having happened. values.seed(spec.id, defaultsOf(spec)); - let ui = null; - if (spec.ui) ui = await load(spec.ui); - let teardown = null; - - if (ui && typeof ui.setup === "function") { - // Kept, not discarded: setup() returns the unsubscribe from on(), and - // dropping it means every re-mount leaves another live listener behind. - teardown = ui.setup({ - values: { - get: () => values.valuesOf(spec.id), - set: (key, value) => values.set(spec.id, key, value), - }, - // Scoped to this module: a module hearing about everyone else's - // edits would have to filter, and filtering needs an id it should - // not need to know. - on: (fn) => values.onChange((id, key, value) => { - if (id === spec.id) fn(key, value); - }), - // The app services a module may use, named explicitly. A short, - // reviewable list beats reaching for a global, which is how a module - // ends up depending on something nobody meant to expose. - shell: services, - }); - } - - // Built AFTER setup, so a control shows what setup settled on. The panel - // is drawn from a snapshot of the values, so building it first meant a - // module that adopted a live value in setup -- the theme, which the page - // applies before any module exists -- had a control showing something the - // app was not set to. const panel = renderPanel(spec, { values: values.valuesOf(spec.id), onChange: (key, value) => values.set(spec.id, key, value), }); - // Only now, with a complete panel AND a setup() that returned, does - // anything reach the DOM. Appending before setup ran left a module whose - // setup threw reported as hidden with its panel still on screen -- which - // is precisely the guarantee this file states it keeps. + // Only a complete panel reaches the DOM. host.appendChild(panel.node); mounted.push({ id: spec.id, panel, - destroy() { - if (typeof teardown === "function") teardown(); - panel.destroy(); - }, + destroy() { panel.destroy(); }, }); } catch (err) { hidden.push({ id: spec.id, why: `${err.name}: ${err.message}` }); diff --git a/app/ui/shell/services.js b/app/ui/shell/services.js deleted file mode 100644 index ba5bb1d1..00000000 --- a/app/ui/shell/services.js +++ /dev/null @@ -1,26 +0,0 @@ -// What the app offers modules beyond the kit. -// -// Deliberately tiny and named one by one. The alternative -- letting a module -// reach for a global -- means the surface is "whatever happens to be on window", -// which nobody can review and nothing can keep stable. - -import { applyDensity, normaliseCols, rememberCols, recallCols } from "../composer/internals/density.js"; - -export const services = { - theme: { - set(choice) { window.ComposerTheme.apply(choice); }, - get: () => window.ComposerTheme.get(), - resolved: () => window.ComposerTheme.resolved(), - }, - density: { - /** What the grids are set to, which the browser remembers between visits. */ - get: () => recallCols("app"), - /** Column count for every adaptive grid on the page. */ - set(cols) { - const n = normaliseCols(cols); - for (const grid of document.querySelectorAll(".cx-gallery")) applyDensity(grid, n); - rememberCols("app", n); - return n; - }, - }, -}; diff --git a/core/config.py b/core/config.py index 353eab40..39d7e66c 100644 --- a/core/config.py +++ b/core/config.py @@ -9,11 +9,9 @@ MODULES_DIR = ROOT / "modules" UI_PREFIX = "/funpack" -# Extension allowlists per served root. These are rule 1 ("a module never styles -# anything") enforced at the transport layer: a .css inside modules/ is a 404, so -# a module cannot ship a stylesheet even if someone writes one. +# Extension allowlist for the served app. modules/ is never served: a module is +# Python and a settings declaration, and draws nothing. APP_EXTS = frozenset({".js", ".css", ".html", ".woff2", ".svg"}) -MODULE_EXTS = frozenset({".js"}) # Where projects are kept. Inside the pack rather than ComfyUI's output tree: a # project is FunPack's own state, and clearing outputs must not take a user's diff --git a/core/contract.py b/core/contract.py index d6c36598..6c7a412a 100644 --- a/core/contract.py +++ b/core/contract.py @@ -81,7 +81,6 @@ class ModuleSpec: # Hook points a sampler must offer before this module can run. Empty means # "any sampler will do", the same narrowing rule traits follow. hooks: List[str] = field(default_factory=list) - ui: Optional[str] = None # served path to its ui.js status: str = "experimental" # or "proven" source: str = "" # dotted import path @@ -109,7 +108,6 @@ class ModuleSpec: "before": list(self.before), "stage": self.stage, "category": self.category, - "ui": self.ui, "status": self.status, "nodes": [_node_id(n) for n in self.nodes], } diff --git a/core/registry.py b/core/registry.py index 85d11c26..cd654339 100644 --- a/core/registry.py +++ b/core/registry.py @@ -132,12 +132,6 @@ def scan(root: Optional[Path] = None, package: Optional[str] = None) -> Registry # live in packages too. continue - # A module's ui.js is optional, and the default is none: most modules - # are a settings schema and no JavaScript at all. - ui_file = feature / "ui.js" - if ui_file.is_file(): - raw["ui"] = f"{config.UI_PREFIX}/modules/{'/'.join(rel.parts)}/ui.js" - try: registry.add(validate(raw, source=dotted)) except SchemaError as exc: diff --git a/core/routes.py b/core/routes.py index 1ebfa41c..38de4b01 100644 --- a/core/routes.py +++ b/core/routes.py @@ -1197,10 +1197,6 @@ def register(routes, prefix=None): async def _app_asset(req): return _serve_under(req, config.APP_DIR, config.APP_EXTS) - @routes.get(P + "/modules/{tail:.*}") - async def _module_asset(req): - return _serve_under(req, config.MODULES_DIR, config.MODULE_EXTS) - @routes.get(P + "/") async def _index(_req): # The frontend: app/ui. diff --git a/core/schema.py b/core/schema.py index bc7e96af..afdd0927 100644 --- a/core/schema.py +++ b/core/schema.py @@ -337,7 +337,7 @@ def validate(announcement: Dict[str, Any], source: str = "") -> ModuleSpec: # region would make it invent one -- which then shows up as "hidden", i.e. # a module reported as broken for having nothing to show. mount = announcement.get("mount") - renders = bool(announcement.get("settings")) or bool(announcement.get("ui")) + renders = bool(announcement.get("settings")) if renders and (not isinstance(mount, str) or not mount.strip()): raise SchemaError( f"module {announcement.get('id') or source or '?'} has settings to render " @@ -401,7 +401,6 @@ def validate(announcement: Dict[str, Any], source: str = "") -> ModuleSpec: before=_ids("before"), stage=stage, category=category, - ui=announcement.get("ui"), status=status, source=source, ) diff --git a/core/tests/fixtures/fixture_modules/timing/audio_clock/ui.js b/core/tests/fixtures/fixture_modules/timing/audio_clock/ui.js deleted file mode 100644 index ff8b4c56..00000000 --- a/core/tests/fixtures/fixture_modules/timing/audio_clock/ui.js +++ /dev/null @@ -1 +0,0 @@ -export default {}; diff --git a/core/tests/test_registry.py b/core/tests/test_registry.py index e2aab3a1..e6cb3a91 100644 --- a/core/tests/test_registry.py +++ b/core/tests/test_registry.py @@ -51,15 +51,6 @@ def test_every_failure_says_why(scanned): assert where and why, f"failure recorded with no reason: {where!r} {why!r}" -def test_a_module_with_a_ui_file_advertises_its_path(scanned): - assert scanned.specs["audio_clock"].ui.endswith("/modules/timing/audio_clock/ui.js") - - -def test_a_module_without_a_ui_file_advertises_none(scanned): - # The default is no JavaScript at all: a settings schema is a whole module. - assert scanned.specs["momentum"].ui is None - - def test_defaults_are_available_without_any_ui(scanned): assert scanned.specs["audio_clock"].defaults() == { "enabled": True, "strength": 0.65, "mode": "beat", diff --git a/core/tests/test_routes.py b/core/tests/test_routes.py index 39a2398d..9cba8141 100644 --- a/core/tests/test_routes.py +++ b/core/tests/test_routes.py @@ -41,7 +41,6 @@ def app(tmp_path, monkeypatch): mod_dir = tmp_path / "modules" / "timing" / "audio_clock" mod_dir.mkdir(parents=True) - (mod_dir / "ui.js").write_text("export default 1;") (mod_dir / "sneaky.css").write_text(".x{}") monkeypatch.setattr(config, "APP_DIR", app_dir) @@ -113,13 +112,9 @@ def test_a_modules_url_cannot_reach_app_content(app): # --- the allowlist on ordinary URLs ---------------------------------------- -def test_module_css_is_refused(app): +def test_modules_are_not_served(app): assert get(app, P + "/modules/timing/audio_clock/sneaky.css")[0] == 404 - - -def test_module_js_is_served(app): - status, body = get(app, P + "/modules/timing/audio_clock/ui.js") - assert status == 200 and "export default" in body + assert get(app, P + "/modules/timing/audio_clock/ui.js")[0] == 404 def test_app_asset_is_served(app): diff --git a/core/tests/test_serve_allowlist.py b/core/tests/test_serve_allowlist.py index 5c64d8b3..8a958dfc 100644 --- a/core/tests/test_serve_allowlist.py +++ b/core/tests/test_serve_allowlist.py @@ -1,8 +1,4 @@ -"""Rule 1, transport half: a module cannot ship anything but JavaScript. - -These exist before there is any UI to enforce them against — the wall goes up -first, so nothing can be built that leans on a hole in it. -""" +"""Rule 1, transport half: only the app is served; modules/ never is.""" import pytest @@ -23,7 +19,6 @@ def tree(tmp_path): mods = tmp_path / "modules" (mods / "timing" / "audio_clock").mkdir(parents=True) - (mods / "timing" / "audio_clock" / "ui.js").write_text("export default {};") (mods / "timing" / "audio_clock" / "sneaky.css").write_text(".x{color:red}") (mods / "timing" / "audio_clock" / "panel.html").write_text("no") (mods / "timing" / "audio_clock" / "module.py").write_text("x = 1") @@ -36,35 +31,8 @@ def app_get(tree, rel): return serve(tree / "app", rel, config.APP_EXTS) -def module_get(tree, rel): - return serve(tree / "modules", rel, config.MODULE_EXTS) - - # --- the allowlist --------------------------------------------------------- -def test_module_js_is_served(tree): - r = module_get(tree, "timing/audio_clock/ui.js") - assert r.status == 200 - assert b"export default" in r.body - - -def test_module_css_is_not_served(tree): - assert module_get(tree, "timing/audio_clock/sneaky.css").status == 404 - - -def test_module_html_is_not_served(tree): - assert module_get(tree, "timing/audio_clock/panel.html").status == 404 - - -def test_module_python_is_not_served(tree): - assert module_get(tree, "timing/audio_clock/module.py").status == 404 - - -def test_module_directory_is_not_an_index(tree): - # modules/ has no .html in its allowlist, so a directory is nothing to serve. - assert module_get(tree, "timing/audio_clock").status == 404 - - def test_app_serves_its_own_css_and_fonts(tree): assert app_get(tree, "composer/composer.css").status == 200 assert app_get(tree, "assets/Inter.woff2").status == 200 @@ -87,10 +55,6 @@ def test_traversal_is_forbidden(tree, rel): assert app_get(tree, rel).status == 403 -def test_traversal_out_of_modules_is_forbidden(tree): - assert module_get(tree, "../app/boot.js").status == 403 - - def test_null_byte_is_forbidden(tree): assert app_get(tree, "boot.js\x00.png").status == 403 @@ -106,7 +70,6 @@ def test_symlink_escape_is_forbidden(tree): def test_js_is_explicitly_text_javascript(tree): # With ES modules a text/plain guess makes the browser refuse the module. assert app_get(tree, "boot.js").content_type == "text/javascript" - assert module_get(tree, "timing/audio_clock/ui.js").content_type == "text/javascript" def test_known_types_are_explicit(): diff --git a/docs/architecture.md b/docs/architecture.md index b79489b1..4f87d0e9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -87,11 +87,10 @@ There are three trust levels in the app code. Know which one you are writing. | You are writing | May style or build DOM? | Checked by | |---|---|---| | A **backend module's settings** (Python only) | No JavaScript at all. The kit draws the rows. | `core/schema.py`, `core/tests/test_shipped_hints.py` | -| An optional **module `ui.js`** (`modules///ui.js`). Not loaded by the app; see [Writing a module](writing-a-module.md#optional-uijs). | No. `setup({ values, on, shell })` and nothing else. | `tools/kit_lint` scans `modules/` | | A **UI feature** in `app/ui/features/` | It builds with kit elements. A few features touch `document` or add `fp-` classes in `frame.css`; new work should not. | review; the kit rules are enforced on `composer/` and `shell/` | | The **kit** itself | Yes. It is the only place that styles. | `tools/kit_lint` `KIT_RULES`, tests in `composer/tests/` | -Be exact about what the lint is. `tools/kit_lint` is a discipline check, not a security boundary: a module `ui.js` runs in the page, so `document` is reachable. First-party modules are reviewed. A third-party module would need real isolation (a worker or another realm) before it could be trusted. +Be exact about what the lint is. `tools/kit_lint` is a discipline check, not a security boundary. Modules run only in Python and are never served to the browser; first-party modules are reviewed. ## Where things live diff --git a/docs/module-reference.md b/docs/module-reference.md index 06751872..022aa4f5 100644 --- a/docs/module-reference.md +++ b/docs/module-reference.md @@ -10,7 +10,7 @@ Read by `core/registry.py` from the module's `__init__.py` (the `FIELDS` table). |---|---|---|---| | `ID` | `str` | none | Required, non-empty. Unique across modules (a second claimant is left out and said). Keep it free of `.`: a fault is traced back to a module from its hook key `funpack.`. | | `TITLE` | `str` | none | Required, non-empty. | -| `MOUNT` | `str` | `""` | Required and non-empty when the module has `SETTINGS` (or a `ui.js`, which the app does not load). | +| `MOUNT` | `str` | `""` | Required and non-empty when the module has `SETTINGS`. | | `SETTINGS` | `dict` | `{}` | `{key: setting}`. Must be a dict. | | `STAGE` | `str` | `"sampling"` | One of `load`, `conditioning`, `latent`, `guidance`, `sampling`, `post`. | | `CATEGORY` | `str` | `""` | `""` or one of `continuity`, `guidance`, `conditioning`, `sampling`, `post`, `system`. Empty shows under "Other". | @@ -25,8 +25,6 @@ Read by `core/registry.py` from the module's `__init__.py` (the `FIELDS` table). | `TRAITS` | callable | none | `traits(model) -> iterable[str]`. | | `PROVIDES` | `dict[str, callable]` | `{}` | Capability name to function. Names must be non-empty strings. | -Not read from constants: `ui`. Core sets it to `/funpack/modules///ui.js` when that file exists. - `CONTRACT_VERSION` in `core/contract.py` is `1`. It is sent to the browser as `contract` in `/funpack/api/modules`. Core does not read a version from modules, so there is nothing for a module to declare. ## Setting keys @@ -183,5 +181,4 @@ Most used: | `/funpack/` | the app (`index.html`) | | `/funpack/api/modules` | the manifest (`?traits=a,b` narrows to a model) | | `/funpack/api/m//...` | a module's own routes | -| `/funpack/modules///ui.js` | a module's `ui.js`. Only `.js` is served from `modules/`. The app does not load it. | | `/funpack/app/...` | the app. Allowed types: `.js .css .html .woff2 .svg`. | diff --git a/docs/testing.md b/docs/testing.md index 61ad392c..64d87879 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -66,7 +66,7 @@ See [Adding an element](ui-kit-reference.md#adding-an-element). The registry-wid ### `tools/kit_lint` -`scan.js` applies `MODULE_RULES` to module `ui.js` files and `KIT_RULES` to the kit. `rule_no_module_styling.test.js` runs it and also runs the files in `composer/tests/lint_fixtures`, so a rule that stops catching its own bad example fails. It is a discipline check, not a security boundary. +`scan.js` applies `KIT_RULES` to the kit and the shell: no markup parsing, no `eval`. `rule_kit_never_interprets.test.js` runs it and checks it still catches a bad example. It is a discipline check, not a security boundary. ## Looking at it diff --git a/docs/ui-design-guidelines.md b/docs/ui-design-guidelines.md index 343af367..83d3077e 100644 --- a/docs/ui-design-guidelines.md +++ b/docs/ui-design-guidelines.md @@ -17,7 +17,7 @@ How FunPack looks and behaves, and why. Read this before you add a screen, a set Only the kit styles anything: `app/ui/composer/` (tokens, internals, elements). A feature builds with kit elements and names no colour, size or class. - A rule test fails the build if kit code parses content as markup or compiles code (`tools/kit_lint` `KIT_RULES`). -- The same scanner is run over module `ui.js` files (the running app does not load them) with stricter rules (`MODULE_RULES`): no inline style, no `classList`, no `document`, no raw hex colour, no `px` size. +- Modules ship no JavaScript, so there is nothing to scan in `modules/`. - A few older features touch `document` or add `fp-` classes in `shell/frame.css`. Do not copy them. New work uses the kit, and if the kit lacks the element, add the element (see [Adding an element](ui-kit-reference.md#adding-an-element)). ## Tokens diff --git a/docs/ui-kit-reference.md b/docs/ui-kit-reference.md index d4b6a191..dda2aefb 100644 --- a/docs/ui-kit-reference.md +++ b/docs/ui-kit-reference.md @@ -180,7 +180,7 @@ Used by elements. A feature does not import them. They exist in `composer/intern |---|---| | `rule_content_is_data.test.js` | Every string prop of every demo is shown as text. | | `rule_unknown_element_hides.test.js` | Asking for a missing element throws and hides the feature. | -| `rule_no_module_styling.test.js` | `tools/kit_lint` finds nothing in `modules/`, and its own fixtures (`tests/lint_fixtures`) still trip every rule. | +| `rule_kit_never_interprets.test.js` | `tools/kit_lint` finds no markup parsing or `eval` in the kit and the shell, and still catches a bad example. | | `register.test.js` | Duplicate definitions are refused. | | `overlays.test.js`, `dismiss.test.js`, `zlayer.test.js`, `portal.test.js` | Layering, Escape and portal behaviour. | | `panel_schema.test.js` | `panel.js` draws every setting type. | diff --git a/docs/writing-a-module.md b/docs/writing-a-module.md index 0bfe377c..d38d7e05 100644 --- a/docs/writing-a-module.md +++ b/docs/writing-a-module.md @@ -132,7 +132,7 @@ Constants at the top of `__init__.py`, uppercase because they are declarations, A declaration that is wrong does not load, and the Log says exactly what is wrong. There is no best-effort mode. -**About `MOUNT`.** Core requires it when a module has settings, so give it. In the app as shipped, though, module settings are drawn by Settings > Engine and the Sampler window, which group by `CATEGORY`, not `MOUNT`. `MOUNT` only matters to the optional `ui.js` path described at the end. +**About `MOUNT`.** Core requires it when a module has settings, so give it. In the app as shipped, though, module settings are drawn by Settings > Engine and the Sampler window, which group by `CATEGORY`, not `MOUNT`. `MOUNT` only matters to the catalogue and the tests, which mount panels by it. ## Settings @@ -182,7 +182,7 @@ What the tests add (`core/tests/test_shipped_hints.py`), so treat them as rules: Two code paths can draw a setting, and they differ: - **Settings > Engine and the Sampler window** (`app/ui/features/settings/field.js`). This is what people use. It draws by `type`: `bool` as a toggle; `int` and `float` as a slider when `ui` is absent or `"slider"` and both `min` and `max` are set, otherwise as a number box; `enum` as a select; `color` as a swatch; `multiline` as a text area; `text` and `path` as a text box. A `ui` of `segmented`, `wheel`, `radioGroup`, `filterList` and so on is accepted by the schema but this page still draws the default for the type. `placeholder` is not used here. -- **`composer/panel.js`** honours every `ui` hint in `UI_HINTS`. It is used by the catalogue and the tests, and by the optional `ui.js` route. +- **`composer/panel.js`** honours every `ui` hint in `UI_HINTS`. It is used by the catalogue and the tests. So: choose `ui` only between `slider` and `number` for numbers, and design for the select and toggle everywhere else. @@ -322,22 +322,9 @@ Research or debug data does not go in the console: put a toggle, a Download butt - Fewest words in docstrings, hints and tooltips. No backstory and no future plans in the repo. - Nothing NSFW in code, tests, comments, commits or docs. Build test data from constructed fixtures. -## Optional `ui.js` +## No JavaScript -A module may have `ui.js` next to its `__init__.py`. Core serves it at `/funpack/modules///ui.js` (only `.js` files are served from `modules/`, so a module cannot ship CSS) and puts the path in its manifest. The contract in `app/ui/shell/panels.js`: - -```js -export function setup({ values, on, shell }) { - // values.get() / values.set(key, value) this module's settings only - // on((key, value) => ...) hears this module's changes only - // shell.theme, shell.density the small list of app services - return () => { /* teardown */ }; -} -``` - -A module's `ui.js` gets **no element factory and no DOM**. It cannot style, build or inject anything: `tools/kit_lint` fails the build on inline styles, class changes, `document`, `window`, `globalThis`, `innerHTML`, hardcoded colours or pixel sizes, stylesheets and `eval`. The settings panel is drawn for it from the declaration. - -**The running app ignores `ui.js` on purpose.** `boot.js` never calls `mountAll`; only the catalogue and the tests do, and no shipped module has a `ui.js`. A module does not get to draw anything. It declares what it wants in `SETTINGS` (type, `ui` hint, range, hint text) and the UI kit builds the control from that declaration. Anything else a person needs to see in the app is a [UI feature](writing-a-ui-feature.md), which is app code and not a module. Do not add a `ui.js`. +A module ships no JavaScript and draws nothing. It declares what it wants in `SETTINGS` (type, `ui` hint, range, hint text) and the UI kit builds the control from that declaration. `modules/` is never served to the browser. Anything else a person needs to see in the app is a [UI feature](writing-a-ui-feature.md), which is app code and not a module. ## Before you open a pull request diff --git a/tools/kit_lint/rules.js b/tools/kit_lint/rules.js index 718ebbdb..0347306a 100644 --- a/tools/kit_lint/rules.js +++ b/tools/kit_lint/rules.js @@ -1,44 +1,11 @@ -// What a module may not do, and what the kit may not do. +// What the kit and the shell may not do. // -// A DISCIPLINE CHECK, NOT A BOUNDARY. Say it plainly, because the difference -// matters: a module's ui.js is an ES module in the page realm, so `document` is -// a global it can reach and no regex can stop it. `globalThis["docu"+"ment"]` -// defeats every rule below and always will; only real isolation -- a worker, -// another realm -- would be a boundary. -// -// What this does buy: the accidental and the casual are caught in the file -// rather than at the moment they would have run, which is the difference -// between a failing test and a support ticket. The bracket-access spellings are -// here because they were the first thing tried and they went straight through. -// -// The runtime half is the real work: the kit hands a module nothing that -// touches the DOM, so following the contract requires no restraint. +// A DISCIPLINE CHECK, NOT A BOUNDARY: it catches the accidental in the file +// rather than at the moment it would have run. Modules need no rules here +// because they ship no JavaScript. -export const MODULE_RULES = [ - { name: "inline-style", why: "presentation belongs to the kit", - test: /\.style\s*[.[]|\[\s*['"`]style['"`]\s*\]|setAttribute\s*\(\s*['"`]style/ }, - { name: "style-attribute", why: "presentation belongs to the kit", - test: /\bstyle\s*=\s*['"`]/ }, - { name: "class-manipulation", why: "kit classes are the kit's; a module names elements, not classes", - test: /\bclassList\b|setAttribute\s*\(\s*['"`]class/ }, - { name: "raw-element", why: "every element comes from the kit, not from a module", - test: /document\s*[.[]|createElementNS|\[\s*['"`]createElement['"`]\s*\]/ }, - { name: "reaching-for-globals", why: "a module is handed what it may use; the rest is not for it", - test: /\bglobalThis\b|\bwindow\s*[.[]|\bself\s*\[/ }, - { name: "markup", why: "content is data; the kit renders it as text", - test: /\binnerHTML\b|\bouterHTML\b|insertAdjacentHTML|\[\s*['"`](inner|outer)HTML['"`]\s*\]/ }, - { name: "stylesheet", why: "a module cannot ship styling, and cannot inject it either", - test: /adoptedStyleSheets|document\s*\.\s*head|<\s*(link|style)\b/ }, - { name: "hardcoded-colour", why: "colour comes from the theme tokens", - test: /#[0-9a-fA-F]{3,8}\b/ }, - { name: "hardcoded-size", why: "sizes come from the spacing and control scales", - test: /\b\d+(\.\d+)?px\b/ }, - { name: "eval", why: "nothing in a module is compiled at runtime", - test: /\beval\s*[([]|new\s+Function\s*\(|\[\s*['"`](eval|Function)['"`]\s*\]/ }, -]; - -// The kit itself may style and may build elements -- that is its job -- but it -// must never interpret content. +// The kit may style and build elements -- that is its job -- but it must never +// interpret content. export const KIT_RULES = [ { name: "markup", why: "the kit renders content as text and never parses it", test: /\binnerHTML\b|\bouterHTML\b|insertAdjacentHTML|document\s*\.\s*write/ },