fix: address review #4 — board name invalidation, SVG tests, test gaps
- Pass board.name to all invalidateBoardCache calls (permission mutations) - Align integration create invalidation with update/delete - Add initial-fetch-failure test for integration-data-cache - Expand SVG XSS test corpus (javascript:, xlink:href, @import, onbegin) - Forbid <style> tags in SVG sanitizer to prevent CSS injection - Fix duplicate export in packages/common/src/index.ts
This commit is contained in:
7 files changed
+58
-5
No files matched your search
@@ -41,4 +41,29 @@ describe("SVG sanitization", () => {
|
||||
const wrapped = `<html><body>${dirty}</body></html>`;
|
||||
expect(sanitizeSvg(dirty)).toBe(purify.sanitize(wrapped, svgSanitizeOptions));
|
||||
});
|
||||
|
||||
it("strips javascript: protocol in href", () => {
|
||||
const dirty = '<svg><a href="javascript:alert(1)"><text>click</text></a></svg>';
|
||||
const clean = sanitizeSvg(dirty);
|
||||
expect(clean).not.toContain("javascript:");
|
||||
});
|
||||
|
||||
it("strips xlink:href with javascript:", () => {
|
||||
const dirty = '<svg><a xlink:href="javascript:alert(1)"><text>click</text></a></svg>';
|
||||
const clean = sanitizeSvg(dirty);
|
||||
expect(clean).not.toContain("javascript:");
|
||||
});
|
||||
|
||||
it("strips style with url() import", () => {
|
||||
const dirty =
|
||||
'<svg><style>@import url("https://evil.com/steal.css");</style><rect width="100" height="100"/></svg>';
|
||||
const clean = sanitizeSvg(dirty);
|
||||
expect(clean).not.toContain("@import");
|
||||
});
|
||||
|
||||
it("strips animate with onbegin handler", () => {
|
||||
const dirty = '<svg><animate onbegin="alert(1)"/></svg>';
|
||||
const clean = sanitizeSvg(dirty);
|
||||
expect(clean).not.toContain("onbegin");
|
||||
});
|
||||
});
|
||||
@@ -3,6 +3,7 @@ import { parseHTML } from "linkedom";
|
||||
|
||||
const svgSanitizeOptions = {
|
||||
USE_PROFILES: { svg: true, svgFilters: true },
|
||||
FORBID_TAGS: ["style"],
|
||||
} as const;
|
||||
|
||||
// ponytail: linkedom stores DOM tree pointers as own value properties; DOMPurify
|
||||
|
||||
@@ -41,6 +41,11 @@ describe("getCachedIntegrationData", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
test("throws on initial fetch failure when no stale data exists", async () => {
|
||||
const fetcher = vi.fn().mockRejectedValue(new Error("upstream down"));
|
||||
await expect(getCachedIntegrationData("integration-a", "q1", fetcher)).rejects.toThrow("upstream down");
|
||||
});
|
||||
|
||||
test("returns stale data when refetch fails", async () => {
|
||||
vi.useFakeTimers();
|
||||
const fetcher = vi
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// ponytail: module-level cache is single-instance only.
|
||||
// Multi-instance deployments (REDIS_IS_EXTERNAL=true) bypass this entirely —
|
||||
// each request calls the upstream fetcher directly.
|
||||
// isMultiInstance is read at module load; bypass behavior is verified via code review, not unit tests.
|
||||
const isMultiInstance = process.env.REDIS_IS_EXTERNAL === "true";
|
||||
|
||||
interface CachedEntry<T> {
|
||||
|
||||
@@ -604,6 +604,12 @@ export const boardRouter = createTRPCRouter({
|
||||
.output(z.void())
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.id), "full");
|
||||
|
||||
const existingBoard = await ctx.db.query.boards.findFirst({
|
||||
where: eq(boards.id, input.id),
|
||||
columns: { name: true },
|
||||
});
|
||||
|
||||
const boardSettings = await getServerSettingByKeyAsync(ctx.db, "board");
|
||||
|
||||
if (
|
||||
@@ -621,7 +627,7 @@ export const boardRouter = createTRPCRouter({
|
||||
.set({ isPublic: input.visibility === "public" })
|
||||
.where(eq(boards.id, input.id));
|
||||
|
||||
invalidateBoardCache(input.id);
|
||||
invalidateBoardCache(input.id, existingBoard?.name);
|
||||
}),
|
||||
deleteBoard: protectedProcedure
|
||||
.meta({
|
||||
@@ -852,6 +858,11 @@ export const boardRouter = createTRPCRouter({
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.id), "modify");
|
||||
|
||||
const existingBoard = await ctx.db.query.boards.findFirst({
|
||||
where: eq(boards.id, input.id),
|
||||
columns: { name: true },
|
||||
});
|
||||
|
||||
await ctx.db
|
||||
.update(boards)
|
||||
.set({
|
||||
@@ -882,7 +893,7 @@ export const boardRouter = createTRPCRouter({
|
||||
})
|
||||
.where(eq(boards.id, input.id));
|
||||
|
||||
invalidateBoardCache(input.id);
|
||||
invalidateBoardCache(input.id, existingBoard?.name);
|
||||
}),
|
||||
saveBoard: protectedProcedure.input(boardSaveSchema).mutation(async ({ input, ctx }) => {
|
||||
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.id), "modify");
|
||||
@@ -1377,6 +1388,11 @@ export const boardRouter = createTRPCRouter({
|
||||
saveUserBoardPermissions: protectedProcedure.input(boardSavePermissionsSchema).mutation(async ({ input, ctx }) => {
|
||||
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.entityId), "full");
|
||||
|
||||
const board = await ctx.db.query.boards.findFirst({
|
||||
where: eq(boards.id, input.entityId),
|
||||
columns: { name: true },
|
||||
});
|
||||
|
||||
const existingUserPermissions = await ctx.db.query.boardUserPermissions.findMany({
|
||||
columns: { userId: true },
|
||||
where: eq(boardUserPermissions.boardId, input.entityId),
|
||||
@@ -1427,11 +1443,16 @@ export const boardRouter = createTRPCRouter({
|
||||
for (const userId of userIds) {
|
||||
invalidateUserCache(userId);
|
||||
}
|
||||
invalidateBoardCache(input.entityId);
|
||||
invalidateBoardCache(input.entityId, board?.name);
|
||||
}),
|
||||
saveGroupBoardPermissions: protectedProcedure.input(boardSavePermissionsSchema).mutation(async ({ input, ctx }) => {
|
||||
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.entityId), "full");
|
||||
|
||||
const board = await ctx.db.query.boards.findFirst({
|
||||
where: eq(boards.id, input.entityId),
|
||||
columns: { name: true },
|
||||
});
|
||||
|
||||
const existingGroupPermissions = await ctx.db.query.boardGroupPermissions.findMany({
|
||||
columns: { groupId: true },
|
||||
where: eq(boardGroupPermissions.boardId, input.entityId),
|
||||
@@ -1490,7 +1511,7 @@ export const boardRouter = createTRPCRouter({
|
||||
invalidateUserCache(member.userId);
|
||||
}
|
||||
}
|
||||
invalidateBoardCache(input.entityId);
|
||||
invalidateBoardCache(input.entityId, board?.name);
|
||||
}),
|
||||
importOldmarrConfig: permissionRequiredProcedure
|
||||
.requiresPermission("board-create")
|
||||
|
||||
@@ -371,6 +371,7 @@ export const integrationRouter = createTRPCRouter({
|
||||
url: input.url,
|
||||
});
|
||||
|
||||
await invalidateIntegrationCacheAsync(integrationId);
|
||||
invalidateIntegrationCache(integrationId);
|
||||
}),
|
||||
update: protectedProcedure.input(integrationUpdateSchema).mutation(async ({ ctx, input }) => {
|
||||
|
||||
@@ -12,4 +12,3 @@ export * from "./error";
|
||||
export * from "./theme";
|
||||
export * from "./function";
|
||||
export * from "./retryable-loader";
|
||||
export * from "./id";
|
||||
Reference in new issue
Block a user