diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/meshbay-client/src/main.js | 20 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/api/webapp.py | 32 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_desktop_shell.py | 79 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_security_headers.py | 40 |
4 files changed, 160 insertions, 11 deletions
diff --git a/packages/meshbay-client/src/main.js b/packages/meshbay-client/src/main.js index ab579a1..83cf772 100644 --- a/packages/meshbay-client/src/main.js +++ b/packages/meshbay-client/src/main.js @@ -112,7 +112,25 @@ const CSP = [ "font-src 'self'", "connect-src 'self' https: wss:", "worker-src 'self'", - `frame-src ${RECAPTCHA_SRC}`, + // `blob:` here and in `frame-src` are one thing, not two: the PDF preview. + // + // `files-app.js` decrypts a PDF in the page, wraps it in a Blob and hands it + // to `<object type="application/pdf">`, so the bytes never leave the + // renderer. Chromium serves that with its own viewer, and the viewer takes + // TWO permissions — it loads the resource as plugin data (`object-src`, + // which was falling back to `default-src 'none'`) and then renders it in an + // internal frame (`frame-src`). Fixing either alone still shows the "this + // browser will not display the PDF inline" fallback, which is how this + // looked like a missing feature rather than a policy. + // + // `'self'` does not cover it: a same-origin `blob:` URL is NOT matched by + // `'self'` in either directive — measured on Chromium 152, this build's and + // Chrome's alike — so the token has to be `blob:` in both. Only page script + // can mint a blob URL and the type is set by our code, so what this admits + // is PDFium parsing bytes that came from a node — exactly what a browser + // does with the same file. + "object-src blob:", + `frame-src blob: ${RECAPTCHA_SRC}`, "frame-ancestors 'none'", "base-uri 'none'", "form-action 'none'", diff --git a/packages/meshbay-hub/src/meshbay_hub/api/webapp.py b/packages/meshbay-hub/src/meshbay_hub/api/webapp.py index 96b93bb..0d9d1f8 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/webapp.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/webapp.py @@ -93,9 +93,13 @@ _NO_STORE = {"Cache-Control": "no-store"} # Content-Security-Policy for the whole hub, applied by a middleware in app.py. # -# This is the *same* policy the desktop client's protocol handler already sends -# for these exact UI files (`meshbay-client/src/main.js`), plus the two reCAPTCHA -# hosts the sign-up widget loads its script, challenge iframe and images from. +# This is the desktop client's policy for these exact UI files +# (`meshbay-client/src/main.js`), which serves the same interface, and the two +# have to be changed together — a directive added here and not there breaks the +# app, and the reverse leaves the browser behind. They differ in two places, on +# purpose: `frame-ancestors` is `'self'` here and `'none'` there (nothing frames +# an `app://` page), and `frame-src` carries `'self'` here for the streamed +# download's hidden iframe, which the client has no service worker for. # `'unsafe-inline'` is style-only — htm/preact set inline `style=` attributes # everywhere; nothing inline executes, and the shell below carries no inline # `<script>`. `'wasm-unsafe-eval'` is required for the Argon2id WASM. The hub's @@ -111,6 +115,26 @@ CSP = "; ".join([ "font-src 'self'", "connect-src 'self' https: wss:", "worker-src 'self'", + # `object-src` exists for one thing, and `frame-src`'s `blob:` for the same + # thing: previewing a PDF without writing it anywhere. + # + # `files-app.js` decrypts the file in the page, wraps it in a Blob and hands + # it to `<object type="application/pdf">`. Chromium renders that with its own + # viewer, which needs TWO permissions — the resource is loaded as plugin data + # (`object-src`) and then rendered in an internal frame (`frame-src`). This + # policy had neither: `object-src` was absent, so it fell back to + # `default-src 'none'`, and the fallback message ("this browser will not + # display the PDF inline") was shown on every platform from the day this CSP + # shipped. It read as a desktop-client limitation because the client has sent + # a CSP since its first launch and the hub had none before this file. + # + # `'self'` does not cover a same-origin `blob:` URL in either directive — + # measured, not assumed, in Chrome 152 against the deployed page — so the + # token is `blob:` and it is needed in both. Widening `object-src` from + # `'none'` admits only what page script minted itself, at a type this code + # sets: PDFium parsing bytes that came from a node, which is what any + # browser does with the same file once it is downloaded. + "object-src blob:", # `'self'` is not decoration: the streamed-download path works by navigating # a hidden iframe to `/_mbdl/<id>` so the service worker is asked for the # response it is holding. Without it Chrome refuses the frame, the worker is @@ -118,7 +142,7 @@ CSP = "; ".join([ # happen — on Firefox and Safari that is the *only* way to write a large # file to disk, so the whole path was dead. Added when reCAPTCHA needed a # frame, which is why nobody connected the two. - f"frame-src 'self' {_RECAPTCHA_SRC}", + f"frame-src 'self' blob: {_RECAPTCHA_SRC}", # `'self'`, not `'none'`, and the difference is one same-origin iframe. # # The threat frame-ancestors answers is clickjacking: a *foreign* page diff --git a/packages/meshbay-hub/tests/test_desktop_shell.py b/packages/meshbay-hub/tests/test_desktop_shell.py index c9d8683..039ea2f 100644 --- a/packages/meshbay-hub/tests/test_desktop_shell.py +++ b/packages/meshbay-hub/tests/test_desktop_shell.py @@ -184,9 +184,15 @@ def _policy() -> str: body = match.group(1) if rec: body = body.replace("${RECAPTCHA_SRC}", rec.group(1)) + # Skip the `//` comments inside the array. Reading one as a directive is + # the mistake this file already records against the packaged unit test, + # which matched the comment explaining why `User=` was absent: parse + # directives, not text. A comment line here yielded a phantom `//` + # directive the moment one was written. + lines = [line.strip() for line in body.splitlines()] return "; ".join( - line.strip().strip('`",').strip('`"') - for line in body.splitlines() if line.strip()) + line.strip('`",').strip('`"') + for line in lines if line and not line.startswith("//")) def _directive(name: str) -> str: @@ -240,6 +246,75 @@ def test_recaptcha_is_the_only_third_party_and_stays_scoped_to_it(): assert tok in hosts, f"unexpected external origin in CSP: {tok}" +def test_the_pdf_preview_has_both_permissions_it_needs(): + """ + The application shows a PDF the same way the browser does, and needs the + same two permissions to do it — this is where it was missing them. + + `files-app.js` decrypts the file in the renderer and hands it to + `<object type="application/pdf">` from a Blob. Chromium's viewer loads that + as plugin data (`object-src`) and renders it in an internal frame + (`frame-src`); with either refused, the "will not display the PDF inline" + fallback is what the user gets. Measured on Electron 44 (Chromium 152): + with both opened the viewer renders, with `plugins` left at its default + `false` — the built-in viewer is not behind that flag, so do not turn it on + to fix a PDF. + + `'self'` would not do: a same-origin `blob:` URL is not matched by it in + either directive. + """ + for name in ("object-src", "frame-src"): + directive = _directive(name) + assert directive, f"no {name} directive in the main process CSP" + sources = directive.split()[1:] + assert "blob:" in sources, ( + f"{name} refuses the decrypted PDF; the preview shows its fallback " + f"message in the desktop client") + assert "*" not in sources + + +def test_the_two_policies_stay_in_step(): + """ + One interface, two policies: this one and the hub's + (`meshbay_hub.api.webapp.CSP`), sent for the very same files. A directive + added to one and not the other is a feature that works in the browser and + not in the application, or the reverse. The PDF preview is what made the + duplication visible: the client sent a policy from its first launch and the + hub sent none until 2026-09-01, so the same markup worked in Chrome and not + in the application, and the difference read as a missing native feature + rather than as a policy nobody had compared. + + Two differences are deliberate and named here. Everything else must match. + """ + from meshbay_hub.api.webapp import CSP as HUB_CSP + + def directives(policy: str) -> dict[str, set[str]]: + out = {} + for part in policy.split(";"): + tokens = part.strip().split() + if tokens: + out[tokens[0]] = set(tokens[1:]) + return out + + app, hub = directives(_policy()), directives(HUB_CSP) + assert set(app) == set(hub), ( + f"a directive exists in one policy only: {set(app) ^ set(hub)}") + + # Nothing frames an `app://` page, so the client refuses every ancestor; + # the hub allows itself, for the streamed download's hidden iframe. + assert app["frame-ancestors"] == {"'none'"} + assert hub["frame-ancestors"] == {"'self'"} + # That same iframe is why the hub's frame-src carries `'self'`. The client + # has no service worker to ask (Chromium refuses one on a custom scheme), + # so it saves through the native dialog and needs no same-origin frame. + assert hub["frame-src"] - app["frame-src"] == {"'self'"} + + for name in sorted(set(app) - {"frame-ancestors", "frame-src"}): + assert app[name] == hub[name], ( + f"{name} has drifted: app {sorted(app[name])} vs " + f"hub {sorted(hub[name])}") + + # ── The bridge ────────────────────────────────────────────────────────────── def test_the_bridge_is_the_only_way_in(): diff --git a/packages/meshbay-hub/tests/test_security_headers.py b/packages/meshbay-hub/tests/test_security_headers.py index 44dd7e8..b068656 100644 --- a/packages/meshbay-hub/tests/test_security_headers.py +++ b/packages/meshbay-hub/tests/test_security_headers.py @@ -46,7 +46,9 @@ async def test_even_a_404_carries_the_headers(client): def test_the_policy_is_locked_down_where_it_matters(): - assert "default-src 'none'" in CSP # covers object-src, etc. + # The catch-all. It no longer "covers object-src" — that directive is set + # explicitly below, and this comment used to say otherwise. + assert "default-src 'none'" in CSP # `'self'`, not `'none'`: every foreign origin is still refused, which is # the whole of the clickjacking protection. What `'self'` adds is this # origin framing itself, which the streamed download needs — see @@ -95,6 +97,38 @@ def test_the_streamed_download_frame_is_allowed(): assert "*" not in frame_src +def test_the_pdf_preview_has_both_permissions_it_needs(): + """ + Two directives govern one feature, and fixing either alone changes nothing + visible. + + `files-app.js` decrypts a PDF in the page and shows it from a Blob through + `<object type="application/pdf">`. Chromium's viewer loads that as plugin + data (`object-src`) and renders it in an internal frame (`frame-src`). + `object-src` was absent, so it fell back to `default-src 'none'` and every + preview showed the "will not display the PDF inline" fallback instead — + measured in Chrome 152 against the deployed page, where the violation was + `object-src` and the fallback was on screen. It had been broken here since + this file started sending a policy (2026-09-01); before that the hub sent + none, which is why the browser was believed to be the working one. + + `'self'` is not what either directive needs: a same-origin `blob:` URL is + not matched by `'self'`, so with `object-src` opened the preview still + failed at `frame-src`. Both carry `blob:`, and neither carries a wildcard. + """ + for name in ("object-src", "frame-src"): + directive = _directive(CSP, name) + assert directive, f"no {name} directive" + sources = directive.split()[1:] + assert "blob:" in sources, ( + f"{name} refuses the decrypted PDF; the preview shows its fallback " + f"message on every browser") + assert "*" not in sources + # A blob URL is minted by this page's own script. Nothing else needs + # these directives, so nothing else belongs in them. + assert "data:" not in sources + + def test_no_foreign_origin_may_frame_this_page(): """The clickjacking property, stated separately from how it is spelled. @@ -122,9 +156,7 @@ def test_the_two_framing_headers_agree(): Checked as a pair rather than one value apiece, because the defect was the disagreement and either one alone reads as correct. """ - import asyncio - - from meshbay_hub.app import create_app # noqa: F401 (import check) + from meshbay_hub.app import create_app # noqa: F401 (import check) ancestors = _directive(CSP, "frame-ancestors").split(" ", 1)[1].strip() expected = {"'none'": "DENY", "'self'": "SAMEORIGIN"}[ancestors] |