Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ jobs:
- name: Web tests
run: bun test

# Checks for in-app React UIs (currently the diff viewer; more cmux React
# surfaces will live alongside it). Add per-app steps here as they land.
# Checks for in-app React webviews (currently the diff viewer; more cmux React
# surfaces will live alongside it).
react-apps-check:
runs-on: ubuntu-latest
steps:
Expand All @@ -136,26 +136,26 @@ jobs:
- name: Setup Bun
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2

- name: Verify generated diff viewer assets
run: ./scripts/build-diff-viewer-app.sh --check
- name: Verify generated webviews assets
run: ./scripts/build-webviews-app.sh --check

- name: Verify React Compiler for diff viewer
run: ./scripts/check-diff-viewer-react-compiler.mjs
- name: Verify React Compiler for webviews
run: ./scripts/check-webviews-react-compiler.mjs

- name: Typecheck diff viewer
working-directory: diff-viewer
- name: Typecheck webviews
working-directory: webviews
run: bun run typecheck

- name: Test diff viewer
working-directory: diff-viewer
- name: Test webviews
working-directory: webviews
run: bun run test

- name: Oxlint diff viewer
working-directory: diff-viewer
- name: Oxlint webviews
working-directory: webviews
run: bun run lint:ci

- name: React Doctor
working-directory: diff-viewer
working-directory: webviews
run: bun run react-doctor:ci

web-db-migrations:
Expand Down
4 changes: 2 additions & 2 deletions CLI/cmux_open.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5587,7 +5587,7 @@ extension CMUXCLI {
try FileManager.default.createDirectory(at: targetDirectory, withIntermediateDirectories: true)

let appSourceDirectory = try diffViewerBundledAppAssetDirectory(nextTo: sourceDirectory)
let appAssetDirectoryName = "cmux-diff-viewer-app"
let appAssetDirectoryName = "cmux-webviews-app"
Comment thread
cursor[bot] marked this conversation as resolved.
let targetAppDirectory = viewerURL.deletingLastPathComponent()
.appendingPathComponent("assets", isDirectory: true)
.appendingPathComponent(appAssetDirectoryName, isDirectory: true)
Expand Down Expand Up @@ -5626,7 +5626,7 @@ extension CMUXCLI {
private func diffViewerBundledAppAssetDirectory(nextTo sourceDirectory: URL) throws -> URL {
let appDirectory = sourceDirectory
.deletingLastPathComponent()
.appendingPathComponent("diff-viewer-app", isDirectory: true)
.appendingPathComponent("webviews-app", isDirectory: true)
Comment on lines 5627 to +5629

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update Swift diff-viewer fixtures for the renamed app bundle

When the Swift CLI tests build their temporary fixture resources, writeTestDiffViewerAssets still creates markdown-viewer/diff-viewer-app, and several CMUXOpenCommandTests assertions still look for cmux-diff-viewer-app/main.mjs; with this lookup now requiring markdown-viewer/webviews-app, those tests either throw Bundled cmux diff viewer app assets not found before generating the viewer or fail the expected asset URL checks. Update the test fixtures/assertions in the same rename so the CLI test suite can pass.

Useful? React with 👍 / 👎.

.standardizedFileURL
let entry = appDirectory.appendingPathComponent("main.mjs", isDirectory: false)
var isDirectory: ObjCBool = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@
set -euo pipefail

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
SRC_DIR="$ROOT/diff-viewer"
OUT_DIR="$ROOT/Resources/markdown-viewer/diff-viewer-app"
SRC_DIR="$ROOT/webviews"
OUT_DIR="$ROOT/Resources/markdown-viewer/webviews-app"

if [ "${1:-}" = "--check" ]; then
tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' EXIT
(
cd "$SRC_DIR"
bun install --frozen-lockfile
CMUX_DIFF_VIEWER_OUT_DIR="$tmp_dir" bun run build
CMUX_WEBVIEWS_OUT_DIR="$tmp_dir" bun run build
)
diff_output="$(mktemp)"
set +e
Expand All @@ -22,10 +22,10 @@ if [ "${1:-}" = "--check" ]; then
cat "$diff_output" >&2
rm -f "$diff_output"
if [ "$diff_status" -eq 1 ]; then
echo "diff viewer app assets are stale; run ./scripts/build-diff-viewer-app.sh" >&2
echo "webviews app assets are stale; run ./scripts/build-webviews-app.sh" >&2
exit 1
fi
echo "failed to compare diff viewer assets (diff exit $diff_status)" >&2
echo "failed to compare webviews assets (diff exit $diff_status)" >&2
exit 2
fi
rm -f "$diff_output"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import { join } from "node:path";
import { fileURLToPath } from "node:url";

const root = join(fileURLToPath(new URL("..", import.meta.url)));
const bundlePath = join(root, "Resources", "markdown-viewer", "diff-viewer-app", "main.mjs");
const bundlePath = join(root, "Resources", "markdown-viewer", "webviews-app", "main.mjs");

const bundle = readFileSync(bundlePath, "utf8");
const compilerCacheCalls = bundle.match(/\b[A-Za-z_$][\w$]*\.c\(\d+\)/g) ?? [];
if (compilerCacheCalls.length < 8) {
console.error("React Compiler cache calls were not found in the generated diff viewer bundle.");
console.error("React Compiler cache calls were not found in the generated webviews bundle.");
process.exit(1);
}

console.log(`React Compiler enabled for diff viewer (${compilerCacheCalls.length} cache sites).`);
console.log(`React Compiler enabled for webviews (${compilerCacheCalls.length} cache sites).`);
10 changes: 5 additions & 5 deletions diff-viewer/README.md → webviews/README.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
# cmux Diff Viewer
# cmux Webviews

This is the source-owned React app for `cmux open diff`.
This is the source-owned React bundle for embedded cmux webviews. It currently ships the `cmux diff` viewer and is structured to host more React-backed webviews.

Build it with:

```sh
./scripts/build-diff-viewer-app.sh
./scripts/build-webviews-app.sh
```

The build output is committed under `Resources/markdown-viewer/diff-viewer-app` because the macOS app serves local static files from its bundled resources. Keep source changes in this directory, then regenerate the bundled asset with the script above.
The build output is committed under `Resources/markdown-viewer/webviews-app` because the macOS app serves local static files from its bundled resources. Keep source changes in this directory, then regenerate the bundled asset with the script above.

React Compiler is enabled in `vite.config.mjs` with the React 19 runtime target. Verify the compiled bundle guard with:

```sh
./scripts/check-diff-viewer-react-compiler.mjs
./scripts/check-webviews-react-compiler.mjs
```

Large public stress samples are available through:
Expand Down
23 changes: 22 additions & 1 deletion diff-viewer/bun.lock → webviews/bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

File renamed without changes.
6 changes: 4 additions & 2 deletions diff-viewer/package.json → webviews/package.json
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
{
"name": "@cmux/diff-viewer",
"name": "@cmux/webviews",
"version": "0.1.0",
"private": true,
"type": "module",
"scripts": {
"build": "bun run typecheck && vite build",
"build": "bun run verify:tanstack-router && bun run typecheck && vite build",
"lint": "oxlint . --react-plugin --jsx-a11y-plugin --import-plugin",
"lint:ci": "oxlint . --react-plugin --jsx-a11y-plugin --import-plugin --deny-warnings",
"lint:fix": "oxlint . --react-plugin --jsx-a11y-plugin --import-plugin --fix",
"test": "bun test",
"typecheck": "tsc --noEmit",
"verify:tanstack-router": "node scripts/verify-tanstack-router-security.mjs",
"react-doctor": "react-doctor . --full --no-score --fail-on none",
"react-doctor:ci": "react-doctor . --full --no-score --fail-on error"
},
"dependencies": {
"@pierre/diffs": "1.2.7",
"@pierre/trees": "1.0.0-beta.4",
"@tanstack/react-router": "1.170.11",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 @tanstack/react-router is declared but not imported anywhere in source

@tanstack/react-router is listed under dependencies, but no file in webviews/src/ imports it. Is this intentional pre-emptive pinning ahead of a follow-up PR that adds router usage, or was an import accidentally left out? If it's proactive pinning only, a comment in package.json (or the PR description) clarifying this would help future maintainers understand why an apparently unused dependency is present and security-verified.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

"@vitejs/plugin-react": "^5.1.2",
"react": "19.2.3",
"react-dom": "19.2.3",
Expand Down
79 changes: 79 additions & 0 deletions webviews/scripts/verify-tanstack-router-security.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { readFileSync } from "node:fs";

const expectedDependencyVersion = "1.170.11";
const expectedLockEntries = new Map([
["@tanstack/history", {
version: "1.162.0",
integrity: "sha512-79pf/RkhteYZTRgcR4F9kbk84P2N8rugQJswxfIqovlbRiT3yI7eBE+5QorIrZaOKktsgzRlXh1l/du/xpl4iA==",
}],
["@tanstack/react-router", {
version: "1.170.11",
integrity: "sha512-gP2vzdyaI8Ow/Uz/MRPfK2wN09YwRI0Y/oF74Wuy9R3KmjbfJv2tLrkM+Onu1xWklSn3ugZarMPJXRE0kzrJTA==",
}],
["@tanstack/react-store", {
version: "0.9.3",
integrity: "sha512-y2iHd/N9OkoQbFJLUX1T9vbc2O9tjH0pQRgTcx1/Nz4IlwLvkgpuglXUx+mXt0g5ZDFrEeDnONPqkbfxXJKwRg==",
}],
["@tanstack/router-core", {
version: "1.171.9",
integrity: "sha512-QM5ZwLT9c5ZcTJW0QQZRRIBC4qjImUyUCXCVyuYVOF9xr76XLsJSX4F2dOxr9VptAv+W+TkWNOYdX8VaO9kdgA==",
}],
["@tanstack/store", {
version: "0.9.3",
integrity: "sha512-8reSzl/qGWGGVKhBoxXPMWzATSbZLZFWhwBAFO9NAyp0TxzfBP0mIrGb8CP8KrQTmvzXlR/vFPPUrHTLBGyFyw==",
}],
]);

const compromisedVersions = new Map([
["@tanstack/history", new Set(["1.161.9", "1.161.12"])],
["@tanstack/react-router", new Set(["1.169.5", "1.169.8"])],
["@tanstack/router-core", new Set(["1.169.5", "1.169.8"])],
Comment on lines +27 to +30

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Expand the GHSA blocklist to all affected packages

When another GHSA-g7cv-rxg3-hmpx-affected TanStack package is added to this lockfile, this guard will still pass for known-malicious versions because the blocklist only includes three packages; the advisory covers many more affected @tanstack/* packages (for example @tanstack/react-router-devtools 1.166.16/1.166.19). Since this script is now the build-time security gate for TanStack lockfile drift, leaving the rest of the affected package/version pairs out makes the verifier give a false sense of coverage for future TanStack additions.

Useful? React with 👍 / 👎.

]);

const packageJSON = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8"));
const lockfile = readFileSync(new URL("../bun.lock", import.meta.url), "utf8");

const dependencyVersion = packageJSON.dependencies?.["@tanstack/react-router"];
if (dependencyVersion !== expectedDependencyVersion) {
fail(`@tanstack/react-router must be exact-pinned to ${expectedDependencyVersion}, found ${dependencyVersion ?? "missing"}`);
}

const lockEntries = parseTanstackLockEntries(lockfile);
for (const [name, expected] of expectedLockEntries) {
const actual = lockEntries.get(name);
if (!actual) {
fail(`bun.lock is missing ${name}`);
}
if (actual.version !== expected.version) {
fail(`${name} must resolve to ${expected.version}, found ${actual.version}`);
}
if (actual.integrity !== expected.integrity) {
fail(`${name}@${expected.version} integrity changed`);
}
}

for (const [name, actual] of lockEntries) {
const badVersions = compromisedVersions.get(name);
if (badVersions?.has(actual.version)) {
fail(`${name}@${actual.version} is blocked by GHSA-g7cv-rxg3-hmpx`);
}
}

console.log(`Verified @tanstack/react-router ${expectedDependencyVersion} and TanStack lockfile entries.`);

function parseTanstackLockEntries(text) {
const entries = new Map();
const linePattern = /^\s+"(@tanstack\/[^"]+)": \["@tanstack\/[^@"]+@([^"]+)",.*"(sha512-[^"]+)"\],?$/gm;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse nested TanStack entries before trusting the guard

When Bun has to keep a second copy of a package for a specific dependency, this lockfile format can use prefixed keys like parent/package while the package spec inside the array still names the real package; there are already entries of that form elsewhere in webviews/bun.lock (for example bundled/nested packages). This regex only matches keys that start exactly with @tanstack/, so a future dependency that pulls a nested compromised @tanstack/history or @tanstack/router-core would not be included in lockEntries and the GHSA blocklist loop would pass even though the bad package is present. Parse the package name from the array spec (or otherwise scan all package records) so nested TanStack copies are checked too.

Useful? React with 👍 / 👎.

for (const match of text.matchAll(linePattern)) {
entries.set(match[1], {
version: match[2],
integrity: match[3],
});
}
return entries;
}
Comment on lines +64 to +74

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Last-write-wins on duplicate package names in lockfile parser

parseTanstackLockEntries accumulates results into a Map keyed by package name (e.g. @tanstack/react-router). If bun.lock ever contains two lines with the same @tanstack/ package name — for instance via an aliased install or a future nested-workspace setup — the Map silently overwrites the first entry with the second. The compromised-version sweep on lines 55-60 then only sees the last occurrence, so a compromised version that happens to appear earlier in the file would be invisible to the check. For a security-critical verifier, collecting all matches into an array and iterating all of them (or failing fast on any duplicate key) would close this gap without changing normal-case behavior.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


function fail(message) {
console.error(message);
process.exit(1);
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ describe("vendored Pierre tree bundle", () => {

expect(tree.getItem("src/viewer-controller.ts")?.getPath()).toBe("src/viewer-controller.ts");

const nextPaths = ["README.md", "diff-viewer/src/App.tsx"];
const nextPaths = ["README.md", "webviews/src/App.tsx"];
tree.resetPaths(nextPaths, {
preparedInput: preparePresortedFileTreeInput(nextPaths),
});

expect(tree.getItem("src/App.tsx")).toBeNull();
expect(tree.getItem("README.md")?.getPath()).toBe("README.md");
expect(tree.getItem("diff-viewer/src/App.tsx")?.getPath()).toBe("diff-viewer/src/App.tsx");
expect(tree.getItem("webviews/src/App.tsx")?.getPath()).toBe("webviews/src/App.tsx");
} finally {
tree.cleanUp();
}
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion diff-viewer/vite.config.mjs → webviews/vite.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import react from "@vitejs/plugin-react";
import tailwindcss from "@tailwindcss/vite";
import { defineConfig } from "vite";

const outDir = process.env.CMUX_DIFF_VIEWER_OUT_DIR ?? "../Resources/markdown-viewer/diff-viewer-app";
const outDir = process.env.CMUX_WEBVIEWS_OUT_DIR ?? "../Resources/markdown-viewer/webviews-app";

export default defineConfig({
define: {
Expand Down
Loading