mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
Merge the review fixes for channel tests
This commit is contained in:
commit
669058d37c
18 files changed
+583
-87
No files matched your search
@@ -12375,7 +12375,8 @@ paths:
|
||||
"502":
|
||||
description: "The mail server, or the way to it, failed. `meta.errorCode` names the cause: `credentials_rejected` (the
|
||||
server refused the SMTP login), `upstream_rejected` (a permanent 5xx rejection, e.g. an unknown mailbox or
|
||||
relay denied), `upstream_error` (a temporary 4xx rejection), `timeout`, or `connection_failed`.
|
||||
relay denied, or an envelope or message refused without a reply code), `upstream_error` (a temporary 4xx
|
||||
rejection), `timeout`, or `connection_failed` (including a TLS handshake or protocol failure).
|
||||
`meta.smtpCode` carries the SMTP reply code when the mail server sent one."
|
||||
content:
|
||||
application/json:
|
||||
@@ -12421,8 +12422,9 @@ paths:
|
||||
(404/410), `rate_limited` (429), `upstream_error` (5xx), `redirected` (3xx, never followed), `timeout`, or
|
||||
`connection_failed`. `meta.upstreamStatus` carries the HTTP status whenever one arrived, and
|
||||
`meta.upstreamBody` up to 200 characters of what the other end answered, with control characters removed; it
|
||||
is omitted when the body was empty or could carry a credential (a token-shaped string, or the saved header
|
||||
value, auth token, topic or URL token)."
|
||||
is omitted when the body was empty or could carry a credential: a token-shaped string, or the saved auth
|
||||
token, the topic, or a credential in the server URL (userinfo, a query value, a path segment of 16 or more
|
||||
characters), literal, JSON-escaped or percent-encoded."
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
@@ -12491,8 +12493,9 @@ paths:
|
||||
(404/410), `rate_limited` (429), `upstream_error` (5xx), `redirected` (3xx, never followed), `timeout`, or
|
||||
`connection_failed`. `meta.upstreamStatus` carries the HTTP status whenever one arrived, and
|
||||
`meta.upstreamBody` up to 200 characters of what the other end answered, with control characters removed; it
|
||||
is omitted when the body was empty or could carry a credential (a token-shaped string, or the saved header
|
||||
value, auth token, topic or URL token)."
|
||||
is omitted when the body was empty or could carry a credential: a token-shaped string, or the saved header
|
||||
value (and its token part), the URL's userinfo, a query value, or a path segment of 16 or more characters
|
||||
(the token in a Discord or Slack webhook URL), literal, JSON-escaped or percent-encoded."
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
|
||||
@@ -274,6 +274,13 @@ short, Gotify's own error text. A `401` means the token is wrong. A `400`
|
||||
means Gotify could not read the body; check that the format is set to
|
||||
Gotify. A Gotify on your own network also needs the operator grant below.
|
||||
|
||||
The detail reaches the card inside the server's `502` response. A reverse
|
||||
proxy that replaces error pages for 5xx responses, such as Traefik's
|
||||
`errors` middleware or nginx with `proxy_intercept_errors on`, swaps that
|
||||
response for its own page, and the card can then only say "Test failed".
|
||||
Exclude the `/api/` paths from the error-page rule to keep the detail. This
|
||||
applies to the ntfy and email tests too.
|
||||
|
||||
## Notification targets on a private network
|
||||
|
||||
Every webhook and ntfy target is resolved at send time and refused when
|
||||
|
||||
@@ -347,6 +347,7 @@ describe("POST /api/admin/notifications/test — a failure keeps its cause (#947
|
||||
upstreamStatus: 400,
|
||||
});
|
||||
expect(body.data.results[0].error).toContain("HTTP 400");
|
||||
expect(body.data.results[0].error).not.toContain("hl-test");
|
||||
});
|
||||
|
||||
it("tests the webhook channel rather than calling it unknown", async () => {
|
||||
@@ -372,8 +373,9 @@ describe("POST /api/admin/notifications/test — a failure keeps its cause (#947
|
||||
success: false,
|
||||
errorCode: "upstream_rejected",
|
||||
upstreamStatus: 400,
|
||||
upstreamBody: "priority must be an integer",
|
||||
});
|
||||
// The relay body belongs to the settings card, not the admin toast.
|
||||
expect(body.data.results[0].upstreamBody).toBeUndefined();
|
||||
});
|
||||
|
||||
it("tests the email channel and forwards the SMTP code", async () => {
|
||||
|
||||
@@ -72,11 +72,12 @@ async function ensureTelegramChannel(userId: string): Promise<boolean> {
|
||||
return true;
|
||||
}
|
||||
|
||||
/** The admin toast renders `errorCode` and the status; the relay body stays on the settings card. */
|
||||
type TestResult = {
|
||||
channel: string;
|
||||
success: boolean;
|
||||
error?: string;
|
||||
} & TestFailureDetail;
|
||||
} & Omit<TestFailureDetail, "upstreamBody">;
|
||||
|
||||
/**
|
||||
* A failed HTTP or SMTP send, with the code, status and SMTP reply code the
|
||||
@@ -87,21 +88,26 @@ function failedSend(
|
||||
channel: string,
|
||||
label: string,
|
||||
result: SendOutcome,
|
||||
context?: string,
|
||||
): TestResult {
|
||||
const detail: TestFailureDetail = result.errorCode
|
||||
? { errorCode: result.errorCode }
|
||||
const { errorCode, upstreamStatus, smtpCode } = result.errorCode
|
||||
? {
|
||||
errorCode: result.errorCode,
|
||||
upstreamStatus: undefined,
|
||||
smtpCode: undefined,
|
||||
}
|
||||
: testFailureDetail(result);
|
||||
const cause = result.errorCode
|
||||
? `refused by the private-origin policy (${result.errorCode})`
|
||||
: detail.errorCode
|
||||
? testFailureSentence(label, detail)
|
||||
: errorCode
|
||||
? testFailureSentence(label, { errorCode, upstreamStatus, smtpCode })
|
||||
: (result.reason ?? "send failed");
|
||||
return {
|
||||
channel,
|
||||
success: false,
|
||||
error: context ? `${cause} (${context})` : cause,
|
||||
...detail,
|
||||
error: cause,
|
||||
...(errorCode ? { errorCode } : {}),
|
||||
...(upstreamStatus !== undefined ? { upstreamStatus } : {}),
|
||||
...(smtpCode !== undefined ? { smtpCode } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -205,14 +211,8 @@ export const POST = apiHandler(async () => {
|
||||
const ntfyResult = await sendViaNtfy(config, payload);
|
||||
success = ntfyResult.ok;
|
||||
if (!success) {
|
||||
results.push(
|
||||
failedSend(
|
||||
"NTFY",
|
||||
"The ntfy server",
|
||||
ntfyResult,
|
||||
`topic: ${config.topic}`,
|
||||
),
|
||||
);
|
||||
// The topic is not echoed: on a public server it is the secret.
|
||||
results.push(failedSend("NTFY", "The ntfy server", ntfyResult));
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
|
||||
@@ -52,7 +52,10 @@ vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
const emitIfSampledMock = vi.fn();
|
||||
vi.mock("@/lib/logging/transports", () => ({
|
||||
emitIfSampled: (...args: unknown[]) => emitIfSampledMock(...args),
|
||||
}));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
@@ -374,6 +377,10 @@ describe("POST /api/settings/{webhook,ntfy}/test — the refusal names itself",
|
||||
upstreamBody: '{"error":"Bad Request","errorCode":400}',
|
||||
});
|
||||
expect(json.error).toContain("HTTP 400");
|
||||
// A user's relay refusing is not an operator error: the event is warn.
|
||||
expect(emitIfSampledMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ level: "warn" }),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -444,6 +451,9 @@ describe("POST /api/settings/{webhook,ntfy}/test — the refusal names itself",
|
||||
expect(response.status).toBe(500);
|
||||
const json = await response.json();
|
||||
expect(json.meta).toBeUndefined();
|
||||
expect(emitIfSampledMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ level: "error" }),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
/**
|
||||
* The admin notification test toast names each failing channel's cause
|
||||
* (review L5): the code the route sends is translated and the status is
|
||||
* shown, so the per-channel fields in the response have a reader.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { describeChannelTestFailures } from "../notification-test-failures";
|
||||
|
||||
const MESSAGES: Record<string, string> = {
|
||||
"settings.testConnection.errors.upstream_rejected":
|
||||
"The other end refused the message",
|
||||
"settings.testConnection.errors.credentials_rejected": "Credentials rejected",
|
||||
"admin.notificationTestFailed": "Failed to send test notification",
|
||||
};
|
||||
const t = (key: string) => MESSAGES[key] ?? key;
|
||||
|
||||
describe("describeChannelTestFailures", () => {
|
||||
it("lists only failing channels, with the translated cause and the status", () => {
|
||||
expect(
|
||||
describeChannelTestFailures(
|
||||
[
|
||||
{ channel: "APNS", success: true },
|
||||
{
|
||||
channel: "WEBHOOK",
|
||||
success: false,
|
||||
error: "The webhook answered HTTP 400.",
|
||||
errorCode: "upstream_rejected",
|
||||
upstreamStatus: 400,
|
||||
},
|
||||
{
|
||||
channel: "EMAIL",
|
||||
success: false,
|
||||
error: "The mail server rejected the credentials (SMTP 535).",
|
||||
errorCode: "credentials_rejected",
|
||||
smtpCode: 535,
|
||||
},
|
||||
],
|
||||
t,
|
||||
),
|
||||
).toEqual([
|
||||
"Webhook: The other end refused the message (HTTP 400)",
|
||||
"Email: Credentials rejected (SMTP 535)",
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to the route's sentence when there is no known code", () => {
|
||||
expect(
|
||||
describeChannelTestFailures(
|
||||
[
|
||||
{
|
||||
channel: "NTFY",
|
||||
success: false,
|
||||
error: "SYSTEM_ALERT disabled in settings",
|
||||
},
|
||||
{ channel: "MYSTERY", success: false, errorCode: "unlisted" },
|
||||
],
|
||||
t,
|
||||
),
|
||||
).toEqual([
|
||||
"ntfy: SYSTEM_ALERT disabled in settings",
|
||||
"MYSTERY: Failed to send test notification",
|
||||
]);
|
||||
});
|
||||
|
||||
it("is what the admin toast renders", () => {
|
||||
const src = readFileSync(
|
||||
join(process.cwd(), "src/components/admin/reminders-section.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
expect(src).toMatch(/describeChannelTestFailures\(data\?\.results/);
|
||||
expect(src).toMatch(/description:/);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,46 @@
|
||||
import {
|
||||
CHANNEL_TYPE_LABELS,
|
||||
type ChannelType,
|
||||
} from "@/lib/notifications/types";
|
||||
|
||||
/** One channel's row in the admin notification test response. */
|
||||
export interface ChannelTestResult {
|
||||
channel: string;
|
||||
success: boolean;
|
||||
error?: string;
|
||||
errorCode?: string;
|
||||
upstreamStatus?: number;
|
||||
smtpCode?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* One line per failing channel for the admin test toast: the channel name,
|
||||
* the cause in the admin's language (the same `settings.testConnection.errors`
|
||||
* vocabulary the settings cards use), and the HTTP status or SMTP reply code.
|
||||
* A row without a known code falls back to the route's own sentence.
|
||||
*/
|
||||
export function describeChannelTestFailures(
|
||||
results: ReadonlyArray<ChannelTestResult>,
|
||||
t: (key: string) => string,
|
||||
): string[] {
|
||||
return results
|
||||
.filter((r) => !r.success)
|
||||
.map((r) => {
|
||||
const label = CHANNEL_TYPE_LABELS[r.channel as ChannelType] ?? r.channel;
|
||||
const key = r.errorCode
|
||||
? `settings.testConnection.errors.${r.errorCode}`
|
||||
: null;
|
||||
const translated = key ? t(key) : null;
|
||||
const cause =
|
||||
translated && translated !== key
|
||||
? translated
|
||||
: (r.error ?? t("admin.notificationTestFailed"));
|
||||
const code =
|
||||
typeof r.upstreamStatus === "number"
|
||||
? ` (HTTP ${r.upstreamStatus})`
|
||||
: typeof r.smtpCode === "number"
|
||||
? ` (SMTP ${r.smtpCode})`
|
||||
: "";
|
||||
return `${label}: ${cause}${code}`;
|
||||
});
|
||||
}
|
||||
@@ -22,6 +22,10 @@ import { useTranslations } from "@/lib/i18n/context";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { useAdminSettings, useUpdateSettings } from "./_shared";
|
||||
import { apiGet, apiPost } from "@/lib/api/api-fetch";
|
||||
import {
|
||||
describeChannelTestFailures,
|
||||
type ChannelTestResult,
|
||||
} from "./notification-test-failures";
|
||||
|
||||
interface NotificationHealth {
|
||||
windowHours: number;
|
||||
@@ -132,19 +136,23 @@ export function RemindersSection() {
|
||||
return apiPost<
|
||||
| {
|
||||
message?: string;
|
||||
results?: Array<{
|
||||
channel: string;
|
||||
success: boolean;
|
||||
error?: string;
|
||||
}>;
|
||||
results?: ChannelTestResult[];
|
||||
}
|
||||
| undefined
|
||||
>("/api/admin/notifications/test");
|
||||
},
|
||||
onSuccess: (data) => {
|
||||
const hasFailures = data?.results?.some((r) => !r.success);
|
||||
if (hasFailures) {
|
||||
toast.error(data?.message ?? t("admin.notificationTestFailed"));
|
||||
const failures = describeChannelTestFailures(data?.results ?? [], t);
|
||||
if (failures.length > 0) {
|
||||
toast.error(data?.message ?? t("admin.notificationTestFailed"), {
|
||||
description: (
|
||||
<ul className="space-y-0.5">
|
||||
{failures.map((line) => (
|
||||
<li key={line}>{line}</li>
|
||||
))}
|
||||
</ul>
|
||||
),
|
||||
});
|
||||
} else {
|
||||
toast.success(data?.message ?? t("admin.notificationTestSuccess"));
|
||||
}
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
/**
|
||||
* How the test-connection button reads a real failure response (#947).
|
||||
*
|
||||
* The component suite is SSR-only, so the button's probe is exercised as the
|
||||
* function the button calls: `apiFetchRaw` is stubbed with the envelope a
|
||||
* notification test route sends, and the result is rendered through the same
|
||||
* callout the button renders. Removing the meta extraction turns this red.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const apiFetchRawMock = vi.fn();
|
||||
vi.mock("@/lib/api/api-fetch", () => ({
|
||||
apiFetchRaw: (...args: unknown[]) => apiFetchRawMock(...args),
|
||||
}));
|
||||
|
||||
import {
|
||||
runConnectionTest,
|
||||
TestConnectionFailure,
|
||||
} from "../test-connection-button";
|
||||
|
||||
function envelope(status: number, body: unknown): Response {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
function renderFailure(
|
||||
result: Awaited<ReturnType<typeof runConnectionTest>>,
|
||||
): string {
|
||||
if (result.kind !== "error") throw new Error("expected a failure");
|
||||
return renderToStaticMarkup(
|
||||
<TestConnectionFailure
|
||||
message={`code:${result.errorCode}`}
|
||||
upstreamStatus={result.upstreamStatus}
|
||||
smtpCode={result.smtpCode}
|
||||
upstreamBody={result.upstreamBody}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
apiFetchRawMock.mockReset();
|
||||
});
|
||||
|
||||
describe("runConnectionTest", () => {
|
||||
it("reads the code, the HTTP status and what the relay said from a 502", async () => {
|
||||
apiFetchRawMock.mockResolvedValue(
|
||||
envelope(502, {
|
||||
data: null,
|
||||
error: "The webhook answered HTTP 400.",
|
||||
meta: {
|
||||
errorCode: "upstream_rejected",
|
||||
upstreamStatus: 400,
|
||||
upstreamBody: "priority must be an integer",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await runConnectionTest("/api/settings/webhook/test");
|
||||
|
||||
expect(apiFetchRawMock).toHaveBeenCalledWith("/api/settings/webhook/test", {
|
||||
method: "POST",
|
||||
});
|
||||
const html = renderFailure(result);
|
||||
expect(html).toContain("code:upstream_rejected (HTTP 400)");
|
||||
expect(html).toContain("priority must be an integer");
|
||||
});
|
||||
|
||||
it("reads the SMTP reply code from an email failure", async () => {
|
||||
apiFetchRawMock.mockResolvedValue(
|
||||
envelope(502, {
|
||||
data: null,
|
||||
error: "The mail server rejected the credentials (SMTP 535).",
|
||||
meta: { errorCode: "credentials_rejected", smtpCode: 535 },
|
||||
}),
|
||||
);
|
||||
|
||||
const html = renderFailure(
|
||||
await runConnectionTest("/api/settings/email/test"),
|
||||
);
|
||||
|
||||
expect(html).toContain("code:credentials_rejected (SMTP 535)");
|
||||
expect(html).not.toContain("test-connection-upstream-body");
|
||||
});
|
||||
|
||||
it("falls back to the generic code when a proxy replaced the body", async () => {
|
||||
apiFetchRawMock.mockResolvedValue(
|
||||
new Response("<html>Bad Gateway</html>", { status: 502 }),
|
||||
);
|
||||
|
||||
expect(await runConnectionTest("/api/settings/ntfy/test")).toEqual({
|
||||
kind: "error",
|
||||
errorCode: "generic",
|
||||
upstreamStatus: undefined,
|
||||
smtpCode: undefined,
|
||||
upstreamBody: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("reports success with the latency", async () => {
|
||||
apiFetchRawMock.mockResolvedValue(
|
||||
envelope(200, { data: { ok: true, latencyMs: 42 }, error: null }),
|
||||
);
|
||||
|
||||
expect(await runConnectionTest("/api/nightscout/test")).toEqual({
|
||||
kind: "ok",
|
||||
latency: 42,
|
||||
});
|
||||
});
|
||||
|
||||
it("is what the button calls", () => {
|
||||
const src = readFileSync(
|
||||
join(process.cwd(), "src/components/settings/test-connection-button.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
expect(src).toMatch(/setResult\(await runConnectionTest\(endpoint\)\)/);
|
||||
});
|
||||
});
|
||||
@@ -82,6 +82,48 @@ export function TestConnectionFailure({
|
||||
);
|
||||
}
|
||||
|
||||
export type ConnectionTestResult =
|
||||
| { kind: "ok"; latency: number }
|
||||
| {
|
||||
kind: "error";
|
||||
errorCode: string;
|
||||
upstreamStatus?: number;
|
||||
smtpCode?: number;
|
||||
upstreamBody?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* The button's probe: POST the endpoint and read the envelope into what the
|
||||
* button shows. Exported so the reading of a real failure response can be
|
||||
* tested without a DOM.
|
||||
*/
|
||||
export async function runConnectionTest(
|
||||
endpoint: string,
|
||||
): Promise<ConnectionTestResult> {
|
||||
try {
|
||||
const res = await apiFetchRaw(endpoint, { method: "POST" });
|
||||
const json = (await res.json().catch(() => ({}))) as TestResponse;
|
||||
|
||||
if (res.ok && json.data?.ok !== false) {
|
||||
return { kind: "ok", latency: json.data?.latencyMs ?? 0 };
|
||||
}
|
||||
const meta = json.meta;
|
||||
return {
|
||||
kind: "error",
|
||||
errorCode: meta?.errorCode ?? "generic",
|
||||
upstreamStatus:
|
||||
typeof meta?.upstreamStatus === "number"
|
||||
? meta.upstreamStatus
|
||||
: undefined,
|
||||
smtpCode: typeof meta?.smtpCode === "number" ? meta.smtpCode : undefined,
|
||||
upstreamBody:
|
||||
typeof meta?.upstreamBody === "string" ? meta.upstreamBody : undefined,
|
||||
};
|
||||
} catch {
|
||||
return { kind: "error", errorCode: "connection_failed" };
|
||||
}
|
||||
}
|
||||
|
||||
export function TestConnectionButton({
|
||||
endpoint,
|
||||
disabled = false,
|
||||
@@ -89,49 +131,13 @@ export function TestConnectionButton({
|
||||
}: TestConnectionButtonProps) {
|
||||
const { t } = useTranslations();
|
||||
const [testing, setTesting] = useState(false);
|
||||
const [result, setResult] = useState<
|
||||
| { kind: "ok"; latency: number }
|
||||
| {
|
||||
kind: "error";
|
||||
errorCode: string;
|
||||
upstreamStatus?: number;
|
||||
smtpCode?: number;
|
||||
upstreamBody?: string;
|
||||
}
|
||||
| null
|
||||
>(null);
|
||||
const [result, setResult] = useState<ConnectionTestResult | null>(null);
|
||||
|
||||
async function handleClick() {
|
||||
setTesting(true);
|
||||
setResult(null);
|
||||
try {
|
||||
const res = await apiFetchRaw(endpoint, { method: "POST" });
|
||||
const json = (await res.json().catch(() => ({}))) as TestResponse;
|
||||
|
||||
if (res.ok && json.data?.ok !== false) {
|
||||
setResult({
|
||||
kind: "ok",
|
||||
latency: json.data?.latencyMs ?? 0,
|
||||
});
|
||||
} else {
|
||||
const meta = json.meta;
|
||||
setResult({
|
||||
kind: "error",
|
||||
errorCode: meta?.errorCode ?? "generic",
|
||||
upstreamStatus:
|
||||
typeof meta?.upstreamStatus === "number"
|
||||
? meta.upstreamStatus
|
||||
: undefined,
|
||||
smtpCode:
|
||||
typeof meta?.smtpCode === "number" ? meta.smtpCode : undefined,
|
||||
upstreamBody:
|
||||
typeof meta?.upstreamBody === "string"
|
||||
? meta.upstreamBody
|
||||
: undefined,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
setResult({ kind: "error", errorCode: "connection_failed" });
|
||||
setResult(await runConnectionTest(endpoint));
|
||||
} finally {
|
||||
setTesting(false);
|
||||
}
|
||||
|
||||
@@ -110,3 +110,33 @@ describe("WideEventBuilder.setHttp", () => {
|
||||
expect(evt.toJSON().http).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("WideEventBuilder.capStatusLevel", () => {
|
||||
it("derives error from a 5xx by default", () => {
|
||||
const evt = new WideEventBuilder("http");
|
||||
evt.finish(502);
|
||||
expect(evt.toJSON().level).toBe("error");
|
||||
});
|
||||
|
||||
it("keeps a capped 5xx at warn", () => {
|
||||
const evt = new WideEventBuilder("http");
|
||||
evt.capStatusLevel("warn");
|
||||
evt.finish(502);
|
||||
expect(evt.toJSON().level).toBe("warn");
|
||||
});
|
||||
|
||||
it("does not lower an explicit elevation", () => {
|
||||
const evt = new WideEventBuilder("http");
|
||||
evt.capStatusLevel("warn");
|
||||
evt.elevateLevel("error");
|
||||
evt.finish(502);
|
||||
expect(evt.toJSON().level).toBe("error");
|
||||
});
|
||||
|
||||
it("leaves a 4xx at warn under the cap", () => {
|
||||
const evt = new WideEventBuilder("http");
|
||||
evt.capStatusLevel("warn");
|
||||
evt.finish(422);
|
||||
expect(evt.toJSON().level).toBe("warn");
|
||||
});
|
||||
});
|
||||
@@ -43,6 +43,7 @@ export class WideEventBuilder {
|
||||
private event: Partial<WideEvent>;
|
||||
private startTime: number;
|
||||
private highestLevel: LogLevel = "info";
|
||||
private statusLevelCeiling: LogLevel = "error";
|
||||
|
||||
constructor(kind: EventKind = "http") {
|
||||
this.startTime = performance.now();
|
||||
@@ -239,6 +240,18 @@ export class WideEventBuilder {
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ceiling for the level `finish()` derives from the HTTP status. A route
|
||||
* whose 5xx reports someone else's refusal (a user's relay answering a
|
||||
* test with 400) caps it at `warn`, so a user's configuration does not
|
||||
* raise an operator-facing error. `elevateLevel` calls and thrown errors
|
||||
* are unaffected.
|
||||
*/
|
||||
capStatusLevel(level: LogLevel): this {
|
||||
this.statusLevelCeiling = level;
|
||||
return this;
|
||||
}
|
||||
|
||||
/** Request/Operation beenden, Duration berechnen */
|
||||
finish(httpStatus?: number): this {
|
||||
this.event.duration_ms = Math.round(performance.now() - this.startTime);
|
||||
@@ -256,8 +269,16 @@ export class WideEventBuilder {
|
||||
this.event.http.status = httpStatus;
|
||||
}
|
||||
if (httpStatus !== undefined) {
|
||||
if (httpStatus >= 500) this.elevateLevel("error");
|
||||
else if (httpStatus >= 400) this.elevateLevel("warn");
|
||||
const derived: LogLevel | null =
|
||||
httpStatus >= 500 ? "error" : httpStatus >= 400 ? "warn" : null;
|
||||
if (derived) {
|
||||
this.elevateLevel(
|
||||
LOG_LEVEL_PRIORITY[derived] >
|
||||
LOG_LEVEL_PRIORITY[this.statusLevelCeiling]
|
||||
? this.statusLevelCeiling
|
||||
: derived,
|
||||
);
|
||||
}
|
||||
}
|
||||
this.event.level = this.highestLevel;
|
||||
return this;
|
||||
|
||||
@@ -95,7 +95,7 @@ describe("secret extraction", () => {
|
||||
secretsInUrl(
|
||||
"https://user:pa%20ss@gotify.example.com/message?token=Abc123",
|
||||
),
|
||||
).toEqual(["Abc123", "user", "pa ss"]);
|
||||
).toEqual(expect.arrayContaining(["Abc123", "user", "pa%20ss", "pa ss"]));
|
||||
expect(secretsInUrl("not a url")).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -108,3 +108,65 @@ describe("secret extraction", () => {
|
||||
expect(secretsInHeaderValue(undefined)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("secret matching survives encoding and malformed parts (review L4)", () => {
|
||||
it("keeps the query token when the userinfo holds a malformed escape", () => {
|
||||
const found = secretsInUrl(
|
||||
"https://us%ZZer:pw@gotify.example.com/message?token=Abc12345",
|
||||
);
|
||||
expect(found).toContain("Abc12345");
|
||||
expect(found).toContain("us%ZZer");
|
||||
});
|
||||
|
||||
it("keeps the other query values when one holds a malformed escape", () => {
|
||||
const found = secretsInUrl(
|
||||
"https://gotify.example.com/message?a=%ZZ&token=Abc12345",
|
||||
);
|
||||
expect(found).toContain("Abc12345");
|
||||
expect(found).toContain("%ZZ");
|
||||
});
|
||||
|
||||
it("treats long path segments as secrets and short ones not", () => {
|
||||
const found = secretsInUrl(
|
||||
"https://discord.com/api/webhooks/123456789012345678/AbCdEfGhIjKlMnOpQrSt",
|
||||
);
|
||||
expect(found).toContain("123456789012345678");
|
||||
expect(found).toContain("AbCdEfGhIjKlMnOpQrSt");
|
||||
expect(found).not.toContain("webhooks");
|
||||
expect(found).not.toContain("api");
|
||||
});
|
||||
|
||||
it("refuses a body echoing a path token", async () => {
|
||||
await expect(
|
||||
readUpstreamBody(
|
||||
res('{"message":"Unknown Webhook AbCdEfGhIjKlMnOpQrSt"}', 404),
|
||||
secretsInUrl(
|
||||
"https://discord.com/api/webhooks/123/AbCdEfGhIjKlMnOpQrSt",
|
||||
),
|
||||
),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("refuses a JSON-escaped echo with an escaped slash", async () => {
|
||||
await expect(
|
||||
readUpstreamBody(res('{"auth":"Basic dXNlcj\\/wYXNz"}'), [
|
||||
"dXNlcj/wYXNz",
|
||||
]),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("refuses a JSON-escaped echo with an escaped quote", async () => {
|
||||
await expect(
|
||||
readUpstreamBody(res('{"got":"ab\\"cd1234"}'), ['ab"cd1234']),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("refuses a percent-encoded echo, in either hex case", async () => {
|
||||
await expect(
|
||||
readUpstreamBody(res("bad token p%40ss%20word!"), ["p@ss word!"]),
|
||||
).resolves.toBeUndefined();
|
||||
await expect(
|
||||
readUpstreamBody(res("bad token a%2fb%2Bc1234"), ["a/b+c1234"]),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -237,3 +237,43 @@ describe("sendViaEmail — the failure names itself for the test button", () =>
|
||||
expect(result.smtpCode).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("sendViaEmail — transport and envelope faults are named too", () => {
|
||||
it.each([
|
||||
["a TLS handshake failure", "ETLS", undefined, "connection_failed"],
|
||||
["a protocol error", "EPROTOCOL", undefined, "connection_failed"],
|
||||
[
|
||||
"an envelope refused without a code",
|
||||
"EENVELOPE",
|
||||
undefined,
|
||||
"upstream_rejected",
|
||||
],
|
||||
[
|
||||
"a message refused without a code",
|
||||
"EMESSAGE",
|
||||
undefined,
|
||||
"upstream_rejected",
|
||||
],
|
||||
[
|
||||
"an envelope refused with a temporary code",
|
||||
"EENVELOPE",
|
||||
451,
|
||||
"upstream_error",
|
||||
],
|
||||
])("%s", async (_label, code, responseCode, failureCode) => {
|
||||
loadEmailConfigMock.mockReturnValue(transport);
|
||||
sendMailMock.mockRejectedValue(
|
||||
Object.assign(new Error(String(code)), {
|
||||
code,
|
||||
...(responseCode !== undefined ? { responseCode } : {}),
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await sendViaEmail(
|
||||
{ recipient: "you@example.com" },
|
||||
payload(),
|
||||
);
|
||||
|
||||
expect(result.failureCode).toBe(failureCode);
|
||||
});
|
||||
});
|
||||
@@ -106,13 +106,20 @@ function classifySmtpError(err: unknown): {
|
||||
: code === "ECONNECTION" ||
|
||||
code === "ESOCKET" ||
|
||||
code === "EDNS" ||
|
||||
code === "ECONNREFUSED"
|
||||
code === "ECONNREFUSED" ||
|
||||
// STARTTLS or TLS handshake failure (typically port 465 set up
|
||||
// as 587 or the reverse) and a server that does not speak SMTP.
|
||||
code === "ETLS" ||
|
||||
code === "EPROTOCOL"
|
||||
? "connection_failed"
|
||||
: smtpCode !== undefined && smtpCode >= 500
|
||||
? "upstream_rejected"
|
||||
: smtpCode !== undefined && smtpCode >= 400
|
||||
? "upstream_error"
|
||||
: undefined;
|
||||
: // An envelope or message refused without a reply code.
|
||||
code === "EENVELOPE" || code === "EMESSAGE"
|
||||
? "upstream_rejected"
|
||||
: undefined;
|
||||
const extras = {
|
||||
...(failureCode ? { failureCode } : {}),
|
||||
...(smtpCode !== undefined ? { smtpCode } : {}),
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { apiError } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { annotate, getEvent } from "@/lib/logging/context";
|
||||
import type { SendOutcome } from "@/lib/notifications/retry-policy";
|
||||
|
||||
/**
|
||||
@@ -78,7 +78,8 @@ export function testFailureSentence(
|
||||
* The other end, or the way to it, failed: 502 with `meta.errorCode`, the
|
||||
* upstream status or SMTP code, and what the relay said when it is short
|
||||
* and holds no secret. A failure the sender could not name is an internal
|
||||
* fault and stays a 500 with `fallbackMessage`.
|
||||
* fault and stays a 500 with `fallbackMessage`. The 502 is recorded at
|
||||
* `warn`, the 500 at `error`.
|
||||
*/
|
||||
export function answerTestDeliveryFailure(
|
||||
result: SendOutcome,
|
||||
@@ -105,5 +106,9 @@ export function answerTestDeliveryFailure(
|
||||
if (!detail.errorCode) {
|
||||
return apiError(opts.fallbackMessage, 500);
|
||||
}
|
||||
// 502 is the honest status: the other end failed, not this request. But
|
||||
// the cause is the user's relay or mail setup, so the wide event stays at
|
||||
// warn rather than raising an operator-facing error on every test press.
|
||||
getEvent()?.capStatusLevel("warn");
|
||||
return apiError(testFailureSentence(opts.label, detail), 502, { ...detail });
|
||||
}
|
||||
@@ -52,6 +52,36 @@ async function readBounded(res: Response): Promise<string | undefined> {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The ways a relay can echo a secret back: literally, JSON-escaped (with and
|
||||
* without the `\/` form some serialisers write for a slash, common inside a
|
||||
* base64 value), and percent-encoded.
|
||||
*/
|
||||
function echoForms(secret: string): string[] {
|
||||
const json = JSON.stringify(secret).slice(1, -1);
|
||||
return [
|
||||
...new Set([
|
||||
secret,
|
||||
json,
|
||||
json.replaceAll("/", "\\/"),
|
||||
encodeURIComponent(secret),
|
||||
]),
|
||||
];
|
||||
}
|
||||
|
||||
/** A value and its decoded form; a malformed escape keeps the raw value. */
|
||||
function withDecoded(part: string): string[] {
|
||||
try {
|
||||
const decoded = decodeURIComponent(part.replaceAll("+", " "));
|
||||
return decoded === part ? [part] : [part, decoded];
|
||||
} catch {
|
||||
return [part];
|
||||
}
|
||||
}
|
||||
|
||||
/** Path segments this long are credentials in Discord, Slack and n8n URLs. */
|
||||
const MIN_SECRET_PATH_SEGMENT_LENGTH = 16;
|
||||
|
||||
/**
|
||||
* Read what a relay answered on a non-2xx, in a form safe to hand back to
|
||||
* the person who pressed the test button.
|
||||
@@ -87,9 +117,16 @@ export async function readUpstreamBody(
|
||||
if (!cleaned) return undefined;
|
||||
|
||||
if (looksSecretShaped(raw) || looksSecretShaped(cleaned)) return undefined;
|
||||
const rawLower = raw.toLowerCase();
|
||||
for (const secret of knownSecrets) {
|
||||
if (!secret || secret.length < MIN_KNOWN_SECRET_LENGTH) continue;
|
||||
if (raw.includes(secret) || cleaned.includes(secret)) return undefined;
|
||||
for (const form of echoForms(secret)) {
|
||||
if (raw.includes(form) || cleaned.includes(form)) return undefined;
|
||||
}
|
||||
// Percent-encoding is case-insensitive in its hex digits.
|
||||
if (rawLower.includes(encodeURIComponent(secret).toLowerCase())) {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
const characters = Array.from(cleaned);
|
||||
@@ -98,19 +135,33 @@ export async function readUpstreamBody(
|
||||
}
|
||||
|
||||
/**
|
||||
* The secrets a URL can carry on its own: userinfo and every query value
|
||||
* (Gotify's `?token=` form).
|
||||
* The secrets a URL can carry on its own: userinfo, every query value
|
||||
* (Gotify's `?token=` form) and every path segment of 16 or more characters
|
||||
* (a Discord or Slack webhook token, an n8n webhook id). Each part is decoded
|
||||
* on its own, so one malformed escape cannot drop the others.
|
||||
*/
|
||||
export function secretsInUrl(url: string): string[] {
|
||||
let parsed: URL;
|
||||
try {
|
||||
const parsed = new URL(url);
|
||||
const found = [...parsed.searchParams.values()];
|
||||
if (parsed.username) found.push(decodeURIComponent(parsed.username));
|
||||
if (parsed.password) found.push(decodeURIComponent(parsed.password));
|
||||
return found;
|
||||
parsed = new URL(url);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
const found: string[] = [];
|
||||
if (parsed.username) found.push(...withDecoded(parsed.username));
|
||||
if (parsed.password) found.push(...withDecoded(parsed.password));
|
||||
for (const pair of parsed.search.replace(/^\?/, "").split("&")) {
|
||||
const eq = pair.indexOf("=");
|
||||
if (eq < 0) continue;
|
||||
const value = pair.slice(eq + 1);
|
||||
if (value) found.push(...withDecoded(value));
|
||||
}
|
||||
for (const segment of parsed.pathname.split("/")) {
|
||||
if (segment.length >= MIN_SECRET_PATH_SEGMENT_LENGTH) {
|
||||
found.push(...withDecoded(segment));
|
||||
}
|
||||
}
|
||||
return [...new Set(found)];
|
||||
}
|
||||
|
||||
/** A header value and, for the `<scheme> <token>` form, the token alone. */
|
||||
|
||||
@@ -705,7 +705,7 @@ export const settingsPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
...stdResponses,
|
||||
"502": {
|
||||
description:
|
||||
"The mail server, or the way to it, failed. `meta.errorCode` names the cause: `credentials_rejected` (the server refused the SMTP login), `upstream_rejected` (a permanent 5xx rejection, e.g. an unknown mailbox or relay denied), `upstream_error` (a temporary 4xx rejection), `timeout`, or `connection_failed`. `meta.smtpCode` carries the SMTP reply code when the mail server sent one.",
|
||||
"The mail server, or the way to it, failed. `meta.errorCode` names the cause: `credentials_rejected` (the server refused the SMTP login), `upstream_rejected` (a permanent 5xx rejection, e.g. an unknown mailbox or relay denied, or an envelope or message refused without a reply code), `upstream_error` (a temporary 4xx rejection), `timeout`, or `connection_failed` (including a TLS handshake or protocol failure). `meta.smtpCode` carries the SMTP reply code when the mail server sent one.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
"500": {
|
||||
@@ -735,7 +735,7 @@ export const settingsPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
...stdResponses,
|
||||
"502": {
|
||||
description:
|
||||
"The ntfy server, or the way to it, failed. `meta.errorCode` names the cause: `upstream_rejected` (the other end refused the payload, e.g. 400/409/415/422), `credentials_rejected` (401/403), `endpoint_not_found` (404/410), `rate_limited` (429), `upstream_error` (5xx), `redirected` (3xx, never followed), `timeout`, or `connection_failed`. `meta.upstreamStatus` carries the HTTP status whenever one arrived, and `meta.upstreamBody` up to 200 characters of what the other end answered, with control characters removed; it is omitted when the body was empty or could carry a credential (a token-shaped string, or the saved header value, auth token, topic or URL token).",
|
||||
"The ntfy server, or the way to it, failed. `meta.errorCode` names the cause: `upstream_rejected` (the other end refused the payload, e.g. 400/409/415/422), `credentials_rejected` (401/403), `endpoint_not_found` (404/410), `rate_limited` (429), `upstream_error` (5xx), `redirected` (3xx, never followed), `timeout`, or `connection_failed`. `meta.upstreamStatus` carries the HTTP status whenever one arrived, and `meta.upstreamBody` up to 200 characters of what the other end answered, with control characters removed; it is omitted when the body was empty or could carry a credential: a token-shaped string, or the saved auth token, the topic, or a credential in the server URL (userinfo, a query value, a path segment of 16 or more characters), literal, JSON-escaped or percent-encoded.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
"500": {
|
||||
@@ -791,7 +791,7 @@ export const settingsPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
...stdResponses,
|
||||
"502": {
|
||||
description:
|
||||
"The relay, or the way to it, failed. `meta.errorCode` names the cause: `upstream_rejected` (the other end refused the payload, e.g. 400/409/415/422), `credentials_rejected` (401/403), `endpoint_not_found` (404/410), `rate_limited` (429), `upstream_error` (5xx), `redirected` (3xx, never followed), `timeout`, or `connection_failed`. `meta.upstreamStatus` carries the HTTP status whenever one arrived, and `meta.upstreamBody` up to 200 characters of what the other end answered, with control characters removed; it is omitted when the body was empty or could carry a credential (a token-shaped string, or the saved header value, auth token, topic or URL token).",
|
||||
"The relay, or the way to it, failed. `meta.errorCode` names the cause: `upstream_rejected` (the other end refused the payload, e.g. 400/409/415/422), `credentials_rejected` (401/403), `endpoint_not_found` (404/410), `rate_limited` (429), `upstream_error` (5xx), `redirected` (3xx, never followed), `timeout`, or `connection_failed`. `meta.upstreamStatus` carries the HTTP status whenever one arrived, and `meta.upstreamBody` up to 200 characters of what the other end answered, with control characters removed; it is omitted when the body was empty or could carry a credential: a token-shaped string, or the saved header value (and its token part), the URL's userinfo, a query value, or a path segment of 16 or more characters (the token in a Discord or Slack webhook URL), literal, JSON-escaped or percent-encoded.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
"500": {
|
||||
|
||||
Reference in new issue
Block a user