Skip to content

Security: brain-bbqs/clip-extractor

SECURITY.md

Security notes

clip-extractor is a fully static, backend-free page (see package.json's description) — there is no server to hold a session, set an httpOnly cookie, or otherwise keep a credential out of client-side JavaScript. The OAuth access/refresh tokens used to call the EMBER-DANDI API have to live somewhere the browser's JS can read them back out. That constraint doesn't go away; the sections below are about managing it deliberately instead of by accident.

These notes intentionally mirror bbqs-uploader's SECURITY.md; this app signs in through the same flow against the same archive, so the same trade-offs apply and are accepted the same way. Keep the two in sync when either changes.

The actual risk is XSS, not "clear text storage" by itself

A token sitting in localStorage/sessionStorage is only exploitable remotely if an attacker can first get JavaScript to execute on this origin (XSS) — at which point they could just make authenticated requests directly, credential theft is a bonus, not the primary damage. So before treating a "clear text storage of sensitive information" scanner alert as something to dismiss or work around, actually check whether that precondition holds:

grep -rn "innerHTML\|outerHTML\|insertAdjacentHTML" src/

For every hit, confirm any dynamic (user-supplied, API-returned, or otherwise non-literal) string is assigned via .textContent (or an element.value type property) rather than concatenated into the HTML string itself. A fixed, hardcoded template assigned via innerHTML is fine — the risk is interpolating untrusted data into HTML source, not the property name. As of this writing, both innerHTML uses in src/ are static glyph literals on the play/pause button in main.ts, and every archive-supplied string (dandiset titles and identifiers, usernames) is rendered through .textContent / createElement / replaceChildren. Keep it that way — this is the property that makes accepting client-side token storage a reasonable call for this app.

Also keep an eye on:

  • Third-party code fetched at runtime. Nothing in index.html loads a CDN <script> tag, and everything the app imports is bundled into its own assets or emitted alongside them, with two exceptions:

    • src/lib/ffmpeg.ts loads ffmpeg.wasm's core (@ffmpeg/core@0.12.10, about 30MB of JS and WASM) from cdn.jsdelivr.net when a range is cut from a video file already in the browser, or a pose overlay video is rendered, and runs it in @ffmpeg/ffmpeg's bundled Web Worker. That core writes the very bytes that get uploaded. It stays on the CDN on purpose: it is GPL-2.0-or-later and this app is MIT, so the app must never bundle, copy or serve it from its own origin. Instead, both files are pinned by SHA-256 beside the version in FFMPEG_CORE: the app downloads the bytes, hashes them (fetchPinned in src/lib/pinned.ts), and only turns them into Blob URLs for ffmpeg.wasm once both match. On a mismatch the encode stops with an error and nothing from the download is loaded. A version bump means recomputing both digests from npm pack @ffmpeg/core@<version> (jsDelivr serves the tarball's files unchanged) and moving the exact-pinned @ffmpeg/core devDependency to the same version: the integration specs serve that copy in the CDN's place, and tests/unit/ffmpegInstance.test.ts checks the pins against it.
    • sleap-io.js carries a last-resort fallback that appends an unpkg.com <script> for mp4box@0.5.4 to the page itself, with no integrity hash, which would run on the main thread with full access to storage. It is reached only if the bundled mp4box chunk fails to load while the mp4box video backend (the last fallback in openLocalBackend in src/main.ts) is being tried. It lives inside the library and is left as it is.

    @talmolab/sleap-io.js's loadSlp reads an .slp file in a Web Worker it builds from a Blob, and that worker importScripts h5wasm from cdn.jsdelivr.net/npm/h5wasm@0.10.2 unless it is handed a URL. src/main.ts hands it h5: { h5wasmUrl }, pointing at the app's own copy of h5wasm's self-contained IIFE build, which Vite emits as an asset, so the worker runs nothing from a third party. tests/integration/cdn.spec.ts checks that parsing a .slp requests that asset and nothing from a CDN. That default URL is still in the bundle, as is @ffmpeg/ffmpeg's default unpkg.com core URL, but neither is reached, because the app always passes its own.

    The ffmpeg Worker has no localStorage or sessionStorage, so it cannot read the stored token directly, but it is third-party code running on this origin. A compromised CDN copy is the other realistic way a token in storage gets exfiltrated without a bug in this app's own code: the digest check takes the CDN out of that chain for the ffmpeg core, and the mp4box fallback is the one path still guarded only by the version in its URL. A change to either pinned version, or to the digests, deserves the same review as a new dependency.

  • Minimal runtime dependencies. Every entry under dependencies in package.json, and what it is for:

    • @brain-bbqs/ember-client: the BBQS apps' shared EMBER client, covering OAuth sign-in (its PKCE verifier in sessionStorage), the stored settings (the localStorage sink described below), archive API calls, the admin-owned dandiset check, upload part planning, and the dandi-etag and MD5 hashing. It depends only on @brain-bbqs/utils and spark-md5; this app takes spark-md5 only transitively through it.
    • @brain-bbqs/ui: the BBQS apps' shared page shell: its stylesheet (src/style.css), and the header, footer, drop target and "What's New" behaviour src/main.ts and src/ui/elements.ts import. The "What's New" modal draws this repository's own CHANGELOG.md, bundled at build time, with createElement and text nodes; it never parses markup, and a link gets an href only for an absolute https: URL. Nothing in it uses innerHTML. It depends only on @brain-bbqs/utils.
    • @brain-bbqs/utils: the BBQS apps' shared helpers (byte formatting, interruption handling, a bounded work queue, filename sanitizing, and the localStorage helpers @brain-bbqs/ember-client stores the settings through). No dependencies and no network.
    • @ffmpeg/ffmpeg: the ffmpeg.wasm wrapper, bundled; the core it runs is the pinned CDN fetch above, run in a Worker.
    • @talmolab/sleap-io.js: reads SLEAP .slp and ndx-pose .nwb pose files, and provides the fallback video backends. It fetches a remote .slp itself over range requests (with no credentials of ours), spawns the h5wasm Worker above, and brings in h5wasm, jsfive, mediabunny, mp4box, pako and yaml of its own.
    • h5wasm: HDF5 compiled to WASM, imported lazily in src/lib/nwb.ts to measure an ndx-pose series; it lands in its own lazily loaded chunk, with the WASM embedded rather than fetched, and reads only from its in-memory filesystem. Its IIFE build is also emitted as an asset of its own, for sleap-io.js's worker above to import.
    • hash-wasm: the incremental SHA-256 in src/lib/etag.ts, run on the main thread over each file before upload, and the digest check on the ffmpeg core in src/lib/pinned.ts. Its WASM is embedded in the main bundle; it touches no network or storage.
    • mediabunny: demuxes, decodes and trims video through the browser's WebCodecs (no WASM). It streams archive and URL-named videos over range requests without credentials of ours, and may start small Workers from its own inline code.

    Every added runtime dependency is something that could be compromised upstream and ship code that reads localStorage; don't add one without a reason.

The admin-owned dandiset check calls a third party, but without our token

listIncomingDandisets (from @brain-bbqs/ember-client, called in src/main.ts) calls a companion service (not part of this repo, currently hosted on PythonAnywhere) to check whether a BBQS/EMBER admin co-owns an "Incoming: " dandiset, once per candidate dandiset on every load of the picker.

That call carries no credentials of ours, and it must stay that way. The service reads the dandiset's owner list from the archive with its own API key and intersects it with the admin roster it holds server-side, rather than borrowing the caller's token to do the read. So the request is a bare unauthenticated GET of a dandiset identifier, and the signed-in user's OAuth token still only ever goes to the archive itself.

An earlier version of this check did forward the user's live access token to that host on every picker load, which put a credential capable of acting as the signed-in user on a machine this repo doesn't control. Do not reintroduce that: if the service ever needs to know something it can't resolve with its own credentials, change the service, not the header. The package's own tests/unit/dandisets.test.ts (in bbqs-web-components) pins the absence of the Authorization header on this call.

What the design does concentrate is credential custody on the service side: alongside the roster, that host stores a long-lived archive API key, which is more powerful than any single user's token, and the account behind it needs enough read access to see the owner list of every sanctioned "Incoming: " dandiset (embargoed ones are invisible to non-owners, so in practice that means an archive superuser or an account co-owning them). That's a deliberate trade of many transient user tokens transiting a third party for one stored credential the admins own and can rotate on their own schedule. Before pointing this at a different or newly-deployed instance of that service, confirm it is served over HTTPS, that it does not log its API key, the roster, or the owner lists it reads, and that the key belongs to an account whose only job is this check.

The residual leak is small and non-identifying: because the endpoint is unauthenticated, anyone can ask whether a given dandiset identifier is BBQS-sanctioned. That reveals nothing about who the admins are, grants no access to embargoed content, and is rate-limited service-side.

This is also not a hard access-control boundary even when working correctly: real upload authorization is still enforced entirely by DANDI's own dandiset ownership permissions. The service's adminOwned answer only curates what this app's picker shows; it grants no capability on its own. When the service cannot be reached at all, the check fails closed (the dandiset is not offered) and the picker says so rather than reporting an empty list as "you have not been added to any datasets".

Handling a "clear text storage" alert on a new credential

  1. Run the innerHTML/XSS check above. If it turns up a real injection point, fix that — it's a bigger problem than where the token sits, and no storage choice below fixes it.
  2. If it doesn't, decide how much persistence the credential actually needs, in order of decreasing exposure:
    • localStorage — survives browser restarts. Lowest friction, largest exposure window (persists until explicitly cleared or signed out).
    • sessionStorage — survives reloads, clears on tab close. Meaningfully smaller window than localStorage, but scanners (CodeQL included) generally flag this the same way — expect to still need step 3.
    • In-memory only (a plain module variable, no Storage API) — cleared on any reload/navigation, not just tab close. Removes the flagged sink entirely, at the cost of re-authenticating on every page load.
  3. If you land on localStorage or sessionStorage, dismiss the resulting alert as an accepted, documented trade-off (link this file) rather than trying to "encrypt" the value client-side first — any decryption key reachable by this app's own JS is reachable by an attacker's injected JS too, so client-side encryption of a client-held secret is not a real mitigation, just a false sense of one.

Accepted here: PR #11 added EMBER sign-in and persists the OAuth token set in localStorage under clip-extractor.settings.v1 (settingsStore in src/lib/settings.ts, the shared createArchiveSettingsStore from @brain-bbqs/ember-client bound to that key), so a signed-in session survives a page reload the way it does in bbqs-uploader. The stored shape is unchanged since the move into the shared package, which tests/unit/settings.test.ts pins against a record written by the release before it. That is the same sink bbqs-uploader carries, accepted here for the same reasons: the XSS check above is clean, sessionStorage would be flagged identically, and in-memory-only would mean signing in again on every page load.

Mechanism note: this repo runs CodeQL via GitHub's default setup, and an inline // codeql[js/clear-text-storage-of-sensitive-data] comment does not suppress the alert there (tried in PR #11, commit 100202c, both on the flagged line's preceding line and further above). The write itself now happens inside the shared packages (createArchiveSettingsStore in @brain-bbqs/ember-client, writing through @brain-bbqs/utils), and createArchiveSettingsStore carries the same marker as documentation of intent. If an alert is raised against it, clearing the check still requires dismissing the alert in the repository's Security tab, which needs admin rights. Expect to do that, not to fix it in code.

OAuth token lifecycle

  • Access tokens use django-oauth-toolkit's unconfigured default lifetime (~10 hours on the EMBER archive, per its settings). ensureFreshOAuth() in src/main.ts refreshes the access token automatically (60s before expiry) before each archive call it guards, using the refresh_token — so in practice a signed-in user isn't prompted to re-authenticate every 10 hours, only when the refresh token itself is invalidated, the user explicitly signs out (which revokes it via /oauth/revoke_token/), or stored settings are cleared.
  • Sign-in is Authorization Code + PKCE (createOAuthClient from @brain-bbqs/ember-client, configured with this app's client id and PKCE key in src/lib/oauth.ts) against a public client with no secret; the PKCE verifier lives in sessionStorage under clip-extractor.oauth-pkce.v1 only between the redirect out and the callback back, and the state parameter is checked on return before the code is exchanged.
  • clip-extractor has its own registered OAuth application, separate from bbqs-uploader's, so either tool can be revoked without affecting the other. Redirect URIs are registered per served location (production, and any preview path that needs sign-in); the app computes its own redirect URI at runtime from origin + pathname.

There aren't any published security advisories