fix(notifications): redact failure details
This commit is contained in:
@@ -4,7 +4,9 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { NotificationEvent, NotificationOutbox } from "../src/main/notification-outbox";
|
||||
import { sendNotification } from "../src/main/notify";
|
||||
import { buildPackageNotificationEvent } from "../src/main/notification-events";
|
||||
import { buildNotifyRequest, sendNotification } from "../src/main/notify";
|
||||
import type { PackageResult } from "../src/shared/types";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
@@ -277,6 +279,93 @@ describe("NotificationOutbox", () => {
|
||||
expect(persisted(filePath).events[0]).toEqual(event("safe"));
|
||||
});
|
||||
|
||||
it("keeps private package failure details out of events, requests, and persisted state", async () => {
|
||||
const filePath = createOutboxFile();
|
||||
const privateDetails = "https://private.example.test/hook C:/Private/target alice@example.test token=SUPERSECRET";
|
||||
const result: PackageResult = {
|
||||
packageId: "pkg-private",
|
||||
name: "Paket",
|
||||
status: "failed",
|
||||
startedAt: 1000,
|
||||
downloadEndedAt: 2000,
|
||||
postProcessStartedAt: 0,
|
||||
completedAt: 2000,
|
||||
downloadDurationSeconds: 1,
|
||||
extractionDurationSeconds: 0,
|
||||
remuxDurationSeconds: 0,
|
||||
postProcessDurationSeconds: 0,
|
||||
totalDurationSeconds: 1,
|
||||
totalBytes: 1000,
|
||||
downloadedBytes: 0,
|
||||
averageDownloadSpeedBps: 0,
|
||||
successfulFiles: 0,
|
||||
failedFiles: 1,
|
||||
cancelledFiles: 0,
|
||||
archiveCount: 0,
|
||||
partCount: 0,
|
||||
outputCount: 0,
|
||||
failurePhase: "download",
|
||||
errorCategory: privateDetails,
|
||||
archiveOperations: [],
|
||||
remuxOperations: []
|
||||
};
|
||||
const notificationEvent = buildPackageNotificationEvent({ generation: 1, result }, 1000);
|
||||
const request = buildNotifyRequest("https://discord.com/api/webhooks/123/abc", {
|
||||
title: notificationEvent.payload.title,
|
||||
message: notificationEvent.payload.description || "",
|
||||
color: notificationEvent.payload.color,
|
||||
fields: notificationEvent.payload.fields,
|
||||
timestamp: notificationEvent.createdAt
|
||||
});
|
||||
const outbox = new NotificationOutbox({ filePath, send: async () => true, now: () => 1000 });
|
||||
|
||||
await outbox.enqueue(notificationEvent);
|
||||
|
||||
const eventText = JSON.stringify(notificationEvent);
|
||||
const requestText = String(request.init.body);
|
||||
const persistedText = fs.readFileSync(filePath, "utf8");
|
||||
const sensitiveValues = [
|
||||
"https://private.example.test/hook",
|
||||
"C:/Private/target",
|
||||
"alice@example.test",
|
||||
"token=SUPERSECRET"
|
||||
];
|
||||
expect(notificationEvent.payload.fields.find((field) => field.name === "Fehler")?.value).toBe("Download · Download");
|
||||
for (const sensitiveValue of sensitiveValues) {
|
||||
expect(eventText).not.toContain(sensitiveValue);
|
||||
expect(requestText).not.toContain(sensitiveValue);
|
||||
expect(persistedText).not.toContain(sensitiveValue);
|
||||
}
|
||||
});
|
||||
|
||||
it("projects private failure details from an existing outbox before persisting again", async () => {
|
||||
const filePath = createOutboxFile();
|
||||
const privateDetails = "https://private.example.test/hook C:/Private/target alice@example.test token=SUPERSECRET";
|
||||
const legacyEvent = event("legacy-private", {
|
||||
payload: {
|
||||
title: "Paket fehlgeschlagen",
|
||||
description: "Paket",
|
||||
fields: [{ name: "Fehler", value: `Download · ${privateDetails}`, inline: false }]
|
||||
}
|
||||
});
|
||||
fs.writeFileSync(filePath, JSON.stringify({
|
||||
version: 1,
|
||||
events: [legacyEvent],
|
||||
lastSuccessAt: 0,
|
||||
lastFailureAt: 0
|
||||
}), "utf8");
|
||||
const outbox = new NotificationOutbox({ filePath, send: async () => true, now: () => 1000 });
|
||||
|
||||
await outbox.enqueue(event("safe"));
|
||||
|
||||
const persistedText = fs.readFileSync(filePath, "utf8");
|
||||
expect(persisted(filePath).events[0].payload.fields[0]?.value).toBe("Download · Download");
|
||||
expect(persistedText).not.toContain("https://private.example.test/hook");
|
||||
expect(persistedText).not.toContain("C:/Private/target");
|
||||
expect(persistedText).not.toContain("alice@example.test");
|
||||
expect(persistedText).not.toContain("token=SUPERSECRET");
|
||||
});
|
||||
|
||||
it("returns after the default three-second shutdown budget when sending hangs", async () => {
|
||||
vi.useFakeTimers();
|
||||
const filePath = createOutboxFile();
|
||||
|
||||
@@ -153,7 +153,7 @@ describe("package lifecycle telemetry", () => {
|
||||
partCount: 16,
|
||||
archiveCount: 1,
|
||||
failurePhase: "extract",
|
||||
errorCategory: "checksum"
|
||||
errorCategory: "Entpacken"
|
||||
}));
|
||||
});
|
||||
|
||||
@@ -170,7 +170,7 @@ describe("package lifecycle telemetry", () => {
|
||||
failedFiles: 1,
|
||||
cancelledFiles: 0,
|
||||
failurePhase: "download",
|
||||
errorCategory: "download-error"
|
||||
errorCategory: "Download"
|
||||
}));
|
||||
});
|
||||
|
||||
@@ -185,10 +185,46 @@ describe("package lifecycle telemetry", () => {
|
||||
status: "failed",
|
||||
failedFiles: 1,
|
||||
failurePhase: "download",
|
||||
errorCategory: "download"
|
||||
errorCategory: "Download"
|
||||
}));
|
||||
});
|
||||
|
||||
it("projects private download failure details to a fixed category", () => {
|
||||
const privateDetails = "https://private.example.test/hook C:/Private/target alice@example.test token=SUPERSECRET";
|
||||
const item = {
|
||||
...downloadItem("item-1", "failed"),
|
||||
lastError: privateDetails,
|
||||
fullStatus: privateDetails
|
||||
};
|
||||
const result = finalizePackageResult(telemetry({
|
||||
package: packageEntry({ status: "failed", itemIds: [item.id] }),
|
||||
items: [item]
|
||||
}));
|
||||
|
||||
expect(result.errorCategory).toBe("Download");
|
||||
expect(result.errorCategory).not.toContain("private.example.test");
|
||||
expect(result.errorCategory).not.toContain("C:/Private/target");
|
||||
expect(result.errorCategory).not.toContain("alice@example.test");
|
||||
expect(result.errorCategory).not.toContain("SUPERSECRET");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["Host ist offline", "Offline"],
|
||||
["Request ETIMEDOUT", "Timeout"],
|
||||
["socket ECONNRESET", "Netzwerk"],
|
||||
["ENOSPC: no space left on device", "Speicherplatz"],
|
||||
["EACCES: permission denied", "Berechtigung"],
|
||||
["nicht näher klassifizierbar", "Download"]
|
||||
])("maps download failure detail %s to %s", (detail, expectedCategory) => {
|
||||
const item = { ...downloadItem("item-1", "failed"), lastError: detail };
|
||||
const result = finalizePackageResult(telemetry({
|
||||
package: packageEntry({ status: "failed", itemIds: [item.id] }),
|
||||
items: [item]
|
||||
}));
|
||||
|
||||
expect(result.errorCategory).toBe(expectedCategory);
|
||||
});
|
||||
|
||||
it("classifies a package with only cancelled work as cancelled", () => {
|
||||
const item = downloadItem("item-1", "cancelled");
|
||||
const result = finalizePackageResult(telemetry({
|
||||
@@ -268,11 +304,19 @@ describe("package lifecycle telemetry", () => {
|
||||
const failedItem = downloadItem("item-1", "failed");
|
||||
const failedArchive = archiveOperation({ status: "failed", errorCategory: "archive-error" });
|
||||
const failedRemux = remuxOperation({ status: "failed", errorCategory: "remux-error" });
|
||||
const failures = [
|
||||
finalizePackageResult(telemetry({ items: [failedItem], archiveOperations: [failedArchive], remuxOperations: [failedRemux], cleanupErrorCategory: "cleanup-error" })),
|
||||
finalizePackageResult(telemetry({ items: [failedItem], archiveOperations: [failedArchive], remuxOperations: [failedRemux] })),
|
||||
finalizePackageResult(telemetry({ items: [failedItem], archiveOperations: [failedArchive] })),
|
||||
finalizePackageResult(telemetry({ items: [failedItem] }))
|
||||
];
|
||||
|
||||
expect(finalizePackageResult(telemetry({ items: [failedItem], archiveOperations: [failedArchive], remuxOperations: [failedRemux], cleanupErrorCategory: "cleanup-error" })).failurePhase).toBe("cleanup");
|
||||
expect(finalizePackageResult(telemetry({ items: [failedItem], archiveOperations: [failedArchive], remuxOperations: [failedRemux] })).failurePhase).toBe("remux");
|
||||
expect(finalizePackageResult(telemetry({ items: [failedItem], archiveOperations: [failedArchive] })).failurePhase).toBe("extract");
|
||||
expect(finalizePackageResult(telemetry({ items: [failedItem] })).failurePhase).toBe("download");
|
||||
expect(failures.map(({ failurePhase, errorCategory }) => ({ failurePhase, errorCategory }))).toEqual([
|
||||
{ failurePhase: "cleanup", errorCategory: "Cleanup" },
|
||||
{ failurePhase: "remux", errorCategory: "Remux" },
|
||||
{ failurePhase: "extract", errorCategory: "Entpacken" },
|
||||
{ failurePhase: "download", errorCategory: "Download" }
|
||||
]);
|
||||
});
|
||||
|
||||
it("uses audio-strip outcomes when no individual remux operation was recorded", () => {
|
||||
|
||||
Reference in New Issue
Block a user