From 20a035930c0bdb686c265161e11b24bebae7c4ad Mon Sep 17 00:00:00 2001 From: Patrick Bertsch Date: Sun, 25 Oct 2026 19:53:44 -0600 Subject: [PATCH 1/4] fix(studio): API key presence only, WiFi tokens not remembered, RunFile checks roots Stored keys are reported by presence only, WiFi tokens are no longer remembered, and RunFile checks the workspace and automation roots. GetAPIKey reports whether a key is saved and never returns it. The key panel starts with an empty field, and an empty save no longer clears a saved key; Remove key still deletes. WiFi tokens are no longer remembered. The token is typed for each connection. The saved tag and the forget control are gone, and Studio deletes the plain-text copy an earlier version kept in localStorage on first start. The host and port are still remembered. RunFile accepts only .probe files inside the open workspace or an automation folder: the check the automation endpoint makes. RunFileAsync makes it before the background start, so a refusal reaches the caller. Tests cover the API key presence check, the run path refusals, and the RunFileAsync refusal. The studio_run_file description and the Studio docs say what changed. --- CHANGELOG.md | 5 ++ internal/mcp/studio.go | 2 +- studio/ai_chat.go | 12 ++-- studio/app.go | 9 ++- studio/frontend/src/main.ts | 88 ++++++------------------ studio/frontend/wailsjs/go/main/App.d.ts | 2 +- studio/keychain.go | 24 +++++-- studio/keychain_test.go | 13 ++-- studio/runfile_roots_test.go | 50 ++++++++++++++ studio/runtrack.go | 4 ++ website/src/content/docs/tools/mcp.md | 2 +- website/src/content/docs/tools/studio.md | 4 +- 12 files changed, 123 insertions(+), 92 deletions(-) create mode 100644 studio/runfile_roots_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index db0aafed..a8e61658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **The recording viewer fits a 390 px phone screen.** Long step names, URLs and error text no longer push the page sideways: the single-column layout shrinks to the screen, long words wrap, and the 16 px side gutters stay. The desktop layout is unchanged. The viewer ships with `probe test --record`, which is not released yet. - **VS Code: the Run Test commands no longer pass `--name` to `probe test`.** This version of the CLI defines no such flag, so those runs failed with an unknown-flag error. "FlutterProbe: Run Test File at Cursor", the CodeLens buttons and the Tests view now run the file that contains the test, and their titles and tooltips say so. Until the CLI has a per-test filter, the extension runs whole files. +### Security +- **Studio no longer returns the Anthropic API key to the window.** The key panel shows whether a key is saved and starts with an empty field. Saving an empty field no longer clears a saved key; use Remove key to delete it. +- **Studio no longer remembers WiFi tokens.** The token is typed for each connection. Studio deletes the plain-text copy that an earlier version kept in local storage on first start. +- **Studio's Run File accepts only `.probe` files inside the open workspace or an automation folder.** Other paths are refused before anything runs, as the automation endpoint already refused them. + ## [0.26.0] - 2026-10-14 ### Added diff --git a/internal/mcp/studio.go b/internal/mcp/studio.go index 522742a7..af98a1ae 100644 --- a/internal/mcp/studio.go +++ b/internal/mcp/studio.go @@ -44,7 +44,7 @@ Studio automation is off by default: if no Studio answers, start it with PROBE_S }, { Name: "studio_run_file", - Description: "Start running a .probe file in Studio. Returns at once; poll studio_run_state for step-by-step progress and studio_results for the verdicts. Fails if Studio is not connected to a device or a run is already in progress.", + Description: "Start running a .probe file in Studio. Returns at once; poll studio_run_state for step-by-step progress and studio_results for the verdicts. The file must be a .probe file inside the open workspace or an automation folder; any other path is refused. Fails if Studio is not connected to a device or a run is already in progress.", InputSchema: mcpSchema{Type: "object", Required: []string{"path"}, Properties: map[string]mcpProp{ "path": {Type: "string", Description: "Absolute path of the .probe file"}, }}, diff --git a/studio/ai_chat.go b/studio/ai_chat.go index 3cbc5601..77966726 100644 --- a/studio/ai_chat.go +++ b/studio/ai_chat.go @@ -57,14 +57,10 @@ type claudeMessage struct { var chatHTTPClient = &http.Client{Timeout: 120 * time.Second} -// GetAPIKey retrieves the stored Anthropic API key from the platform keychain. -// Returns an empty string (not an error) when no key is stored. -func (a *App) GetAPIKey() (string, error) { - key, err := keychainGet() - if err != nil { - return "", nil - } - return key, nil +// GetAPIKey reports whether an Anthropic API key is saved in the platform keychain. It never returns the key: the +// settings panel only needs to show that one is saved. +func (a *App) GetAPIKey() (bool, error) { + return keychainHas(keychainService, keychainUser) } // SetAPIKey stores the Anthropic API key in the platform keychain. diff --git a/studio/app.go b/studio/app.go index 65bceae8..de199ce6 100644 --- a/studio/app.go +++ b/studio/app.go @@ -905,7 +905,14 @@ type RunResult struct { // // The connection must be established (Connect first). Lint warnings are not // surfaced here — use Lint() for that; this method runs whatever parses. -func (a *App) RunFile(path string) ([]RunResult, error) { +// +// The path must be a .probe file inside the workspace a person opened or an automation folder: the check the +// automation endpoint makes, applied to every caller. Other paths are refused before anything runs. +func (a *App) RunFile(requested string) ([]RunResult, error) { + path, err := probeFileWithinRoots(a.automationRoots(), requested) + if err != nil { + return nil, err + } a.mu.Lock() conn := a.conn a.mu.Unlock() diff --git a/studio/frontend/src/main.ts b/studio/frontend/src/main.ts index babb3ddf..b1046d0e 100644 --- a/studio/frontend/src/main.ts +++ b/studio/frontend/src/main.ts @@ -413,45 +413,12 @@ $("btn-settings-save").addEventListener("click", async () => { type WiFiDevice = { name: string; host: string; port: number; version: string }; -const WIFI_TOKENS_KEY = "fp.studio.wifi-tokens"; - -// Token store: deviceName → token. Persisted in localStorage so a user who -// has connected once doesn't need to paste the token again on subsequent -// launches. Keyed by the Bonjour instance name (typically the device's -// hostname) since the IP can change across DHCP leases but the name is -// stable. Tokens for an LAN-only test agent inside a desktop app are an -// acceptable trust boundary; this is not a credential vault. -function loadTokenStore(): Record { - try { - const raw = localStorage.getItem(WIFI_TOKENS_KEY); - return raw ? (JSON.parse(raw) as Record) : {}; - } catch { - return {}; - } -} - -function saveTokenStore(store: Record) { - try { - localStorage.setItem(WIFI_TOKENS_KEY, JSON.stringify(store)); - } catch { - // localStorage full or denied — non-fatal, user just re-enters next time - } -} - -function rememberToken(deviceName: string, token: string) { - const store = loadTokenStore(); - store[deviceName] = token; - saveTokenStore(store); -} - -function forgetToken(deviceName: string) { - const store = loadTokenStore(); - delete store[deviceName]; - saveTokenStore(store); -} - -function recalledToken(deviceName: string): string | undefined { - return loadTokenStore()[deviceName]; +// Earlier versions kept WiFi tokens in plain text in localStorage. Studio no longer remembers tokens, so remove that copy +// on start. +try { + localStorage.removeItem("fp.studio.wifi-tokens"); +} catch { + /* storage unavailable: nothing to remove */ } let wifiSelected: WiFiDevice | null = null; @@ -485,28 +452,13 @@ function selectWiFiDevice(dev: WiFiDevice) { for (const li of Array.from($("wifi-devices").children) as HTMLElement[]) { li.classList.toggle("selected", li.dataset.key === `${dev.host}:${dev.port}`); } - const remembered = recalledToken(dev.name); $("wifi-selected-label").textContent = `Connecting to ${dev.name} (${dev.host}:${dev.port})`; ($("wifi-token-row") as HTMLElement).hidden = false; const input = $("wifi-token-input") as HTMLInputElement; - input.value = remembered ?? ""; + input.value = ""; input.focus(); } -function renderForgetButton(deviceName: string, container: HTMLElement) { - const btn = document.createElement("button"); - btn.className = "btn-mini wifi-forget"; - btn.title = `Forget remembered token for ${deviceName}`; - btn.textContent = "✕"; - btn.addEventListener("click", (e) => { - e.stopPropagation(); - forgetToken(deviceName); - btn.remove(); - toast(`Forgot token for ${deviceName}`, "info", 1500); - }); - container.appendChild(btn); -} - EventsOn("wifi:device-found", (dev: WiFiDevice) => { const key = `${dev.host}:${dev.port}`; if (wifiSeen.has(key)) return; @@ -525,14 +477,6 @@ EventsOn("wifi:device-found", (dev: WiFiDevice) => { hostEl.className = "wifi-host"; hostEl.textContent = `${dev.host}:${dev.port}${dev.version ? ` · v${dev.version}` : ""}`; li.append(nameEl, hostEl); - if (recalledToken(dev.name)) { - const savedTag = document.createElement("span"); - savedTag.className = "wifi-saved-tag"; - savedTag.textContent = "🔑 saved"; - savedTag.title = "Token remembered from a previous connection"; - li.appendChild(savedTag); - renderForgetButton(dev.name, li); - } li.addEventListener("click", () => selectWiFiDevice(dev)); list.appendChild(li); }); @@ -618,7 +562,6 @@ $("btn-wifi-connect").addEventListener("click", async () => { setStatus("connecting", `Connecting to ${wifiSelected.host}…`); try { await ConnectWiFi(wifiSelected.host, wifiSelected.port, token); - rememberToken(wifiSelected.name, token); setStatus( "connected", `Connected · ${wifiSelected.name}`, @@ -1271,12 +1214,16 @@ $("btn-chat-key").addEventListener("click", openApiKeyOverlay); // ---- API key overlay ---------------------------------------------------- async function openApiKeyOverlay() { + // The stored key is never sent to the page: the field starts empty, and the placeholder says whether one is saved. + const input = $("api-key-input") as HTMLInputElement; + input.value = ""; + let saved = false; try { - const key = (await GetAPIKey()) as string; - ($("api-key-input") as HTMLInputElement).value = key || ""; + saved = await GetAPIKey(); } catch { - ($("api-key-input") as HTMLInputElement).value = ""; + saved = false; } + input.placeholder = saved ? "A key is saved. Type a new key to replace it." : "sk-ant-…"; $("api-key-overlay").hidden = false; } @@ -1291,9 +1238,14 @@ $("api-key-overlay").addEventListener("click", (e) => { $("btn-api-key-save").addEventListener("click", async () => { const key = ($("api-key-input") as HTMLInputElement).value.trim(); + // An empty field must not clear a saved key: removing one is the separate Remove key action. + if (!key) { + toast("Type a key to save it, or use Remove key to delete the saved one.", "info"); + return; + } try { await SetAPIKey(key); - toast(key ? "API key saved to keychain." : "API key cleared.", "success"); + toast("API key saved to keychain.", "success"); closeApiKeyOverlay(); } catch (err) { toast(`Failed to save key: ${err}`, "error"); diff --git a/studio/frontend/wailsjs/go/main/App.d.ts b/studio/frontend/wailsjs/go/main/App.d.ts index df76a265..ea464186 100755 --- a/studio/frontend/wailsjs/go/main/App.d.ts +++ b/studio/frontend/wailsjs/go/main/App.d.ts @@ -16,7 +16,7 @@ export function DeleteAPIKey():Promise; export function Disconnect():Promise; -export function GetAPIKey():Promise; +export function GetAPIKey():Promise; export function GetDeviceSettings():Promise; diff --git a/studio/keychain.go b/studio/keychain.go index 9d9211e5..47cd1da0 100644 --- a/studio/keychain.go +++ b/studio/keychain.go @@ -1,12 +1,28 @@ package main -import "github.com/zalando/go-keyring" +import ( + "errors" + + "github.com/zalando/go-keyring" +) const ( keychainService = "flutterprobe.studio" keychainUser = "anthropic-api-key" ) -func keychainGet() (string, error) { return keyring.Get(keychainService, keychainUser) } -func keychainSet(key string) error { return keyring.Set(keychainService, keychainUser, key) } -func keychainDelete() error { return keyring.Delete(keychainService, keychainUser) } +func keychainGet() (string, error) { return keyring.Get(keychainService, keychainUser) } +func keychainSet(key string) error { return keyring.Set(keychainService, keychainUser, key) } +func keychainDelete() error { return keyring.Delete(keychainService, keychainUser) } + +// keychainHas reports whether an item is saved. A missing item is not an error; anything else is. +func keychainHas(service, user string) (bool, error) { + _, err := keyring.Get(service, user) + if errors.Is(err, keyring.ErrNotFound) { + return false, nil + } + if err != nil { + return false, err + } + return true, nil +} diff --git a/studio/keychain_test.go b/studio/keychain_test.go index e2028715..c0311406 100644 --- a/studio/keychain_test.go +++ b/studio/keychain_test.go @@ -12,21 +12,22 @@ func TestAPIKeyRoundTripThroughTheKeychain(t *testing.T) { keyring.MockInit() a := &App{} - if got, err := a.GetAPIKey(); err != nil || got != "" { - t.Fatalf("no key stored: got %q, err %v; want an empty string and no error", got, err) + // GetAPIKey reports presence only, so the test checks what it reports, not a key value. + if has, err := a.GetAPIKey(); err != nil || has { + t.Fatalf("no key stored: has %v, err %v; want false and no error", has, err) } if err := a.SetAPIKey("test-key-value"); err != nil { t.Fatal(err) } - if got, _ := a.GetAPIKey(); got != "test-key-value" { - t.Errorf("stored key = %q", got) + if has, _ := a.GetAPIKey(); !has { + t.Error("key stored but GetAPIKey reports none") } // An empty key means "forget it". if err := a.SetAPIKey(""); err != nil { t.Fatal(err) } - if got, _ := a.GetAPIKey(); got != "" { - t.Errorf("after clearing, key = %q", got) + if has, _ := a.GetAPIKey(); has { + t.Error("after clearing, GetAPIKey still reports a key") } if err := a.DeleteAPIKey(); err == nil { t.Error("deleting a key that is not there should report it") diff --git a/studio/runfile_roots_test.go b/studio/runfile_roots_test.go new file mode 100644 index 00000000..a5a5f30c --- /dev/null +++ b/studio/runfile_roots_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// RunFile runs only .probe files inside the workspace a person opened or an automation folder. The check runs before +// the connection check, so a refused path fails the same way whether or not a device is connected. +func TestRunFileRefusesPathsOutsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + inside := filepath.Join(workspace, "smoke.probe") + notProbe := filepath.Join(workspace, "notes.txt") + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + for _, p := range []string{inside, notProbe, outside} { + if err := os.WriteFile(p, []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + } + a := &App{} + a.SetWorkspace(workspace) + + if _, err := a.RunFile(outside); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a .probe file outside the workspace: err %v; want the outside-the-folders refusal", err) + } + if _, err := a.RunFile(notProbe); err == nil || !strings.Contains(err.Error(), "only .probe files") { + t.Errorf("a non-.probe file in the workspace: err %v; want the suffix refusal", err) + } + // A file inside the workspace passes the check; with no device connected it stops at the connection check. + if _, err := a.RunFile(inside); err != errNotConnected { + t.Errorf("a .probe file inside the workspace: err %v; want errNotConnected (the path check passed)", err) + } +} + +// RunFileAsync refuses the path at once, so the caller hears about it instead of a background run failing silently. +func TestRunFileAsyncRefusesPathsOutsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + if err := os.WriteFile(outside, []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + a := &App{conn: &connection{}} + a.SetWorkspace(workspace) + + if err := a.RunFileAsync(outside); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("RunFileAsync on a path outside the workspace: err %v; want the outside-the-folders refusal", err) + } +} diff --git a/studio/runtrack.go b/studio/runtrack.go index 46dcfb67..ed9332c1 100644 --- a/studio/runtrack.go +++ b/studio/runtrack.go @@ -100,6 +100,10 @@ func (a *App) RunFileAsync(path string) error { if busy { return errRunInProgress } + // Refuse a path outside the allowed folders now, not inside the goroutine, so the caller hears about it. + if _, err := probeFileWithinRoots(a.automationRoots(), path); err != nil { + return err + } go func() { _, _ = a.RunFile(path) }() return nil } diff --git a/website/src/content/docs/tools/mcp.md b/website/src/content/docs/tools/mcp.md index e0343a56..8e70a1d7 100644 --- a/website/src/content/docs/tools/mcp.md +++ b/website/src/content/docs/tools/mcp.md @@ -42,7 +42,7 @@ FlutterProbe ships an MCP (Model Context Protocol) server as a standalone binary | Tool | Description | |---|---| | `get_report` | Read the most recently modified JSON test run report | -| `studio_open_workspace`, `studio_list_devices`, `studio_connect` (a device id, or `host`/`port`/`token` of a phone on the network), `studio_run_file`, `studio_cancel`, `studio_run_state`, `studio_results`, `studio_screenshot`, `studio_widget_tree`, `studio_ui_state` | Drive a running Probe Studio (started with `PROBE_STUDIO_AUTOMATION=1`) without a window: open a workspace, connect to a device, run a `.probe` file and read step-by-step progress | +| `studio_open_workspace`, `studio_list_devices`, `studio_connect` (a device id, or `host`/`port`/`token` of a phone on the network), `studio_run_file`, `studio_cancel`, `studio_run_state`, `studio_results`, `studio_screenshot`, `studio_widget_tree`, `studio_ui_state` | Drive a running Probe Studio (started with `PROBE_STUDIO_AUTOMATION=1`) without a window: open a workspace, connect to a device, run a `.probe` file inside the open workspace (other paths are refused) and read step-by-step progress | | `perf_trend` | Show how a measured performance number (memory, CPU, frames, data) changed over past runs | | `perf_compare` | Compare two performance baseline files and list what got worse | | `generate_report` | Generate a standalone HTML report from a JSON results file | diff --git a/website/src/content/docs/tools/studio.md b/website/src/content/docs/tools/studio.md index 21c045ad..794b2ce9 100644 --- a/website/src/content/docs/tools/studio.md +++ b/website/src/content/docs/tools/studio.md @@ -80,7 +80,7 @@ The runner is a library, not a subprocess. The connection-stability work in v0.6 Studio connects to physical iOS and Android devices through the exact same `internal/device` package the `probe` CLI uses — there is no separate device-management code path for Studio. - **USB** — plug in the device, launch the app with `--dart-define=PROBE_AGENT=true`, click ↻ to refresh the device picker, and pick the entry tagged `physical`. Physical iOS gets an `iproxy` tunnel automatically before Studio dials the agent; physical Android goes through `adb forward`, the same path as emulators. Connection errors for a missing `iproxy` or `adb` binary surface inline with an actionable fix (e.g. "install via: brew install libimobiledevice"). -- **WiFi (preferred for iOS)** — matching the CLI's own documented preference, WiFi is more stable than USB-C for physical iOS: a USB-C cable's charge/data mode-flipping can drop the `iproxy` tunnel mid-run, where WiFi has no such failure mode. Build the Flutter app with `--dart-define=PROBE_WIFI=true` and pass an advertiser to `ProbeAgent.start` (see [WiFi auto-discovery](#wifi-auto-discovery)), click the 📡 button to open the WiFi discovery overlay, and pick the device once it appears via mDNS (`_flutterprobe._tcp`). Paste the token printed in the app's `PROBE_TOKEN=...` log line — Studio remembers it per device in local storage for next time (with a "forget" control). +- **WiFi (preferred for iOS)** — matching the CLI's own documented preference, WiFi is more stable than USB-C for physical iOS: a USB-C cable's charge/data mode-flipping can drop the `iproxy` tunnel mid-run, where WiFi has no such failure mode. Build the Flutter app with `--dart-define=PROBE_WIFI=true` and pass an advertiser to `ProbeAgent.start` (see [WiFi auto-discovery](#wifi-auto-discovery)), click the 📡 button to open the WiFi discovery overlay, and pick the device once it appears via mDNS (`_flutterprobe._tcp`). Paste the token printed in the app's `PROBE_TOKEN=...` log line — Studio does not keep it: paste it again each time. - The exact conditions for a physical iPhone over WiFi, with a three-command check, are in [Physical iPhone over WiFi](/platform/ios/#physical-iphone-over-wifi-cli-and-studio-what-must-be-true). - Devices that don't advertise over mDNS (no advertiser in the build, older agent versions, or networks that block mDNS): use **Connect manually** in the WiFi overlay (host, port, token) — see [Manual WiFi entry](#manual-wifi-entry) — or the CLI's `--host --token `. @@ -158,7 +158,7 @@ wails build - No multi-device side-by-side view. - No per-step replay / time-travel debugging — see the Results pane for pass/fail, but stack inspection requires re-running. -- The AI chat pane talks to the Anthropic API only, with your own key stored in the OS keychain. Each message sends the open file. The failures of the last run and the connected device's widget tree are sent only when you turn them on, under **AI chat context** in Workspace settings, or in `probe.yaml` under `studio.ai_context` (`run_results` and `widget_tree`). **Both default to off**: until you change them, the chat sends only the open file. When either is on, a line above the chat input says what is sent. Before the widget tree is sent, obscured fields and credential-looking text (API keys such as `sk-…`, GitHub tokens such as `ghp_…`, `Bearer` values, and long hex or base64 runs) are redacted. Each part is cut to a size limit and marked when it is cut. The chat is not exposed through the automation endpoint or the MCP server; use the standalone `probe-mcp` binary with Claude Desktop or Cursor for MCP-driven work. +- The AI chat pane talks to the Anthropic API only, with your own key stored in the OS keychain. The panel never shows the key again: it shows only whether one is saved. Each message sends the open file. The failures of the last run and the connected device's widget tree are sent only when you turn them on, under **AI chat context** in Workspace settings, or in `probe.yaml` under `studio.ai_context` (`run_results` and `widget_tree`). **Both default to off**: until you change them, the chat sends only the open file. When either is on, a line above the chat input says what is sent. Before the widget tree is sent, obscured fields and credential-looking text (API keys such as `sk-…`, GitHub tokens such as `ghp_…`, `Bearer` values, and long hex or base64 runs) are redacted. Each part is cut to a size limit and marked when it is cut. The chat is not exposed through the automation endpoint or the MCP server; use the standalone `probe-mcp` binary with Claude Desktop or Cursor for MCP-driven work. ## Configuration From 95e320fb093f830355c7b8de41307b25af37dfe5 Mon Sep 17 00:00:00 2001 From: Patrick Bertsch Date: Sun, 25 Oct 2026 18:36:06 -0600 Subject: [PATCH 2/4] fix(studio): ReadFile checks roots like RunFile ReadFile reads .probe files only. The editor calls it only for the .probe entries of the open workspace tree, and the automation endpoint calls it only on a path it has already checked, so that call is unchanged. It now checks the path the way RunFile does: an absolute path, resolved through every symlink, inside the workspace a person opened or an automation folder, and a regular .probe file. A ".." segment is refused before anything else, on either separator, including one that would stay inside the workspace. Tests cover a file inside and outside the workspace, a non-.probe file, a ".." path that climbs out and one that stays inside, a relative path, and a symlink that points out. The round-trip and headless tests now open the workspace first, as the page does. --- CHANGELOG.md | 1 + studio/app.go | 28 ++++++--- studio/app_test.go | 1 + studio/headless_test.go | 3 +- studio/readfile_roots_test.go | 106 ++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 studio/readfile_roots_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a8e61658..a8f8700e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **Studio no longer returns the Anthropic API key to the window.** The key panel shows whether a key is saved and starts with an empty field. Saving an empty field no longer clears a saved key; use Remove key to delete it. - **Studio no longer remembers WiFi tokens.** The token is typed for each connection. Studio deletes the plain-text copy that an earlier version kept in local storage on first start. - **Studio's Run File accepts only `.probe` files inside the open workspace or an automation folder.** Other paths are refused before anything runs, as the automation endpoint already refused them. +- **Studio opens only `.probe` files inside the open workspace or an automation folder.** The editor's open action uses the same check as Run File, and a path with `..` is refused. ## [0.26.0] - 2026-10-14 diff --git a/studio/app.go b/studio/app.go index de199ce6..2440b171 100644 --- a/studio/app.go +++ b/studio/app.go @@ -241,23 +241,35 @@ func (a *App) ListDir(dir string) ([]FileEntry, error) { return out, nil } -// ReadFile returns the contents of a .probe file. Path traversal is rejected -// to keep the UI from accidentally opening anything outside the workspace. +// ReadFile returns the contents of a .probe file inside the workspace a person opened or an automation folder. The +// editor opens only the .probe entries of the open workspace tree, so those are the only files it reads. The check is +// the one RunFile makes: an absolute path, resolved through every symlink, inside an allowed root, and a regular +// .probe file. A ".." segment is refused before anything else, so a path cannot climb out of a root. func (a *App) ReadFile(path string) (string, error) { - if !strings.HasSuffix(path, ".probe") { - return "", fmt.Errorf("only .probe files can be opened") - } - clean := filepath.Clean(path) - if strings.Contains(clean, "..") { + if hasDotDotSegment(path) { return "", fmt.Errorf("path traversal rejected: %s", path) } - b, err := os.ReadFile(clean) + real, err := probeFileWithinRoots(a.automationRoots(), path) + if err != nil { + return "", err + } + b, err := os.ReadFile(real) if err != nil { return "", err } return string(b), nil } +// hasDotDotSegment reports whether any segment of p is "..", on either path separator. +func hasDotDotSegment(p string) bool { + for _, part := range strings.FieldsFunc(p, func(r rune) bool { return r == '/' || r == '\\' }) { + if part == ".." { + return true + } + } + return false +} + // WriteFile saves content to a .probe path. Creates parent directories. func (a *App) WriteFile(path, content string) error { if !strings.HasSuffix(path, ".probe") { diff --git a/studio/app_test.go b/studio/app_test.go index a8e7540a..2209b14a 100644 --- a/studio/app_test.go +++ b/studio/app_test.go @@ -179,6 +179,7 @@ func TestApp_WriteFile_ReadFile_RoundTrip(t *testing.T) { a := NewApp() dir := t.TempDir() path := filepath.Join(dir, "sample.probe") + a.SetWorkspace(dir) // ReadFile reads inside the workspace a person opened content := "test \"round trip\"\n wait 1 seconds\n" if err := a.WriteFile(path, content); err != nil { diff --git a/studio/headless_test.go b/studio/headless_test.go index 1bf9766d..40b47bf6 100644 --- a/studio/headless_test.go +++ b/studio/headless_test.go @@ -98,8 +98,9 @@ func TestHeadlessCallNeedsTheTokenAndALocalCaller(t *testing.T) { } func TestHeadlessCallsAppMethodsLikeWails(t *testing.T) { - s, _ := startHeadlessTest(t) + s, a := startHeadlessTest(t) dir := t.TempDir() + a.SetWorkspace(dir) // the page opens the workspace before it lists or reads its files if err := os.WriteFile(filepath.Join(dir, "a.probe"), []byte("test \"x\"\n"), 0o644); err != nil { t.Fatal(err) } diff --git a/studio/readfile_roots_test.go b/studio/readfile_roots_test.go new file mode 100644 index 00000000..c755b023 --- /dev/null +++ b/studio/readfile_roots_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ReadFile reads .probe files inside the workspace a person opened or an automation folder: the check RunFile makes. +func TestReadFileReadsAProbeFileInsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + path := filepath.Join(workspace, "smoke.probe") + if err := os.WriteFile(path, []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if got, err := a.ReadFile(path); err != nil || got != "test \"x\"\n" { + t.Fatalf("ReadFile inside the workspace: got %q, err %v", got, err) + } +} + +func TestReadFileRefusesPathsOutsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + if err := os.WriteFile(outside, []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if _, err := a.ReadFile(outside); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a .probe file outside the workspace: err %v; want the outside-the-folders refusal", err) + } +} + +func TestReadFileRefusesANonProbeFileInsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + notes := filepath.Join(workspace, "notes.txt") + if err := os.WriteFile(notes, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if _, err := a.ReadFile(notes); err == nil || !strings.Contains(err.Error(), "only .probe files") { + t.Errorf("a non-.probe file inside the workspace: err %v; want the suffix refusal", err) + } +} + +// A ".." segment is refused outright, even one that would stay inside the workspace, so no path relies on climbing. +func TestReadFileRefusesDotDotSegments(t *testing.T) { + workspace := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + if err := os.WriteFile(outside, []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, "smoke.probe"), []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(workspace, "sub"), 0o755); err != nil { + t.Fatal(err) + } + // Built by hand: filepath.Join would clean the ".." away before ReadFile sees it. + sep := string(filepath.Separator) + climbOut := workspace + sep + ".." + sep + filepath.Base(filepath.Dir(outside)) + sep + filepath.Base(outside) + stayInside := workspace + sep + "sub" + sep + ".." + sep + "smoke.probe" + relativeClimb := ".." + sep + "smoke.probe" + a := &App{} + a.SetWorkspace(workspace) + for _, p := range []string{climbOut, stayInside, relativeClimb} { + if _, err := a.ReadFile(p); err == nil || !strings.Contains(err.Error(), "path traversal rejected") { + t.Errorf("ReadFile(%q): err %v; want the path traversal refusal", p, err) + } + } +} + +// Only absolute paths are read, so a bare name cannot resolve against whatever directory the process is in. +func TestReadFileNeedsAnAbsolutePath(t *testing.T) { + workspace := t.TempDir() + if err := os.WriteFile(filepath.Join(workspace, "smoke.probe"), []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if _, err := a.ReadFile("smoke.probe"); err == nil || !strings.Contains(err.Error(), "use an absolute path") { + t.Errorf("a relative path: err %v; want the absolute-path refusal", err) + } +} + +// A symlink inside the workspace that points outside is refused, because the check resolves links before comparing. +func TestReadFileRefusesASymlinkOutOfTheWorkspace(t *testing.T) { + workspace := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + if err := os.WriteFile(outside, []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(workspace, "link.probe") + if err := os.Symlink(outside, link); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if _, err := a.ReadFile(link); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a symlink out of the workspace: err %v; want the outside-the-folders refusal", err) + } +} From 7ba71aa5349c5265de0b4442774c2cbf1d361227 Mon Sep 17 00:00:00 2001 From: Patrick Bertsch Date: Sun, 25 Oct 2026 18:40:37 -0600 Subject: [PATCH 3/4] fix(studio): WriteFile and ListDir follow the rule ReadFile and RunFile follow Studio file access now has one rule. WriteFile saves .probe files, and creates the folders above them, only inside the workspace a person opened or an automation folder. ListDir lists only folders inside those roots. With no workspace open it lists nothing, and a folder that does not exist yet lists as empty. Both use the helper that ReadFile, RunFile and the automation endpoint already use, so the rule lives in one place (resolveInRoots). It refuses a relative path, a ".." segment on either separator, and any path whose symlinks resolve outside the roots. For writes it also resolves the nearest existing folder, so a new file under a folder that is a link out is refused, and a folder named like a .probe file is refused as a target. The automation endpoint's open_workspace and list_dir use the same helper, so they now refuse ".." as well. The file tree's up button stops at the workspace root. Tests cover a new file inside the workspace, outside paths, a non-.probe name, ".." on both separators, a file and a new file under a link out, a relative path, a folder named like a probe file, the workspace listing with hidden entries left out, a missing folder, folders outside, and no workspace open. The existing ListDir and round-trip tests open the workspace first, as the page does. Not covered: LoadWorkspaceSettings and SaveWorkspaceSettings still take a folder and read or write probe.yaml in it without this check. They need the UI call order checked, and the browser suite runs only in CI. --- CHANGELOG.md | 1 + studio/app.go | 48 ++++---- studio/app_test.go | 1 + studio/automation.go | 59 +++++++++- studio/frontend/src/main.ts | 2 + studio/write_list_roots_test.go | 193 ++++++++++++++++++++++++++++++++ 6 files changed, 284 insertions(+), 20 deletions(-) create mode 100644 studio/write_list_roots_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a8f8700e..b71bb1d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **Studio no longer remembers WiFi tokens.** The token is typed for each connection. Studio deletes the plain-text copy that an earlier version kept in local storage on first start. - **Studio's Run File accepts only `.probe` files inside the open workspace or an automation folder.** Other paths are refused before anything runs, as the automation endpoint already refused them. - **Studio opens only `.probe` files inside the open workspace or an automation folder.** The editor's open action uses the same check as Run File, and a path with `..` is refused. +- **Studio saves `.probe` files and lists folders only inside the open workspace or an automation folder.** Save and the file tree use the same rule as Read File and Run File, including the `..` and symlink checks. With no workspace open, the file tree is empty. ## [0.26.0] - 2026-10-14 diff --git a/studio/app.go b/studio/app.go index 2440b171..feb01c1d 100644 --- a/studio/app.go +++ b/studio/app.go @@ -212,18 +212,30 @@ func (a *App) workspaceConfig() *config.Config { return cfg } -// ListDir returns immediate children of dir. Hidden entries (dot-prefixed) -// are filtered. Returns empty slice + nil on missing directories so the UI -// can render an empty state without special-casing errors. +// ListDir returns immediate children of a folder inside the workspace a person opened or an automation folder. Hidden +// entries (dot-prefixed) are filtered. A folder that does not exist yet lists as empty, so the UI can render an empty +// state without special-casing errors; with no workspace open there is nothing to list. func (a *App) ListDir(dir string) ([]FileEntry, error) { - if dir == "" { - dir = "." + roots := a.automationRoots() + if len(roots) == 0 { + return []FileEntry{}, nil } - entries, err := os.ReadDir(dir) + real, err := resolveInRoots(roots, dir, true) + if err != nil { + return nil, err + } + info, err := os.Stat(real) + if os.IsNotExist(err) { + return []FileEntry{}, nil + } + if err != nil { + return nil, err + } + if !info.IsDir() { + return nil, fmt.Errorf("%s is not a folder", dir) + } + entries, err := os.ReadDir(real) if err != nil { - if os.IsNotExist(err) { - return []FileEntry{}, nil - } return nil, err } out := make([]FileEntry, 0, len(entries)) @@ -234,7 +246,7 @@ func (a *App) ListDir(dir string) ([]FileEntry, error) { } out = append(out, FileEntry{ Name: name, - Path: filepath.Join(dir, name), + Path: filepath.Join(filepath.Clean(dir), name), IsDir: e.IsDir(), }) } @@ -270,19 +282,17 @@ func hasDotDotSegment(p string) bool { return false } -// WriteFile saves content to a .probe path. Creates parent directories. +// WriteFile saves content to a .probe file inside the workspace a person opened or an automation folder. Missing +// folders are created inside the same check, so a folder that is a link out of the workspace refuses the write. func (a *App) WriteFile(path, content string) error { - if !strings.HasSuffix(path, ".probe") { - return fmt.Errorf("only .probe files can be written") - } - clean := filepath.Clean(path) - if strings.Contains(clean, "..") { - return fmt.Errorf("path traversal rejected: %s", path) + real, err := probeFileForWriteWithinRoots(a.automationRoots(), path) + if err != nil { + return err } - if err := os.MkdirAll(filepath.Dir(clean), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(real), 0o755); err != nil { return err } - return os.WriteFile(clean, []byte(content), 0o644) + return os.WriteFile(real, []byte(content), 0o644) } // ---- Lint ---- diff --git a/studio/app_test.go b/studio/app_test.go index 2209b14a..92795d53 100644 --- a/studio/app_test.go +++ b/studio/app_test.go @@ -247,6 +247,7 @@ func TestApp_ListDir_MissingDirReturnsEmpty(t *testing.T) { func TestApp_ListDir_FiltersHiddenEntries(t *testing.T) { a := NewApp() dir := t.TempDir() + a.SetWorkspace(dir) // the page opens the workspace before it lists it for _, name := range []string{"visible.probe", ".hidden"} { if err := os.WriteFile(filepath.Join(dir, name), []byte("x"), 0o644); err != nil { t.Fatal(err) diff --git a/studio/automation.go b/studio/automation.go index 66ddafe7..98cc7c95 100644 --- a/studio/automation.go +++ b/studio/automation.go @@ -187,13 +187,25 @@ func (s *automationServer) handleRPC(w http.ResponseWriter, r *http.Request) { // decided with filepath.Rel, so a sibling such as /ws-evil is not inside /ws and a symlink inside // a root that points out of it does not get out. func withinRoots(roots []string, p string) (string, error) { + return resolveInRoots(roots, p, false) +} + +// resolveInRoots is the one rule for every file and folder Studio touches, for the person in the window and for the +// automation endpoint alike: the path must be absolute, must have no ".." segment on either separator, and after its +// symlinks are resolved it must be one of the roots or inside one. With create set, the final part may not exist yet +// (a new file, or the folders above it): the nearest existing parent is resolved and checked, and the missing part is +// taken as written, which cannot hold a symlink. The resolved path is returned, and callers use that, not p. +func resolveInRoots(roots []string, p string, create bool) (string, error) { if len(roots) == 0 { return "", fmt.Errorf("no folder is open to automation: open a workspace in Studio, or set PROBE_STUDIO_WORKSPACE or PROBE_STUDIO_AUTOMATION_ROOTS") } if !filepath.IsAbs(p) { return "", fmt.Errorf("%s: use an absolute path", p) } - real, err := filepath.EvalSymlinks(p) + if hasDotDotSegment(p) { + return "", fmt.Errorf("path traversal rejected: %s", p) + } + real, err := resolveExisting(filepath.Clean(p), create) if err != nil { return "", fmt.Errorf("%s: %v", p, err) } @@ -214,6 +226,51 @@ func withinRoots(roots []string, p string) (string, error) { return "", fmt.Errorf("%s is outside the folders automation may use", p) } +// resolveExisting returns clean with every symlink that exists resolved. With create set, the parts that do not exist +// yet are appended to the resolved nearest existing parent; a missing part cannot be a symlink, so nothing is hidden. +func resolveExisting(clean string, create bool) (string, error) { + if !create { + return filepath.EvalSymlinks(clean) + } + existing, missing := clean, []string{} + for { + _, err := os.Lstat(existing) + if err == nil { + break + } + if !os.IsNotExist(err) { + return "", err + } + parent := filepath.Dir(existing) + if parent == existing { + return "", fmt.Errorf("no existing folder above it") + } + missing = append([]string{filepath.Base(existing)}, missing...) + existing = parent + } + real, err := filepath.EvalSymlinks(existing) + if err != nil { + return "", err + } + return filepath.Join(append([]string{real}, missing...)...), nil +} + +// probeFileForWriteWithinRoots is the write side of probeFileWithinRoots: the file may be new, its folders may be new, +// and its name must end in .probe. A path that is an existing folder is refused. +func probeFileForWriteWithinRoots(roots []string, p string) (string, error) { + real, err := resolveInRoots(roots, p, true) + if err != nil { + return "", err + } + if !strings.HasSuffix(real, ".probe") { + return "", fmt.Errorf("%s: only .probe files can be written", p) + } + if info, err := os.Lstat(real); err == nil && info.IsDir() { + return "", fmt.Errorf("%s is a folder", p) + } + return real, nil +} + // probeFileWithinRoots is withinRoots for a .probe file; the suffix is checked on the // resolved path, so a link named x.probe that points at another file does not pass. func probeFileWithinRoots(roots []string, p string) (string, error) { diff --git a/studio/frontend/src/main.ts b/studio/frontend/src/main.ts index b1046d0e..d7d4abca 100644 --- a/studio/frontend/src/main.ts +++ b/studio/frontend/src/main.ts @@ -242,6 +242,8 @@ async function onFileClick(path: string, isDir: boolean) { $("btn-files-up").addEventListener("click", () => { if (currentDir === "." || currentDir === "/") return; const parent = currentDir.split("/").slice(0, -1).join("/") || "."; + // Studio lists only inside the open workspace, so the up button stops at its root. + if (workspaceRoot && !(parent === workspaceRoot || parent.startsWith(`${workspaceRoot}/`))) return; refreshFiles(parent); }); diff --git a/studio/write_list_roots_test.go b/studio/write_list_roots_test.go new file mode 100644 index 00000000..cbc1b693 --- /dev/null +++ b/studio/write_list_roots_test.go @@ -0,0 +1,193 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// WriteFile and ListDir follow the rule ReadFile and RunFile follow: an absolute path, no ".." segment on either +// separator, every symlink resolved, and a place inside the workspace a person opened or an automation folder. + +func TestWriteFileWritesANewProbeFileInsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + path := filepath.Join(workspace, "flows", "login", "smoke.probe") // the folders do not exist yet + if err := a.WriteFile(path, "test \"x\"\n"); err != nil { + t.Fatalf("WriteFile inside the workspace: %v", err) + } + if got, err := os.ReadFile(path); err != nil || string(got) != "test \"x\"\n" { + t.Fatalf("the file is not where WriteFile put it: %q, %v", got, err) + } +} + +func TestWriteFileRefusesPathsOutsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + a := &App{} + a.SetWorkspace(workspace) + if err := a.WriteFile(outside, "x"); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a new .probe file outside the workspace: err %v; want the outside-the-folders refusal", err) + } + if _, err := os.Stat(outside); !os.IsNotExist(err) { + t.Errorf("a refused write still created the file: %v", err) + } +} + +func TestWriteFileRefusesANonProbeName(t *testing.T) { + workspace := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + if err := a.WriteFile(filepath.Join(workspace, "notes.txt"), "x"); err == nil || !strings.Contains(err.Error(), "only .probe files can be written") { + t.Errorf("a non-.probe name: err %v; want the suffix refusal", err) + } +} + +// Both separators count, and a ".." that would stay inside the workspace is refused too. +func TestWriteFileRefusesDotDotSegments(t *testing.T) { + workspace := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + sep := string(filepath.Separator) + for _, p := range []string{ + workspace + sep + ".." + sep + "evil.probe", + workspace + sep + "sub" + sep + ".." + sep + "smoke.probe", + workspace + "/..\\evil.probe", + } { + if err := a.WriteFile(p, "x"); err == nil || !strings.Contains(err.Error(), "path traversal rejected") { + t.Errorf("WriteFile(%q): err %v; want the path traversal refusal", p, err) + } + } +} + +func TestWriteFileRefusesAFileThatLinksOut(t *testing.T) { + workspace := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere.probe") + if err := os.WriteFile(outside, []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(workspace, "link.probe") + if err := os.Symlink(outside, link); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if err := a.WriteFile(link, "overwritten"); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a link out of the workspace: err %v; want the outside-the-folders refusal", err) + } + if got, _ := os.ReadFile(outside); string(got) != "original" { + t.Errorf("the file outside the workspace was changed through the link: %q", got) + } +} + +// The folders may not exist yet, so a new file under a folder that is a link out of the workspace must be refused. +func TestWriteFileRefusesANewFileUnderAFolderThatLinksOut(t *testing.T) { + workspace := t.TempDir() + outsideDir := t.TempDir() + if err := os.Symlink(outsideDir, filepath.Join(workspace, "linked")); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + path := filepath.Join(workspace, "linked", "new.probe") + if err := a.WriteFile(path, "x"); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a new file under a linked folder: err %v; want the outside-the-folders refusal", err) + } + if _, err := os.Stat(filepath.Join(outsideDir, "new.probe")); !os.IsNotExist(err) { + t.Errorf("a file was created outside the workspace through the link: %v", err) + } +} + +func TestWriteFileNeedsAnAbsolutePath(t *testing.T) { + workspace := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + if err := a.WriteFile("smoke.probe", "x"); err == nil || !strings.Contains(err.Error(), "use an absolute path") { + t.Errorf("a relative path: err %v; want the absolute-path refusal", err) + } +} + +func TestWriteFileRefusesAFolderNamedLikeAProbeFile(t *testing.T) { + workspace := t.TempDir() + if err := os.Mkdir(filepath.Join(workspace, "folder.probe"), 0o755); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + if err := a.WriteFile(filepath.Join(workspace, "folder.probe"), "x"); err == nil || !strings.Contains(err.Error(), "is a folder") { + t.Errorf("a folder with a .probe name: err %v; want the is-a-folder refusal", err) + } +} + +func TestListDirListsTheWorkspace(t *testing.T) { + workspace := t.TempDir() + if err := os.WriteFile(filepath.Join(workspace, "smoke.probe"), []byte("test \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(workspace, "flows"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, ".hidden"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + a := &App{} + a.SetWorkspace(workspace) + entries, err := a.ListDir(workspace) + if err != nil { + t.Fatalf("ListDir of the workspace: %v", err) + } + seen := map[string]bool{} + for _, e := range entries { + seen[e.Name] = true + if e.Path != filepath.Join(workspace, e.Name) { + t.Errorf("entry %q has path %q, want the path under the folder it was listed from", e.Name, e.Path) + } + } + if len(entries) != 2 || !seen["smoke.probe"] || !seen["flows"] { + t.Errorf("ListDir of the workspace: got %v; want smoke.probe and flows, hidden entries left out", entries) + } +} + +func TestListDirRefusesFoldersOutsideTheWorkspace(t *testing.T) { + workspace := t.TempDir() + outside := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + if _, err := a.ListDir(outside); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a folder outside the workspace: err %v; want the outside-the-folders refusal", err) + } + // A missing folder outside is refused the same way, so the answer does not reveal what exists there. + if _, err := a.ListDir(filepath.Join(outside, "missing")); err == nil || !strings.Contains(err.Error(), "outside the folders automation may use") { + t.Errorf("a missing folder outside the workspace: err %v; want the outside-the-folders refusal", err) + } +} + +func TestListDirRefusesDotDotSegments(t *testing.T) { + workspace := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + if _, err := a.ListDir(workspace + string(filepath.Separator) + ".."); err == nil || !strings.Contains(err.Error(), "path traversal rejected") { + t.Errorf("a .. folder: err %v; want the path traversal refusal", err) + } +} + +func TestListDirShowsAMissingFolderInsideTheWorkspaceAsEmpty(t *testing.T) { + workspace := t.TempDir() + a := &App{} + a.SetWorkspace(workspace) + if entries, err := a.ListDir(filepath.Join(workspace, "not-yet")); err != nil || len(entries) != 0 { + t.Errorf("a missing folder inside the workspace: entries %v, err %v; want an empty list and no error", entries, err) + } +} + +// Before a workspace is open the file tree starts on a relative folder; there is nothing to list yet. +func TestListDirWithNoWorkspaceListsNothing(t *testing.T) { + t.Setenv("PROBE_STUDIO_WORKSPACE", "") + t.Setenv("PROBE_STUDIO_AUTOMATION_ROOTS", "") + a := &App{} + if entries, err := a.ListDir("tests"); err != nil || len(entries) != 0 { + t.Errorf("no workspace open: entries %v, err %v; want an empty list and no error", entries, err) + } +} From bd807eda89a0eb7e09af227b5f2f71cafbc095ce Mon Sep 17 00:00:00 2001 From: Patrick Bertsch Date: Mon, 26 Oct 2026 21:08:13 -0600 Subject: [PATCH 4/4] fix(studio): keep WiFi tokens in the OS keychain, one item per device A discovered device's WiFi token is saved in the platform keychain under the device's name, so the next connection needs no paste. ConnectWiFiSaved reads the token in Go, and the window never receives it. A token is saved only after ConnectWiFi accepts it. The plain-text copy an earlier version kept in localStorage is deleted on start. Tests cover the per-device round trip, refusal of an empty device name or token, and that ConnectWiFiSaved runs the same target checks as ConnectWiFi. --- CHANGELOG.md | 2 +- studio/frontend/src/main.ts | 78 ++++++++++++++++++++---- studio/frontend/wailsjs/go/main/App.d.ts | 8 +++ studio/frontend/wailsjs/go/main/App.js | 16 +++++ studio/keychain.go | 21 +++++++ studio/wifi_token.go | 47 ++++++++++++++ studio/wifi_token_test.go | 70 +++++++++++++++++++++ website/src/content/docs/tools/studio.md | 4 +- 8 files changed, 232 insertions(+), 14 deletions(-) create mode 100644 studio/wifi_token.go create mode 100644 studio/wifi_token_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cba092d..41ad0bf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,7 +66,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Security - **Studio no longer returns the Anthropic API key to the window.** The key panel shows whether a key is saved and starts with an empty field. Saving an empty field no longer clears a saved key; use Remove key to delete it. -- **Studio no longer remembers WiFi tokens.** The token is typed for each connection. Studio deletes the plain-text copy that an earlier version kept in local storage on first start. +- **Studio keeps each WiFi token in the OS keychain, under its device, and never returns it to the window.** A device with a saved token shows a 🔑 saved tag, and connecting to it needs no paste. Its ✕ control forgets the token. Studio deletes the plain-text copy that an earlier version kept in local storage on first start. - **Studio's Run File accepts only `.probe` files inside the open workspace or an automation folder.** Other paths are refused before anything runs, as the automation endpoint already refused them. - **Studio opens only `.probe` files inside the open workspace or an automation folder.** The editor's open action uses the same check as Run File, and a path with `..` is refused. - **Studio saves `.probe` files and lists folders only inside the open workspace or an automation folder.** Save and the file tree use the same rule as Read File and Run File, including the `..` and symlink checks. With no workspace open, the file tree is empty. diff --git a/studio/frontend/src/main.ts b/studio/frontend/src/main.ts index cfd9a19e..4e77c926 100644 --- a/studio/frontend/src/main.ts +++ b/studio/frontend/src/main.ts @@ -12,8 +12,10 @@ import { Chat, Connect, ConnectWiFi, + ConnectWiFiSaved, DeleteAPIKey, Disconnect, + ForgetWiFiToken, GetAPIKey, InitialWorkspace, SetWorkspace, @@ -24,6 +26,7 @@ import { PickWorkspace, ReadFile, RunFile, + SaveWiFiToken, SaveWorkspaceSettings, SetAPIKey, StartRecording, @@ -32,6 +35,7 @@ import { StopRecording, StopWiFiDiscovery, TestWiFi, + WiFiTokenSaved, WriteFile, } from "../wailsjs/go/main/App"; import { main } from "../wailsjs/go/models"; @@ -415,8 +419,8 @@ $("btn-settings-save").addEventListener("click", async () => { type WiFiDevice = { name: string; host: string; port: number; version: string }; -// Earlier versions kept WiFi tokens in plain text in localStorage. Studio no longer remembers tokens, so remove that copy -// on start. +// Earlier versions kept WiFi tokens in plain text in localStorage. Those copies are removed on start; tokens now live in +// the OS keychain, one item per device. try { localStorage.removeItem("fp.studio.wifi-tokens"); } catch { @@ -449,18 +453,57 @@ function closeWiFiOverlay() { StopWiFiDiscovery(); } -function selectWiFiDevice(dev: WiFiDevice) { +async function selectWiFiDevice(dev: WiFiDevice) { wifiSelected = dev; for (const li of Array.from($("wifi-devices").children) as HTMLElement[]) { li.classList.toggle("selected", li.dataset.key === `${dev.host}:${dev.port}`); } - $("wifi-selected-label").textContent = `Connecting to ${dev.name} (${dev.host}:${dev.port})`; ($("wifi-token-row") as HTMLElement).hidden = false; const input = $("wifi-token-input") as HTMLInputElement; input.value = ""; + // A saved token is used when the field is left blank. The window never receives the saved token itself. + const saved = await savedWiFiToken(dev.name); + if (wifiSelected !== dev) return; + $("wifi-selected-label").textContent = + `Connecting to ${dev.name} (${dev.host}:${dev.port})` + + (saved ? " · a saved token is used if the field is blank" : ""); input.focus(); } +// Whether a WiFi token is saved for the device. A keychain that cannot be read counts as none saved. +async function savedWiFiToken(device: string): Promise { + try { + return await WiFiTokenSaved(device); + } catch { + return false; + } +} + +// Marks a device that has a saved token, with the ✕ control that forgets it. +async function markSavedWiFi(li: HTMLElement, dev: WiFiDevice) { + if (!(await savedWiFiToken(dev.name))) return; + const tag = document.createElement("span"); + tag.className = "wifi-saved-tag"; + tag.textContent = "🔑 saved"; + tag.title = "A token is saved for this device"; + const forget = document.createElement("button"); + forget.className = "btn-mini wifi-forget"; + forget.title = `Forget the saved token for ${dev.name}`; + forget.textContent = "✕"; + forget.addEventListener("click", async (e) => { + e.stopPropagation(); + try { + await ForgetWiFiToken(dev.name); + tag.remove(); + forget.remove(); + toast(`Forgot the token for ${dev.name}`, "info", 1500); + } catch (err) { + toast(`Could not forget the token: ${err}`, "error"); + } + }); + li.append(tag, forget); +} + EventsOn("wifi:device-found", (dev: WiFiDevice) => { const key = `${dev.host}:${dev.port}`; if (wifiSeen.has(key)) return; @@ -479,6 +522,7 @@ EventsOn("wifi:device-found", (dev: WiFiDevice) => { hostEl.className = "wifi-host"; hostEl.textContent = `${dev.host}:${dev.port}${dev.version ? ` · v${dev.version}` : ""}`; li.append(nameEl, hostEl); + void markSavedWiFi(li, dev); li.addEventListener("click", () => selectWiFiDevice(dev)); list.appendChild(li); }); @@ -555,19 +599,31 @@ $("btn-wifi-manual-test").addEventListener("click", () => manualWiFi(false)); $("btn-wifi-manual-connect").addEventListener("click", () => manualWiFi(true)); $("btn-wifi-connect").addEventListener("click", async () => { - if (!wifiSelected) return; - const token = ($("wifi-token-input") as HTMLInputElement).value.trim(); - if (!token) { + const dev = wifiSelected; + if (!dev) return; + const typed = ($("wifi-token-input") as HTMLInputElement).value.trim(); + const useSaved = !typed && (await savedWiFiToken(dev.name)); + if (!typed && !useSaved) { toast("Token is required.", "error"); return; } - setStatus("connecting", `Connecting to ${wifiSelected.host}…`); + setStatus("connecting", `Connecting to ${dev.host}…`); try { - await ConnectWiFi(wifiSelected.host, wifiSelected.port, token); + if (useSaved) { + await ConnectWiFiSaved(dev.host, dev.port, dev.name); + } else { + await ConnectWiFi(dev.host, dev.port, typed); + // Keep the typed token only once it has connected. A keychain that refuses the save does not undo the connection. + try { + await SaveWiFiToken(dev.name, typed); + } catch { + toast("Connected, but the token could not be saved to the keychain.", "info", 6000); + } + } setStatus( "connected", - `Connected · ${wifiSelected.name}`, - `${wifiSelected.name} (${wifiSelected.host}:${wifiSelected.port}) · WiFi transport` + `Connected · ${dev.name}`, + `${dev.name} (${dev.host}:${dev.port}) · WiFi transport` ); closeWiFiOverlay(); startStream(); diff --git a/studio/frontend/wailsjs/go/main/App.d.ts b/studio/frontend/wailsjs/go/main/App.d.ts index ea464186..c849eed1 100755 --- a/studio/frontend/wailsjs/go/main/App.d.ts +++ b/studio/frontend/wailsjs/go/main/App.d.ts @@ -12,10 +12,14 @@ export function Connect(arg1:string):Promise; export function ConnectWiFi(arg1:string,arg2:number,arg3:string):Promise; +export function ConnectWiFiSaved(arg1:string,arg2:number,arg3:string):Promise; + export function DeleteAPIKey():Promise; export function Disconnect():Promise; +export function ForgetWiFiToken(arg1:string):Promise; + export function GetAPIKey():Promise; export function GetDeviceSettings():Promise; @@ -44,6 +48,8 @@ export function RunFileAsync(arg1:string):Promise; export function RunState():Promise; +export function SaveWiFiToken(arg1:string,arg2:string):Promise; + export function SaveWorkspaceSettings(arg1:string,arg2:main.WorkspaceSettings):Promise; export function SetAPIKey(arg1:string):Promise; @@ -64,4 +70,6 @@ export function TakeScreenshot():Promise; export function TestWiFi(arg1:string,arg2:number,arg3:string):Promise; +export function WiFiTokenSaved(arg1:string):Promise; + export function WriteFile(arg1:string,arg2:string):Promise; diff --git a/studio/frontend/wailsjs/go/main/App.js b/studio/frontend/wailsjs/go/main/App.js index ab0ad82b..f932b256 100755 --- a/studio/frontend/wailsjs/go/main/App.js +++ b/studio/frontend/wailsjs/go/main/App.js @@ -22,6 +22,10 @@ export function ConnectWiFi(arg1, arg2, arg3) { return window['go']['main']['App']['ConnectWiFi'](arg1, arg2, arg3); } +export function ConnectWiFiSaved(arg1, arg2, arg3) { + return window['go']['main']['App']['ConnectWiFiSaved'](arg1, arg2, arg3); +} + export function DeleteAPIKey() { return window['go']['main']['App']['DeleteAPIKey'](); } @@ -30,6 +34,10 @@ export function Disconnect() { return window['go']['main']['App']['Disconnect'](); } +export function ForgetWiFiToken(arg1) { + return window['go']['main']['App']['ForgetWiFiToken'](arg1); +} + export function GetAPIKey() { return window['go']['main']['App']['GetAPIKey'](); } @@ -86,6 +94,10 @@ export function RunState() { return window['go']['main']['App']['RunState'](); } +export function SaveWiFiToken(arg1, arg2) { + return window['go']['main']['App']['SaveWiFiToken'](arg1, arg2); +} + export function SaveWorkspaceSettings(arg1, arg2) { return window['go']['main']['App']['SaveWorkspaceSettings'](arg1, arg2); } @@ -126,6 +138,10 @@ export function TestWiFi(arg1, arg2, arg3) { return window['go']['main']['App']['TestWiFi'](arg1, arg2, arg3); } +export function WiFiTokenSaved(arg1) { + return window['go']['main']['App']['WiFiTokenSaved'](arg1); +} + export function WriteFile(arg1, arg2) { return window['go']['main']['App']['WriteFile'](arg1, arg2); } diff --git a/studio/keychain.go b/studio/keychain.go index 47cd1da0..d00d0843 100644 --- a/studio/keychain.go +++ b/studio/keychain.go @@ -26,3 +26,24 @@ func keychainHas(service, user string) (bool, error) { } return true, nil } + +// keychainGetItem returns the value saved under user, or "" when nothing is saved. +func keychainGetItem(user string) (string, error) { + value, err := keyring.Get(keychainService, user) + if errors.Is(err, keyring.ErrNotFound) { + return "", nil + } + return value, err +} + +// keychainSetItem saves value under user. +func keychainSetItem(user, value string) error { return keyring.Set(keychainService, user, value) } + +// keychainDeleteItem removes the item saved under user. A missing item is not an error. +func keychainDeleteItem(user string) error { + err := keyring.Delete(keychainService, user) + if errors.Is(err, keyring.ErrNotFound) { + return nil + } + return err +} diff --git a/studio/wifi_token.go b/studio/wifi_token.go new file mode 100644 index 00000000..46739b3d --- /dev/null +++ b/studio/wifi_token.go @@ -0,0 +1,47 @@ +package main + +import ( + "fmt" + "strings" +) + +// wifiTokenUser is the keychain account that holds one device's WiFi token. The device name is its Bonjour instance +// name, which stays the same when its IP address changes, so a saved token follows the device. +func wifiTokenUser(device string) string { return "wifi-token:" + device } + +// WiFiTokenSaved reports whether a WiFi token is saved for the named device. The token itself is not returned: +// ConnectWiFiSaved reads it in Go when the device is connected. +func (a *App) WiFiTokenSaved(device string) (bool, error) { + if strings.TrimSpace(device) == "" { + return false, nil + } + return keychainHas(keychainService, wifiTokenUser(device)) +} + +// SaveWiFiToken keeps a WiFi token for the named device in the platform keychain. The window calls it only after +// ConnectWiFi has accepted the token, so a mistyped token is never saved. +func (a *App) SaveWiFiToken(device, token string) error { + token = strings.TrimSpace(token) + if strings.TrimSpace(device) == "" || token == "" { + return fmt.Errorf("a device name and a token are both needed") + } + return keychainSetItem(wifiTokenUser(device), token) +} + +// ForgetWiFiToken removes the WiFi token saved for the named device. +func (a *App) ForgetWiFiToken(device string) error { + return keychainDeleteItem(wifiTokenUser(device)) +} + +// ConnectWiFiSaved connects to a device with the token saved for it. The token is read here, in Go, so the window +// never receives it. Every check ConnectWiFi makes still applies, and its errors do not repeat the token. +func (a *App) ConnectWiFiSaved(host string, port int, device string) (ConnectionStatus, error) { + token, err := keychainGetItem(wifiTokenUser(device)) + if err != nil { + return ConnectionStatus{}, fmt.Errorf("the saved token for %s could not be read: %v", device, err) + } + if token == "" { + return ConnectionStatus{}, fmt.Errorf("no token is saved for %s; paste its token once to save it", device) + } + return a.ConnectWiFi(host, port, token) +} diff --git a/studio/wifi_token_test.go b/studio/wifi_token_test.go new file mode 100644 index 00000000..f145c1cd --- /dev/null +++ b/studio/wifi_token_test.go @@ -0,0 +1,70 @@ +package main + +import ( + "context" + "strings" + "testing" + + "github.com/zalando/go-keyring" +) + +// A WiFi token is kept for each device in the platform keychain. keyring.MockInit keeps the test off the login keychain. +func TestWiFiTokenIsSavedPerDeviceAndForgottenOneAtATime(t *testing.T) { + keyring.MockInit() + a := &App{} + + if saved, err := a.WiFiTokenSaved("Pat's phone"); err != nil || saved { + t.Fatalf("nothing saved yet: saved %v, err %v; want false and no error", saved, err) + } + if err := a.SaveWiFiToken("Pat's phone", goodToken); err != nil { + t.Fatal(err) + } + if err := a.SaveWiFiToken("Desk phone", goodToken); err != nil { + t.Fatal(err) + } + if saved, _ := a.WiFiTokenSaved("Pat's phone"); !saved { + t.Error("a token was saved for Pat's phone, but WiFiTokenSaved reports none") + } + if saved, _ := a.WiFiTokenSaved("Kitchen tablet"); saved { + t.Error("a device with no saved token reports one") + } + if err := a.ForgetWiFiToken("Pat's phone"); err != nil { + t.Fatal(err) + } + if saved, _ := a.WiFiTokenSaved("Pat's phone"); saved { + t.Error("after forgetting, Pat's phone still reports a saved token") + } + if saved, _ := a.WiFiTokenSaved("Desk phone"); !saved { + t.Error("forgetting one device removed the token saved for another") + } +} + +func TestSaveWiFiTokenRefusesAnEmptyDeviceOrToken(t *testing.T) { + keyring.MockInit() + a := &App{} + if err := a.SaveWiFiToken("", goodToken); err == nil { + t.Error("a token with no device name was saved") + } + if err := a.SaveWiFiToken("Desk phone", " "); err == nil { + t.Error("a blank token was saved") + } +} + +// ConnectWiFiSaved needs a saved token, and it runs the same target checks as ConnectWiFi: a public address is refused +// before anything is dialled, and the refusal does not repeat the token. +func TestConnectWiFiSavedUsesTheSavedTokenAndTheSameChecks(t *testing.T) { + keyring.MockInit() + t.Setenv(allowPublicEnv, "") + a := &App{ctx: context.Background(), lookupHost: stubResolver(nil)} + + if _, err := a.ConnectWiFiSaved("8.8.8.8", 48686, "Desk phone"); err == nil || !strings.Contains(err.Error(), "no token is saved") { + t.Fatalf("no saved token: err %v; want a message that no token is saved", err) + } + if err := a.SaveWiFiToken("Desk phone", goodToken); err != nil { + t.Fatal(err) + } + _, err := a.ConnectWiFiSaved("8.8.8.8", 48686, "Desk phone") + if err == nil || strings.Contains(err.Error(), goodToken) { + t.Fatalf("public target with a saved token: err %v; want a refusal that does not repeat the token", err) + } +} diff --git a/website/src/content/docs/tools/studio.md b/website/src/content/docs/tools/studio.md index 439de61e..adc4d076 100644 --- a/website/src/content/docs/tools/studio.md +++ b/website/src/content/docs/tools/studio.md @@ -80,7 +80,7 @@ The runner is a library, not a subprocess. The connection-stability work in v0.6 Studio connects to physical iOS and Android devices through the exact same `internal/device` package the `probe` CLI uses — there is no separate device-management code path for Studio. - **USB** — plug in the device, launch the app with `--dart-define=PROBE_AGENT=true`, click ↻ to refresh the device picker, and pick the entry tagged `physical`. Physical iOS gets an `iproxy` tunnel automatically before Studio dials the agent; physical Android goes through `adb forward`, the same path as emulators. Connection errors for a missing `iproxy` or `adb` binary surface inline with an actionable fix (e.g. "install via: brew install libimobiledevice"). -- **WiFi (preferred for iOS)** — matching the CLI's own documented preference, WiFi is more stable than USB-C for physical iOS: a USB-C cable's charge/data mode-flipping can drop the `iproxy` tunnel mid-run, where WiFi has no such failure mode. Build the Flutter app with `--dart-define=PROBE_WIFI=true` and pass an advertiser to `ProbeAgent.start` (see [WiFi auto-discovery](#wifi-auto-discovery)), click the 📡 button to open the WiFi discovery overlay, and pick the device once it appears via mDNS (`_flutterprobe._tcp`). Paste the token printed in the app's `PROBE_TOKEN=...` log line — Studio does not keep it: paste it again each time. +- **WiFi (preferred for iOS)** — matching the CLI's own documented preference, WiFi is more stable than USB-C for physical iOS: a USB-C cable's charge/data mode-flipping can drop the `iproxy` tunnel mid-run, where WiFi has no such failure mode. Build the Flutter app with `--dart-define=PROBE_WIFI=true` and pass an advertiser to `ProbeAgent.start` (see [WiFi auto-discovery](#wifi-auto-discovery)), click the 📡 button to open the WiFi discovery overlay, and pick the device once it appears via mDNS (`_flutterprobe._tcp`). Paste the token printed in the app's `PROBE_TOKEN=...` log line. Studio keeps it in the OS keychain for that device, so the next connection needs no paste. A device with a saved token shows a 🔑 saved tag, and its ✕ control forgets the token. - The exact conditions for a physical iPhone over WiFi, with a three-command check, are in [Physical iPhone over WiFi](/platform/ios/#physical-iphone-over-wifi-cli-and-studio-what-must-be-true). - Devices that don't advertise over mDNS (no advertiser in the build, older agent versions, or networks that block mDNS): use **Connect manually** in the WiFi overlay (host, port, token) — see [Manual WiFi entry](#manual-wifi-entry) — or the CLI's `--host --token `. @@ -173,7 +173,7 @@ Studio bugs go to the same tracker as the CLI: [github.com/AlphaWaveSystems/flut The 📡 overlay has a **Connect manually (host, port, token)** section for a phone that does not show up in the discovered list. - **Host**: an IPv4 address, an IPv6 address or a hostname (such as `iphone.local`): no `ws://`, path or port. **Port**: 1 to 65535, default 48686 (the port the app printed as `PROBE_PORT=` if it moved). **Token**: the value after `PROBE_TOKEN=` in the app's console (8 to 128 letters, digits and `_ . ~ -`; without the `PROBE_TOKEN=` part). -- **Test connection** dials the agent and pings it without making it the active connection, and says whether the agent is reachable and the token accepted. **Connect** does the same and then connects. The host and port are remembered for next time; the token never is. +- **Test connection** dials the agent and pings it without making it the active connection, and says whether the agent is reachable and the token accepted. **Connect** does the same and then connects. The host and port are remembered for next time. A manually entered device does not save its token, so paste it each time; a device found over mDNS keeps its token in the keychain. - When it fails, the message says what to check: the phone and this computer on the same network (and `curl http://:/probe/status` from this computer), the app in the foreground with the screen unlocked, a **profile build made with `--dart-define=PROBE_AGENT=true` and `--dart-define=PROBE_WIFI=true`** (a production or debug build does not listen), the port, and a fresh token (a new one is created whenever the app restarts). - An agent can do the same through the automation endpoint (`test_wifi`, `connect_wifi`) or the MCP tool `studio_connect` with `host`, `port` and `token` (and `test_only` to only check).