From 9bf94c547c1da1922f7be75153b839256d649982 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Sun, 16 Aug 2026 08:35:09 +0700 Subject: [PATCH 1/3] feat(sandbox): create a sandbox from a custom image, and check the name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a sandbox always downloaded its guest image — over a gigabyte, and impossible on a machine that cannot reach the storage host. The Add sandbox dialog now takes an optional image file. Empty means download as before; a path means use that file, read where it sits rather than copied, and persisted so reset re-provisions from the same file. An image the user picked has three ways to be wrong that a create would otherwise discover minutes in, or worse, only when the guest fails to boot: it is not a sandbox image, it is for another OS, or it is for another architecture. inspect.ts streams the tarball to answer that, reading only the first bytes of each member so a 1.3 GB image is judged in milliseconds (measured: 1-127ms across the four published images). Format comes from the members: a kernel plus a disk is the macOS image, a Linux root filesystem is the one Windows and Linux share — which is why those two cannot be distinguished, and accepting one image for both is correct rather than a gap. Architecture comes from the kernel's own magic (arm64 stamps "ARM\x64" at offset 56, x86 stamps "HdrS" at 514 — both start "MZ", so the offsets are what tell them apart), from the build's arch marker in a rootfs, or failing that from the first ELF header in the tree. An image whose architecture cannot be determined is allowed through: refusing a valid image because the probe could not prove its arch is worse than the rare mismatch, which the guest reports loudly on first boot anyway. The main process is the authority — the same check runs on add, so a stale or hand-crafted call cannot get an unusable image past the dialog. The name field is checked while typing too. It becomes the sandbox id, its data directory, and the WSL distribution name, so a duplicate (compared case-insensitively, since macOS and Windows would give both sandboxes one directory), an over-long name, or an unsafe character used to surface only after a create had started and downloaded its image. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 18 ++ DIVERGENCES.md | 2 +- packages/app/src/context/platform.tsx | 3 + packages/app/src/i18n/en.ts | 19 +- packages/app/src/i18n/es.ts | 19 +- packages/app/src/sandbox/dialog-add.tsx | 122 ++++++++++-- packages/app/src/sandbox/name-model.test.ts | 49 +++++ packages/app/src/sandbox/name-model.ts | 31 +++ packages/app/src/sandbox/types.ts | 31 ++- packages/codev-sandbox/src/bwrap/provider.ts | 6 +- packages/codev-sandbox/src/image.test.ts | 42 ++++ packages/codev-sandbox/src/image.ts | 27 ++- packages/codev-sandbox/src/index.ts | 1 + packages/codev-sandbox/src/inspect.test.ts | 159 +++++++++++++++ packages/codev-sandbox/src/inspect.ts | 183 ++++++++++++++++++ packages/codev-sandbox/src/provider.ts | 4 + packages/codev-sandbox/src/vz/provider.ts | 11 +- packages/codev-sandbox/src/wsl/provider.ts | 6 +- .../desktop/src/main/sandbox/controller.ts | 4 + packages/desktop/src/main/sandbox/ipc.ts | 48 ++++- packages/desktop/src/preload/index.ts | 4 +- packages/desktop/src/renderer/index.tsx | 13 ++ 22 files changed, 769 insertions(+), 33 deletions(-) create mode 100644 packages/app/src/sandbox/name-model.test.ts create mode 100644 packages/app/src/sandbox/name-model.ts create mode 100644 packages/codev-sandbox/src/inspect.test.ts create mode 100644 packages/codev-sandbox/src/inspect.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0300f29034..8fd134e978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,24 @@ file is the human-readable summary of what changed and why. ### Desktop / Web UI +- **A sandbox can be created from an image file you already have** — creating + a sandbox always downloaded its environment, which is well over a gigabyte + and impossible on a machine with no route to the internet. **Add sandbox** + now has an optional **Sandbox image** field: leave it empty and the right + image is downloaded as before, or choose a file you already have — one built + in-house, or one a colleague copied to you. The file is checked as soon as + you choose it, and the dialog says plainly what is wrong when it cannot be + used: it is not a sandbox image at all, it is an image for a different + operating system (a macOS image on Windows, say), or it is built for a + different kind of processor (an Intel image on an Apple-silicon Mac). The + file is read where it sits rather than copied, and it is remembered, so + resetting the sandbox rebuilds it from the same file. +- **The sandbox name is checked as you type** — a name that was already taken, + or too long for the environments it names, was only rejected once creation + had started, which on a fresh machine meant waiting out a large download + first. Duplicate names, over-long names, and characters that are unsafe in a + Windows distribution or a folder name are now caught in the dialog, with a + message saying what to change. - **The office skills now work inside a sandbox** — the document skills (Word, Excel, PowerPoint, PDF) were installed into the sandbox image, but several pieces they depend on were not, so they failed once you actually diff --git a/DIVERGENCES.md b/DIVERGENCES.md index a065f83149..9f7bfd9e3a 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -197,7 +197,7 @@ from the squash message first, or use a plain merge. | Dual-path WSL install: `packages/desktop/src/main/wsl/{runtime,servers}.ts` | `runtime.ts`: three internals gain `export` (`runPowerShell`, `runInteractiveCommand`, `resolveSystem32Command`) — no behavior change. `servers.ts`: the `install-wsl` job calls `installWslRuntimeDualPath` and the `install-distro` job calls `installWslDistroDualPath` (both from the fork-only `install-bundle.ts`, which holds all new logic) instead of `installWslRuntimeElevated` / `installWslDistro` directly; both return the same `WslCommandResult` shape, so the surrounding job/error handling is untouched. | The customer fleet sits behind whitelist-only proxies that cannot reach Microsoft's servers. The dual path downloads the pinned bundle (`wsl.zip`: runtime MSI + Ubuntu-24.04 image) from the codev storage host, enables VirtualMachinePlatform via offline dism + installs the MSI behind one UAC prompt, and falls back to upstream's `wsl --install` / `--web-download` paths only when the bundle cannot be *obtained* — a completed-but-failed elevated install is returned as-is, never retried behind a second UAC prompt. | Keep the two call swaps and the three `export` keywords; take upstream's surrounding changes. If upstream reworks `installWslRuntimeElevated` / `installWslDistro`, keep them exported from `runtime.ts` — `install-bundle.ts` calls them as the fallback path. | | Async settings stores: `packages/desktop/src/main/{store,index}.ts`, `packages/desktop/package.json` | `store.ts` no longer uses electron-store: `getStore` now returns the fork-only `store-file.ts` store — in-memory state, flushed write-behind through `store-writer.ts`'s serialized latest-wins async atomic writer (temp → fsync → rename via `fs.promises`, so the fsync runs on the libuv threadpool). Adds `flushStoreWrites(timeoutMs)`; `removeStoreFile`/`removeStoreFileIfEmpty` discard/flush the writer around the delete so a late flush cannot resurrect the file. `index.ts`: `stopSidecars` awaits `flushStoreWrites(1500)`. `package.json`: the `electron-store` dependency is removed. On-disk format is unchanged (flat keys, tab-indented JSON, no extension) — existing files load as-is; a corrupt file now starts empty with a logged error instead of throwing from every accessor. | conf v15's `store.set` is a synchronous fsync on the Electron main thread, and one of those parked the whole app in uninterruptible disk sleep (kernel journal stall + lost ext4 fast-commit wakeup, observed live during a sandbox mount-apply restart): event loop dead, IPC dead, quit impossible. conf also rereads the file on every access, which makes write-behind unsound with the stock class (a set on one key reads a stale file and drops a still-queued update), so the storage layer had to be replaced rather than patched. | Keep the fork store; take upstream's surrounding changes. If upstream adds electron-store options (schema, migrations, dot-notation) or new store call sites, extend `store-file.ts` to match — its API is the superset the desktop actually uses (`get/set/has/delete/clear/store/path`). Do not reintroduce a synchronous write path on the main thread. | | Desktop sandbox wiring: `packages/desktop/src/main/{index,store-keys}.ts`, `packages/desktop/package.json` | Hooks the fork-only `src/main/sandbox/{bundle,controller,ipc}.ts` modules into startup: `index.ts` constructs the sandbox controller beside the WSL servers controller, adds `sandboxes.stopAll()` to `stopSidecars`, registers the `sandbox-*` IPC channels next to the WSL ones, and calls `sandboxes.initialize()` (all platforms) once the local server is ready; `store-keys.ts` gains `SANDBOXES_KEY = "sandboxes"`; `package.json` gains the `@codev/sandbox` and `@codev/gateway` workspace devDependencies (bundled into the main process by electron-vite, so not runtime dependencies; gateway supplies the sandbox egress broker's credentials and config fragment). | The desktop app is where sandboxes are created and run; `createSandboxController` generalizes the proven WSL-servers controller pattern to the per-platform `@codev/sandbox` providers (macOS vz / Windows wsl / Linux bwrap). | Keep the wiring lines and take upstream's surrounding changes; they are additive and order-insensitive apart from `initialize()` needing the local server ready first. Do not rename the `sandbox-*` channels or the `sandboxes` store key — preload/renderer and persisted sandbox configs depend on them. | -| Desktop sandbox renderer slice: `packages/app/package.json`, `packages/app/src/{index.ts,app.tsx}`, `packages/app/src/context/{platform,server}.tsx`, `packages/app/src/components/{settings-v2/servers.tsx,dialog-select-server.tsx}`, `packages/app/src/wsl/settings.tsx`, `packages/app/src/i18n/{en,es}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx` | Surfaces the fork-only sandbox modules (`packages/app/src/sandbox/*`, `packages/desktop/src/renderer/sandbox/*`) in the UI: app `package.json` exports `./sandbox/types` (mirror of `./wsl/types`); `index.ts` re-exports `useSandboxes` + the sandbox types; `app.tsx` mounts `SandboxesProvider` inside `WslServersProvider`; `platform.tsx` gains optional `sandboxes?: SandboxesPlatform`; `server.tsx` adds the third `ServerConnection.Sidecar` variant `{ variant: "sandbox"; name; projectDir?; mounts? }` with key `sandbox:` (`projectDir` is the host folder mounted into the guest and `mounts` the extra host folders with their resolved guest paths — the guest only ever sees its own mount points, so this is the only way the app can name a sandbox's projects after the folders it works on); `settings-v2/servers.tsx` filters sandbox rows out of the generic list and renders `SandboxServerSettings` beside the WSL section; `dialog-select-server.tsx` `handleRemove` routes `sandbox:` keys to `platform.sandboxes.remove` (like `wsl:` keys) and moves the `tabs.removeServer(key)` + `server.remove(key)` pair *above* both awaited platform removals; `wsl/settings.tsx` `AddServerMenu` shows the menu when either wslServers or sandboxes exist and gains an "Add sandbox" item; i18n gains the `sandbox.*` key family (including the optional per-sandbox CPU/memory fields, macOS-only since only the vz backend applies them; `sandbox.add.projectDirHint`, the standing explanation above the folder field; the `sandbox.mounts.*` family for the manage-folders dialog that adds/removes extra mounts on an existing sandbox; `sandbox.job.queued`, shown while a sandbox's create/reset/destroy waits its turn behind another sandbox's job; and `sandbox.server.stop` for the menu entry that shuts a sandbox down until it is explicitly started again); preload exposes `api.sandboxes` over the `sandbox-*` channels and `renderer/index.tsx` wires the platform api, loading gate, ready-connection list, and the `startupServer` fallback (fork-only wrapper that validates `sandbox:` default keys the way `availableStartupServer` validates `wsl:` ones). | Ready sandboxes must appear as first-class servers and be manageable from settings; the slice mirrors the WSL vertical exactly so upstream conflicts resolve the same way in both. Removing a server destroys a disk and takes seconds, and the connection leaves the list the moment it finishes, so the tab cleanup has to happen before the await rather than after it. | Keep every listed edit; take upstream's surrounding changes. If upstream adds a `Sidecar` variant or reworks `AddServerMenu`, re-apply the sandbox variant/menu item on top. The `sandbox:` key format is persisted as the default-server key and as `config.id` in the main-process store — never change one side alone. Keep the tab cleanup ahead of the platform removals in `handleRemove`; if upstream reorders it back, re-apply. | +| Desktop sandbox renderer slice: `packages/app/package.json`, `packages/app/src/{index.ts,app.tsx}`, `packages/app/src/context/{platform,server}.tsx`, `packages/app/src/components/{settings-v2/servers.tsx,dialog-select-server.tsx}`, `packages/app/src/wsl/settings.tsx`, `packages/app/src/i18n/{en,es}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx` | Surfaces the fork-only sandbox modules (`packages/app/src/sandbox/*`, `packages/desktop/src/renderer/sandbox/*`) in the UI: app `package.json` exports `./sandbox/types` (mirror of `./wsl/types`); `index.ts` re-exports `useSandboxes` + the sandbox types; `app.tsx` mounts `SandboxesProvider` inside `WslServersProvider`; `platform.tsx` gains optional `sandboxes?: SandboxesPlatform` and, on the desktop variant, `openFilePathDialog` (a path-only file picker — the existing attachment picker reads the file's bytes, which is unusable for a multi-gigabyte guest image), implemented in `renderer/index.tsx` over the existing `open-file-picker` channel and releasing the read token immediately; `server.tsx` adds the third `ServerConnection.Sidecar` variant `{ variant: "sandbox"; name; projectDir?; mounts? }` with key `sandbox:` (`projectDir` is the host folder mounted into the guest and `mounts` the extra host folders with their resolved guest paths — the guest only ever sees its own mount points, so this is the only way the app can name a sandbox's projects after the folders it works on); `settings-v2/servers.tsx` filters sandbox rows out of the generic list and renders `SandboxServerSettings` beside the WSL section; `dialog-select-server.tsx` `handleRemove` routes `sandbox:` keys to `platform.sandboxes.remove` (like `wsl:` keys) and moves the `tabs.removeServer(key)` + `server.remove(key)` pair *above* both awaited platform removals; `wsl/settings.tsx` `AddServerMenu` shows the menu when either wslServers or sandboxes exist and gains an "Add sandbox" item; i18n gains the `sandbox.*` key family (including the optional per-sandbox CPU/memory fields, macOS-only since only the vz backend applies them; `sandbox.add.projectDirHint`, the standing explanation above the folder field; the `sandbox.mounts.*` family for the manage-folders dialog that adds/removes extra mounts on an existing sandbox; `sandbox.job.queued`, shown while a sandbox's create/reset/destroy waits its turn behind another sandbox's job; `sandbox.server.stop` for the menu entry that shuts a sandbox down until it is explicitly started again; and the `sandbox.add.image*` / `sandbox.add.name*` / `sandbox.arch.*` families for the optional custom-image field and the sandbox-name checks); preload exposes `api.sandboxes` over the `sandbox-*` channels and `renderer/index.tsx` wires the platform api, loading gate, ready-connection list, and the `startupServer` fallback (fork-only wrapper that validates `sandbox:` default keys the way `availableStartupServer` validates `wsl:` ones). | Ready sandboxes must appear as first-class servers and be manageable from settings; the slice mirrors the WSL vertical exactly so upstream conflicts resolve the same way in both. Removing a server destroys a disk and takes seconds, and the connection leaves the list the moment it finishes, so the tab cleanup has to happen before the await rather than after it. | Keep every listed edit; take upstream's surrounding changes. If upstream adds a `Sidecar` variant or reworks `AddServerMenu`, re-apply the sandbox variant/menu item on top. The `sandbox:` key format is persisted as the default-server key and as `config.id` in the main-process store — never change one side alone. Keep the tab cleanup ahead of the platform removals in `handleRemove`; if upstream reorders it back, re-apply. | | Desktop identity: `packages/desktop/src/main/{index,store}.ts`, `packages/desktop/electron-builder.config{,.test}.ts`, `packages/desktop/scripts/copy-metainfo.ts`, `packages/desktop/resources/linux/opencode-desktop.desktop` (deleted), `packages/app/src/pages/layout/{deep-links.ts,helpers.test.ts}`, `packages/codev-brand/test/brand.test.ts` | appId `ai.opencode.desktop{,.beta,.dev}` → `ai.codev.desktop{,.beta,.dev}` (both copies: `APP_IDS` in main and in the builder config, plus the unpackaged fallback); deep-link scheme `opencode://` → `codev://` in all three places that must agree — `protocols.schemes`, `app.setAsDefaultProtocolClient`, and the renderer's `parseUrl` — plus the `argv.startsWith` filter the second-instance handler uses; `rpm.packageName` `opencode`/`opencode-beta`/`opencode-dev` → `codev`/`codev-beta`/`codev-dev`; the hidden `opencode-desktop.desktop` legacy launcher and its `deb`/`rpm` fpm wiring deleted. `src/main/migrate.ts` keeps the old ids — they name the *Tauri* app's directories, not ours. | The appId is the whole desktop identity: `app.setPath("userData", join(appData, appId))`, so it also names the data directory, and it drives the macOS bundle id, the Windows AppUserModelId and NSIS keys, the Linux `.desktop` filename, `executableName` and `StartupWMClass`, and the AppStream metainfo id. Leaving it as opencode meant the fork's app identified itself as upstream to the OS. Renamed as a clean cut with no data migration and no back-compat shim, on the explicit basis that there is no installed base: existing profiles under `appData/ai.opencode.desktop*` are simply orphaned (settings, session history, sandboxes and the downloaded guest image), and a packaged update across the rename would install beside the old app rather than replace it. The legacy launcher went with it — it existed to rescue GNOME/KDE pins from a superseded packaging id, and its `Exec=/opt/OpenCode/...` no longer matched any install layout. | Keep our ids and scheme; take upstream's other config. Never rename the `TAURI_APP_IDS` in `migrate.ts` to match — a blind find-and-replace there points the Tauri migration at a directory that never existed. All three deep-link sites have to move together or links open nothing; `electron-builder.config.test.ts` covers the ids, the package names and the scheme per channel. | diff --git a/packages/app/src/context/platform.tsx b/packages/app/src/context/platform.tsx index cebdbe47c5..23844fe982 100644 --- a/packages/app/src/context/platform.tsx +++ b/packages/app/src/context/platform.tsx @@ -133,6 +133,9 @@ export type Platform = PlatformBase & platform: "desktop" os?: DesktopOS openDirectoryPickerDialog(opts?: OpenDirectoryPickerOptions): Promise + /** Pick one file and get its path back. Unlike the attachment picker + * this never reads the file, which matters for multi-gigabyte ones. */ + openFilePathDialog(opts?: { title?: string; extensions?: string[] }): Promise } ) diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 13cf45d6f8..7e7e8bc78c 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -395,7 +395,24 @@ export const dict = { "sandbox.job.reset": "Resetting...", "sandbox.job.queued": "Waiting for another sandbox...", "sandbox.add.namePlaceholder": "Sandbox name", - "sandbox.add.nameInvalid": "Use letters, numbers, dots, dashes, or underscores.", + "sandbox.add.nameInvalid": "Start with a letter or number, then use letters, numbers, dots, dashes, or underscores.", + "sandbox.add.nameTooLong": "Keep the name to {{max}} characters or fewer.", + "sandbox.add.nameDuplicate": "You already have a sandbox with this name.", + "sandbox.arch.x64": "Intel/AMD (x86_64)", + "sandbox.arch.arm64": "ARM64 (Apple silicon)", + "sandbox.add.image": "Sandbox image", + "sandbox.add.imagePlaceholder": "Sandbox image file (optional)", + "sandbox.add.imageHint": + "Leave this empty and the right image is downloaded for you. Choose a file only if you have one already \u2014 an image built for this computer, or one someone shared with you.", + "sandbox.add.imageClear": "Clear", + "sandbox.add.imageChecking": "Checking the image\u2026", + "sandbox.add.imageOk": "This image can be used on this computer.", + "sandbox.add.imageUnreadable": "That file could not be read. Check it still exists and you have permission to open it.", + "sandbox.add.imageNotArchive": "That is not a sandbox image. A sandbox image is a .tar.gz file.", + "sandbox.add.imageNotSandbox": "That .tar.gz is not a sandbox image \u2014 it does not contain a sandbox environment.", + "sandbox.add.imageWrongOsMac": "That image is for macOS sandboxes. This computer needs the Windows/Linux image.", + "sandbox.add.imageWrongOsOther": "That image is for Windows/Linux sandboxes. This computer needs the macOS image.", + "sandbox.add.imageWrongArch": "That image is built for {{found}} machines, which this computer is not.", "sandbox.add.projectDir": "Project folder", "sandbox.add.projectDirHint": "Choose the folder you want to work on. The sandbox can only reach folders you mount from this computer — you can mount more later — and the agent edits files in them without asking.", diff --git a/packages/app/src/i18n/es.ts b/packages/app/src/i18n/es.ts index 4850bcaa41..7f35c31788 100644 --- a/packages/app/src/i18n/es.ts +++ b/packages/app/src/i18n/es.ts @@ -386,7 +386,24 @@ export const dict = { "sandbox.job.reset": "Restableciendo...", "sandbox.job.queued": "Esperando a otro sandbox...", "sandbox.add.namePlaceholder": "Nombre del sandbox", - "sandbox.add.nameInvalid": "Usa letras, números, puntos, guiones o guiones bajos.", + "sandbox.add.nameInvalid": "Empieza con una letra o un n\u00famero y usa despu\u00e9s letras, n\u00fameros, puntos, guiones o guiones bajos.", + "sandbox.add.nameTooLong": "Usa como m\u00e1ximo {{max}} caracteres.", + "sandbox.add.nameDuplicate": "Ya tienes un sandbox con este nombre.", + "sandbox.arch.x64": "Intel/AMD (x86_64)", + "sandbox.arch.arm64": "ARM64 (Apple silicon)", + "sandbox.add.image": "Imagen del sandbox", + "sandbox.add.imagePlaceholder": "Archivo de imagen del sandbox (opcional)", + "sandbox.add.imageHint": + "D\u00e9jalo vac\u00edo y se descargar\u00e1 la imagen adecuada. Elige un archivo solo si ya tienes uno: una imagen creada para este equipo o una que te hayan compartido.", + "sandbox.add.imageClear": "Quitar", + "sandbox.add.imageChecking": "Comprobando la imagen\u2026", + "sandbox.add.imageOk": "Esta imagen se puede usar en este equipo.", + "sandbox.add.imageUnreadable": "No se pudo leer ese archivo. Comprueba que sigue existiendo y que tienes permiso para abrirlo.", + "sandbox.add.imageNotArchive": "Eso no es una imagen de sandbox. Una imagen de sandbox es un archivo .tar.gz.", + "sandbox.add.imageNotSandbox": "Ese .tar.gz no es una imagen de sandbox: no contiene un entorno de sandbox.", + "sandbox.add.imageWrongOsMac": "Esa imagen es para sandboxes de macOS. Este equipo necesita la imagen de Windows/Linux.", + "sandbox.add.imageWrongOsOther": "Esa imagen es para sandboxes de Windows/Linux. Este equipo necesita la imagen de macOS.", + "sandbox.add.imageWrongArch": "Esa imagen est\u00e1 hecha para equipos {{found}}, y este no lo es.", "sandbox.add.projectDir": "Carpeta del proyecto", "sandbox.add.projectDirHint": "Elige la carpeta en la que quieres trabajar. El sandbox solo puede acceder a las carpetas de este equipo que montes — puedes montar más después — y el agente edita archivos en ellas sin preguntar.", diff --git a/packages/app/src/sandbox/dialog-add.tsx b/packages/app/src/sandbox/dialog-add.tsx index 2f2f503f35..cb7d949083 100644 --- a/packages/app/src/sandbox/dialog-add.tsx +++ b/packages/app/src/sandbox/dialog-add.tsx @@ -9,14 +9,11 @@ import { Show } from "solid-js" import { createStore } from "solid-js/store" import { useLanguage } from "@/context/language" import { usePlatform } from "@/context/platform" -import type { SandboxConfig } from "./types" +import type { SandboxConfig, SandboxImageProblem } from "./types" import { useSandboxes } from "./context" +import { sandboxNameUsable, validateSandboxName } from "./name-model" import { sandboxResourcesValid } from "./resources" -// Distribution and directory names are derived from the sandbox name across -// every backend, so keep it to characters that are safe in all of them. -const NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/ - // onCreated fires with the new sandbox before the dialog closes. Callers that // need to react to a creation must use it rather than watching the sandbox // list: that list arrives over a separate IPC event, so it is still empty for @@ -27,7 +24,20 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => const dialog = useDialog() const sandboxes = useSandboxes() const api = platform.sandboxes! - const [store, setStore] = createStore({ name: "", projectDir: "", cpus: "", memoryGiB: "", adding: false }) + const [store, setStore] = createStore({ + name: "", + projectDir: "", + cpus: "", + memoryGiB: "", + adding: false, + imagePath: "", + imageChecking: false, + imageProblem: null as SandboxImageProblem | null, + // The path the verdict in imageProblem belongs to. A field edited since + // then is unchecked, not approved — without this a typed path would show + // as fine until the create failed on it. + imageCheckedPath: null as string | null, + }) // Only the macOS VM backend applies per-sandbox limits; WSL's are VM-global // and bubblewrap has none, so the fields appear only where they take effect. @@ -51,12 +61,64 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => const value = availability() return value?.supported ? value.warning : null } - const nameInvalid = () => store.name.length > 0 && !NAME_PATTERN.test(store.name) + // Checked against the sandboxes that already exist, so a name collision is + // reported while typing rather than by a create that fails after minutes of + // downloading. + const takenNames = () => (sandboxes.state.data?.sandboxes ?? []).map((item) => item.config.name) + const nameError = () => validateSandboxName(store.name, takenNames()) + const nameMessage = () => { + const error = nameError() + if (!error) return null + if (error.code === "too-long") return language.t("sandbox.add.nameTooLong", { max: error.max }) + if (error.code === "duplicate") return language.t("sandbox.add.nameDuplicate") + return language.t("sandbox.add.nameInvalid") + } + const imageMessage = () => { + const problem = store.imageProblem + if (!problem) return null + if (problem.code === "wrong-arch") + return language.t("sandbox.add.imageWrongArch", { + found: language.t(problem.found === "x64" ? "sandbox.arch.x64" : "sandbox.arch.arm64"), + }) + if (problem.code === "wrong-os") + return language.t(problem.found === "vz" ? "sandbox.add.imageWrongOsMac" : "sandbox.add.imageWrongOsOther") + if (problem.code === "not-gzip") return language.t("sandbox.add.imageNotArchive") + if (problem.code === "unreadable") return language.t("sandbox.add.imageUnreadable") + return language.t("sandbox.add.imageNotSandbox") + } const canCreate = () => availability()?.supported === true && - NAME_PATTERN.test(store.name) && + sandboxNameUsable(store.name, takenNames()) && store.projectDir.length > 0 && - resourcesValid() + resourcesValid() && + // An unchecked image is not an approved one. + (!store.imagePath || (store.imageCheckedPath === store.imagePath && !store.imageProblem)) + + // The main process is the authority on what counts as usable here; the + // dialog only renders its verdict. Runs when a file is picked and when a + // hand-typed path is committed, never per keystroke. + const checkImage = async (path: string) => { + if (!path || store.imageCheckedPath === path) return + setStore({ imageChecking: true, imageProblem: null }) + // A failed round-trip is reported as unreadable rather than as "fine": + // null is the answer for a usable image, so it must not double as an error. + const problem = await api.inspectImage(path).catch((): SandboxImageProblem => ({ code: "unreadable" })) + // The field can move on during the round-trip; a stale verdict must not + // be applied to a path the user has since changed. + if (store.imagePath !== path) return + setStore({ imageChecking: false, imageProblem: problem, imageCheckedPath: path }) + } + + const pickImage = () => { + if (platform.platform !== "desktop") return + void platform + .openFilePathDialog({ title: language.t("sandbox.add.image"), extensions: ["gz", "tgz"] }) + .then((path) => { + if (!path) return + setStore({ imagePath: path, imageProblem: null, imageCheckedPath: null }) + return checkImage(path) + }) + } const pickProjectDir = () => { if (platform.platform !== "desktop") return @@ -85,6 +147,7 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => store.name, store.projectDir, resourcesApply() ? { cpus: cpus(), memoryMiB: memoryMiB() } : undefined, + store.imagePath || undefined, ) if (created) { props.onCreated?.(created) @@ -129,9 +192,7 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => disabled={store.adding} onInput={(event) => setStore("name", event.currentTarget.value)} /> - -

{language.t("sandbox.add.nameInvalid")}

- + {(text) =>

{text()}

} {/* The folder is the sandbox's only reach into this machine and is fixed for its lifetime, so say so before the picker opens rather than leaving it to be discovered. */} @@ -149,6 +210,43 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => {language.t("sandbox.add.choose")} + {/* Optional: most people want the published image downloaded for + them, so the hint leads with that and explains what the field is + for before offering the picker. */} +

{language.t("sandbox.add.imageHint")}

+
+ + setStore({ imagePath: event.currentTarget.value, imageProblem: null, imageChecking: false }) + } + onChange={(event) => void checkImage(event.currentTarget.value)} + /> + + {language.t("sandbox.add.choose")} + + + + setStore({ imagePath: "", imageProblem: null, imageChecking: false, imageCheckedPath: null }) + } + > + {language.t("sandbox.add.imageClear")} + + +
+ +

{language.t("sandbox.add.imageChecking")}

+ + {(text) =>

{text()}

} + +

{language.t("sandbox.add.imageOk")}

+
{ + test("accepts ordinary names", () => { + expect(validateSandboxName("work", [])).toBeNull() + expect(validateSandboxName("codev-hub", [])).toBeNull() + expect(validateSandboxName("Repo_2.0", [])).toBeNull() + }) + + test("says nothing about an empty field", () => { + // Nothing has been typed yet — that is not an error to show anyone. + expect(validateSandboxName("", [])).toBeNull() + expect(sandboxNameUsable("", [])).toBe(false) + }) + + test("rejects characters that are unsafe as a distro or directory name", () => { + expect(validateSandboxName("my sandbox", [])).toEqual({ code: "charset" }) + expect(validateSandboxName("work/tmp", [])).toEqual({ code: "charset" }) + expect(validateSandboxName("hé", [])).toEqual({ code: "charset" }) + }) + + test("rejects a leading dot or dash", () => { + // A leading dot hides the data directory; a leading dash reads as a flag + // to the wsl CLI. + expect(validateSandboxName(".hidden", [])).toEqual({ code: "charset" }) + expect(validateSandboxName("-flag", [])).toEqual({ code: "charset" }) + }) + + test("rejects a name past the length limit", () => { + expect(validateSandboxName("a".repeat(SANDBOX_NAME_MAX), [])).toBeNull() + expect(validateSandboxName("a".repeat(SANDBOX_NAME_MAX + 1), [])).toEqual({ + code: "too-long", + max: SANDBOX_NAME_MAX, + }) + }) + + test("rejects a name already in use, ignoring case", () => { + expect(validateSandboxName("work", ["work"])).toEqual({ code: "duplicate" }) + // macOS and Windows would give both sandboxes the same data directory. + expect(validateSandboxName("Work", ["work"])).toEqual({ code: "duplicate" }) + expect(validateSandboxName("work2", ["work"])).toBeNull() + }) + + test("reports the charset problem before the duplicate one", () => { + // Both are wrong; the message that tells the user what to type wins. + expect(validateSandboxName("bad name", ["bad name"])).toEqual({ code: "charset" }) + }) +}) diff --git a/packages/app/src/sandbox/name-model.ts b/packages/app/src/sandbox/name-model.ts new file mode 100644 index 0000000000..d6c9350043 --- /dev/null +++ b/packages/app/src/sandbox/name-model.ts @@ -0,0 +1,31 @@ +// Pure validation for the sandbox name field. The name is not just a label: it +// becomes the sandbox's id (`sandbox:`), its data directory, and on +// Windows the WSL distribution name — so it has to be checked before a create +// starts, not after one fails minutes into an image download. + +export type SandboxNameError = { code: "charset" } | { code: "too-long"; max: number } | { code: "duplicate" } + +// Distribution and directory names derive from this, and a create that fails +// on a too-long path does so only after the download. 32 is comfortably under +// every limit involved while leaving room for readable names. +export const SANDBOX_NAME_MAX = 32 + +// Must start alphanumeric (a leading dot hides the data directory, a leading +// dash reads as a flag to the wsl CLI) and stay in characters that are safe as +// both a WSL distribution name and a directory name on all three platforms. +const SANDBOX_NAME_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/ + +export function validateSandboxName(name: string, existing: string[]): SandboxNameError | null { + if (!name.length) return null + if (!SANDBOX_NAME_PATTERN.test(name)) return { code: "charset" } + if (name.length > SANDBOX_NAME_MAX) return { code: "too-long", max: SANDBOX_NAME_MAX } + // Compared case-insensitively: macOS and Windows filesystems treat "Work" + // and "work" as one directory, so two such sandboxes would fight over the + // same data directory even though their ids differ. + if (existing.some((taken) => taken.toLowerCase() === name.toLowerCase())) return { code: "duplicate" } + return null +} + +export function sandboxNameUsable(name: string, existing: string[]) { + return name.length > 0 && validateSandboxName(name, existing) === null +} diff --git a/packages/app/src/sandbox/types.ts b/packages/app/src/sandbox/types.ts index 0ed6370be9..b6b3b9110b 100644 --- a/packages/app/src/sandbox/types.ts +++ b/packages/app/src/sandbox/types.ts @@ -5,6 +5,8 @@ export type SandboxBackend = "vz" | "wsl" | "bwrap" +export type SandboxArch = "x64" | "arm64" + export type SandboxConfig = { id: string name: string @@ -13,6 +15,9 @@ export type SandboxConfig = { // Extra host folders shared into the guest alongside projectDir; absent // means none. Objects so a readOnly flag can be added without migration. mounts?: SandboxMount[] + // A guest image file the user supplied instead of the published one; reset + // re-provisions from it, so it is kept for the sandbox's life. + imagePath?: string // Set when the user stopped this sandbox, so it stays down across relaunches // instead of booting with the rest. stopped?: boolean @@ -68,16 +73,34 @@ export type SandboxesState = { export type SandboxesEvent = { type: "state"; state: SandboxesState } -export type SandboxAvailability = - | { supported: true; warning: string | null } - | { supported: false; message: string } +// What a guest image turned out to be, mirroring @codev/sandbox's inspector. +// "vz" is the macOS kernel+disk pair; "rootfs" is the tarball Windows and +// Linux share, so those two cannot be told apart — by design. +export type SandboxImageFormat = "vz" | "rootfs" + +export type SandboxImageProblem = + | { code: "unreadable" } + | { code: "not-gzip" } + | { code: "not-a-sandbox-image" } + | { code: "wrong-os"; found: SandboxImageFormat } + | { code: "wrong-arch"; found: SandboxArch } + +export type SandboxAvailability = { supported: true; warning: string | null } | { supported: false; message: string } export type SandboxesPlatform = { getState(): Promise subscribe(cb: (event: SandboxesEvent) => void): () => void availability(): Promise // Resolves undefined when the create was cancelled mid-download. - add(name: string, projectDir: string, resources?: SandboxResources): Promise + add( + name: string, + projectDir: string, + resources?: SandboxResources, + imagePath?: string, + ): Promise + // Checks a candidate custom image against this machine before it is used. + // Null means it is usable here. + inspectImage(path: string): Promise // Aborts the in-flight create/reset job for this sandbox name, if any. cancel(name: string): Promise remove(id: string): Promise diff --git a/packages/codev-sandbox/src/bwrap/provider.ts b/packages/codev-sandbox/src/bwrap/provider.ts index 19037fea79..676e15b31d 100644 --- a/packages/codev-sandbox/src/bwrap/provider.ts +++ b/packages/codev-sandbox/src/bwrap/provider.ts @@ -9,7 +9,7 @@ import { sandboxBootEnv, } from "../boot" import { createBrokerServer, type SandboxEgress } from "../broker" -import { readImageMarker, stageAsset, writeImageMarker } from "../image" +import { localImageAsset, readImageMarker, stageAsset, writeImageMarker } from "../image" import type { SandboxAsset } from "../manifest" import { resolveGuestMounts } from "../mounts" import type { SandboxAvailability, SandboxConfig, SandboxJobOptions, SandboxProvider } from "../provider" @@ -67,7 +67,9 @@ export function createBwrapProvider(options: BwrapProviderOptions): SandboxProvi const startTimeoutMs = options.startTimeoutMs ?? 30_000 const prepare = async (config: SandboxConfig, opts?: SandboxJobOptions) => { - const asset = await options.resolveImage({ signal: opts?.signal }) + const asset = config.imagePath + ? await localImageAsset(config.imagePath) + : await options.resolveImage({ signal: opts?.signal }) const dirs = sandboxDirs(options.dataDir, config.name) // reset wipes the rootfs (it is the guest's mutable state), so a matching // marker alone is not proof — the rootfs must actually be there. diff --git a/packages/codev-sandbox/src/image.test.ts b/packages/codev-sandbox/src/image.test.ts index 4ef8e97251..2fe89880a0 100644 --- a/packages/codev-sandbox/src/image.test.ts +++ b/packages/codev-sandbox/src/image.test.ts @@ -3,9 +3,11 @@ import { createHash } from "node:crypto" import { mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises" import { tmpdir } from "node:os" import { join } from "node:path" +import { pathToFileURL } from "node:url" import { imageMarkerPath, isAssetStaged, + localImageAsset, partialAssetPath, readImageMarker, stageAsset, @@ -254,3 +256,43 @@ test("image markers round-trip and read as null when unreadable", async () => { await rm(dir, { recursive: true, force: true }) }) + +// A user-supplied image is described the same way a published one is, so the +// providers' marker-and-stage path needs no special case beyond staging. +test("describes a local image file as an asset", async () => { + const dir = await mkdtemp(join(tmpdir(), "codev-local-image-")) + const path = join(dir, "vz-arm64.tar.gz") + await writeFile(path, payload) + const asset = await localImageAsset(path) + expect(asset).toEqual({ url: pathToFileURL(path).href, sha256: payloadSha256, bytes: payload.byteLength }) + await rm(dir, { recursive: true, force: true }) +}) + +test("rejects a local image that is not there", async () => { + expect(localImageAsset(join(tmpdir(), "codev-missing-image.tar.gz"))).rejects.toThrow("does not exist") +}) + +test("stages a local image where it lies instead of copying it into the cache", async () => { + const dir = await mkdtemp(join(tmpdir(), "codev-local-image-")) + const cacheDir = join(dir, "cache") + const path = join(dir, "vz-arm64.tar.gz") + await writeFile(path, payload) + const asset = await localImageAsset(path) + const reported: number[] = [] + expect(await stageAsset(cacheDir, asset, { onProgress: (progress) => reported.push(progress.bytes) })).toBe(path) + // Nothing was downloaded or duplicated, and the dialog still sees a + // completed progress report rather than a stalled bar. + expect(await readdir(cacheDir).catch(() => null)).toBeNull() + expect(reported).toEqual([payload.byteLength]) + await rm(dir, { recursive: true, force: true }) +}) + +test("refuses a local image that has since been deleted", async () => { + const dir = await mkdtemp(join(tmpdir(), "codev-local-image-")) + const path = join(dir, "vz-arm64.tar.gz") + await writeFile(path, payload) + const asset = await localImageAsset(path) + await rm(path) + expect(stageAsset(join(dir, "cache"), asset)).rejects.toThrow("no longer available") + await rm(dir, { recursive: true, force: true }) +}) diff --git a/packages/codev-sandbox/src/image.ts b/packages/codev-sandbox/src/image.ts index 98effbc729..1922121258 100644 --- a/packages/codev-sandbox/src/image.ts +++ b/packages/codev-sandbox/src/image.ts @@ -3,6 +3,7 @@ import { createReadStream, createWriteStream } from "node:fs" import { mkdir, readdir, readFile, rename, rm, stat, writeFile } from "node:fs/promises" import { finished } from "node:stream/promises" import { basename, dirname, join } from "node:path" +import { fileURLToPath, pathToFileURL } from "node:url" import type { SandboxAsset } from "./manifest" import type { SandboxProgress } from "./provider" @@ -32,7 +33,29 @@ export async function isAssetStaged(cacheDir: string, asset: SandboxAsset) { return !!file && file.isFile() && file.size === asset.bytes } +// Describes a guest image the user supplied from disk instead of the manifest. +// Same shape as a published asset so the providers' marker-and-stage path is +// unchanged, and the digest means a reset still re-provisions when the file +// behind it has changed. +export async function localImageAsset(path: string): Promise { + const file = await stat(path).catch(() => null) + if (!file?.isFile()) throw new Error(`sandbox image file does not exist: ${path}`) + const hash = createHash("sha256") + for await (const chunk of createReadStream(path)) hash.update(chunk as Buffer) + return { url: pathToFileURL(path).href, sha256: hash.digest("hex"), bytes: file.size } +} + export async function stageAsset(cacheDir: string, asset: SandboxAsset, opts?: StageAssetOptions): Promise { + // A custom image is used where it lies: there is nothing to download, and + // copying gigabytes into the cache would only duplicate what the user + // already has on disk. + if (asset.url.startsWith("file:")) { + const path = fileURLToPath(asset.url) + const file = await stat(path).catch(() => null) + if (!file?.isFile()) throw new Error(`custom sandbox image is no longer available: ${path}`) + opts?.onProgress?.({ bytes: asset.bytes, totalBytes: asset.bytes }) + return path + } if (await isAssetStaged(cacheDir, asset)) return stagedAssetPath(cacheDir, asset) await mkdir(cacheDir, { recursive: true }) await removeStalePartials(cacheDir, asset) @@ -165,9 +188,7 @@ async function verifyPartial(partial: string, asset: SandboxAsset) { const digest = hash.digest("hex") if (digest === asset.sha256) return await rm(partial, { force: true }) - throw new Error( - `sandbox asset failed verification for ${asset.url}: expected sha256 ${asset.sha256}, got ${digest}`, - ) + throw new Error(`sandbox asset failed verification for ${asset.url}: expected sha256 ${asset.sha256}, got ${digest}`) } async function partialSize(partial: string) { diff --git a/packages/codev-sandbox/src/index.ts b/packages/codev-sandbox/src/index.ts index fae08b2fc4..7c10a724db 100644 --- a/packages/codev-sandbox/src/index.ts +++ b/packages/codev-sandbox/src/index.ts @@ -15,3 +15,4 @@ export * from "./vz/protocol" export * from "./vz/provider" export * from "./wsl/cli" export * from "./wsl/provider" +export * from "./inspect" diff --git a/packages/codev-sandbox/src/inspect.test.ts b/packages/codev-sandbox/src/inspect.test.ts new file mode 100644 index 0000000000..3f765a8e06 --- /dev/null +++ b/packages/codev-sandbox/src/inspect.test.ts @@ -0,0 +1,159 @@ +import { expect, test } from "bun:test" +import { mkdtemp, writeFile } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { gzipSync } from "node:zlib" +import { inspectSandboxImage, validateSandboxImage } from "./inspect" + +// Minimal ustar writer: the inspector reads names, sizes and the first bytes +// of each member, so the fixtures only need to be structurally real. +function tarHeader(name: string, size: number) { + const header = Buffer.alloc(512) + header.write(name, 0, 100, "utf8") + header.write("000644 \0", 100, 8) + header.write("000000 \0", 108, 8) + header.write("000000 \0", 116, 8) + header.write(`${size.toString(8).padStart(11, "0")}\0`, 124, 12) + header.write("00000000000 ", 136, 12) + header.write(" ", 148, 8) + header.write("0", 156, 1) + header.write("ustar\x0000", 257, 8) + let sum = 0 + for (const byte of header) sum += byte + header.write(`${sum.toString(8).padStart(6, "0")}\0 `, 148, 8) + return header +} + +function tar(entries: { name: string; data?: Buffer }[]) { + const blocks: Buffer[] = [] + for (const entry of entries) { + const data = entry.data ?? Buffer.alloc(0) + blocks.push(tarHeader(entry.name, data.length), data) + const padding = (512 - (data.length % 512)) % 512 + if (padding) blocks.push(Buffer.alloc(padding)) + } + blocks.push(Buffer.alloc(1024)) + return Buffer.concat(blocks) +} + +async function fixture(name: string, contents: Buffer) { + const dir = await mkdtemp(join(tmpdir(), "codev-inspect-")) + const path = join(dir, name) + await writeFile(path, contents) + return path +} + +// Both kernels are EFI-stub PE images opening with "MZ"; the arch magic sits +// further in, which is exactly what kernelArch keys on. +function arm64Kernel() { + const kernel = Buffer.alloc(2048) + kernel.write("MZ", 0, "latin1") + kernel.write("ARM\x64", 56, "latin1") + return kernel +} + +function x64Kernel() { + const kernel = Buffer.alloc(2048) + kernel.write("MZ", 0, "latin1") + kernel.write("HdrS", 514, "latin1") + return kernel +} + +function elf(machine: number) { + const binary = Buffer.alloc(64) + binary.writeUInt32BE(0x7f454c46, 0) + binary.writeUInt16LE(machine, 18) + return binary +} + +test("recognizes a vz image and its architecture", async () => { + const arm = await fixture("vz.tar.gz", gzipSync(tar([{ name: "vmlinux", data: arm64Kernel() }, { name: "rootfs.img" }]))) + expect(await inspectSandboxImage(arm)).toEqual({ kind: "image", format: "vz", arch: "arm64" }) + const intel = await fixture("vz.tar.gz", gzipSync(tar([{ name: "vmlinux", data: x64Kernel() }, { name: "rootfs.img" }]))) + expect(await inspectSandboxImage(intel)).toEqual({ kind: "image", format: "vz", arch: "x64" }) +}) + +test("reads a rootfs image's architecture from the build's version marker", async () => { + const path = await fixture( + "bwrap.tar.gz", + gzipSync( + tar([ + { name: "etc/" }, + { name: "etc/hostname", data: Buffer.from("guest\n") }, + { name: "etc/codev-office-versions", data: Buffer.from("dotnet=8.0.423\npandoc=3.10.1\narch=amd64\n") }, + ]), + ), + ) + expect(await inspectSandboxImage(path)).toEqual({ kind: "image", format: "rootfs", arch: "x64" }) +}) + +// An image built without the office layer has no marker file, so the arch has +// to come from the first ELF in the tree. +test("falls back to an ELF header when the marker file is absent", async () => { + const path = await fixture( + "bwrap.tar.gz", + gzipSync(tar([{ name: "etc/" }, { name: "usr/bin/node", data: elf(0xb7) }])), + ) + expect(await inspectSandboxImage(path)).toEqual({ kind: "image", format: "rootfs", arch: "arm64" }) +}) + +test("reports an archive that is not a sandbox image", async () => { + const path = await fixture( + "holiday.tar.gz", + gzipSync(tar([{ name: "photos/" }, { name: "photos/beach.jpg", data: Buffer.from("\xff\xd8\xff\xe0", "latin1") }])), + ) + expect(await inspectSandboxImage(path)).toEqual({ kind: "not-a-sandbox-image" }) +}) + +test("reports a file that is not a gzip archive at all", async () => { + const path = await fixture("screenshot.png", Buffer.from("\x89PNG\r\n\x1a\n", "latin1")) + expect(await inspectSandboxImage(path)).toEqual({ kind: "not-gzip" }) +}) + +test("reports a missing or empty file", async () => { + expect(await inspectSandboxImage(join(tmpdir(), "codev-does-not-exist.tar.gz"))).toEqual({ kind: "unreadable" }) + expect(await inspectSandboxImage(await fixture("empty.tar.gz", Buffer.alloc(0)))).toEqual({ kind: "unreadable" }) +}) + +test("accepts an image that matches the machine", () => { + const inspection = { kind: "image", format: "vz", arch: "arm64" } as const + expect(validateSandboxImage(inspection, { backend: "vz", arch: "arm64" })).toBeNull() +}) + +test("rejects an image built for the other architecture", () => { + const inspection = { kind: "image", format: "vz", arch: "x64" } as const + expect(validateSandboxImage(inspection, { backend: "vz", arch: "arm64" })).toEqual({ + code: "wrong-arch", + found: "x64", + }) +}) + +test("rejects an image built for a different operating system", () => { + // A Windows/Linux rootfs tarball offered to the macOS backend... + expect(validateSandboxImage({ kind: "image", format: "rootfs", arch: "arm64" }, { backend: "vz", arch: "arm64" })).toEqual( + { code: "wrong-os", found: "rootfs" }, + ) + // ...and a macOS image offered to Windows. + expect(validateSandboxImage({ kind: "image", format: "vz", arch: "x64" }, { backend: "wsl", arch: "x64" })).toEqual({ + code: "wrong-os", + found: "vz", + }) +}) + +// Windows and Linux consume the same rootfs tarball, so neither can be told +// from the other — accepting it for both is correct, not a gap. +test("accepts one rootfs image for both Windows and Linux", () => { + const inspection = { kind: "image", format: "rootfs", arch: "x64" } as const + expect(validateSandboxImage(inspection, { backend: "wsl", arch: "x64" })).toBeNull() + expect(validateSandboxImage(inspection, { backend: "bwrap", arch: "x64" })).toBeNull() +}) + +test("allows an image whose architecture could not be determined", () => { + const inspection = { kind: "image", format: "rootfs", arch: null } as const + expect(validateSandboxImage(inspection, { backend: "bwrap", arch: "arm64" })).toBeNull() +}) + +test("passes the unusable-file reasons straight through", () => { + expect(validateSandboxImage({ kind: "not-gzip" }, { backend: "vz", arch: "arm64" })).toEqual({ code: "not-gzip" }) + expect(validateSandboxImage({ kind: "unreadable" }, { backend: "vz", arch: "arm64" })).toEqual({ code: "unreadable" }) +}) diff --git a/packages/codev-sandbox/src/inspect.ts b/packages/codev-sandbox/src/inspect.ts new file mode 100644 index 0000000000..17c3b715df --- /dev/null +++ b/packages/codev-sandbox/src/inspect.ts @@ -0,0 +1,183 @@ +import { createReadStream } from "node:fs" +import { open, stat } from "node:fs/promises" +import { createGunzip } from "node:zlib" +import type { SandboxArch, SandboxBackend } from "./provider" + +// A guest image is one of two shapes. "vz" is the macOS pair (a kernel plus an +// ext4 disk); "rootfs" is the Debian root filesystem tarball that bubblewrap +// and `wsl --import` both consume — the same file serves Windows and Linux, so +// those two are indistinguishable by design. +export type SandboxImageFormat = "vz" | "rootfs" + +export type SandboxImageInspection = + | { kind: "image"; format: SandboxImageFormat; arch: SandboxArch | null } + // Deliberately separate reasons: "this is not a sandbox image" and "this is + // not even an archive" call for different advice to the user. + | { kind: "unreadable" } + | { kind: "not-gzip" } + | { kind: "not-a-sandbox-image" } + +export function imageFormatForBackend(backend: SandboxBackend): SandboxImageFormat { + return backend === "vz" ? "vz" : "rootfs" +} + +// Only the first bytes of any member are read, so this stays cheap on a +// multi-gigabyte tarball: the vz kernel is the first member, and a rootfs +// carries its arch marker among the early etc/ entries. The budget bounds the +// pathological case (an archive with neither) rather than the normal one. +const SCAN_BUDGET_BYTES = 256 * 1024 * 1024 +const TAR_BLOCK = 512 +// Enough to cover the x86 kernel's "HdrS" at offset 514. +const PEEK_BYTES = 1024 + +export async function inspectSandboxImage(path: string): Promise { + const file = await stat(path).catch(() => null) + if (!file?.isFile() || file.size === 0) return { kind: "unreadable" } + const handle = await open(path, "r").catch(() => null) + if (!handle) return { kind: "unreadable" } + const magic = Buffer.alloc(2) + await handle.read(magic, 0, 2, 0) + await handle.close() + // gzip: 1f 8b. Checked up front so a .png or a plain tar fails fast with a + // reason we can explain, instead of surfacing as a zlib stream error. + if (magic[0] !== 0x1f || magic[1] !== 0x8b) return { kind: "not-gzip" } + + const source = createReadStream(path).pipe(createGunzip()) + let format: SandboxImageFormat | null = null + let arch: SandboxArch | null = null + try { + for await (const entry of tarEntries(source, SCAN_BUDGET_BYTES)) { + if (entry.name === "vmlinux" || entry.name === "rootfs.img" || entry.name === "initrd") { + format = "vz" + if (entry.name === "vmlinux") arch = kernelArch(entry.peek) ?? arch + } else if (looksLikeRootfs(entry.name)) { + format ??= "rootfs" + } + // Written by the image build; the cheapest and most direct answer. + if (entry.name === "etc/codev-office-versions") arch = versionsArch(entry.peek) ?? arch + // Any ELF in the tree answers it too, for an image built without the + // office layer (and so without the marker file above). + if (!arch) arch = elfArch(entry.peek) ?? arch + if (format && arch) break + } + } catch { + // A truncated or corrupt stream mid-scan: whatever was learned before the + // failure is not trustworthy enough to accept. + return { kind: "not-a-sandbox-image" } + } finally { + source.destroy() + } + if (!format) return { kind: "not-a-sandbox-image" } + return { kind: "image", format, arch } +} + +export type SandboxImageProblem = + | { code: "unreadable" } + | { code: "not-gzip" } + | { code: "not-a-sandbox-image" } + | { code: "wrong-os"; found: SandboxImageFormat } + | { code: "wrong-arch"; found: SandboxArch } + +// Null means usable. An image whose architecture could not be determined is +// allowed through with no problem reported: refusing a valid image because the +// probe could not prove its arch would be worse than the rare mismatch that +// slips past, and the guest fails loudly on first boot either way. +export function validateSandboxImage( + inspection: SandboxImageInspection, + expected: { backend: SandboxBackend; arch: SandboxArch }, +): SandboxImageProblem | null { + if (inspection.kind !== "image") return { code: inspection.kind } + if (inspection.format !== imageFormatForBackend(expected.backend)) + return { code: "wrong-os", found: inspection.format } + if (inspection.arch && inspection.arch !== expected.arch) return { code: "wrong-arch", found: inspection.arch } + return null +} + +type TarEntry = { name: string; peek: Buffer } + +async function* tarEntries(source: AsyncIterable, budget: number): AsyncGenerator { + const iterator = source[Symbol.asyncIterator]() + let buffer: Buffer = Buffer.alloc(0) + let read = 0 + const pull = async () => { + const next = await iterator.next() + if (next.done) return false + buffer = buffer.length ? Buffer.concat([buffer, next.value]) : next.value + read += next.value.length + return true + } + const ensure = async (bytes: number) => { + while (buffer.length < bytes) if (!(await pull())) return false + return true + } + const take = (bytes: number) => { + const out = buffer.subarray(0, bytes) + buffer = buffer.subarray(bytes) + return out + } + while (read < budget) { + if (!(await ensure(TAR_BLOCK))) return + const header = take(TAR_BLOCK) + // Two zero blocks terminate the archive; one is enough to stop scanning. + if (header[0] === 0) return + const name = cstring(header.subarray(0, 100)) + const size = Number.parseInt(cstring(header.subarray(124, 136)).trim() || "0", 8) + if (!Number.isFinite(size) || size < 0) return + const padded = Math.ceil(size / TAR_BLOCK) * TAR_BLOCK + const peekLength = Math.min(size, PEEK_BYTES) + let peek = Buffer.alloc(0) + if (peekLength > 0) { + if (!(await ensure(peekLength))) return + peek = Buffer.from(take(peekLength)) + } + let skip = padded - peekLength + while (skip > 0) { + if (buffer.length === 0 && !(await pull())) return + skip -= take(Math.min(skip, buffer.length)).length + } + yield { name, peek } + } +} + +function cstring(buffer: Buffer) { + const end = buffer.indexOf(0) + return buffer.subarray(0, end === -1 ? buffer.length : end).toString("utf8") +} + +// The top-level directories a Linux root filesystem always has. Matched on the +// tar's own entry names, which carry no leading slash. +const ROOTFS_ENTRIES = new Set(["etc", "usr", "bin", "sbin", "var", "opt", "lib"]) + +function looksLikeRootfs(name: string) { + const top = name.replace(/\/+$/, "").split("/")[0] ?? "" + return ROOTFS_ENTRIES.has(top) +} + +// Both architectures' kernels are EFI-stub PE images starting "MZ", so the +// magic further in is what actually distinguishes them: arm64 stamps "ARM\x64" +// at offset 56, x86 stamps "HdrS" at 514 (verified against both built kernels). +function kernelArch(peek: Buffer): SandboxArch | null { + if (peek.length >= 60 && peek.subarray(56, 60).toString("latin1") === "ARM\x64") return "arm64" + if (peek.length >= 518 && peek.subarray(514, 518).toString("latin1") === "HdrS") return "x64" + return null +} + +function versionsArch(peek: Buffer): SandboxArch | null { + const match = /^arch=(\S+)$/m.exec(peek.toString("utf8")) + if (!match) return null + // The file records the Debian architecture the image was built for. + if (match[1] === "amd64") return "x64" + if (match[1] === "arm64") return "arm64" + return null +} + +const ELF_MACHINE_X86_64 = 0x3e +const ELF_MACHINE_AARCH64 = 0xb7 + +function elfArch(peek: Buffer): SandboxArch | null { + if (peek.length < 20 || peek.readUInt32BE(0) !== 0x7f454c46) return null + const machine = peek.readUInt16LE(18) + if (machine === ELF_MACHINE_X86_64) return "x64" + if (machine === ELF_MACHINE_AARCH64) return "arm64" + return null +} diff --git a/packages/codev-sandbox/src/provider.ts b/packages/codev-sandbox/src/provider.ts index ac13379dc8..993567eb2a 100644 --- a/packages/codev-sandbox/src/provider.ts +++ b/packages/codev-sandbox/src/provider.ts @@ -16,6 +16,10 @@ export type SandboxConfig = { // instead of booting with the rest. Absent means start it, which is what // every sandbox persisted before this existed wants. stopped?: boolean + // A guest image file the user supplied instead of the published one. Kept + // for the sandbox's life because reset re-provisions from it; absent means + // download the manifest's image, which is the normal path. + imagePath?: string // Per-sandbox limits, applied by the vz backend at boot. Omitted means the // provider default. WSL and bubblewrap ignore them: WSL's limits live in // the user's VM-global .wslconfig, and bubblewrap has no cgroup plumbing diff --git a/packages/codev-sandbox/src/vz/provider.ts b/packages/codev-sandbox/src/vz/provider.ts index f6c18be052..33f34c8a4f 100644 --- a/packages/codev-sandbox/src/vz/provider.ts +++ b/packages/codev-sandbox/src/vz/provider.ts @@ -5,7 +5,7 @@ import { checkSandboxHealth, generateSandboxCredentials, pollSandboxHealth, sand import { createBrokerServer, type SandboxEgress } from "../broker" import { forwardTcpToUnix } from "../bwrap/bridge" import { runCommand, unpackRootfs } from "../bwrap/rootfs" -import { readImageMarker, stageAsset, writeImageMarker } from "../image" +import { localImageAsset, readImageMarker, stageAsset, writeImageMarker } from "../image" import type { SandboxAsset } from "../manifest" import { resolveGuestMounts } from "../mounts" import { macosSandboxAvailability } from "../policy" @@ -95,8 +95,13 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { await copyFile(join(dirs.image, VZ_DISK_FILE), dirs.disk, constants.COPYFILE_FICLONE) } + // A custom image is used from where it lies; otherwise the published one + // is resolved from the manifest and downloaded. + const resolveAsset = (config: SandboxConfig, opts?: SandboxJobOptions) => + config.imagePath ? localImageAsset(config.imagePath) : options.resolveImage({ signal: opts?.signal }) + const prepare = async (config: SandboxConfig, opts?: SandboxJobOptions) => { - await provision(config, await options.resolveImage({ signal: opts?.signal }), opts) + await provision(config, await resolveAsset(config, opts), opts) } return { @@ -298,7 +303,7 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { // pristine disk; when it has moved on, reset doubles as the upgrade path // and re-provisions from the current image. await rm(dirs.disk, { force: true }) - await provision(config, await options.resolveImage(opts), opts) + await provision(config, await resolveAsset(config, opts), opts) }, } } diff --git a/packages/codev-sandbox/src/wsl/provider.ts b/packages/codev-sandbox/src/wsl/provider.ts index 226c8bcd0a..6bedbcb191 100644 --- a/packages/codev-sandbox/src/wsl/provider.ts +++ b/packages/codev-sandbox/src/wsl/provider.ts @@ -10,7 +10,7 @@ import { sandboxBootEnv, } from "../boot" import { createBrokerServer, type SandboxEgress } from "../broker" -import { readImageMarker, stageAsset, writeImageMarker } from "../image" +import { localImageAsset, readImageMarker, stageAsset, writeImageMarker } from "../image" import { wslFirewallScript } from "./firewall" import type { SandboxAsset } from "../manifest" import { windowsBuildNumber, windowsSandboxAvailability } from "../policy" @@ -109,7 +109,9 @@ export function createWslProvider(options: WslProviderOptions): SandboxProvider const prepare = async (config: SandboxConfig, opts?: SandboxJobOptions) => { const distro = wslDistroName(config.name) - const asset = await options.resolveImage({ signal: opts?.signal }) + const asset = config.imagePath + ? await localImageAsset(config.imagePath) + : await options.resolveImage({ signal: opts?.signal }) const dirs = wslSandboxDirs(options.dataDir, config.name) if (await registered(distro)) { // Registered on the current image: nothing to do. A stale (or unknown) diff --git a/packages/desktop/src/main/sandbox/controller.ts b/packages/desktop/src/main/sandbox/controller.ts index 3f2ba5c19d..db7991d874 100644 --- a/packages/desktop/src/main/sandbox/controller.ts +++ b/packages/desktop/src/main/sandbox/controller.ts @@ -39,6 +39,8 @@ export function createDesktopSandboxController(logger: ControllerLogger) { // The backend every new sandbox on this machine gets; null means the // platform has no provider and add must be rejected up front. backend: provider?.backend ?? null, + // What a custom image has to match to be usable here. + arch: sandboxArch() ?? null, availability: (): Promise => { if (!provider) return Promise.resolve({ supported: false, message: `sandboxes are not supported on ${process.platform}` }) @@ -125,6 +127,8 @@ function normalizePersistedSandbox(value: unknown): SandboxConfig[] { ...(mounts.length ? { mounts } : {}), // Only an explicit true keeps a sandbox down; anything else starts it. ...(record.stopped === true ? { stopped: true } : {}), + // Kept for the sandbox's life: reset re-provisions from the same file. + ...(typeof record.imagePath === "string" && record.imagePath.length ? { imagePath: record.imagePath } : {}), ...(sandboxResourcesValid(resources) ? resources : {}), }, ] diff --git a/packages/desktop/src/main/sandbox/ipc.ts b/packages/desktop/src/main/sandbox/ipc.ts index 59418eba18..11e24c06c9 100644 --- a/packages/desktop/src/main/sandbox/ipc.ts +++ b/packages/desktop/src/main/sandbox/ipc.ts @@ -1,8 +1,14 @@ import { stat } from "node:fs/promises" import { app, ipcMain } from "electron" import type { IpcMainInvokeEvent } from "electron" -import { sandboxResourcesValid, sandboxServerId, validateMountPaths } from "@codev/sandbox" -import type { MountValidationError } from "@codev/sandbox" +import { + inspectSandboxImage, + sandboxResourcesValid, + sandboxServerId, + validateMountPaths, + validateSandboxImage, +} from "@codev/sandbox" +import type { MountValidationError, SandboxImageInspection } from "@codev/sandbox" import type { DesktopSandboxController } from "./controller" // Unlike the WSL channels there is no unavailable-stub variant: the controller @@ -40,17 +46,32 @@ export function registerSandboxIpcHandlers(controller: DesktopSandboxController) ipcMain.handle("sandbox-unsubscribe", (event) => unsubscribe(event.sender.id)) ipcMain.handle("sandbox-get-state", () => controller.getState()) ipcMain.handle("sandbox-availability", () => controller.availability()) + // Inspects a candidate image so the dialog can report the problem while the + // user is still filling the form. Returns the same problem shape the add + // path enforces, so the two can never disagree about what is acceptable. + ipcMain.handle("sandbox-inspect-image", async (_event: IpcMainInvokeEvent, path: unknown) => + sandboxImageProblem(controller, await inspectSandboxImage(requireSandboxIpcString("image path", path))), + ) ipcMain.handle( "sandbox-add", - (_event: IpcMainInvokeEvent, name: unknown, projectDir: unknown, resources: unknown) => { + async (_event: IpcMainInvokeEvent, name: unknown, projectDir: unknown, resources: unknown, image: unknown) => { const backend = controller.backend if (!backend) throw new Error(`sandboxes are not supported on ${process.platform}`) const sandboxName = requireSandboxIpcString("sandbox name", name) + // The dialog checks the same things for inline feedback; this is the + // authoritative gate, so a stale or hand-crafted call cannot get an + // unusable image past it. + const imagePath = image === undefined || image === null ? undefined : requireSandboxIpcString("image path", image) + if (imagePath) { + const problem = sandboxImageProblem(controller, await inspectSandboxImage(imagePath)) + if (problem) throw new Error(imageProblemMessage(problem)) + } return controller.addSandbox({ id: sandboxServerId(sandboxName), name: sandboxName, backend, projectDir: requireSandboxIpcString("project directory", projectDir), + ...(imagePath ? { imagePath } : {}), ...requireSandboxResources(resources), }) }, @@ -89,6 +110,27 @@ export function registerSandboxIpcHandlers(controller: DesktopSandboxController) }) } +// Neither can be missing in practice — a platform with no provider rejects add +// before this runs — but the validator needs both, so an absent one is treated +// as "cannot vouch for this image" rather than silently passing it. +function sandboxImageProblem(controller: DesktopSandboxController, inspection: SandboxImageInspection) { + if (!controller.backend || !controller.arch) return { code: "not-a-sandbox-image" } as const + return validateSandboxImage(inspection, { backend: controller.backend, arch: controller.arch }) +} + +// The renderer renders these from its own i18n copy; this text is the fallback +// for a direct IPC call and for the error a failed add surfaces. +function imageProblemMessage(problem: NonNullable>) { + if (problem.code === "unreadable") return "the image file could not be read" + if (problem.code === "not-gzip") return "that file is not a sandbox image (expected a .tar.gz archive)" + if (problem.code === "not-a-sandbox-image") return "that archive is not a sandbox image" + if (problem.code === "wrong-os") + return problem.found === "vz" + ? "that is a macOS sandbox image; this computer needs the Windows/Linux image" + : "that is a Windows/Linux sandbox image; this computer needs the macOS image" + return `that image is built for ${problem.found === "x64" ? "Intel/x86_64" : "ARM64"} machines` +} + const SANDBOX_MOUNTS_MAX = 16 function requireSandboxMounts(value: unknown) { diff --git a/packages/desktop/src/preload/index.ts b/packages/desktop/src/preload/index.ts index b66a48344b..95bed50744 100644 --- a/packages/desktop/src/preload/index.ts +++ b/packages/desktop/src/preload/index.ts @@ -48,7 +48,9 @@ const api: ElectronAPI = { } }, availability: () => ipcRenderer.invoke("sandbox-availability"), - add: (name, projectDir, resources) => ipcRenderer.invoke("sandbox-add", name, projectDir, resources), + add: (name, projectDir, resources, imagePath) => + ipcRenderer.invoke("sandbox-add", name, projectDir, resources, imagePath), + inspectImage: (path) => ipcRenderer.invoke("sandbox-inspect-image", path), cancel: (name) => ipcRenderer.invoke("sandbox-cancel", name), remove: (id) => ipcRenderer.invoke("sandbox-remove", id), reset: (id) => ipcRenderer.invoke("sandbox-reset", id), diff --git a/packages/desktop/src/renderer/index.tsx b/packages/desktop/src/renderer/index.tsx index fde029197f..3e1b500795 100644 --- a/packages/desktop/src/renderer/index.tsx +++ b/packages/desktop/src/renderer/index.tsx @@ -183,6 +183,19 @@ const createPlatform = (windowState: DesktopWindowState): Platform => { }) }, + async openFilePathDialog(opts) { + const result = await window.api.openFilePicker({ + multiple: false, + title: opts?.title ?? t("desktop.dialog.chooseFile"), + extensions: opts?.extensions, + }) + if (!result) return null + // Only the path is wanted, so release the read token immediately rather + // than holding it open for a file that may be gigabytes. + await window.api.releasePickedFiles(result.token) + return result.files[0]?.path ?? null + }, + async openAttachmentPickerDialog(opts, onFile) { const result = await window.api.openFilePicker({ multiple: opts?.multiple ?? false, From 65c90b49edd1bccc6dda5e80519399c425c4099e Mon Sep 17 00:00:00 2001 From: minhn4 Date: Sun, 16 Aug 2026 08:45:47 +0700 Subject: [PATCH 2/3] fix(desktop): pick a sandbox image without the attachment size budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Choosing an image left the field empty. open-file-picker is the attachment picker: it stats every file, enforces MAX_ATTACHMENT_BYTES (20 MB) and mints a read token for the renderer. A guest image is over a gigabyte, so the handler threw, the invoke rejected, and the dialog's un-caught .then never ran — the picker closed and nothing appeared, with no error to explain it. Picking a path is not attaching a file: nothing is read, so it gets its own channel with no budget and no read authorization. Its filters also carry an "All files" group, so an image with an unusual name stays selectable. The dialog now reports a failed pick instead of dropping it. That silence is what made a 20 MB limit look like the app ignoring the click. Co-Authored-By: Claude Opus 5 (1M context) --- packages/app/src/sandbox/dialog-add.tsx | 9 +++++++++ packages/desktop/src/main/ipc.ts | 19 +++++++++++++++++++ packages/desktop/src/preload/index.ts | 1 + packages/desktop/src/preload/types.ts | 2 ++ packages/desktop/src/renderer/index.tsx | 8 +------- 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/app/src/sandbox/dialog-add.tsx b/packages/app/src/sandbox/dialog-add.tsx index cb7d949083..ecca8fa563 100644 --- a/packages/app/src/sandbox/dialog-add.tsx +++ b/packages/app/src/sandbox/dialog-add.tsx @@ -118,6 +118,15 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => setStore({ imagePath: path, imageProblem: null, imageCheckedPath: null }) return checkImage(path) }) + // A picker that fails has to say so: swallowing it leaves the field + // empty after the user chose a file, which reads as the app ignoring them. + .catch((error) => + showToast({ + variant: "error", + title: language.t("common.requestFailed"), + description: error instanceof Error ? error.message : String(error), + }), + ) } const pickProjectDir = () => { diff --git a/packages/desktop/src/main/ipc.ts b/packages/desktop/src/main/ipc.ts index 4563b688ac..349493f0fa 100644 --- a/packages/desktop/src/main/ipc.ts +++ b/packages/desktop/src/main/ipc.ts @@ -157,6 +157,25 @@ export function registerIpcHandlers(deps: Deps) { }, ) + // Picking a path is not attaching a file: nothing is read, so the 20 MB + // attachment budget must not apply (a sandbox guest image is gigabytes) and + // no read authorization is minted. The filters keep an "All files" group so + // an image with an unusual name is still selectable. + ipcMain.handle( + "open-file-path-picker", + async (_event: IpcMainInvokeEvent, opts?: { title?: string; extensions?: string[] }) => { + const result = await dialog.showOpenDialog({ + properties: ["openFile"], + title: opts?.title ?? "Choose a file", + filters: opts?.extensions?.length + ? [{ name: "Files", extensions: opts.extensions }, { name: "All files", extensions: ["*"] }] + : undefined, + }) + if (result.canceled) return null + return result.filePaths[0] ?? null + }, + ) + ipcMain.handle("read-picked-file", async (event: IpcMainInvokeEvent, token: string, filePath: string) => { return pickedFiles.read(event.sender.id, token, filePath) }) diff --git a/packages/desktop/src/preload/index.ts b/packages/desktop/src/preload/index.ts index 95bed50744..5fc4a9a077 100644 --- a/packages/desktop/src/preload/index.ts +++ b/packages/desktop/src/preload/index.ts @@ -112,6 +112,7 @@ const api: ElectronAPI = { openDirectoryPicker: (opts) => ipcRenderer.invoke("open-directory-picker", opts), openFilePicker: (opts) => ipcRenderer.invoke("open-file-picker", opts), + openFilePathPicker: (opts) => ipcRenderer.invoke("open-file-path-picker", opts), readPickedFile: (token, path) => ipcRenderer.invoke("read-picked-file", token, path), releasePickedFiles: (token) => ipcRenderer.invoke("release-picked-files", token), getPathForFile: (file) => webUtils.getPathForFile(file), diff --git a/packages/desktop/src/preload/types.ts b/packages/desktop/src/preload/types.ts index 02749b9ebb..284e92052d 100644 --- a/packages/desktop/src/preload/types.ts +++ b/packages/desktop/src/preload/types.ts @@ -93,6 +93,8 @@ export type ElectronAPI = { defaultPath?: string extensions?: string[] }) => Promise<{ token: string; files: { path: string; name: string; size: number }[] } | null> + // Path only: never reads the file, so no attachment size budget applies. + openFilePathPicker: (opts?: { title?: string; extensions?: string[] }) => Promise readPickedFile: (token: string, path: string) => Promise releasePickedFiles: (token: string) => Promise getPathForFile: (file: File) => string diff --git a/packages/desktop/src/renderer/index.tsx b/packages/desktop/src/renderer/index.tsx index 3e1b500795..1716de3e2e 100644 --- a/packages/desktop/src/renderer/index.tsx +++ b/packages/desktop/src/renderer/index.tsx @@ -184,16 +184,10 @@ const createPlatform = (windowState: DesktopWindowState): Platform => { }, async openFilePathDialog(opts) { - const result = await window.api.openFilePicker({ - multiple: false, + return window.api.openFilePathPicker({ title: opts?.title ?? t("desktop.dialog.chooseFile"), extensions: opts?.extensions, }) - if (!result) return null - // Only the path is wanted, so release the read token immediately rather - // than holding it open for a file that may be gigabytes. - await window.api.releasePickedFiles(result.token) - return result.files[0]?.path ?? null }, async openAttachmentPickerDialog(opts, onFile) { From ae40cc7409d00f2a2b812c44b867f7403678d60d Mon Sep 17 00:00:00 2001 From: minhn4 Date: Sun, 16 Aug 2026 08:57:41 +0700 Subject: [PATCH 3/3] fix(app): say what the create is doing instead of always "downloading" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status line under the Add sandbox form read the create job's progress and treated any value as a download in flight. That progress covers fetching the image and nothing else, so it was wrong in three situations, and wrong for the whole slow part of each: - a custom image reports its full size in one go, so the dialog claimed to be downloading a file the user had picked off their own disk; - a finished download leaves progress pinned at 100% for the entire unpack, so the bar sat full while still saying "Downloading"; - an image already in the cache reports nothing, so the idle hint about first downloads stayed up for the whole create. The phase now comes from what the progress is not saying — complete, or absent, means the fetch is over and the sandbox is being built — and the dialog says "Creating the sandbox…" for that stretch. The download hint is also dropped once a custom image is chosen, since nothing will be downloaded. The rule is in create-phase.ts with tests rather than inline in the JSX; it is small, pure, and had already been got wrong three ways by eye. Co-Authored-By: Claude Opus 5 (1M context) --- packages/app/src/i18n/en.ts | 1 + packages/app/src/i18n/es.ts | 1 + packages/app/src/sandbox/create-phase.test.ts | 40 +++++++++++++++++++ packages/app/src/sandbox/create-phase.ts | 18 +++++++++ packages/app/src/sandbox/dialog-add.tsx | 30 +++++++------- 5 files changed, 74 insertions(+), 16 deletions(-) create mode 100644 packages/app/src/sandbox/create-phase.test.ts create mode 100644 packages/app/src/sandbox/create-phase.ts diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 7e7e8bc78c..f3546cb3d9 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -423,6 +423,7 @@ export const dict = { "sandbox.add.resourcesInvalid": "CPUs must be 1-32 and memory 1-64 GB.", "sandbox.add.downloadHint": "The first sandbox downloads its image; on a slow connection this can take a while.", "sandbox.add.downloadProgress": "Downloading image… {{done}}/{{total}} MB", + "sandbox.add.creating": "Creating the sandbox\u2026 this takes a few minutes.", "sandbox.mounts.title": "Sandbox folders", "sandbox.mounts.hint": "Folders mounted here are the only ones on this computer the sandbox can reach, and the agent edits files in them without asking.", diff --git a/packages/app/src/i18n/es.ts b/packages/app/src/i18n/es.ts index 7f35c31788..1afb02c99b 100644 --- a/packages/app/src/i18n/es.ts +++ b/packages/app/src/i18n/es.ts @@ -414,6 +414,7 @@ export const dict = { "sandbox.add.resourcesInvalid": "Las CPU deben ser 1-32 y la memoria 1-64 GB.", "sandbox.add.downloadHint": "El primer sandbox descarga su imagen; con una conexión lenta puede tardar un poco.", "sandbox.add.downloadProgress": "Descargando imagen… {{done}}/{{total}} MB", + "sandbox.add.creating": "Creando el sandbox\u2026 esto tarda unos minutos.", "sandbox.mounts.title": "Carpetas del sandbox", "sandbox.mounts.hint": "Las carpetas montadas aquí son las únicas de este equipo a las que el sandbox puede acceder, y el agente edita archivos en ellas sin preguntar.", diff --git a/packages/app/src/sandbox/create-phase.test.ts b/packages/app/src/sandbox/create-phase.test.ts new file mode 100644 index 0000000000..50964c23ff --- /dev/null +++ b/packages/app/src/sandbox/create-phase.test.ts @@ -0,0 +1,40 @@ +import { expect, test } from "bun:test" +import { sandboxCreatePhase } from "./create-phase" +import type { SandboxJob } from "./types" + +const job = (over: Partial): SandboxJob => ({ + kind: "create", + id: "sandbox:work", + startedAt: 0, + status: "running", + ...over, +}) + +test("is idle before the create starts", () => { + expect(sandboxCreatePhase({ adding: false })).toBe("idle") + // A job belonging to some other sandbox must not make this dialog busy. + expect(sandboxCreatePhase({ adding: false, job: job({}) })).toBe("idle") +}) + +test("waits while the job is queued behind another sandbox", () => { + expect(sandboxCreatePhase({ adding: true, job: job({ status: "queued" }) })).toBe("queued") +}) + +test("downloads only while bytes are still arriving", () => { + expect(sandboxCreatePhase({ adding: true, job: job({ progress: { bytes: 5, totalBytes: 100 } }) })).toBe( + "downloading", + ) +}) + +// The bug this exists for: progress stops updating at 100% and stays there for +// the whole unpack, so the dialog used to claim it was still downloading. +test("switches to creating once the download completes", () => { + expect(sandboxCreatePhase({ adding: true, job: job({ progress: { bytes: 100, totalBytes: 100 } }) })).toBe("creating") +}) + +// A cached image reports nothing at all, and a custom image reports its full +// size in one go — neither is a download. +test("is creating when no bytes are ever fetched", () => { + expect(sandboxCreatePhase({ adding: true, job: job({}) })).toBe("creating") + expect(sandboxCreatePhase({ adding: true })).toBe("creating") +}) diff --git a/packages/app/src/sandbox/create-phase.ts b/packages/app/src/sandbox/create-phase.ts new file mode 100644 index 0000000000..0e8310b80b --- /dev/null +++ b/packages/app/src/sandbox/create-phase.ts @@ -0,0 +1,18 @@ +import type { SandboxJob } from "./types" + +// What an in-flight create is doing, for the Add sandbox dialog's status line. +// +// The job carries progress for fetching the image and nothing else, so the +// phase has to be read from what that progress is *not* saying: a report that +// has reached its total, or no report at all, means the fetch is done (or was +// never needed) and the slow part — unpacking and building the sandbox — is +// under way. Getting this wrong leaves the dialog claiming to download a file +// it already has, or one the user supplied from their own disk. +export type SandboxCreatePhase = "idle" | "queued" | "downloading" | "creating" + +export function sandboxCreatePhase(input: { adding: boolean; job?: SandboxJob }): SandboxCreatePhase { + if (!input.adding) return "idle" + if (input.job?.status === "queued") return "queued" + const reported = input.job?.progress + return reported && reported.bytes < reported.totalBytes ? "downloading" : "creating" +} diff --git a/packages/app/src/sandbox/dialog-add.tsx b/packages/app/src/sandbox/dialog-add.tsx index ecca8fa563..d1f9e31013 100644 --- a/packages/app/src/sandbox/dialog-add.tsx +++ b/packages/app/src/sandbox/dialog-add.tsx @@ -11,6 +11,7 @@ import { useLanguage } from "@/context/language" import { usePlatform } from "@/context/platform" import type { SandboxConfig, SandboxImageProblem } from "./types" import { useSandboxes } from "./context" +import { sandboxCreatePhase } from "./create-phase" import { sandboxNameUsable, validateSandboxName } from "./name-model" import { sandboxResourcesValid } from "./resources" @@ -136,15 +137,18 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) => }) } - // This create's image-download progress, pushed by the main process while - // the add is in flight. Jobs are queued, so the create may still be waiting - // behind another sandbox's job: no progress until it starts running. - const progress = () => { - if (!store.adding) return undefined - return sandboxes.state.data?.jobs.find((job) => job.kind === "create" && job.status === "running")?.progress + const createJob = () => sandboxes.state.data?.jobs.find((entry) => entry.kind === "create") + const createPhase = () => sandboxCreatePhase({ adding: store.adding, job: createJob() }) + const downloadProgress = () => (createPhase() === "downloading" ? createJob()?.progress : undefined) + // Null while the bar is up: the byte counter under it already says what is + // happening, and a custom image needs no download warning at all. + const statusMessage = () => { + const phase = createPhase() + if (phase === "queued") return language.t("sandbox.job.queued") + if (phase === "creating") return language.t("sandbox.add.creating") + if (phase === "downloading") return null + return store.imagePath ? null : language.t("sandbox.add.downloadHint") } - const queued = () => - store.adding && sandboxes.state.data?.jobs.some((job) => job.kind === "create" && job.status === "queued") const create = async () => { if (!canCreate() || store.adding) return @@ -279,14 +283,7 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) =>

{language.t("sandbox.add.resourcesInvalid")}

- - {queued() ? language.t("sandbox.job.queued") : language.t("sandbox.add.downloadHint")} -

- } - > + {(current) => (
@@ -304,6 +301,7 @@ export function DialogAddSandbox(props: { onCreated?: (config: SandboxConfig) =>
)} + {(text) =>

{text()}

}