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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { LuFolderOpen } from "react-icons/lu";
import { useOpenNew } from "renderer/react-query/projects";
import { useCreateBranchWorkspace } from "renderer/react-query/workspaces";
import { STROKE_WIDTH } from "./constants";
import { OrganizationDropdown } from "./WorkspaceSidebarHeader/OrganizationDropdown";

interface WorkspaceSidebarFooterProps {
isCollapsed?: boolean;
Expand Down Expand Up @@ -55,7 +56,7 @@ export function WorkspaceSidebarFooter({

if (isCollapsed) {
return (
<div className="border-t border-border p-2 flex justify-center">
<div className="border-t border-border p-2 flex flex-col items-center gap-2">
<Tooltip delayDuration={300}>
<TooltipTrigger asChild>
<Button
Expand All @@ -70,12 +71,13 @@ export function WorkspaceSidebarFooter({
</TooltipTrigger>
<TooltipContent side="right">Add repository</TooltipContent>
</Tooltip>
<OrganizationDropdown isCollapsed />
</div>
);
}

return (
<div className="border-t border-border p-2">
<div className="border-t border-border p-2 flex flex-col gap-1">
<Button
variant="ghost"
size="sm"
Expand All @@ -86,6 +88,7 @@ export function WorkspaceSidebarFooter({
<LuFolderOpen className="w-4 h-4" strokeWidth={STROKE_WIDTH} />
<span>Add repository</span>
</Button>
<OrganizationDropdown />
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,29 @@ import {
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuSeparator,
DropdownMenuShortcut,
DropdownMenuSub,
DropdownMenuSubContent,
DropdownMenuSubTrigger,
DropdownMenuTrigger,
} from "@superset/ui/dropdown-menu";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { useLiveQuery } from "@tanstack/react-db";
import { FaDiscord, FaXTwitter } from "react-icons/fa6";
import {
HiCheck,
HiChevronUpDown,
HiOutlineArrowRightOnRectangle,
HiOutlineCog6Tooth,
HiOutlineCommandLine,
HiOutlineEnvelope,
HiOutlineUserGroup,
} from "react-icons/hi2";
import { useAuth } from "renderer/contexts/AuthProvider";
import { useCollections } from "renderer/contexts/CollectionsProvider";
import { trpc } from "renderer/lib/trpc";
import { useOpenSettings } from "renderer/stores/app-state";
import { useHotkeyText } from "renderer/stores/hotkeys/store";

interface OrganizationDropdownProps {
isCollapsed?: boolean;
Expand All @@ -34,7 +41,9 @@ export function OrganizationDropdown({
const collections = useCollections();
const setActiveOrg = trpc.auth.setActiveOrganization.useMutation();
const signOut = trpc.auth.signOut.useMutation();
const openUrl = trpc.external.openUrl.useMutation();
const openSettings = useOpenSettings();
const hotkeysShortcut = useHotkeyText("SHOW_HOTKEYS");

Comment on lines +44 to 47
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.

⚠️ Potential issue | 🟠 Major

Don’t silently ignore openUrl failures; add pending/disabled + contextual error logging. (Guideline: “Never swallow errors silently”)
Right now openUrl.mutate(...) has no local handling, so failures can be invisible and users can spam-click items while pending.

Proposed patch
 export function OrganizationDropdown({
 	isCollapsed = false,
 }: OrganizationDropdownProps) {
@@
 	const signOut = trpc.auth.signOut.useMutation();
 	const openUrl = trpc.external.openUrl.useMutation();
@@
 	const hotkeysShortcut = useHotkeyText("SHOW_HOTKEYS");
+
+	type OpenUrlInput = Parameters<typeof openUrl.mutateAsync>[0];
+	const handleOpenUrl = async (input: OpenUrlInput) => {
+		try {
+			await openUrl.mutateAsync(input);
+		} catch (err) {
+			console.error("[external/openUrl] failed to open url", err);
+		}
+	};
@@
 						<DropdownMenuItem
-							onSelect={() => openUrl.mutate("https://discord.gg/superset")}
+							onSelect={() => handleOpenUrl("https://discord.gg/superset" as OpenUrlInput)}
 							className="gap-2"
+							disabled={openUrl.isPending}
 						>
@@
 						<DropdownMenuItem
-							onSelect={() => openUrl.mutate("mailto:founders@superset.sh")}
+							onSelect={() => handleOpenUrl("mailto:founders@superset.sh" as OpenUrlInput)}
 							className="gap-2"
+							disabled={openUrl.isPending}
 						>
@@
 						<DropdownMenuItem
-							onSelect={() => openUrl.mutate("https://x.com/supersetsh")}
+							onSelect={() => handleOpenUrl("https://x.com/supersetsh" as OpenUrlInput)}
 							className="gap-2"
+							disabled={openUrl.isPending}
 						>

Also applies to: 189-209

const activeOrganizationId = session?.session?.activeOrganizationId;

Expand Down Expand Up @@ -102,13 +111,30 @@ export function OrganizationDropdown({
{activeOrganization && (
<>
{/* Settings */}
<DropdownMenuItem onSelect={() => openSettings()}>
<DropdownMenuItem onSelect={() => openSettings()} className="gap-2">
<HiOutlineCog6Tooth className="h-4 w-4" />
<span>Settings</span>
</DropdownMenuItem>

{/* Team management */}
<DropdownMenuItem onSelect={() => openSettings("team")}>
<span>Invite and manage members</span>
<DropdownMenuItem
onSelect={() => openSettings("team")}
className="gap-2"
>
<HiOutlineUserGroup className="h-4 w-4" />
<span>Team</span>
</DropdownMenuItem>

{/* Hotkeys */}
<DropdownMenuItem
onSelect={() => openSettings("keyboard")}
className="gap-2"
>
<HiOutlineCommandLine className="h-4 w-4" />
<span className="flex-1">Hotkeys</span>
{hotkeysShortcut !== "Unassigned" && (
<DropdownMenuShortcut>{hotkeysShortcut}</DropdownMenuShortcut>
)}
</DropdownMenuItem>

<DropdownMenuSeparator />
Expand Down Expand Up @@ -153,6 +179,39 @@ export function OrganizationDropdown({
</>
)}

{/* Contact Us */}
<DropdownMenuSub>
<DropdownMenuSubTrigger className="gap-2">
<HiOutlineEnvelope className="h-4 w-4" />
<span>Contact us</span>
</DropdownMenuSubTrigger>
<DropdownMenuSubContent>
<DropdownMenuItem
onSelect={() => openUrl.mutate("https://discord.gg/superset")}
className="gap-2"
>
<FaDiscord className="h-4 w-4" />
<span>Discord</span>
</DropdownMenuItem>
<DropdownMenuItem
onSelect={() => openUrl.mutate("mailto:founders@superset.sh")}
className="gap-2"
>
<HiOutlineEnvelope className="h-4 w-4" />
<span>Email founders</span>
</DropdownMenuItem>
<DropdownMenuItem
onSelect={() => openUrl.mutate("https://x.com/supersetsh")}
className="gap-2"
>
<FaXTwitter className="h-4 w-4" />
<span>X (Twitter)</span>
</DropdownMenuItem>
</DropdownMenuSubContent>
</DropdownMenuSub>
Comment on lines +182 to +211
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the target file to understand the context
cat -n "apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx" | head -250

Repository: superset-sh/superset

Length of output: 8770


🏁 Script executed:

# Find where openUrl is defined as a tRPC handler
fd . apps/desktop --type f --name "*.ts" --o --name "*.tsx" | xargs rg -l "openUrl" | head -20

Repository: superset-sh/superset

Length of output: 863


🏁 Script executed:

# Search for tRPC external handlers/setup
rg "trpc\.external\." apps/desktop/src --type ts --type tsx -A 5 -B 5

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Check for URL validation or allowlist patterns in the codebase
rg "allowlist|allow.*list|validateUrl|scheme.*check" apps/desktop/src --type ts --type tsx -A 3

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

cat -n "apps/desktop/src/lib/trpc/routers/external/index.ts"

Repository: superset-sh/superset

Length of output: 3270


🏁 Script executed:

# Search for shell.openExternal usage and any validation
rg "shell\.openExternal|openExternal" apps/desktop/src --type ts -B 5 -A 10

Repository: superset-sh/superset

Length of output: 7788


🏁 Script executed:

# Check if there are any URL validation utilities or patterns
rg "url|URL|scheme|protocol" apps/desktop/src/lib/trpc/routers/external -B 2 -A 5

Repository: superset-sh/superset

Length of output: 5370


🏁 Script executed:

rg "COMPANY\." apps/desktop/src --type ts -B 2 -A 2 | head -50

Repository: superset-sh/superset

Length of output: 4103


🏁 Script executed:

# Find where COMPANY is defined
fd . apps/desktop/src -name "constants*" -o -name "config*" | xargs cat 2>/dev/null | grep -A 10 -B 2 "COMPANY\|DISCORD\|MAIL"

Repository: superset-sh/superset

Length of output: 294


🏁 Script executed:

# Search for COMPANY definition
rg "^const COMPANY|export.*COMPANY" apps/desktop/src --type ts -A 10

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

rg "export const COMPANY\s*=" apps/desktop --type ts -A 20

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Search more broadly for COMPANY definition
rg "COMPANY\s*=" apps/desktop --type ts | head -5

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Find config or constants files in main and renderer
find apps/desktop/src -type f \( -name "config.ts" -o -name "constants.ts" -o -name "company.ts" \)

Repository: superset-sh/superset

Length of output: 476


🏁 Script executed:

cat -n apps/desktop/src/shared/constants.ts

Repository: superset-sh/superset

Length of output: 2126


🏁 Script executed:

# Also check if COMPANY is imported from somewhere
rg "import.*COMPANY|from.*constants" apps/desktop/src/main/lib/menu.ts apps/desktop/src/renderer/screens/main/components/TopBar/SupportMenu.tsx

Repository: superset-sh/superset

Length of output: 278


🏁 Script executed:

cat -n $(find apps -name "constants.ts" -path "*/shared/*")

Repository: superset-sh/superset

Length of output: 2126


🏁 Script executed:

# Check the shared constants more directly
find . -path "*/shared/constants*" -type f

Repository: superset-sh/superset

Length of output: 156


🏁 Script executed:

rg "COMPANY" apps/desktop/src/shared/constants.ts

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check the entire shared/constants directory
ls -la apps/desktop/src/shared/constants/

Repository: superset-sh/superset

Length of output: 238


🏁 Script executed:

# If it's in a different file, search across shared
rg "export.*COMPANY" apps/desktop/src/shared --type ts -A 15

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check tsconfig to understand the @superset/shared alias
rg "@superset/shared" apps/desktop/tsconfig.json

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Find where COMPANY is actually defined by searching all files
rg "COMPANY\s*:" apps/desktop/src --type ts | head -20

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Try a broader search in apps folder
rg "COMPANY.*DISCORD\|DISCORD.*superset" apps --type ts -B 5 | head -40

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Look at tsconfig to understand path aliases
cat apps/desktop/tsconfig.json | grep -A 5 '"@superset'

Repository: superset-sh/superset

Length of output: 211


🏁 Script executed:

# Search in packages folder for COMPANY
rg "COMPANY" packages --type ts | head -10

Repository: superset-sh/superset

Length of output: 524


🏁 Script executed:

# Check if COMPANY is defined inline in the importing files
cat -n apps/desktop/src/main/lib/menu.ts | head -100

Repository: superset-sh/superset

Length of output: 3186


🏁 Script executed:

cat -n packages/shared/src/constants.ts | grep -A 30 "export const COMPANY"

Repository: superset-sh/superset

Length of output: 1319


🏁 Script executed:

# Verify the specific URLs being used in OrganizationDropdown vs COMPANY constants
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/OrganizationDropdown.tsx | grep -E "discord|founders|x\.com"

Repository: superset-sh/superset

Length of output: 333


🏁 Script executed:

# Check if there are other usages of these inconsistent URLs
rg "discord\.gg/superset" apps/desktop/src --type ts

Repository: superset-sh/superset

Length of output: 248


🏁 Script executed:

# Verify the Electron security practices - check what Electron docs recommend
web_search
Electron shell.openExternal security best practices URL validation allowlist

Repository: superset-sh/superset

Length of output: 161


🌐 Web query:

Electron shell.openExternal security best practices URL validation allowlist

💡 Result:

Key practices to safely use shell.openExternal in Electron

  1. Restrict protocols — only allow safe schemes (http, https). Block or reject file:, javascript:, data:, vbscript:, and app-specific protocols unless explicitly needed. Enforce this in main process code. [1]

  2. Use a domain / host allowlist (preferred) or a robust denylist:

    • If possible maintain an explicit allowlist of trusted domains (exact host or hostname+path rules).
    • If allowlist isn’t feasible, require that the URL matches a safe pattern, then show user confirmation. [1][2]
  3. Canonicalize and parse URLs with a standard parser (WHATWG URL) before checking — do not do string-only checks (slashes, %-encoding, userinfo, IDN homographs). Example:

    • let u = new URL(input);
    • if (!['http:','https:'].includes(u.protocol)) reject;
    • if (!allowlist.has(u.hostname)) prompt or reject.
  4. Validate caller / origin — only permit openExternal when the request originates from a trusted renderer or a validated IPC channel. Authenticate sender in main process. [1][2]

  5. Show the full destination and require explicit user consent for non-allowlisted hosts (display hostname, port, and scheme). Avoid silent opens. [1][3]

  6. Sanitize inputs — percent-decode/normalize and reject suspicious userinfo (user:pass@host) or embedded credentials. Block URLs containing newlines, control chars, or CR/LF injection. [2]

  7. Log and rate-limit external-open attempts for auditing and detection of abuse. Monitor telemetry for unexpected hosts/protocols. [2][3]

  8. Keep Electron up to date and follow upstream security guidance — many published CVEs stem from using openExternal without validation. Review Electron’s security checklist and vendor advisories when designing your checks. [1][3]

Minimal example (main process, pseudo-JS)

  • Note: run these checks in main before calling shell.openExternal.
    const allowlist = new Set(['example.com','docs.example.com']);
    function safeOpenExternal(raw) {
    try {
    const u = new URL(raw);
    if (!['http:','https:'].includes(u.protocol)) throw new Error('bad-protocol');
    if (!allowlist.has(u.hostname)) return promptUserAndMaybeOpen(u); // or reject
    require('electron').shell.openExternal(u.toString());
    } catch (e) {
    // reject / log
    }
    }

Citations

  • Electron security checklist (do not use shell.openExternal with untrusted content). [1]
  • Real-world advisories/patches where lack of URL checks led to RCE/abuse (Wire, WebCatalog, Bruno examples). [2][3]

Add URL allowlist validation to trpc.external.openUrl and extract hardcoded URLs to constants.

The openUrl handler accepts any string without validating scheme or host, creating a security vulnerability. Per Electron best practices, validate URLs against a whitelist before calling shell.openExternal(). Additionally, extract the three hardcoded URLs/emails (lines 190, 197, 204) to constants referencing COMPANY from @superset/shared/constants. Note the Discord URL differs from the centralized constant (https://discord.gg/superset vs https://discord.gg/cZeD9WYcV7) and the X URL differs from COMPANY.X_URL (supersetsh vs superset_sh).


<DropdownMenuSeparator />

{/* Sign out - ALWAYS show so users can never get trapped */}
<DropdownMenuItem onSelect={handleSignOut} className="gap-2">
<HiOutlineArrowRightOnRectangle className="h-4 w-4" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
} from "renderer/stores/app-state";
import { STROKE_WIDTH, STROKE_WIDTH_THIN } from "../constants";
import { NewWorkspaceButton } from "./NewWorkspaceButton";
import { OrganizationDropdown } from "./OrganizationDropdown";

interface WorkspaceSidebarHeaderProps {
isCollapsed?: boolean;
Expand Down Expand Up @@ -86,8 +85,6 @@ export function WorkspaceSidebarHeader({
<TooltipContent side="right">Toggle sidebar</TooltipContent>
</Tooltip>

<OrganizationDropdown isCollapsed />

<Tooltip delayDuration={300}>
<TooltipTrigger asChild>
<button
Expand Down Expand Up @@ -158,8 +155,6 @@ export function WorkspaceSidebarHeader({
<TooltipContent side="right">Toggle sidebar</TooltipContent>
</Tooltip>

<OrganizationDropdown />

<button
type="button"
onClick={handleClick}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@ import { smoothScrollToBottom } from "../utils";

interface ScrollToBottomButtonProps {
terminal: Terminal | null;
isHovered: boolean;
}

export function ScrollToBottomButton({ terminal }: ScrollToBottomButtonProps) {
const [isVisible, setIsVisible] = useState(false);
export function ScrollToBottomButton({
terminal,
isHovered,
}: ScrollToBottomButtonProps) {
const [isNotAtBottom, setIsNotAtBottom] = useState(false);
Comment thread
AviPeltz marked this conversation as resolved.
const shortcutText = useHotkeyText("SCROLL_TO_BOTTOM");
const showShortcut = shortcutText !== "Unassigned";

const checkScrollPosition = useCallback(() => {
if (!terminal) return;
const buffer = terminal.buffer.active;
const isAtBottom = buffer.viewportY >= buffer.baseY;
setIsVisible(!isAtBottom);
setIsNotAtBottom(!isAtBottom);
}, [terminal]);

useEffect(() => {
Expand All @@ -46,21 +50,25 @@ export function ScrollToBottomButton({ terminal }: ScrollToBottomButtonProps) {
}
};

const isVisible = isNotAtBottom && isHovered;

return (
<div
className={cn(
"absolute bottom-4 left-1/2 z-10 -translate-x-1/2 transition-all duration-200",
"absolute bottom-4 right-4 z-10 transition-all duration-200",
isVisible
? "translate-y-0 opacity-100"
: "pointer-events-none translate-y-2 opacity-0",
)}
aria-hidden={!isVisible}
>
<Tooltip delayDuration={500}>
<Tooltip delayDuration={300}>
<TooltipTrigger asChild>
<button
type="button"
onClick={handleClick}
className="flex size-8 items-center justify-center rounded-full border border-border bg-background text-muted-foreground transition-colors hover:bg-muted hover:text-foreground"
tabIndex={isVisible ? 0 : -1}
className="flex size-8 items-center justify-center rounded-full border border-border/50 bg-background/80 text-muted-foreground backdrop-blur-sm transition-colors hover:bg-muted/90 hover:text-foreground"
>
<HiArrowDown className="size-4" />
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => {
const commandBufferRef = useRef("");
const [subscriptionEnabled, setSubscriptionEnabled] = useState(false);
const [isSearchOpen, setIsSearchOpen] = useState(false);
const [isHovered, setIsHovered] = useState(false);
const [xtermInstance, setXtermInstance] = useState<XTerm | null>(null);
const [terminalCwd, setTerminalCwd] = useState<string | null>(null);
const [cwdConfirmed, setCwdConfirmed] = useState(false);
Expand Down Expand Up @@ -613,13 +614,15 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => {
style={{ backgroundColor: terminalBg }}
onDragOver={handleDragOver}
onDrop={handleDrop}
onMouseEnter={() => setIsHovered(true)}
onMouseLeave={() => setIsHovered(false)}
>
<TerminalSearch
searchAddon={searchAddonRef.current}
isOpen={isSearchOpen}
onClose={() => setIsSearchOpen(false)}
/>
<ScrollToBottomButton terminal={xtermInstance} />
<ScrollToBottomButton terminal={xtermInstance} isHovered={isHovered} />
<div ref={terminalRef} className="h-full w-full" />
</div>
);
Expand Down