mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-13 06:32:03 +00:00
fix(import): bound what an export may inflate to, allow one import per account, and never leave the upload behind
The extractor held the inflated export.xml to a flat 8 GiB whatever the member occupied, and the header ratio check reads sizes the archive writes itself: a member declaring an honest ratio could inflate at 1000:1, so an 8 MB upload could fill the temp directory. The streamed byte count is now held to 200 times the member's compressed size on disk (never less than 64 MiB, never more than 8 GiB), and a stored member may not grow at all. An account may have one Apple Health import queued or running. A second upload with different bytes is refused with 409 import.apple_health.busy and the running job's id; the check and the new row are one transaction under an advisory lock. The staged upload is the person's export in plain text. It is now removed on every exit that leaves no worker to read it: a refused or broken multipart body, no worker running, a queue that takes no job (the row is failed as well), the admin route's missing or unknown target account, and an account deleted before the worker reached it. The worker removes the files before recording a failure, so a row that went with a deleted account no longer strands them. The 15-minute import reconcile pass also sweeps staged files older than the longest an import may run.
This commit is contained in:
1 parent
1c7785c23b
commit
1b4594e69f
16 files changed
+549
-57
No files matched your search
@@ -0,0 +1,52 @@
|
||||
/**
|
||||
* The sweep removes staged exports no running import can own, and nothing
|
||||
* else. Mutations that must turn this red: drop the age check (the fresh
|
||||
* upload of a running import goes), or widen the name pattern (a file the app
|
||||
* did not write goes).
|
||||
*/
|
||||
import { mkdtempSync, readdirSync, utimesSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
STAGING_MAX_AGE_MS,
|
||||
sweepStaleImportStaging,
|
||||
} from "../apple-health-staging";
|
||||
|
||||
describe("sweepStaleImportStaging", () => {
|
||||
it("removes old staged uploads and extracted XML, and leaves the rest", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "healthlog-sweep-"));
|
||||
const uuid = "0b7f3c2a-1d4e-4f5a-9b8c-7d6e5f4a3b2c";
|
||||
const names = {
|
||||
oldUpload: `healthlog-apple-health-import-${uuid}.bin`,
|
||||
oldAdminUpload: `healthlog-admin-apple-health-import-${uuid}.bin`,
|
||||
oldXml: `healthlog-import-${"ab".repeat(12)}.xml`,
|
||||
freshUpload: `healthlog-upload-${uuid}.bin`,
|
||||
foreign: "somebody-elses-file.bin",
|
||||
};
|
||||
for (const name of Object.values(names)) {
|
||||
writeFileSync(join(dir, name), "x");
|
||||
}
|
||||
const now = Date.now();
|
||||
const old = new Date(now - STAGING_MAX_AGE_MS - 60_000);
|
||||
for (const name of [
|
||||
names.oldUpload,
|
||||
names.oldAdminUpload,
|
||||
names.oldXml,
|
||||
names.foreign,
|
||||
]) {
|
||||
utimesSync(join(dir, name), old, old);
|
||||
}
|
||||
|
||||
expect(await sweepStaleImportStaging(dir, now)).toBe(3);
|
||||
expect(readdirSync(dir).sort()).toEqual(
|
||||
[names.foreign, names.freshUpload].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it("answers zero for a directory that is not there", async () => {
|
||||
expect(await sweepStaleImportStaging("/nonexistent/healthlog")).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -6,9 +6,12 @@ import { deflateRawSync } from "node:zlib";
|
||||
import { pipeline } from "node:stream/promises";
|
||||
import { Readable } from "node:stream";
|
||||
|
||||
import { readdirSync } from "node:fs";
|
||||
|
||||
import {
|
||||
createByteCap,
|
||||
extractExportXml,
|
||||
memberOutputCap,
|
||||
readCentralDirectory,
|
||||
} from "../unzip-export-xml";
|
||||
|
||||
@@ -22,6 +25,8 @@ function buildMinimalZip(
|
||||
filename: string,
|
||||
payload: Buffer,
|
||||
method: 0 | 8 = 8,
|
||||
/** What the headers claim the member inflates to; the truth by default. */
|
||||
declaredSize?: number,
|
||||
): Buffer {
|
||||
const compressed = method === 8 ? deflateRawSync(payload) : payload;
|
||||
const crc32 = (() => {
|
||||
@@ -54,7 +59,7 @@ function buildMinimalZip(
|
||||
localHeader.writeUInt16LE(0, 12); // date
|
||||
localHeader.writeUInt32LE(crc32, 14);
|
||||
localHeader.writeUInt32LE(compressed.length, 18);
|
||||
localHeader.writeUInt32LE(payload.length, 22);
|
||||
localHeader.writeUInt32LE(declaredSize ?? payload.length, 22);
|
||||
localHeader.writeUInt16LE(nameBuf.length, 26);
|
||||
localHeader.writeUInt16LE(0, 28); // extra len
|
||||
|
||||
@@ -69,7 +74,7 @@ function buildMinimalZip(
|
||||
cdh.writeUInt16LE(0, 14); // date
|
||||
cdh.writeUInt32LE(crc32, 16);
|
||||
cdh.writeUInt32LE(compressed.length, 20);
|
||||
cdh.writeUInt32LE(payload.length, 24);
|
||||
cdh.writeUInt32LE(declaredSize ?? payload.length, 24);
|
||||
cdh.writeUInt16LE(nameBuf.length, 28);
|
||||
cdh.writeUInt16LE(0, 30); // extra
|
||||
cdh.writeUInt16LE(0, 32); // comment
|
||||
@@ -166,6 +171,30 @@ describe("extractExportXml", () => {
|
||||
expect(seenDeclaredBytes).toBe(payload.length);
|
||||
});
|
||||
|
||||
// A member whose headers claim an honest ratio and whose deflate stream
|
||||
// inflates a thousandfold. The header check passes it; only the count of
|
||||
// the bytes actually produced, held to the compressed size on disk, stops
|
||||
// it, and it must leave nothing behind in the staging directory.
|
||||
it("refuses a member that inflates past its compressed size's cap, whatever it declares", async () => {
|
||||
const payload = Buffer.alloc(80 * 1024 * 1024);
|
||||
const compressedLength = deflateRawSync(payload).length;
|
||||
const zip = buildMinimalZip(
|
||||
"apple_health_export/export.xml",
|
||||
payload,
|
||||
8,
|
||||
compressedLength * 2,
|
||||
);
|
||||
const tmp = mkdtempSync(join(tmpdir(), "healthlog-unzip-"));
|
||||
const zipPath = join(tmp, "export.zip");
|
||||
writeFileSync(zipPath, zip);
|
||||
const staged = () =>
|
||||
readdirSync(tmpdir()).filter((n) => n.startsWith("healthlog-import-"));
|
||||
const before = new Set(staged());
|
||||
|
||||
await expect(extractExportXml(zipPath)).rejects.toThrow(/zip bomb/);
|
||||
expect(staged().filter((n) => !before.has(n))).toEqual([]);
|
||||
});
|
||||
|
||||
it("throws when the export.xml member is missing", async () => {
|
||||
const payload = Buffer.from("noop");
|
||||
const zip = buildMinimalZip("other-file.txt", payload);
|
||||
@@ -209,3 +238,26 @@ describe("createByteCap", () => {
|
||||
).rejects.toThrow(/exceeds the 6-byte cap/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("memberOutputCap", () => {
|
||||
it("holds a deflated member to 200 times what it occupies, with a floor", () => {
|
||||
expect(
|
||||
memberOutputCap({ compressionMethod: 8, compressedSize: 1024 }),
|
||||
).toBe(64 * 1024 * 1024);
|
||||
expect(
|
||||
memberOutputCap({
|
||||
compressionMethod: 8,
|
||||
compressedSize: 10 * 1024 * 1024,
|
||||
}),
|
||||
).toBe(2000 * 1024 * 1024);
|
||||
expect(
|
||||
memberOutputCap({ compressionMethod: 8, compressedSize: 1024 ** 3 }),
|
||||
).toBe(8 * 1024 ** 3);
|
||||
});
|
||||
|
||||
it("lets a stored member grow by nothing", () => {
|
||||
expect(memberOutputCap({ compressionMethod: 0, compressedSize: 500 })).toBe(
|
||||
500,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* The staged Apple Health upload, from the request that writes it to the job
|
||||
* that consumes it, and what happens to it on every way out.
|
||||
*
|
||||
* The upload is the person's whole health export in plain text, written to
|
||||
* the shared temp directory by the kick-off request and read by the import
|
||||
* worker. Until v1.39.3 several exits left it there: a worker that was not
|
||||
* running (503), a queue that did not accept the job, an account deleted
|
||||
* before the worker reached it, and nothing ever swept what was left. Every
|
||||
* exit now removes it, and a periodic sweep removes anything older than the
|
||||
* longest an import may run, whatever left it.
|
||||
*
|
||||
* It also holds the rule of one running import per account. An import holds
|
||||
* the archive, the extracted XML (up to 8 GiB) and a worker slot; a second
|
||||
* one for the same account only doubles that and races the first on the same
|
||||
* rows. The check and the new row are one transaction under an advisory lock,
|
||||
* so two uploads that arrive together cannot both pass.
|
||||
*/
|
||||
import { readdir, rm, stat } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import type { Prisma, PrismaClient } from "@/generated/prisma/client";
|
||||
|
||||
/** An import in one of these holds its staged files and a worker slot. */
|
||||
export const ACTIVE_IMPORT_STATUSES = [
|
||||
"queued",
|
||||
"unpacking",
|
||||
"parsing",
|
||||
"upserting",
|
||||
] as const;
|
||||
|
||||
/**
|
||||
* An active row older than this is not holding anything any more: the queue
|
||||
* expires an import after six hours (`APPLE_HEALTH_IMPORT_SEND_OPTIONS`) and
|
||||
* the reconcile pass fails it soon after. It must not block the account.
|
||||
*/
|
||||
const ACTIVE_IMPORT_MAX_AGE_MS = 7 * 60 * 60 * 1000;
|
||||
|
||||
/** Staged files older than this belong to no running import. */
|
||||
export const STAGING_MAX_AGE_MS = ACTIVE_IMPORT_MAX_AGE_MS;
|
||||
|
||||
/** The names the kick-off and the extractor give their temp files. */
|
||||
const STAGING_NAME =
|
||||
/^healthlog-(?:apple-health-import|admin-apple-health-import|upload)-[0-9a-f-]{36}\.bin$|^healthlog-import-[0-9a-f]{24}\.xml$/;
|
||||
|
||||
/** The error code a second concurrent import is refused with. */
|
||||
export const IMPORT_BUSY_CODE = "import.apple_health.busy";
|
||||
|
||||
export type CreateImportJobResult =
|
||||
{ created: { id: string } } | { busy: { id: string; status: string } };
|
||||
|
||||
/**
|
||||
* Create the ImportJob row unless the account already has an import running.
|
||||
* `data` is the row as the caller would create it.
|
||||
*/
|
||||
export async function createImportJobUnlessBusy(
|
||||
prisma: PrismaClient,
|
||||
data: Prisma.ImportJobUncheckedCreateInput,
|
||||
now: Date = new Date(),
|
||||
): Promise<CreateImportJobResult> {
|
||||
return prisma.$transaction(async (tx) => {
|
||||
await tx.$executeRaw`SELECT pg_advisory_xact_lock(hashtext(${`apple-health-import:${data.userId}`}))`;
|
||||
const running = await tx.importJob.findFirst({
|
||||
where: {
|
||||
userId: data.userId,
|
||||
status: { in: [...ACTIVE_IMPORT_STATUSES] },
|
||||
startedAt: { gte: new Date(now.getTime() - ACTIVE_IMPORT_MAX_AGE_MS) },
|
||||
},
|
||||
orderBy: { startedAt: "desc" },
|
||||
select: { id: true, status: true },
|
||||
});
|
||||
if (running) return { busy: running };
|
||||
const created = await tx.importJob.create({ data, select: { id: true } });
|
||||
return { created };
|
||||
});
|
||||
}
|
||||
|
||||
/** Remove a staged file. Never throws; a file already gone is fine. */
|
||||
export async function discardStagedUpload(path: string): Promise<void> {
|
||||
await rm(path, { force: true }).catch(() => {});
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove staged uploads and extracted XML files older than
|
||||
* `STAGING_MAX_AGE_MS` from `dir`. Only names this app writes are touched.
|
||||
* Answers how many were removed.
|
||||
*/
|
||||
export async function sweepStaleImportStaging(
|
||||
dir: string = tmpdir(),
|
||||
now: number = Date.now(),
|
||||
): Promise<number> {
|
||||
let names: string[];
|
||||
try {
|
||||
names = await readdir(dir);
|
||||
} catch {
|
||||
return 0;
|
||||
}
|
||||
let removed = 0;
|
||||
for (const name of names) {
|
||||
if (!STAGING_NAME.test(name)) continue;
|
||||
const path = join(dir, name);
|
||||
try {
|
||||
const info = await stat(path);
|
||||
if (!info.isFile()) continue;
|
||||
if (now - info.mtimeMs < STAGING_MAX_AGE_MS) continue;
|
||||
await rm(path, { force: true });
|
||||
removed++;
|
||||
} catch {
|
||||
// Gone between the listing and here, or not ours to remove.
|
||||
}
|
||||
}
|
||||
return removed;
|
||||
}
|
||||
@@ -73,6 +73,39 @@ const MAX_DECOMPRESSED_BYTES = 8 * 1024 * 1024 * 1024;
|
||||
*/
|
||||
const MAX_COMPRESSION_RATIO = 200;
|
||||
|
||||
/**
|
||||
* Floor for the output cap of a small member. The ratio cap alone would
|
||||
* refuse a tiny, legitimately very compressible export; below this many
|
||||
* bytes of output the ratio is not what protects the disk.
|
||||
*/
|
||||
const MIN_OUTPUT_CAP_BYTES = 64 * 1024 * 1024;
|
||||
|
||||
/**
|
||||
* How many bytes one member may inflate to, from what it ACTUALLY occupies
|
||||
* in the archive rather than what its header claims. The header is the
|
||||
* attacker's to write: a member declaring an honest-looking 10:1 ratio can
|
||||
* still inflate at 1000:1, and with only the flat 8 GiB ceiling an 8 MB
|
||||
* upload could fill `/tmp` with 8 GB before anything noticed. The streamed
|
||||
* byte count is held to the ratio the preflight states, applied to the
|
||||
* compressed bytes on disk, and never above the absolute ceiling. A stored
|
||||
* member cannot grow at all.
|
||||
*/
|
||||
export function memberOutputCap(entry: {
|
||||
compressionMethod: number;
|
||||
compressedSize: number;
|
||||
}): number {
|
||||
if (entry.compressionMethod === 0) {
|
||||
return Math.min(entry.compressedSize, MAX_DECOMPRESSED_BYTES);
|
||||
}
|
||||
return Math.min(
|
||||
MAX_DECOMPRESSED_BYTES,
|
||||
Math.max(
|
||||
MIN_OUTPUT_CAP_BYTES,
|
||||
entry.compressedSize * MAX_COMPRESSION_RATIO,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Cap on the central-directory slice the extractor is willing to hold
|
||||
* in memory. A real Apple export's central directory is a few KB (one
|
||||
@@ -636,7 +669,7 @@ async function streamEntryToFile(
|
||||
}
|
||||
|
||||
const dest = createWriteStream(destPath);
|
||||
const cap = createByteCap(MAX_DECOMPRESSED_BYTES);
|
||||
const cap = createByteCap(memberOutputCap(entry));
|
||||
|
||||
try {
|
||||
if (entry.compressedSize === 0) {
|
||||
|
||||
Reference in new issue
Block a user