Skip to content

Commit c816dfc

Browse files
Fix(toast): action XSS and dispose toast group on hook destroy (#43)
* Fix(toast): action XSS and dispose toast group on hook destroy * update toast test
1 parent b3a8230 commit c816dfc

10 files changed

Lines changed: 213 additions & 40 deletions

File tree

assets/components/toast.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,15 @@ export class ToastItem<T = unknown> extends Component<ToastItemProps<T>, Api> {
168168
this.parts.action.hidden = false;
169169
this.spreadProps(this.parts.action, this.api.getActionTriggerProps());
170170
const label = this.latestProps.action?.label ?? "";
171-
if (this.parts.action.innerHTML !== label) {
172-
this.parts.action.innerHTML = label;
171+
if (this.parts.action.textContent !== label) {
172+
this.parts.action.textContent = label;
173173
}
174174
const extraClasses = actionClassTokens(this.latestProps.action);
175175
if (extraClasses.length) this.parts.action.classList.add(...extraClasses);
176176
} else {
177177
this.parts.action.hidden = true;
178-
if (this.parts.action.innerHTML) {
179-
this.parts.action.innerHTML = "";
178+
if (this.parts.action.textContent) {
179+
this.parts.action.textContent = "";
180180
}
181181
}
182182

assets/hooks/toast.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { Hook } from "phoenix_live_view";
22
import type { HookInterface, CallbackRef } from "phoenix_live_view/assets/js/types/view_hook";
33

4-
import { createToastGroup, getToastStore } from "../components/toast";
4+
import { createToastGroup, disposeToastGroup, getToastStore } from "../components/toast";
55
import type { ActionOptions } from "@zag-js/toast";
66
import type { Placement } from "@zag-js/toast";
77
import type { Options } from "@zag-js/toast";
@@ -354,6 +354,9 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
354354
this.removeHandleEvent(handler);
355355
}
356356
}
357+
if (this.groupId) {
358+
disposeToastGroup(this.groupId);
359+
}
357360
},
358361
};
359362

assets/test/component/toast.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,30 @@ describe("toast group API", () => {
6262
expect(container.dataset.toastGroup).toBeUndefined();
6363
expect(container.dataset.toastGroupId).toBeUndefined();
6464
});
65+
66+
it("renders action label as text without HTML injection", () => {
67+
const container = document.createElement("div");
68+
container.id = groupId;
69+
container.setAttribute("phx-hook", "Toast");
70+
document.body.appendChild(container);
71+
72+
const { group, store } = createToastGroup(container, { id: groupId });
73+
const malicious = '<img src=x onerror="window.__toastXss=1">';
74+
store.create({
75+
id: "t-xss",
76+
title: "Title",
77+
type: "info",
78+
action: { label: malicious, onClick: () => {} },
79+
});
80+
group.render();
81+
82+
const action = container.querySelector<HTMLElement>(
83+
'[data-scope="toast"][data-part="action-trigger"]'
84+
);
85+
expect(action).toBeTruthy();
86+
expect(action!.textContent).toBe(malicious);
87+
expect(action!.querySelector("img")).toBeNull();
88+
89+
document.body.removeChild(container);
90+
});
6591
});

assets/test/helpers/mock-hook.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { expect, vi } from "vitest";
2+
import type { CallbackRef } from "phoenix_live_view/assets/js/types/view_hook";
3+
import { mockLiveSocket } from "./mock-live-socket";
4+
5+
export function mockHookJs() {
6+
return {
7+
exec: vi.fn(),
8+
show: vi.fn(),
9+
hide: vi.fn(),
10+
toggle: vi.fn(),
11+
addClass: vi.fn(),
12+
removeClass: vi.fn(),
13+
toggleClass: vi.fn(),
14+
transition: vi.fn(),
15+
setAttribute: vi.fn(),
16+
removeAttribute: vi.fn(),
17+
toggleAttribute: vi.fn(),
18+
push: vi.fn(),
19+
navigate: vi.fn(),
20+
patch: vi.fn(),
21+
ignoreAttributes: vi.fn(),
22+
};
23+
}
24+
25+
export type MockHookJs = ReturnType<typeof mockHookJs>;
26+
27+
type BaseHookContext<E extends HTMLElement> = {
28+
el: E;
29+
pushEvent: ReturnType<typeof vi.fn>;
30+
js: () => MockHookJs;
31+
liveSocket: ReturnType<typeof mockLiveSocket>["ctx"]["liveSocket"];
32+
handleEvent: ReturnType<
33+
typeof vi.fn<(event: string, callback: (payload: unknown) => void) => CallbackRef>
34+
>;
35+
removeHandleEvent: ReturnType<typeof vi.fn>;
36+
};
37+
38+
type MockHookContextOptions<Extra extends Record<string, unknown>> = {
39+
connected?: boolean;
40+
overrides?: Extra;
41+
};
42+
43+
export function mockHookContext<
44+
E extends HTMLElement,
45+
Extra extends Record<string, unknown> = Record<string, never>,
46+
>(el: E, opts: MockHookContextOptions<Extra> = {}) {
47+
const connected = opts.connected ?? false;
48+
const { ctx, patch, navigate } = mockLiveSocket(connected);
49+
const jsCommands = mockHookJs();
50+
jsCommands.patch = patch;
51+
jsCommands.navigate = navigate;
52+
53+
const base: BaseHookContext<E> = {
54+
el,
55+
pushEvent: vi.fn(),
56+
js: () => jsCommands,
57+
liveSocket: ctx.liveSocket,
58+
handleEvent: vi.fn(
59+
(event: string, callback: (payload: unknown) => void): CallbackRef => ({
60+
event,
61+
callback,
62+
})
63+
),
64+
removeHandleEvent: vi.fn(),
65+
};
66+
67+
const hook = { ...base, ...opts.overrides } as BaseHookContext<E> & Extra;
68+
69+
return { hook, patch, navigate, jsCommands, liveSocket: ctx.liveSocket };
70+
}
71+
72+
type HookLifecycle = "mounted" | "destroyed" | "updated" | "beforeUpdate";
73+
74+
export function callHookLifecycle(
75+
hookModule: Partial<Record<HookLifecycle, (this: never) => void>>,
76+
hook: object,
77+
lifecycle: HookLifecycle
78+
): void {
79+
const fn = hookModule[lifecycle];
80+
expect(fn).toBeDefined();
81+
fn!.call(hook as never);
82+
}
83+
84+
export function callHookMounted(
85+
hookModule: Partial<Record<"mounted", (this: never) => void>>,
86+
hook: object
87+
): void {
88+
callHookLifecycle(hookModule, hook, "mounted");
89+
}
90+
91+
export function callHookDestroyed(
92+
hookModule: Partial<Record<"destroyed", (this: never) => void>>,
93+
hook: object
94+
): void {
95+
callHookLifecycle(hookModule, hook, "destroyed");
96+
}

assets/test/hooks/toast.test.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
import { describe, expect, it } from "vitest";
2-
import { parseActionSpec } from "../../hooks/toast";
1+
import { describe, expect, it, afterEach } from "vitest";
2+
import type { CallbackRef } from "phoenix_live_view/assets/js/types/view_hook";
3+
import { parseActionSpec, Toast } from "../../hooks/toast";
4+
import { getToastStore } from "../../components/toast";
5+
import { callHookDestroyed, callHookMounted, mockHookContext } from "../helpers/mock-hook";
36

47
describe("parseActionSpec", () => {
58
it("parses valid exec_js action", () => {
@@ -28,3 +31,34 @@ describe("parseActionSpec", () => {
2831
expect(parseActionSpec({ label: "X", effects: [{ kind: "other", encoded: "x" }] })).toBeNull();
2932
});
3033
});
34+
35+
describe("Toast hook lifecycle", () => {
36+
const groupId = "toast-hook-dispose-test";
37+
38+
afterEach(() => {
39+
document.getElementById(groupId)?.remove();
40+
});
41+
42+
it("destroyed disposes the toast group store", () => {
43+
const el = document.createElement("div");
44+
el.id = groupId;
45+
document.body.appendChild(el);
46+
47+
const { hook } = mockHookContext(el, {
48+
connected: false,
49+
overrides: {
50+
groupId: "",
51+
handlers: [] as CallbackRef[],
52+
domListeners: [] as Array<{ el: HTMLElement; name: string; fn: EventListener }>,
53+
},
54+
});
55+
56+
callHookMounted(Toast, hook);
57+
expect(getToastStore(groupId)).toBeDefined();
58+
expect(el.dataset.toastGroup).toBe("true");
59+
60+
callHookDestroyed(Toast, hook);
61+
expect(getToastStore(groupId)).toBeUndefined();
62+
expect(el.dataset.toastGroup).toBeUndefined();
63+
});
64+
});

lib/corex/toast/payload.ex

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ defmodule Corex.Toast.Payload do
44
import Corex.Helpers, only: [maybe_put: 3]
55

66
alias Corex.Toast.Action, as: ToastAction
7-
alias Phoenix.HTML.Safe
8-
alias Phoenix.LiveView.{JS, Rendered}
7+
alias Phoenix.LiveView.JS
98

109
@toast_type_strings Map.new(~W(info success error warning loading)a, &{&1, Atom.to_string(&1)})
1110

@@ -106,15 +105,6 @@ defmodule Corex.Toast.Payload do
106105
defp class_string(_), do: nil
107106

108107
defp action_label_html(label) when is_binary(label), do: label
109-
110-
defp action_label_html(%Rendered{} = rendered) do
111-
rendered |> Safe.to_iodata() |> IO.iodata_to_binary()
112-
end
113-
114-
defp action_label_html({:safe, _} = safe) do
115-
safe |> Safe.to_iodata() |> IO.iodata_to_binary()
116-
end
117-
118108
defp action_label_html(_), do: nil
119109

120110
defp encode_js_ops(%JS{} = js), do: Phoenix.json_library().encode!(js.ops)

priv/static/corex.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40320,6 +40320,16 @@ ${err}`);
4032040320
container.dataset.toastGroupId = groupId;
4032140321
return { group: group2, store: store2 };
4032240322
}
40323+
function disposeToastGroup(groupId) {
40324+
const group2 = toastGroups.get(groupId);
40325+
if (!group2) return;
40326+
const container = group2.el;
40327+
group2.destroy();
40328+
toastGroups.delete(groupId);
40329+
toastStores.delete(groupId);
40330+
delete container.dataset.toastGroup;
40331+
delete container.dataset.toastGroupId;
40332+
}
4032340333
function getToastStore(groupId) {
4032440334
if (groupId) return toastStores.get(groupId);
4032540335
const el = document.querySelector("[data-toast-group]");
@@ -41044,15 +41054,15 @@ ${err}`);
4104441054
this.parts.action.hidden = false;
4104541055
this.spreadProps(this.parts.action, this.api.getActionTriggerProps());
4104641056
const label = (_e = (_d = this.latestProps.action) == null ? void 0 : _d.label) != null ? _e : "";
41047-
if (this.parts.action.innerHTML !== label) {
41048-
this.parts.action.innerHTML = label;
41057+
if (this.parts.action.textContent !== label) {
41058+
this.parts.action.textContent = label;
4104941059
}
4105041060
const extraClasses = actionClassTokens(this.latestProps.action);
4105141061
if (extraClasses.length) this.parts.action.classList.add(...extraClasses);
4105241062
} else {
4105341063
this.parts.action.hidden = true;
41054-
if (this.parts.action.innerHTML) {
41055-
this.parts.action.innerHTML = "";
41064+
if (this.parts.action.textContent) {
41065+
this.parts.action.textContent = "";
4105641066
}
4105741067
}
4105841068
const duration = this.duration;
@@ -41352,6 +41362,9 @@ ${err}`);
4135241362
this.removeHandleEvent(handler);
4135341363
}
4135441364
}
41365+
if (this.groupId) {
41366+
disposeToastGroup(this.groupId);
41367+
}
4135541368
}
4135641369
};
4135741370
}

priv/static/corex.min.js

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

priv/static/toast.mjs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,15 +1260,15 @@ var ToastItem = class extends Component {
12601260
this.parts.action.hidden = false;
12611261
this.spreadProps(this.parts.action, this.api.getActionTriggerProps());
12621262
const label = this.latestProps.action?.label ?? "";
1263-
if (this.parts.action.innerHTML !== label) {
1264-
this.parts.action.innerHTML = label;
1263+
if (this.parts.action.textContent !== label) {
1264+
this.parts.action.textContent = label;
12651265
}
12661266
const extraClasses = actionClassTokens(this.latestProps.action);
12671267
if (extraClasses.length) this.parts.action.classList.add(...extraClasses);
12681268
} else {
12691269
this.parts.action.hidden = true;
1270-
if (this.parts.action.innerHTML) {
1271-
this.parts.action.innerHTML = "";
1270+
if (this.parts.action.textContent) {
1271+
this.parts.action.textContent = "";
12721272
}
12731273
}
12741274
const duration = this.duration;
@@ -1378,6 +1378,16 @@ function createToastGroup(container, options) {
13781378
container.dataset.toastGroupId = groupId;
13791379
return { group: group2, store };
13801380
}
1381+
function disposeToastGroup(groupId) {
1382+
const group2 = toastGroups.get(groupId);
1383+
if (!group2) return;
1384+
const container = group2.el;
1385+
group2.destroy();
1386+
toastGroups.delete(groupId);
1387+
toastStores.delete(groupId);
1388+
delete container.dataset.toastGroup;
1389+
delete container.dataset.toastGroupId;
1390+
}
13811391
function getToastStore(groupId) {
13821392
if (groupId) return toastStores.get(groupId);
13831393
const el = document.querySelector("[data-toast-group]");
@@ -1643,6 +1653,9 @@ var ToastHook = {
16431653
this.removeHandleEvent(handler);
16441654
}
16451655
}
1656+
if (this.groupId) {
1657+
disposeToastGroup(this.groupId);
1658+
}
16461659
}
16471660
};
16481661
export {

test/components/toast_test.exs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -405,15 +405,13 @@ defmodule Corex.ToastTest do
405405
assert Payload.normalize_action(%{label: "Go"}) == nil
406406
end
407407

408-
test "normalize_action encodes rendered and safe labels" do
408+
test "normalize_action rejects rendered and safe labels" do
409409
assigns = %{}
410410
rendered = ~H"Run"
411-
action = Payload.normalize_action(%{label: rendered, js: JS.push("go")})
412-
assert action["label"] =~ "Run"
411+
assert Payload.normalize_action(%{label: rendered, js: JS.push("go")}) == nil
413412

414413
safe = {:safe, "<b>Go</b>"}
415-
action2 = Payload.normalize_action(%{label: safe, js: JS.push("go")})
416-
assert action2["label"] =~ "Go"
414+
assert Payload.normalize_action(%{label: safe, js: JS.push("go")}) == nil
417415
end
418416

419417
test "update_detail drops unknown keys and nil priority" do

0 commit comments

Comments
 (0)