feat(packs): apps install from their own repos — and exo leaves this one by liorrutenberg · Pull Request #51 · ssvlabs/brains-desktop

liorrutenberg · 2026-08-11T23:47:01Z

Apps stop being something this repo carries. A pack now lives in its own git repo, and the user installs it by pasting a URL into Settings → Apps — clone, build, verify, install, gate open, tab in the ＋ menu, without a restart. The acceptance test is exo itself: it left this tree, and it comes back through the front door.

The mechanism

The installer is deterministic code, not a session: clone (the user's own git creds, so private repos work) → validate the convention → run the pack's declared build → verify the artifact → atomic swap into <data-root>/packs/<id>/ → write installed.json (pinned commit + hash of every file) → materialize → open the gate → complete. A headless sanity session runs last, report-only. Installing means trusting the repo's build script; the Apps panel says so in those words.
UI loads at runtime, sharing the host's singletons. The host publishes a narrow facade (svelte/internal/client, a registration surface that refuses reserved ids, the room components a pack composes) on window.__BRAINS_SHARED__; a pack compiles against it with those imports external and emits one self-contained IIFE with its CSS inside. WKWebView silently fails import() of local files, so the webview asks pack_ui_source(id), native re-verifies the file's hash against the record, and the text is injected as a script. No asset-protocol scope, no second Svelte runtime, no restart.
Agents and contexts ride the machinery that already exists: an installed pack's declarations merge as a third manifest layer (shipped > local > installed) and flow through the materializer into its own nested workspace, exactly like a tracked pack did. Scheduler and workspace resolver share one Arc<RwLock<Manifest>>, so a post-install reload reaches both — no run in the wrong cwd.
~/.brains becomes a manifest source, deliberately and narrowly. The old invariant said raw manifests in the data root never load; that still holds. What loads now is a hash-verified installed pack whose file set must exactly equal its record — an unrecorded file, a tampered byte, or a directory whose name disagrees with its record contributes nothing, anywhere.

exo left the tree

src/apps/exo is gone; it lives in its own repo and installs like anything else. With it goes this repo's one personal-data exception — CLAUDE.md and README now say there is none, and no tracked file names a pack. The three-state proof (off / on / not installed) survives as an eval check that builds a neutral pack, installs it through the real pipeline, and asserts the states end to end.

Review

An external codex pass over the scope produced 27 findings: fixed or refuted with proof, and three of the refutations were reopened after my own audit and then fixed for real — gate-before-materialize ordering, install/uninstall serialization, and a component-classification heuristic that happened to work by accident. Two SetupGate copy bugs the owner caught are in here too: a debug build claiming the keychain it never reads, and a failing check whose label read as a verdict.

npm run check · npm test (1660) · cargo test --workspace (856) · every lint · npm run build — all green, by exit code, on this branch, each one re-run by the lead rather than taken on a worker's word.

Rebased onto dev after #49 merged. That branch renamed the vocabulary (pack → app, BRAINS_ENABLE_PACKS → BRAINS_ENABLE_APPS, installed-packs → installed-apps) and its daily hygiene sweep declared itself inside the folder this PR deletes. Both are handled: prose and identifiers here follow dev's names, and the sweep's skill and its 9am declaration moved into the extracted exo repo, so it keeps running as an installed app's agent.

Two notes for review. This scope's own internals still say "pack" — the brains-packs crate, pack_* IPC, docs/packs/, <data-root>/packs/<id>/ — so a mechanical rename to dev's one vocabulary is a good follow-up, deliberately not folded in here. And the last commit is plain cargo fmt over four files that arrived unformatted with #49: cargo fmt --check fails on dev as it stands, and that gate is permanent, so this un-reds it. No semantic change, easy to drop if you'd rather it land separately.

🤖 Generated with Claude Code

Chris-ssvlabs · 2026-08-12T10:39:22Z

+  async function updatePack(pack: PackInfo) {
+    installError = null;
+    try {
+      const jobId = await packInstall(pack.source, pack.gitRef ?? undefined);


🔴 updatePack re-installs via pack_install, but Installer::install_inner returns InstalledCollision whenever packs/<id> already exists — the Update button always fails, after a full clone+build. Add an upgrade path (uninstall-then-swap or skip the collision check for same-id updates).

Chris-ssvlabs · 2026-08-12T10:39:22Z

+    Building,
+    Validating,
+    Installing,
+    Done { pack_id: String },


🔴 InstallPhase::Done { pack_id } serializes as pack_id — enum-level rename_all = "camelCase" renames variants only, not struct-variant fields (verified with serde 1.x). The frontend (client.ts { phase: "done"; packId: string }, AppsPanel job.phase.packId) reads packId → undefined, so the post-install live-load calls loadExternalPack({ id: undefined }) and the no-restart UI load never happens. Add rename_all_fields = "camelCase" or #[serde(rename)] on the field.

Chris-ssvlabs · 2026-08-12T10:39:22Z

+                            brains_context::ReconcileMode::SkillsOnly,
+                            Some(&registry_clone),
+                        ) {
+                            eprintln!("[brains-packs] materialization failed: {e}");


🔴 The install path materializes with the boot-time ContextEngine (context_arc), which is immutable ("loaded once. Nothing mutates after boot") and cannot contain the just-installed pack's context — run_materializer iterates context.manifest().contexts, so the new pack's area is never materialized. The gate then opens and agents arm, but the fail-closed workspace resolver errors for an open-gated unmaterialized area: the pack's agents and context are broken until app restart, contradicting the no-restart install. Rebuild/replace the context engine (load_context_engine with data_root) after install and share it like the agent manifest.

Chris-ssvlabs · 2026-08-12T10:39:22Z

+
+/// Get status of a pack by id.
+#[tauri::command]
+pub fn pack_status(id: String, packs: State<'_, PacksState>) -> CmdResult<PackStatus> {


🟡 pack_status and pack_ui_source join the webview-supplied id into a filesystem path (packs_root.pack_path(&id)) with no is_valid_pack_id check — uninstall validates the id "before ANY path is formed", but these sibling entry points accept traversal ids (../…, absolute paths). Validate the id at every command that forms a path.

Chris-ssvlabs · 2026-08-12T10:39:22Z

+
+                // 3. NOW open the gate + save settings
+                if let Ok(mut settings) = settings_arc.lock() {
+                    settings.set_app_enabled(&pack_id, true);


🟡 The gate opens and settings save unconditionally even when the manifest reload (manifest_result is Err) or run_materializer failed — both are only eprintln'd, so step 3 proceeds and the "#7: gate must not open until the pack's context/skills are ready" ordering is violated on any failure. Fail the job instead of opening the gate.

Chris-ssvlabs · 2026-08-12T10:39:22Z

+                    // Check for duplicate agent id
+                    if manifest.agents.iter().any(|a| a.id == agent.id) {
+                        eprintln!(
+                            "[brains-packs] agent {} already in manifest — shipped/local wins",


🟡 Agent ids are derived from skill folder names (morning, daily, …) and checked globally, so two independent installed packs with the same folder name collide: the later pack's agent is silently dropped with a log that misattributes the winner as "shipped/local wins". Namespace agent ids by pack id (or refuse at install) so one pack cannot shadow another's cron.

Chris-ssvlabs · 2026-08-12T10:39:22Z

    ],
    "security": {
-      "csp": "default-src 'self'; script-src 'self' 'unsafe-inline' https:; style-src 'self' 'unsafe-inline' https:; img-src 'self' data: blob: https:; font-src 'self' data: https:; media-src 'self' asset: http://asset.localhost data: blob: https:; frame-src 'self' data: blob:; connect-src 'self' ipc: http://ipc.localhost https://*.anthropic.com",
+      "csp": "default-src 'self'; script-src 'self' 'unsafe-inline' https: asset: http://asset.localhost; style-src 'self' 'unsafe-inline' https: asset:; img-src 'self' data: blob: https:; font-src 'self' data: https:; media-src 'self' asset: http://asset.localhost data: blob: https:; frame-src 'self' data: blob:; connect-src 'self' ipc: http://ipc.localhost https://*.anthropic.com asset: http://asset.localhost",


🟡 script-src (and connect-src/style-src) gain asset: and http://asset.localhost, but pack UI is delivered via the pack_ui_source IPC and injected inline — the PR itself states no asset-protocol scope is used. With assetProtocol already enabled for $HOME/.brains/previews/*, this leftover widening makes any file placed in that scope loadable as a script. Revert the script-src/connect-src additions.

Chris-ssvlabs · 2026-08-12T10:39:22Z

+        .unwrap();
+
+        // Set env to point at our test manifest
+        std::env::set_var(MANIFEST_ENV, resources.join(MANIFEST_FILE_NAME));


🟡 installed_pack_boot_composition_merges_verified_rejects_tampered sets process-global MANIFEST_ENV while cargo runs tests on parallel threads — concurrent tests calling manifest_candidates (e.g. boot_tests::assembles_engines_over_a_fresh_data_dir) can pick up this test's temp manifest, and a panic before remove_var leaks it for the rest of the run. The test already passes explicit candidates to load_agent_manifest, so drop the env mutation (or serialize with a guard).

Chris-ssvlabs · 2026-08-12T10:39:22Z

+      if (packError) {
+        reject(new Error(`${packError.error}\n${packError.stack ?? ""}`));
+      } else {
+        resolve();


🟡 A pack index.js with a top-level syntax error is reported as success: the injected wrapper's try/catch cannot catch a parse error of its own script block, script.onerror does not fire for inline parse errors, so neither __BRAINS_PACK_LOAD_OK__ nor __BRAINS_PACK_LOAD_ERR__ is written and the promise hits resolve() — the exact silent load failure the #17 comment forbids. Check for the pack id in __BRAINS_PACK_LOAD_OK__ instead of only checking the error list.

Chris-ssvlabs · 2026-08-12T10:39:22Z

+  });
+
+  // Check if render is a Svelte component (has $$ or render method)
+  const isSvelteComponent = $derived(


🟡 isSvelteComponent uses the "$$" in render || "render" in render heuristic that #20 removed from AppShell for being accidental: Svelte 5 function components have neither property, so the Component half of the declared Props union is misclassified and would be invoked as render(container). Either require the marked render shape and drop the Component branch, or reuse isExternalPackRender here.

liorrutenberg · 2026-08-12T12:05:14Z

Thorough review — thank you. Read it all; on my own pass every one of the 14 is legitimate, so all 14 are being fixed rather than argued with. Three of them undercut claims this PR makes, which is the useful kind of catch:

Update was dead (installer refuses an existing target) — so the Update button always failed after a full clone and build. Getting a real upgrade path plus an end-to-end test: install → update → new version live, record's commit changed, data dir untouched.
InstallPhase::Done { pack_id } serializes snake_case — enum-level rename_all renames variants, not struct-variant fields. That means the post-install live load never fired and the "no restart" claim in the description is currently false for the install path; boot-time loading masked it in our own proof. Fixing the attribute and pinning the wire shape in a test.
Materialization runs against the boot ContextEngine, which by construction cannot contain the app just installed — so the gate opens over an unmaterialized area and the fail-closed resolver errors. This is the honest completion of the ordering fix; it was half-done.

The other eleven are all going in too, including the ones that matter beyond this PR: id validation at every path-forming command (not just uninstall), git stderr leaking a user:token@ URL into job state and logs, the timestamp-XOR-pid "uuid", the prelude path that skipped containment on its way to being executed, the leftover asset: CSP widening from the approach we abandoned, and the inline-parse-error path that still resolved as success — the exact silent-failure class we'd already fixed once elsewhere.

Two notes on what you'll see when it's pushed: a vocabulary rename landed first (pack→app throughout, install dir now <data-root>/installed/<id> since apps/<id> is per-app data), so line refs will have moved — each finding was located by content. And a small addition the owner asked for: after the deterministic Rust sanity check, a report-only agentic pass runs a declared skill to confirm the install looks coherent.

Will re-request review once it's all in and the gauntlet is green.

sebastian-ssvlabs

Reviewed-at: 9c861e2

sebastian-ssvlabs · 2026-08-12T12:38:13Z

+};
+
+// Also expose transport for external pack component loading
+window.__BRAINS_TRANSPORT__ = getTransport();


🔴 Critical — installed pack UI runs with zero execution sandbox, not just a "narrow facade" gap.

window.__BRAINS_TRANSPORT__ = getTransport() publishes the raw transport globally. TauriTransport.invoke(cmd, args) (src/layout/core/runtime/transport/tauri.ts) is a thin, unfiltered passthrough to @tauri-apps/api/core's invoke — it can call any registered #[tauri::command] by name, not just pack-facing ones (agents, settings, runs, attachments, browser/browser_engine, recording, guard, etc. — see src-tauri/src/commands/mod.rs).

Combined with external-packs.ts::loadExternalPack, which injects the pack's built index.js via document.createElement('script'); script.textContent = code; document.head.appendChild(script) — i.e. runs the pack's code in the same top-level window/document as the host app, not an iframe, not a separate webview — a malicious (or supply-chain-compromised) pack's UI code has:

Full DOM access to the whole running app (session/chat content, board data, other apps' panes).

Full native IPC access via window.__BRAINS_TRANSPORT__.invoke(...), bypassing the packRegistryFacade/hostPaneFacade/spineFacade/brainsFacade shim entirely — a pack doesn't need those facades to do damage, it can just call any command directly.

capabilities/default.json only grants core:default, core:window:allow-start-dragging, shell:allow-open, notification:default, global-shortcut:default at the window level — Tauri's ACL doesn't scope per-script-origin within a window, and app-defined commands (registered via generate_handler!, not through the plugin/permission system) aren't gated by it at all. The capability file's own doc comment already acknowledges this class of issue for board frames ("a retained board frame runs brains-authored HTML with allow-same-origin and therefore reaches this same IPC") — this PR extends the same unbounded reach to arbitrary third-party pack code, permanently, for the lifetime of the app session.

All of the install-time hardening in path_safety.rs / installer.rs / installed.rs (containment checks, hash verification, symlink rejection) governs where files land on disk and what gets read at boot — none of it constrains what the loaded JS can do once it's running. That's a materially different, and larger, trust boundary than "stays inside its pack directory." Worth treating as a blocker or at minimum an explicit, documented risk acceptance (the PR body's "UI loads at runtime, sharing the host's singletons" undersells this — a shared Svelte runtime is not the same risk class as a shared unrestricted IPC transport).

sebastian-ssvlabs · 2026-08-12T12:38:13Z

+        return Ok(PackStatus::Installing { job });
+    }
+
+    let pack_path = packs.packs_root.pack_path(&id);


🟠 pack_status (here) and pack_ui_source (line 385) build a filesystem path directly from the caller-supplied id via packs.packs_root.pack_path(&id), with no is_valid_pack_id check — unlike Installer::uninstall, which explicitly validates id format before any path is formed (its own comment calls this out as the security invariant). PacksRoot::pack_path is a plain self.root.join(id); a relative id containing .. components produces a PathBuf that resolves outside the packs root when the OS actually touches it (an absolute id replaces the base entirely per Path::join semantics).

Exploitability is limited today because InstalledPack::load_and_verify still needs a valid, hash-matching installed.json at the traversed target to return anything useful, and its error variants (e.g. NotFound(PathBuf)) get surfaced back through CmdError — which is at minimum a path-existence oracle outside the intended sandbox, and becomes more relevant given the pack-runtime IPC exposure flagged above (any installed pack can already call this command with an arbitrary id). Recommend routing both through is_valid_pack_id the same way uninstall does, for defense-in-depth and consistency with the stated invariant.

sebastian-ssvlabs · 2026-08-12T12:38:13Z

    ],
    "security": {
-      "csp": "default-src 'self'; script-src 'self' 'unsafe-inline' https:; style-src 'self' 'unsafe-inline' https:; img-src 'self' data: blob: https:; font-src 'self' data: https:; media-src 'self' asset: http://asset.localhost data: blob: https:; frame-src 'self' data: blob:; connect-src 'self' ipc: http://ipc.localhost https://*.anthropic.com",
+      "csp": "default-src 'self'; script-src 'self' 'unsafe-inline' https: asset: http://asset.localhost; style-src 'self' 'unsafe-inline' https: asset:; img-src 'self' data: blob: https:; font-src 'self' data: https:; media-src 'self' asset: http://asset.localhost data: blob: https:; frame-src 'self' data: blob:; connect-src 'self' ipc: http://ipc.localhost https://*.anthropic.com asset: http://asset.localhost",


🟠 This widens script-src/style-src/connect-src to include asset: and http://asset.localhost (previously excluded from script-src/style-src on purpose — dangerousDisableAssetCspModification for those two directives was already set, implying a deliberate prior choice to keep the asset protocol out of executable/style contexts even though it was enabled for media-src). The PR description states pack UI needs "No asset-protocol scope" and assetProtocol.scope here is unchanged (still just recordings/*-mic.wav and previews/*), so this change doesn't appear to be required by the packs mechanism itself. Widening script-src is the most sensitive of the three to loosen without a stated reason — worth confirming this is intentional/needed and unrelated to packs, or splitting it out if it's incidental.

- brains-packs crate: hash-verified installed.json records, path containment (relative-only, ..-free, symlink-refused), convention validation matching the build scripts, job registry, deterministic clone→validate→build→verify→swap installer targeting a self-contained dist/ - boot: installed packs merge as a third manifest layer (shipped > local > installed); shared Arc<RwLock<Manifest>> between scheduler and workspace resolver so post-install reloads reach both - IPC: pack_install / pack_uninstall / pack_list / pack_status / pack_install_status / packs_reload; install opens the gate, uninstall closes it - manifests.rs invariant rewritten: raw data-root manifests still never load; only hash-verified packs/<id> records contribute, via the merge layer - post-install sanity hook seam (report-only) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- host exposes svelte/internal/client + $core/app-registry as shared singletons (globalThis shims + shared chunks); pack builds with those externalized - WKWebView finding: dynamic import() from asset:// fails silently — packs load via fetch + script-tag injection and must build as IIFE, not ESM - proven live: asset:// load, tab from the ＋ menu, runes counter reactivity, no CSP violations; dev-conf asset scope entry documented for release - docs/packs/SPIKE-VERDICT.md carries mechanism, convention constraints, version-coupling and restart-vs-live findings; hello-pack under scripts/spike/ Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- docs/packs/AUTHORING.md written from the enforcement code (validate.rs, installer.rs): repo shape, build contract, host sharing + version coupling, install lifecycle, hello-pack walk-through - hello-pack upgraded to the canonical convention: resolveId/load virtual-module plugin over window.__BRAINS_SHARED__ (no regex over emitted chunks), IIFE output, CSS injected by the JS — dist/ is index.js + pack.json only Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- Settings → Apps: install from git URL (honest copy about running the repo's build), job-phase progress, installed list with update / uninstall (+ optional data purge); Developer panel keeps the gate toggles - loader productized: pack_ui_source IPC serves index.js only after re-verifying its hash against installed.json — asset-scope approach dropped entirely; packs load at boot and live-add after install, no restart - op-table guard test: every pack command provably registered Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- P0: uninstall id validated + containment-checked before any path forms - verification: installed tree must exactly equal the record (no unrecorded files), record bound to its directory, prelude validated, deny_unknown_fields - installer: true atomic swap (sibling dir + rename, temp+rename record), built pack.json identity must match source, git URL userinfo stripped - activation: materialize before gate-open before job-complete; uninstall refuses while an install job is active; terminal jobs leave active indexes - reload: reuses boot's resource_dir, keeps current manifest on empty/failed rebuild; shims narrowed to a facade that refuses reserved ids; shared-chunk matching pinned to node_modules/svelte; external UI marked explicitly (EXTERNAL_PACK_MARKER) instead of the $$ heuristic - SetupGate: token message is build-aware; blocking line names the domain (brains account) instead of echoing a verdict-shaped label Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ipeline, parity proven - src/apps/exo deleted; exo lives in its own repo and installs from a git URL - CLAUDE.md + README rewritten: this repo tracks NO pack, and the personal-data rule has no exceptions - validator brought back in line with the build scripts (details.agent, .mjs preludes) — the divergence had silently stripped pack features - pack_log IPC: the loader reports every step and surfaces execution errors natively; a silent load failure is itself a bug - e2e installer test: real git repo through clone → validate → swap → record → verify, plus the tamper negative; installed.json round-trips its own types - three-state proof re-anchored on an installed pack instead of a tracked one Proven in the running app: install, UI bundle executes, tab opens and renders the workstation room, three agents armed on their crons, gate off retracts and disarms, gate on restores, uninstall leaves a clean tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Four files landed unformatted with #49; rustfmt's own output, no semantic change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Complete closure of the vocabulary rename across the external apps scope: Crate: brains-packs → brains-apps IPC: pack_* → app_* (list, install, uninstall, status, reload, ui_source, log) Types: InstalledPack → InstalledApp, PacksRoot → AppsRoot, etc. Frontend: external-packs.ts → external-apps.ts, pack-shims → app-shims File format: pack.json → app.json Directory: <data-root>/packs/ → <data-root>/installed/ (apps/ was taken by per-app runtime state) Wire contract unchanged: window.__BRAINS_SHARED__ stays as-is. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Fixes from PR #51's code review: 1. Add update() method for atomic app updates 2. Serde rename appId field in Done variant 3. Rebuild context engine after install 4. Validate app id in all path-forming commands 5. Fail job on manifest/materialization failure 6. Derive occupied set from manifest, not const 7. sanitize_url() made public 8. Atomic job id generation 9. Remove unsafe find_prelude_path 10. Namespace agent ids as {app_id}/{skill_folder} 11. Remove asset:// from CSP 12. Remove env var mutation in parallel test 13. Check positive signal (id in OK list) 14. Use explicit marker instead of $ heuristic Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Base automatically changed from chore/hygiene to dev August 11, 2026 23:57

liorrutenberg force-pushed the packs/external-apps branch from 4f61d28 to 9c861e2 Compare August 12, 2026 08:29

Chris-ssvlabs reviewed Aug 12, 2026

View reviewed changes

sebastian-ssvlabs reviewed Aug 12, 2026

View reviewed changes

liorrutenberg and others added 10 commits August 12, 2026 19:30

chore: the size exception says what the file now measures

0305c82

style: cargo fmt — the gate is permanent, dev arrived red

67758c6

Four files landed unformatted with #49; rustfmt's own output, no semantic change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

liorrutenberg force-pushed the packs/external-apps branch from 9c861e2 to 1ab2fa5 Compare August 12, 2026 16:39

Uh oh!

Conversation

liorrutenberg commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liorrutenberg commented Aug 12, 2026

Uh oh!

sebastian-ssvlabs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

liorrutenberg commented Aug 11, 2026 •

edited

Loading