mirror of
https://github.com/xCyanGrizzly/DragonsStash.git
synced 2026-07-22 23:12:02 +00:00
fix(worker): silence false-positive integrity-test notifications
The pre-upload integrity test is advisory (never blocks upload), but it raised a WARNING SystemNotification whenever `7z t` failed — most often because large 7z archives OOM-kill the test process (SIGKILL / exit 137) in the memory-limited worker container, which is a tool limitation, not corruption. Classify failures as encrypted | corrupt | inconclusive; suppress notifications for inconclusive (debug log only) while still proceeding with the upload as before. Genuine corruption now uses the INTEGRITY_AUDIT notification type instead of the misleading HASH_MISMATCH; encrypted archives still notify. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -5,21 +5,28 @@ import { childLogger } from "../util/logger.js";
|
|||||||
const execFileAsync = promisify(execFile);
|
const execFileAsync = promisify(execFile);
|
||||||
const log = childLogger("integrity");
|
const log = childLogger("integrity");
|
||||||
|
|
||||||
|
export type IntegrityFailureKind = "encrypted" | "corrupt" | "inconclusive";
|
||||||
|
|
||||||
export type IntegrityResult =
|
export type IntegrityResult =
|
||||||
| { ok: true }
|
| { ok: true }
|
||||||
| { ok: false; reason: string };
|
| { ok: false; reason: string; kind: IntegrityFailureKind };
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test that the archive can be read end-to-end without errors, BEFORE we
|
* Test that the archive can be read end-to-end without errors, BEFORE we
|
||||||
* spend bandwidth uploading it to the destination channel. Catches:
|
* spend bandwidth uploading it to the destination channel.
|
||||||
* - Truncated downloads (rare given our size check, but cheap to confirm)
|
|
||||||
* - CRC errors inside the archive
|
|
||||||
* - Bad central directories
|
|
||||||
* - Encrypted archives (we report them as failures rather than upload
|
|
||||||
* a file users can't extract)
|
|
||||||
*
|
*
|
||||||
* Returns { ok: true } if the archive is intact. Returns
|
* Failures are classified so the caller can react appropriately:
|
||||||
* { ok: false, reason } otherwise. Logs at warn level on failure.
|
* - "encrypted" — password-protected; users can't extract it. Actionable.
|
||||||
|
* - "corrupt" — genuine CRC / structural error (truncated download, bad
|
||||||
|
* central directory, CRC mismatch). Actionable.
|
||||||
|
* - "inconclusive" — the test tool itself was killed (OOM) or timed out,
|
||||||
|
* typically on very large 7z archives in a memory-limited
|
||||||
|
* container (exit 137 / SIGKILL). This is a TOOL
|
||||||
|
* LIMITATION, not corruption — callers should NOT raise a
|
||||||
|
* user-facing alarm for it.
|
||||||
|
*
|
||||||
|
* Returns { ok: true } if the archive is intact, otherwise
|
||||||
|
* { ok: false, reason, kind }.
|
||||||
*
|
*
|
||||||
* For multipart archives, pass the first part. unzip / unrar / 7z all
|
* For multipart archives, pass the first part. unzip / unrar / 7z all
|
||||||
* auto-discover sibling parts.
|
* auto-discover sibling parts.
|
||||||
@@ -42,7 +49,7 @@ export async function testArchiveIntegrity(
|
|||||||
maxBuffer: 10 * 1024 * 1024,
|
maxBuffer: 10 * 1024 * 1024,
|
||||||
});
|
});
|
||||||
if (stderr && stderr.trim()) {
|
if (stderr && stderr.trim()) {
|
||||||
return { ok: false, reason: `unzip -t reported: ${stderr.slice(0, 500)}` };
|
return { ok: false, kind: "corrupt", reason: `unzip -t reported: ${stderr.slice(0, 500)}` };
|
||||||
}
|
}
|
||||||
return { ok: true };
|
return { ok: true };
|
||||||
}
|
}
|
||||||
@@ -58,7 +65,7 @@ export async function testArchiveIntegrity(
|
|||||||
if (/All OK/i.test(combined)) {
|
if (/All OK/i.test(combined)) {
|
||||||
return { ok: true };
|
return { ok: true };
|
||||||
}
|
}
|
||||||
return { ok: false, reason: `unrar t did not report "All OK": ${combined.slice(-500)}` };
|
return { ok: false, kind: "corrupt", reason: `unrar t did not report "All OK": ${combined.slice(-500)}` };
|
||||||
}
|
}
|
||||||
|
|
||||||
if (archiveType === "SEVEN_Z") {
|
if (archiveType === "SEVEN_Z") {
|
||||||
@@ -70,24 +77,52 @@ export async function testArchiveIntegrity(
|
|||||||
if (/Everything is Ok/i.test(combined)) {
|
if (/Everything is Ok/i.test(combined)) {
|
||||||
return { ok: true };
|
return { ok: true };
|
||||||
}
|
}
|
||||||
return { ok: false, reason: `7z t did not report "Everything is Ok": ${combined.slice(-500)}` };
|
return { ok: false, kind: "corrupt", reason: `7z t did not report "Everything is Ok": ${combined.slice(-500)}` };
|
||||||
}
|
}
|
||||||
|
|
||||||
return { ok: false, reason: `Unknown archive type: ${archiveType}` };
|
return { ok: false, kind: "corrupt", reason: `Unknown archive type: ${archiveType}` };
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const msg = err instanceof Error ? err.message : String(err);
|
const msg = err instanceof Error ? err.message : String(err);
|
||||||
// execFile throws on non-zero exit. Try to extract the most useful part.
|
// execFile throws on non-zero exit, on timeout, and when killed by a signal.
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
const e = err as {
|
||||||
const stderr = (err as any)?.stderr as string | undefined;
|
stderr?: unknown;
|
||||||
|
stdout?: unknown;
|
||||||
|
signal?: string | null;
|
||||||
|
killed?: boolean;
|
||||||
|
code?: number | string | null;
|
||||||
|
};
|
||||||
|
const stderr = typeof e.stderr === "string" ? e.stderr : "";
|
||||||
|
const stdout = typeof e.stdout === "string" ? e.stdout : "";
|
||||||
const detail = stderr ? `: ${stderr.slice(0, 500)}` : "";
|
const detail = stderr ? `: ${stderr.slice(0, 500)}` : "";
|
||||||
|
const haystack = `${msg}\n${stdout}\n${stderr}`;
|
||||||
|
|
||||||
// Specifically flag encrypted archives so the caller can record a more
|
// Encrypted archives — users can't extract them, so flag clearly.
|
||||||
// specific SkipReason / notification.
|
if (/password|encrypted|wrong password|enter password/i.test(haystack)) {
|
||||||
if (/password|encrypted|need.*password/i.test(`${msg}${detail}`)) {
|
return { ok: false, kind: "encrypted", reason: `Archive is encrypted (password protected): ${msg}${detail}` };
|
||||||
return { ok: false, reason: `Archive is encrypted (password protected): ${msg}${detail}` };
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Inconclusive — the test tool was killed or timed out rather than
|
||||||
|
// reporting corruption. Common on large 7z in memory-limited containers,
|
||||||
|
// where `7z t` gets OOM-killed (SIGKILL / exit 137) mid-decompression.
|
||||||
|
// That's a tool limitation, not a corrupt archive.
|
||||||
|
const killedBySignal = e.signal === "SIGKILL" || e.signal === "SIGTERM";
|
||||||
|
const killedExitCode = e.code === 137 || e.code === 143; // 128 + SIGKILL/SIGTERM
|
||||||
|
const timedOut = e.killed === true;
|
||||||
|
const maxBufferExceeded = e.code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER";
|
||||||
|
if (killedBySignal || killedExitCode || timedOut || maxBufferExceeded) {
|
||||||
|
log.debug(
|
||||||
|
{ err, archiveType, firstPartPath, signal: e.signal, code: e.code, killed: e.killed },
|
||||||
|
"Archive integrity test inconclusive (tool killed or timed out)"
|
||||||
|
);
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
kind: "inconclusive",
|
||||||
|
reason: `Integrity test could not complete (tool killed or timed out — likely OOM on a large archive): ${msg}`,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// Genuine failure: a real non-zero exit with error output.
|
||||||
log.debug({ err, archiveType, firstPartPath }, "Archive integrity test failed");
|
log.debug({ err, archiveType, firstPartPath }, "Archive integrity test failed");
|
||||||
return { ok: false, reason: `Integrity test failed: ${msg}${detail}` };
|
return { ok: false, kind: "corrupt", reason: `Integrity test failed: ${msg}${detail}` };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1941,34 +1941,45 @@ async function processOneArchiveSet(
|
|||||||
if (!isMultipartZip) {
|
if (!isMultipartZip) {
|
||||||
const integrity = await testArchiveIntegrity(archType, tempPaths[0]);
|
const integrity = await testArchiveIntegrity(archType, tempPaths[0]);
|
||||||
if (!integrity.ok) {
|
if (!integrity.ok) {
|
||||||
// Detect encryption specifically — those won't extract for users
|
if (integrity.kind === "inconclusive") {
|
||||||
// even if we upload them. Surface clearly via notification but
|
// The test tool was killed (OOM) or timed out — typically a large
|
||||||
// STILL proceed: the user can audit and decide what to do.
|
// 7z in a memory-limited container. This is a tool limitation, NOT
|
||||||
const isEncrypted = /encrypted/i.test(integrity.reason);
|
// corruption, so log quietly and DON'T raise a notification. The
|
||||||
accountLog.warn(
|
// upload proceeds exactly as before.
|
||||||
{ fileName: archiveName, reason: integrity.reason.slice(0, 200), isEncrypted },
|
accountLog.debug(
|
||||||
"Archive integrity test failed — proceeding with upload anyway (advisory check)"
|
{ fileName: archiveName, reason: integrity.reason.slice(0, 200) },
|
||||||
);
|
"Archive integrity test inconclusive — proceeding with upload (advisory check)"
|
||||||
try {
|
);
|
||||||
await db.systemNotification.create({
|
} else {
|
||||||
data: {
|
// Encrypted (won't extract for users) or genuinely corrupt — surface
|
||||||
type: isEncrypted ? "UPLOAD_FAILED" : "HASH_MISMATCH",
|
// clearly via notification but STILL proceed: the user can audit and
|
||||||
severity: "WARNING",
|
// decide what to do.
|
||||||
title: isEncrypted
|
const isEncrypted = integrity.kind === "encrypted";
|
||||||
? `Archive may be encrypted: ${archiveName}`
|
accountLog.warn(
|
||||||
: `Integrity test reported issues: ${archiveName}`,
|
{ fileName: archiveName, reason: integrity.reason.slice(0, 200), kind: integrity.kind },
|
||||||
message: integrity.reason.slice(0, 1000),
|
"Archive integrity test failed — proceeding with upload anyway (advisory check)"
|
||||||
context: {
|
);
|
||||||
fileName: archiveName,
|
try {
|
||||||
sourceChannelId: channel.id,
|
await db.systemNotification.create({
|
||||||
sourceMessageId: Number(archiveSet.parts[0].id),
|
data: {
|
||||||
archiveType: archType,
|
type: isEncrypted ? "UPLOAD_FAILED" : "INTEGRITY_AUDIT",
|
||||||
advisory: true,
|
severity: "WARNING",
|
||||||
|
title: isEncrypted
|
||||||
|
? `Archive may be encrypted: ${archiveName}`
|
||||||
|
: `Integrity test reported issues: ${archiveName}`,
|
||||||
|
message: integrity.reason.slice(0, 1000),
|
||||||
|
context: {
|
||||||
|
fileName: archiveName,
|
||||||
|
sourceChannelId: channel.id,
|
||||||
|
sourceMessageId: Number(archiveSet.parts[0].id),
|
||||||
|
archiveType: archType,
|
||||||
|
advisory: true,
|
||||||
|
},
|
||||||
},
|
},
|
||||||
},
|
});
|
||||||
});
|
} catch {
|
||||||
} catch {
|
// Best-effort notification
|
||||||
// Best-effort notification
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user