diff options
| -rw-r--r-- | packages/core/src/notifications/ntfy.ts | 56 | ||||
| -rw-r--r-- | packages/core/tests/notifications/ntfy.test.ts | 45 | ||||
| -rw-r--r-- | packages/frontend/src/lib/components/SettingsPanel.svelte | 57 |
3 files changed, 130 insertions, 28 deletions
diff --git a/packages/core/src/notifications/ntfy.ts b/packages/core/src/notifications/ntfy.ts index 07ce33b..1f5101c 100644 --- a/packages/core/src/notifications/ntfy.ts +++ b/packages/core/src/notifications/ntfy.ts @@ -28,9 +28,17 @@ export type FetchLike = ( ) => Promise<{ ok: boolean; status: number; statusText?: string; text(): Promise<string> }>; /** - * Validate a ntfy topic URL. Accepts only `http(s)://host/topic` with a - * non-empty topic path. Returns `null` on success, a human-readable error - * string on failure. + * ntfy topic-name rules: 1–64 chars, ASCII alphanumerics + `-` and `_`. Sourced + * from the ntfy server (cf. binwiederhier/ntfy issue #1451 — longer names + * silently 404). Matching this client-side keeps users from saving topic URLs + * that look fine but only fail at publish time. + */ +const NTFY_TOPIC_RE = /^[A-Za-z0-9_-]{1,64}$/; + +/** + * Validate a ntfy topic URL. Accepts only `http(s)://host/topic` where + * `topic` is a single path segment of 1–64 chars matching `[A-Za-z0-9_-]`. + * Returns `null` on success, a human-readable error string on failure. */ export function validateTopicUrl(topicUrl: string): string | null { const trimmed = topicUrl.trim(); @@ -44,9 +52,15 @@ export function validateTopicUrl(topicUrl: string): string | null { if (url.protocol !== "http:" && url.protocol !== "https:") { return "Topic URL must use http:// or https://"; } - // Path must be a non-empty topic (more than just "/") + // Path must be exactly one topic segment. const topic = url.pathname.replace(/^\/+|\/+$/g, ""); if (!topic) return "Topic URL must include a topic name (e.g. https://ntfy.sh/my-topic)"; + if (topic.includes("/")) { + return "Topic URL must point at a single topic (no extra path segments)"; + } + if (!NTFY_TOPIC_RE.test(topic)) { + return "Topic name must be 1–64 characters, letters/numbers/underscore/hyphen only"; + } return null; } @@ -79,17 +93,17 @@ export async function sendNtfy( } const headers: Record<string, string> = { - // ntfy treats the title/priority/tags/click headers as ASCII-only. Strip - // control chars from the title; the body is sent UTF-8 verbatim. + // ntfy is tolerant of non-ASCII in the Title header but many proxies + // aren't — sanitizeHeader strips CR/LF/control chars (injection guard) + // and leaves UTF-8 in place. Body is sent verbatim as UTF-8. Title: sanitizeHeader(event.title), Priority: String(priority), "Content-Type": "text/plain; charset=utf-8", }; if (tags.length > 0) headers.Tags = tags.map(sanitizeHeader).join(","); - if (event.clickUrl) headers.Click = event.clickUrl; - if (config.authToken && config.authToken.trim() !== "") { - headers.Authorization = `Bearer ${config.authToken.trim()}`; - } + if (event.clickUrl) headers.Click = sanitizeHeader(event.clickUrl); + const authValue = buildAuthHeaderValue(config.authToken); + if (authValue) headers.Authorization = authValue; // Per-request abort so a hung server doesn't pin a Bun worker forever. const controller = new AbortController(); @@ -119,9 +133,27 @@ export async function sendNtfy( } } +/** + * Build the `Authorization` header value from the user-configured token. + * + * - Empty/blank ⇒ no header. + * - Already starts with `Bearer ` / `Basic ` / etc. (RFC 7235 scheme + space) + * ⇒ used verbatim. Lets users paste a complete header for Basic auth or + * any other scheme their private ntfy server supports. + * - Otherwise ⇒ prefixed with `Bearer ` (the common case). + */ +function buildAuthHeaderValue(rawToken: string): string | null { + const trimmed = (rawToken ?? "").trim(); + if (!trimmed) return null; + if (/^[A-Za-z][A-Za-z0-9._~+/-]*\s+\S/.test(trimmed)) { + // Already includes a scheme token (e.g. "Bearer xyz", "Basic dXNlcjpw"). + return sanitizeHeader(trimmed); + } + return `Bearer ${sanitizeHeader(trimmed)}`; +} + function sanitizeHeader(value: string): string { - // Strip CR/LF (header injection guard) and trim. ntfy is tolerant of - // non-ASCII in titles, but we still drop control chars. + // Strip CR/LF (header injection guard) and other control chars, then trim. // biome-ignore lint/suspicious/noControlCharactersInRegex: intentional return value.replace(/[\r\n\u0000-\u001f]+/g, " ").trim(); } diff --git a/packages/core/tests/notifications/ntfy.test.ts b/packages/core/tests/notifications/ntfy.test.ts index 3fb1d51..c183847 100644 --- a/packages/core/tests/notifications/ntfy.test.ts +++ b/packages/core/tests/notifications/ntfy.test.ts @@ -61,6 +61,27 @@ describe("validateTopicUrl", () => { expect(validateTopicUrl("https://ntfy.sh")).toMatch(/topic/); expect(validateTopicUrl("https://ntfy.sh/")).toMatch(/topic/); }); + + it("rejects multi-segment paths (topic must be a single segment)", () => { + expect(validateTopicUrl("https://ntfy.sh/team/sub")).toMatch(/single topic/); + }); + + it("rejects topic names with disallowed characters", () => { + expect(validateTopicUrl("https://ntfy.sh/has.dot")).toMatch(/letters\/numbers/); + expect(validateTopicUrl("https://ntfy.sh/has space")).toMatch(/letters\/numbers/); + expect(validateTopicUrl("https://ntfy.sh/has%20enc")).toMatch(/letters\/numbers/); + }); + + it("rejects topic names longer than 64 chars", () => { + const long = "a".repeat(65); + expect(validateTopicUrl(`https://ntfy.sh/${long}`)).toMatch(/64/); + }); + + it("accepts 64-char topic names and underscore/hyphen mixes", () => { + const maxlen = "a".repeat(64); + expect(validateTopicUrl(`https://ntfy.sh/${maxlen}`)).toBeNull(); + expect(validateTopicUrl("https://ntfy.sh/My_Topic-123")).toBeNull(); + }); }); describe("sendNtfy", () => { @@ -91,13 +112,23 @@ describe("sendNtfy", () => { expect(init.headers.Tags).toBe("rotating_light"); }); - it("attaches Authorization header when authToken is set", async () => { + it("attaches Authorization header with Bearer prefix when authToken is a bare token", async () => { const fetchImpl = makeFetch(); await sendNtfy(makeConfig({ authToken: "tk_secret " }), makeEvent(), fetchImpl); const init = fetchImpl.mock.calls[0][1]; expect(init.headers.Authorization).toBe("Bearer tk_secret"); }); + it("passes a pre-prefixed Authorization value (Basic, custom schemes) through verbatim", async () => { + const fetchImpl = makeFetch(); + await sendNtfy(makeConfig({ authToken: "Basic dXNlcjpwYXNz" }), makeEvent(), fetchImpl); + expect(fetchImpl.mock.calls[0][1].headers.Authorization).toBe("Basic dXNlcjpwYXNz"); + + const fetchImpl2 = makeFetch(); + await sendNtfy(makeConfig({ authToken: "Bearer already_prefixed" }), makeEvent(), fetchImpl2); + expect(fetchImpl2.mock.calls[0][1].headers.Authorization).toBe("Bearer already_prefixed"); + }); + it("omits Authorization when authToken is blank", async () => { const fetchImpl = makeFetch(); await sendNtfy(makeConfig({ authToken: " " }), makeEvent(), fetchImpl); @@ -112,6 +143,18 @@ describe("sendNtfy", () => { expect(init.headers.Click).toBe("https://example.com/tab/abc"); }); + it("sanitizes Click header (CR/LF injection guard)", async () => { + const fetchImpl = makeFetch(); + await sendNtfy( + makeConfig(), + makeEvent({ clickUrl: "https://example.com/\r\nInjected: yes" }), + fetchImpl, + ); + const v = fetchImpl.mock.calls[0][1].headers.Click; + expect(v).not.toContain("\n"); + expect(v).not.toContain("\r"); + }); + it("appends short tab tag when tabId is set", async () => { const fetchImpl = makeFetch(); await sendNtfy( diff --git a/packages/frontend/src/lib/components/SettingsPanel.svelte b/packages/frontend/src/lib/components/SettingsPanel.svelte index 1d9ebf7..8031500 100644 --- a/packages/frontend/src/lib/components/SettingsPanel.svelte +++ b/packages/frontend/src/lib/components/SettingsPanel.svelte @@ -72,6 +72,7 @@ let ntfySaveOk = $state(false); let ntfyTesting = $state(false); let ntfyTestResult = $state<string | null>(null); let ntfyTestOk = $state(false); +let ntfyClearingToken = $state(false); function onChunkLimitChange(e: Event): void { const input = e.target as HTMLInputElement; @@ -212,22 +213,43 @@ async function sendNtfyTest(): Promise<void> { } } -function clearNtfyAuthToken(): void { +async function clearNtfyAuthToken(): Promise<void> { // `""` ⇒ explicit clear on save (vs. `undefined` which keeps existing). - ntfyAuthTokenInput = ""; - ntfy = { ...ntfy, hasAuthToken: false }; - // Send a save with explicit empty string to clear server-side. - void (async () => { - try { - await fetch(`${apiBase}/notifications`, { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ authToken: "" }), - }); - } catch { - // ignore + // Optimistic local state on failure caused a real bug pre-review: UI showed + // the token cleared while the server still held it, then "Save" treated the + // blank input as "keep existing" and silently re-armed the old token. Await + // the response and only flip local state on success. + ntfyClearingToken = true; + ntfySaveError = null; + try { + const res = await fetch(`${apiBase}/notifications`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ authToken: "" }), + }); + const data = (await res.json().catch(() => ({}))) as { + config?: NtfyConfigView; + error?: string; + }; + if (!res.ok) { + ntfySaveError = data.error ?? `Clear failed (HTTP ${res.status})`; + return; } - })(); + ntfyAuthTokenInput = ""; + if (data.config) { + ntfy = { + ...DEFAULT_NTFY, + ...data.config, + events: { ...DEFAULT_NTFY.events, ...(data.config.events ?? {}) }, + }; + } else { + ntfy = { ...ntfy, hasAuthToken: false }; + } + } catch (e) { + ntfySaveError = e instanceof Error ? e.message : "Network error"; + } finally { + ntfyClearingToken = false; + } } async function toggleAutoExpand(): Promise<void> { @@ -421,9 +443,14 @@ $effect(() => { <button type="button" class="btn btn-xs btn-ghost btn-outline self-start" + disabled={ntfyClearingToken} onclick={clearNtfyAuthToken} > - Clear stored token + {#if ntfyClearingToken} + <span class="loading loading-spinner loading-xs"></span> + {:else} + Clear stored token + {/if} </button> {/if} </label> |