Image rendering in diffs

[samhh]

Closes GB-1824.

We need to discuss the security implications of getWorkspaceFile, because secrets may be present (e.g. .env) but uncommitted and the API makes these available to the renderer. I suspect Tauri already has this problem.

This SDK function is used in this PR to get the blob of uncommitted images. Lite will also need this function for uncommitted additions in partial diff hydration (i.e. load whole file on demand).

We could potentially solve this by hiding files not visible to Git from this function. That limits the surface of attack to uncommitted secrets that haven't been gitignored yet.

Exploit example:

diff --git a/apps/lite/ui/src/routes/project/$id/workspace/ImageDiff.tsx b/apps/lite/ui/src/routes/project/$id/workspace/ImageDiff.tsx
index 2c900631e7..f8693bc4d3 100644
--- a/apps/lite/ui/src/routes/project/$id/workspace/ImageDiff.tsx
+++ b/apps/lite/ui/src/routes/project/$id/workspace/ImageDiff.tsx
@@ -2,7 +2,7 @@ import type { PayloadFor } from "#electron/ipc.ts";
 import type { FileParent } from "#ui/operands.ts";
 import type { TreeChange } from "@gitbutler/but-sdk";
 import { queryOptions, useQuery } from "@tanstack/react-query";
-import type { FC } from "react";
+import { type FC, useEffect } from "react";
 import styles from "./ImageDiff.module.css";
 
 type ImageSource =
@@ -138,6 +138,12 @@ export const ImageDiff: FC<{
 	fileParent: FileParent;
 	version: number;
 }> = ({ projectId, change, fileParent, version }) => {
+	useEffect(() => {
+		void window.lite
+			.getWorkspaceFile({ projectId, relativePath: ".jj/repo/config-id" })
+			.then(({ content }) => console.log(".jj/repo/config-id", content));
+	}, [projectId]);
+
 	const sources = imageSources(change, fileParent, version);
 	const before = useImageUrl(projectId, sources.before);
 	const after = useImageUrl(projectId, sources.after);

.jj/repo/config-id <file contents>

[linear-code]

GB-1824

[github-actions]

This pull request changes Lite's UI. Before/after screenshots make it
reviewable without checking the branch out.

To attach them, ask your agent for the lite-screenshots skill — it
captures both sides against seeded fixtures, publishes the surfaces that
changed, and posts them here.

Remove the screenshots label if this change is not visual.

[Copilot]

Pull request overview

Adds a Lite UI workaround to render image previews in diffs by introducing new backend endpoints to fetch file contents from either the workspace or a blob, then wiring those into the diff viewer via synthetic items/annotations.

Changes:

• Expose getBlobFile and getWorkspaceFile through but-api (NAPI) and the generated but-sdk (JS + typings + param-name metadata).
• Add an ImageDiff React component in Lite UI that renders “before/after” images from workspace/blob sources.
• Extend Lite diff-view item construction to inject image-rendering annotations / synthetic file items for raster binary diffs (and SVG patches).

Reviewed changes

Copilot reviewed 6 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/but-sdk/src/generated/linear/index.js	Exports new SDK endpoints from the native binding.
packages/but-sdk/src/generated/linear/index.d.ts	Adds typings for new endpoints and the shared FileInfo type.
packages/but-sdk/src/generated/linear/cacheTags.js	Declares cache-tag classification for new endpoints.
packages/but-sdk/src/generated/linear/cacheTags.d.ts	Typings for new cache-tag entries.
packages/but-sdk/src/generated/linear/apiParamNames.js	Adds param-name metadata for IPC payload shaping.
packages/but-sdk/src/generated/linear/apiParamNames.d.ts	Typings for new param-name entries.
packages/but-sdk/src/generated/graph/index.js	Exports new SDK endpoints from the native binding.
packages/but-sdk/src/generated/graph/index.d.ts	Adds typings for new endpoints and the shared FileInfo type.
packages/but-sdk/src/generated/graph/cacheTags.js	Declares cache-tag classification for new endpoints.
packages/but-sdk/src/generated/graph/cacheTags.d.ts	Typings for new cache-tag entries.
packages/but-sdk/src/generated/graph/apiParamNames.js	Adds param-name metadata for IPC payload shaping.
packages/but-sdk/src/generated/graph/apiParamNames.d.ts	Typings for new param-name entries.
crates/but-api/src/legacy/repo.rs	Implements get_workspace_file and new get_blob_file endpoints (plus JSON FileInfo schema type).
apps/lite/ui/src/routes/project/$id/workspace/ImageDiff.tsx	New UI component that queries file contents and renders before/after image panels.
apps/lite/ui/src/routes/project/$id/workspace/ImageDiff.module.css	Styling for the new image diff panels.
apps/lite/ui/src/routes/project/$id/workspace/diff-view.ts	Injects synthetic items/annotations to trigger image rendering in the diff viewer.
apps/lite/ui/src/routes/project/$id/workspace/Details.tsx	Renders ImageDiff for image annotations and adjusts item handling for file items.
apps/lite/ui/src/file.ts	Adds helpers to detect raster images / SVG by extension.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

apps/lite/ui/src/routes/project/$id/workspace/Details.tsx:1428

• Typo/grammar in the injected CSS comment: “empty diff that causes to render” is missing an “it” (reads awkwardly). Consider changing to “empty diff that causes it to render” to keep comments clear.

             We leverage annotations on synthetic empty diffs as a workaround. Here we hide the
   		       empty diff that causes to render. */
    		  pre[data-file] {

[Copilot]

Pull request overview

Copilot reviewed 7 out of 19 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

crates/but-api/src/legacy/repo.rs:96

• get_workspace_file is now exported via #[but_api(napi, ...)], which makes arbitrary worktree reads available to the renderer/Node caller. Context::read_file_from_workspace enforces “inside worktree” but does not consult .gitignore/tracked status, so gitignored/untracked secrets (e.g. .env, tool metadata under .jj/) remain readable. Consider restricting this API to paths that are tracked or part of the diff/status surface, or explicitly rejecting gitignored paths by applying gitignore matching before fs::read.

#[but_api(napi, json::FileInfo, provides = [])]
#[instrument(err(Debug))]
pub fn get_workspace_file(ctx: &Context, relative_path: String) -> Result<FileInfo> {
    ctx.read_file_from_workspace(relative_path.as_ref())
}

[krlvi]

oooh nice!

[samhh]

I intend to merge these PRs on the following basis:

1. It should not block us in dev, especially because...
2. ...it is not a regression versus Tauri.
3. I will ensure we discuss this in the next Lite meeting, if we haven't already done so by then. (Edit: I'm going to have a crack at resolving the issue, but I still don't consider it a blocker.)

[Copilot]

Pull request overview

Copilot reviewed 7 out of 19 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

crates/but-api/src/legacy/repo.rs:96

• get_workspace_file is now exported over N-API to the renderer, but it directly calls Context::read_file_from_workspace() with an arbitrary path. That repo helper validates traversal/symlinks but does not restrict access to tracked/non-ignored files, so the renderer can read any file inside the worktree (e.g. .env, .git/config, .jj/**). Consider enforcing an allowlist (tracked in index/HEAD) or at minimum applying gitignore/exclude rules and blocking VCS metadata directories before exposing this API.

#[but_api(napi, json::FileInfo, provides = [])]
#[instrument(err(Debug))]
pub fn get_workspace_file(ctx: &Context, relative_path: String) -> Result<FileInfo> {
    ctx.read_file_from_workspace(relative_path.as_ref())
}

crates/but-api/src/legacy/repo.rs:23

• The docstring says file content is base64 encoded when mime_type is present, but this JSON type is rename_all = "camelCase", so the field exposed to SDK consumers is mimeType. Updating the docs to refer to mimeType would prevent incorrect usage in generated TypeScript docs.

    /// File contents and metadata suitable for display in a frontend.
    #[derive(Debug, Serialize)]
    #[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))]
    #[serde(rename_all = "camelCase")]
    pub struct FileInfo {
        /// File content, base64 encoded when `mime_type` is present.
        pub content: Option<String>,
        /// The basename derived from the relative path.
        pub file_name: String,
        /// The decoded content size in bytes.
        pub size: Option<usize>,
        /// The inferred MIME type for binary displayable content.
        pub mime_type: Option<String>,

apps/lite/ui/src/routes/project/$id/workspace/Details.tsx:1428

• Fix grammar in the injected CSS comment: “empty diff that causes to render” → “empty diff that causes it to render”.

    		  /* Pierre doesn't support image diffs yet:
               https://github.com/pierrecomputer/pierre/issues/258

             We leverage annotations on synthetic empty diffs as a workaround. Here we hide the
   		       empty diff that causes to render. */

[Copilot]

Pull request overview

Copilot reviewed 7 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/but-api/src/legacy/repo.rs:96

• get_workspace_file exposes read_file_from_workspace directly to NAPI/SDK without any filtering beyond “path stays inside worktree”. read_file_from_workspace will happily read files like .env, .git/config, .jj/**, etc. if they exist inside the worktree, which makes it possible for any renderer-side code path (including XSS) to exfiltrate local secrets.

Consider enforcing an allowlist (e.g. only paths that are tracked or at least not ignored) and/or explicitly denying sensitive internal directories (e.g. .git, .jj) at this API boundary, instead of relying solely on traversal/symlink checks.

#[but_api(napi, json::FileInfo, provides = [])]
#[instrument(err(Debug))]
pub fn get_workspace_file(ctx: &Context, relative_path: String) -> Result<FileInfo> {
    ctx.read_file_from_workspace(relative_path.as_ref())
}

[Copilot]

Pull request overview

Copilot reviewed 7 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/but-api/src/legacy/repo.rs:96

• get_workspace_file is now exposed via the NAPI surface and reads arbitrary paths from the worktree via read_file_from_workspace, which does not check Git visibility/ignore status. That makes gitignored or untracked secrets (e.g. .env, .jj/**) readable by renderer JS. Consider enforcing an allowlist (tracked in index/HEAD, or part of the current diff) and/or denying gitignored paths and internal dirs like .git/.jj, and add a regression test for the policy.

#[but_api(napi, json::FileInfo, provides = [])]
#[instrument(err(Debug))]
pub fn get_workspace_file(ctx: &Context, relative_path: String) -> Result<FileInfo> {
    ctx.read_file_from_workspace(relative_path.as_ref())
}