-
Notifications
You must be signed in to change notification settings - Fork 981
feat(desktop): add Windows build and runtime support #2100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -247,6 +247,15 @@ protocol.registerSchemesAsPrivileged([ | |||||||||||||||||||||||||||||||||||||||||||||||
| supportFetchAPI: true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| scheme: "superset-app", | ||||||||||||||||||||||||||||||||||||||||||||||||
| privileges: { | ||||||||||||||||||||||||||||||||||||||||||||||||
| standard: true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| secure: true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| supportFetchAPI: true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| corsEnabled: true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ]); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const gotTheLock = app.requestSingleInstanceLock(); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -283,6 +292,48 @@ if (!gotTheLock) { | |||||||||||||||||||||||||||||||||||||||||||||||
| .fromPartition("persist:superset") | ||||||||||||||||||||||||||||||||||||||||||||||||
| .protocol.handle("superset-icon", iconProtocolHandler); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Register custom protocol for serving renderer files. | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Dynamic imports (code-split chunks) fail on file:// protocol in Electron on Windows. | ||||||||||||||||||||||||||||||||||||||||||||||||
| // The superset-app:// protocol serves files from the renderer dist directory with | ||||||||||||||||||||||||||||||||||||||||||||||||
| // proper CORS and module support, enabling lazy-loaded route components. | ||||||||||||||||||||||||||||||||||||||||||||||||
| const rendererDir = path.join(__dirname, "../renderer"); | ||||||||||||||||||||||||||||||||||||||||||||||||
| const appProtocolHandler = (request: Request) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| let urlPath = new URL(request.url).pathname; | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Remove leading slash on Windows | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (urlPath.startsWith("/")) urlPath = urlPath.slice(1); | ||||||||||||||||||||||||||||||||||||||||||||||||
| const filePath = path.join(rendererDir, urlPath); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return net.fetch(pathToFileURL(filePath).toString()); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+299
to
+306
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Path traversal vulnerability in protocol handler. The 🛡️ Proposed fix: Validate resolved path stays within rendererDir const appProtocolHandler = (request: Request) => {
let urlPath = new URL(request.url).pathname;
// Remove leading slash on Windows
if (urlPath.startsWith("/")) urlPath = urlPath.slice(1);
+ // Decode URL-encoded characters and normalize
+ urlPath = decodeURIComponent(urlPath);
const filePath = path.join(rendererDir, urlPath);
+ // Prevent path traversal attacks
+ const resolvedPath = path.resolve(filePath);
+ if (!resolvedPath.startsWith(path.resolve(rendererDir) + path.sep) && resolvedPath !== path.resolve(rendererDir)) {
+ return new Response("Forbidden", { status: 403 });
+ }
return net.fetch(pathToFileURL(filePath).toString());
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| protocol.handle("superset-app", appProtocolHandler); | ||||||||||||||||||||||||||||||||||||||||||||||||
| session | ||||||||||||||||||||||||||||||||||||||||||||||||
| .fromPartition("persist:superset") | ||||||||||||||||||||||||||||||||||||||||||||||||
| .protocol.handle("superset-app", appProtocolHandler); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // On Windows, the custom superset-app:// protocol origin is not recognized by | ||||||||||||||||||||||||||||||||||||||||||||||||
| // the API server's CORS policy. Bypass CORS for API requests by modifying headers. | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (PLATFORM.IS_WINDOWS) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const appSession = session.fromPartition("persist:superset"); | ||||||||||||||||||||||||||||||||||||||||||||||||
| appSession.webRequest.onBeforeSendHeaders( | ||||||||||||||||||||||||||||||||||||||||||||||||
| { urls: ["https://api.superset.sh/*", "https://*.posthog.com/*", "https://*.sentry.io/*", "https://app.outlit.ai/*"] }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| (details, callback) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| // Replace custom protocol origin with the expected web origin | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (details.requestHeaders.Origin === "superset-app://app") { | ||||||||||||||||||||||||||||||||||||||||||||||||
| delete details.requestHeaders.Origin; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| callback({ requestHeaders: details.requestHeaders }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
| appSession.webRequest.onHeadersReceived( | ||||||||||||||||||||||||||||||||||||||||||||||||
| { urls: ["https://api.superset.sh/*"] }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| (details, callback) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const headers = details.responseHeaders ?? {}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| headers["access-control-allow-origin"] = ["superset-app://app"]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| headers["access-control-allow-credentials"] = ["true"]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| callback({ responseHeaders: headers }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+316
to
+334
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded URLs in CORS bypass may drift from environment configuration. The URL patterns are hardcoded, but Additionally:
♻️ Suggested approach: Build URL patterns from env if (PLATFORM.IS_WINDOWS) {
+ // Import env from ./env.main or access via shared config
+ const corsUrls = [
+ `${env.NEXT_PUBLIC_API_URL}/*`,
+ `${env.NEXT_PUBLIC_STREAMS_URL}/*`,
+ `${env.NEXT_PUBLIC_POSTHOG_HOST}/*`,
+ "https://*.sentry.io/*",
+ "https://app.outlit.ai/*",
+ ];
const appSession = session.fromPartition("persist:superset");
appSession.webRequest.onBeforeSendHeaders(
- { urls: ["https://api.superset.sh/*", "https://*.posthog.com/*", "https://*.sentry.io/*", "https://app.outlit.ai/*"] },
+ { urls: corsUrls },
(details, callback) => {Also consider applying 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| ensureProjectIconsDir(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| setWorkspaceDockIcon(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| initSentry(); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -64,12 +64,12 @@ export function CollectionsProvider({ children }: { children: ReactNode }) { | |||||||||||||||
| preloadActiveOrganizationCollections(activeOrganizationId); | ||||||||||||||||
| }, [activeOrganizationId]); | ||||||||||||||||
|
|
||||||||||||||||
| if (useElectricCloud === undefined) { | ||||||||||||||||
| return null; | ||||||||||||||||
| } | ||||||||||||||||
| // When PostHog is not configured (no key), feature flags stay undefined forever. | ||||||||||||||||
| // Default to false (use proxy) so the app doesn't block rendering. | ||||||||||||||||
| const isElectricCloud = useElectricCloud ?? false; | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Defaulting Prompt for AI agents
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| setElectricUrl( | ||||||||||||||||
| useElectricCloud | ||||||||||||||||
| isElectricCloud | ||||||||||||||||
| ? env.NEXT_PUBLIC_ELECTRIC_URL | ||||||||||||||||
| : env.NEXT_PUBLIC_ELECTRIC_PROXY_URL, | ||||||||||||||||
| ); | ||||||||||||||||
|
Comment on lines
+67
to
75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts'
echo '--- collections.ts (first 260 lines) ---'
head -260 "$FILE" | nl -ba
echo
echo '--- electricUrl state + collection cache references ---'
rg -n -C3 'electricUrl|setElectricUrl|getCollections|preloadCollections|Map|cache' "$FILE"Repository: superset-sh/superset Length of output: 164 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts'
echo '--- collections.ts (first 260 lines with line numbers) ---'
head -260 "$FILE" | cat -n
echo
echo '--- electricUrl state + collection cache references ---'
rg -n -C3 'electricUrl|setElectricUrl|getCollections|preloadCollections|Map|cache' "$FILE" || trueRepository: superset-sh/superset Length of output: 15682 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsx'
echo '--- CollectionsProvider.tsx (lines 40-80) ---'
sed -n '40,80p' "$FILE" | cat -n -v
echo
echo '--- useEffect blocks and related logic ---'
rg -n -B2 -A5 'useEffect|useElectricCloud|setElectricUrl|preloadActiveOrganizationCollections' "$FILE"Repository: superset-sh/superset Length of output: 3509 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- Searching for FEATURE_FLAGS.ELECTRIC_CLOUD definition ---'
rg -n 'FEATURE_FLAGS|ELECTRIC_CLOUD' apps/desktop/src/renderer --type ts --type tsx -B2 -A2 | head -50
echo
echo '--- Checking PostHog hook behavior in imports ---'
rg -n 'useFeatureFlagEnabled' apps/desktop/src/renderer --type ts --type tsx | head -20Repository: superset-sh/superset Length of output: 170 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- Searching for FEATURE_FLAGS.ELECTRIC_CLOUD definition ---'
rg -n 'FEATURE_FLAGS|ELECTRIC_CLOUD' apps/desktop/src/renderer -g '*.ts' -g '*.tsx' -B2 -A2 | head -60
echo
echo '--- Checking env.renderer for PostHog key ---'
cat apps/desktop/src/renderer/env.renderer.ts | cat -nRepository: superset-sh/superset Length of output: 10539 Don't treat a loading flag the same as a disabled flag. Line 69 collapses both "PostHog is not configured" and "the flag has not resolved yet" into Suggested fix- // When PostHog is not configured (no key), feature flags stay undefined forever.
- // Default to false (use proxy) so the app doesn't block rendering.
- const isElectricCloud = useElectricCloud ?? false;
-
- setElectricUrl(
- isElectricCloud
- ? env.NEXT_PUBLIC_ELECTRIC_URL
- : env.NEXT_PUBLIC_ELECTRIC_PROXY_URL,
- );
+ const isPostHogConfigured = Boolean(env.NEXT_PUBLIC_POSTHOG_KEY);
+ const isElectricCloud = isPostHogConfigured ? useElectricCloud : false;
+ const electricBaseUrl =
+ isElectricCloud === true
+ ? env.NEXT_PUBLIC_ELECTRIC_URL
+ : env.NEXT_PUBLIC_ELECTRIC_PROXY_URL;
+
+ useEffect(() => {
+ setElectricUrl(electricBaseUrl);
+ }, [electricBaseUrl]);
- useEffect(() => {
- preloadActiveOrganizationCollections(activeOrganizationId);
- }, [activeOrganizationId]);
+ useEffect(() => {
+ if (isPostHogConfigured && isElectricCloud === undefined) return;
+ preloadActiveOrganizationCollections(activeOrganizationId);
+ }, [activeOrganizationId, electricBaseUrl, isPostHogConfigured, isElectricCloud]);🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /** | ||
| * Cross-platform postinstall script. | ||
| * | ||
| * Replaces the bash-only postinstall.sh so that `bun install` works on | ||
| * Windows, macOS and Linux without special flags. | ||
| * | ||
| * Steps: | ||
| * 1. Guard against infinite recursion (electron-builder install-app-deps | ||
| * can trigger nested bun installs which would re-run this script). | ||
| * 2. Run sherif for workspace validation. | ||
| * 3. Install native dependencies for the desktop app. | ||
| */ | ||
|
|
||
| import { execSync } from "node:child_process"; | ||
|
|
||
| // Prevent infinite recursion during postinstall | ||
| if (process.env.SUPERSET_POSTINSTALL_RUNNING) { | ||
| process.exit(0); | ||
| } | ||
| process.env.SUPERSET_POSTINSTALL_RUNNING = "1"; | ||
|
|
||
| const env = { ...process.env, SUPERSET_POSTINSTALL_RUNNING: "1" }; | ||
|
|
||
| /** Run a command, inheriting stdio so output is visible. */ | ||
| function run(cmd) { | ||
| execSync(cmd, { stdio: "inherit", env }); | ||
| } | ||
|
|
||
| /** Run a command but don't fail if it errors (for optional native deps on Windows). */ | ||
| function tryRun(cmd, label) { | ||
| try { | ||
| execSync(cmd, { stdio: "inherit", env }); | ||
| } catch { | ||
| console.warn(`[postinstall] ${label} failed (non-fatal on Windows) — continuing`); | ||
| } | ||
| } | ||
|
|
||
| // Run sherif for workspace validation | ||
| run("sherif"); | ||
|
|
||
| // Install native dependencies for desktop app. | ||
| // On Windows, native module compilation may fail if Visual Studio Build Tools | ||
| // are not installed. This is non-fatal — prebuilt binaries will be used when available. | ||
| if (process.platform === "win32") { | ||
| tryRun("bun run --filter=@superset/desktop install:deps", "install:deps"); | ||
| } else { | ||
| run("bun run --filter=@superset/desktop install:deps"); | ||
| } | ||
|
Comment on lines
+29
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't swallow every Windows This makes Suggested guard import { execSync } from "node:child_process";
+import { existsSync } from "node:fs";
+import { join } from "node:path";
@@
function tryRun(cmd, label) {
try {
execSync(cmd, { stdio: "inherit", env });
} catch {
console.warn(`[postinstall] ${label} failed (non-fatal on Windows) — continuing`);
}
}
+
+function assertDesktopNativeDepsInstalled() {
+ const required = [
+ "better-sqlite3",
+ "node-pty",
+ "@ast-grep/napi",
+ "libsql",
+ ];
+ const missing = required.filter(
+ (name) =>
+ !existsSync(join("apps/desktop/node_modules", name, "package.json")),
+ );
+ if (missing.length) {
+ throw new Error(
+ `[postinstall] Missing desktop native deps after install:deps: ${missing.join(", ")}`,
+ );
+ }
+}
@@
if (process.platform === "win32") {
tryRun("bun run --filter=@superset/desktop install:deps", "install:deps");
+ assertDesktopNativeDepsInstalled();
} else {
run("bun run --filter=@superset/desktop install:deps");
}🤖 Prompt for AI Agents |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Sanitize
superset-app://paths before joining torendererDir; as written,../traversal can escape the renderer directory and expose arbitrary local files via the custom protocol.Prompt for AI agents