fix: address v2 stack review findings
This commit is contained in:
1 parent
53b518960c
commit
263702210a
48 files changed
+1235
-530
No files matched your search
@@ -3,11 +3,9 @@ import { describe, expect, test } from "vitest";
|
||||
import { getSafeAssistantAttachmentImageSource, getSafeAssistantMarkdownImageSource } from "./assistant-markdown-image";
|
||||
|
||||
describe("getSafeAssistantMarkdownImageSource", () => {
|
||||
test("allows Homarr-relative and ordinary remote images", () => {
|
||||
test("allows Homarr-relative images without automatically loading remote images", () => {
|
||||
expect(getSafeAssistantMarkdownImageSource("/logo/logo.png")).toBe("/logo/logo.png");
|
||||
expect(getSafeAssistantMarkdownImageSource("https://cdn.example.com/icons/discord.svg")).toBe(
|
||||
"https://cdn.example.com/icons/discord.svg",
|
||||
);
|
||||
expect(getSafeAssistantMarkdownImageSource("https://cdn.example.com/icons/discord.svg")).toBeNull();
|
||||
});
|
||||
|
||||
test("rejects unsafe protocols, protocol-relative URLs, and embedded credentials", () => {
|
||||
|
||||
@@ -4,9 +4,12 @@ export const getSafeAssistantMarkdownImageSource = (source: string | Blob | unde
|
||||
if (typeof source !== "string" || !source) return null;
|
||||
if (source.startsWith("/") && !source.startsWith("//")) return source;
|
||||
|
||||
return getSafeAssistantHttpUrl(source);
|
||||
return null;
|
||||
};
|
||||
|
||||
export const getSafeAssistantExternalMarkdownImageSource = (source: string | Blob | undefined) =>
|
||||
getSafeAssistantHttpUrl(source);
|
||||
|
||||
const safeAttachmentDataUrl =
|
||||
/^data:image\/(?:gif|jpeg|png|webp);base64,(?:[a-z\d+/]{4})*(?:[a-z\d+/]{2}==|[a-z\d+/]{3}=)?$/iu;
|
||||
|
||||
|
||||
@@ -141,7 +141,11 @@ import { getAssistantDirectiveTranslationKey, parseAssistantDirectives } from ".
|
||||
import { remarkAssistantDirectives, resolveAssistantDirectiveEntity } from "./assistant-markdown-directives";
|
||||
import type { AssistantDirectiveEntity } from "./assistant-markdown-directives";
|
||||
import { normalizeAssistantMarkdown } from "./assistant-markdown";
|
||||
import { getSafeAssistantAttachmentImageSource, getSafeAssistantMarkdownImageSource } from "./assistant-markdown-image";
|
||||
import {
|
||||
getSafeAssistantAttachmentImageSource,
|
||||
getSafeAssistantExternalMarkdownImageSource,
|
||||
getSafeAssistantMarkdownImageSource,
|
||||
} from "./assistant-markdown-image";
|
||||
import {
|
||||
getAssistantContextBreakdown,
|
||||
getAssistantConversationUsage,
|
||||
@@ -212,7 +216,8 @@ const ContextDirectiveChip = ({
|
||||
}: DirectiveChipProps & { iconUrl?: string }) => {
|
||||
const t = useScopedI18n("common.assistant");
|
||||
const Icon = contextIcons[directiveType as keyof typeof contextIcons] ?? IconAt;
|
||||
const safeIconUrl = getSafeAssistantMarkdownImageSource(iconUrl);
|
||||
const safeIconUrl =
|
||||
getSafeAssistantMarkdownImageSource(iconUrl) ?? getSafeAssistantExternalMarkdownImageSource(iconUrl);
|
||||
const translationKey = getAssistantDirectiveTranslationKey(directiveType);
|
||||
const typeLabel = t(`mentions.${translationKey}`);
|
||||
|
||||
@@ -284,13 +289,30 @@ const MarkdownTable = ({ children, ...props }: ComponentPropsWithoutRef<"table">
|
||||
|
||||
const MarkdownImage = ({ src, alt = "", ...props }: ComponentPropsWithoutRef<"img">) => {
|
||||
const t = useScopedI18n("common.assistant.image");
|
||||
const [approvedExternalSource, setApprovedExternalSource] = useState<string | null>(null);
|
||||
const safeSource = getSafeAssistantMarkdownImageSource(src);
|
||||
if (!safeSource) return null;
|
||||
const externalSource = getSafeAssistantExternalMarkdownImageSource(src);
|
||||
const approvedSource = externalSource === approvedExternalSource ? externalSource : null;
|
||||
const displaySource = safeSource ?? approvedSource;
|
||||
|
||||
if (!displaySource && externalSource) {
|
||||
return (
|
||||
<Button
|
||||
variant="default"
|
||||
size="compact-sm"
|
||||
leftSection={<IconPhoto size={14} aria-hidden />}
|
||||
onClick={() => setApprovedExternalSource(externalSource)}
|
||||
>
|
||||
{t("loadExternal", { host: new URL(externalSource).host })}
|
||||
</Button>
|
||||
);
|
||||
}
|
||||
if (!displaySource) return null;
|
||||
|
||||
return (
|
||||
<AssistantImage
|
||||
{...props}
|
||||
source={safeSource}
|
||||
source={displaySource}
|
||||
alt={alt}
|
||||
className={classes.markdownImage}
|
||||
loadingLabel={t("loading")}
|
||||
@@ -2099,6 +2121,8 @@ const AutoApprovalControl = () => {
|
||||
width="min(19rem, calc(100vw - 1rem))"
|
||||
shadow="md"
|
||||
withinPortal
|
||||
trapFocus
|
||||
returnFocus
|
||||
>
|
||||
<Popover.Target>
|
||||
<ActionIcon
|
||||
@@ -2106,6 +2130,11 @@ const AutoApprovalControl = () => {
|
||||
variant={enabled ? "light" : "subtle"}
|
||||
color={enabled ? "green" : "gray"}
|
||||
onClick={() => setOpened((current) => !current)}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key !== "Escape" || !opened) return;
|
||||
event.stopPropagation();
|
||||
setOpened(false);
|
||||
}}
|
||||
aria-label={t("label")}
|
||||
aria-pressed={enabled}
|
||||
title={enabled ? t("enabled") : t("label")}
|
||||
@@ -2113,7 +2142,13 @@ const AutoApprovalControl = () => {
|
||||
<IconShieldCheck size={17} />
|
||||
</ActionIcon>
|
||||
</Popover.Target>
|
||||
<Popover.Dropdown>
|
||||
<Popover.Dropdown
|
||||
onKeyDown={(event) => {
|
||||
if (event.key !== "Escape") return;
|
||||
event.stopPropagation();
|
||||
setOpened(false);
|
||||
}}
|
||||
>
|
||||
<Stack gap="sm">
|
||||
<Box>
|
||||
<Text size="sm" fw={700}>
|
||||
@@ -2124,6 +2159,7 @@ const AutoApprovalControl = () => {
|
||||
</Text>
|
||||
</Box>
|
||||
<Button
|
||||
data-autofocus
|
||||
fullWidth
|
||||
variant={enabled ? "default" : "light"}
|
||||
color={enabled ? "gray" : "green"}
|
||||
|
||||
Reference in new issue
Block a user