diff --git a/.env.example b/.env.example index 3c02cfbec..31cee47cc 100644 --- a/.env.example +++ b/.env.example @@ -207,12 +207,20 @@ API_TOKEN_HMAC_KEY="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32" # the backup silently. The backup runs at 02:30 Europe/Berlin via pg-boss cron. # Restore with `pnpm tsx scripts/restore-backup.ts `. # See docs/ops/backup-restore.md for the format spec + step-by-step. +# +# The worker never deletes a BACKUP object -- its grant is PutObject, GetObject +# and AbortMultipartUpload -- so BACKUP_RETENTION_DAYS is the number you set +# the bucket's own lifecycle rule to, not something the worker enforces. The +# one key it does delete is its own connection test's `_healthcheck/.bin`, +# and that delete is best-effort: without DeleteObject on that prefix every +# Test-connection press leaves one 1-byte orphan behind, silently. +# # BACKUP_S3_ENDPOINT="https://.r2.cloudflarestorage.com" # BACKUP_S3_BUCKET="healthlog-backups" # BACKUP_S3_ACCESS_KEY="" # BACKUP_S3_SECRET_KEY="" # BACKUP_S3_REGION="auto" # "auto" for Cloudflare R2; real region for AWS S3 -# BACKUP_RETENTION_DAYS="30" # objects older than this are pruned (best-effort; prefer a bucket lifecycle rule) +# BACKUP_RETENTION_DAYS="30" # the number for your bucket lifecycle rule (see above) # BACKUP_ENCRYPTION_KEY="" # 64-hex-char or base64 32-byte; SEPARATE from ENCRYPTION_KEY diff --git a/.env.production.example b/.env.production.example index 4a56a53b6..27486de2b 100644 --- a/.env.production.example +++ b/.env.production.example @@ -315,9 +315,12 @@ API_TOKEN_HMAC_KEY="" # BACKUP_S3_REGION="auto" # # How long backup objects are meant to live, in days. Default 30, minimum 1. -# The worker never deletes an object -- its grant is PutObject, GetObject and -# AbortMultipartUpload -- so this number is what you set the bucket's own -# lifecycle rule to. See docs/ops/backup-restore.md. +# The worker never deletes a BACKUP object -- its grant is PutObject, GetObject +# and AbortMultipartUpload -- so this number is what you set the bucket's own +# lifecycle rule to. The one key it does delete is its own connection test's +# `_healthcheck/.bin`, and that delete is best-effort: without DeleteObject +# on that prefix every Test-connection press leaves one 1-byte orphan behind, +# silently. See docs/ops/backup-restore.md. # BACKUP_RETENTION_DAYS="30" diff --git a/docs/ops/backup-restore.md b/docs/ops/backup-restore.md index 0ac2d7ad5..64e7854e9 100644 --- a/docs/ops/backup-restore.md +++ b/docs/ops/backup-restore.md @@ -41,7 +41,7 @@ the restore script needs no flag to tell them apart. | `BACKUP_S3_ACCESS_KEY` | yes | | | `BACKUP_S3_SECRET_KEY` | yes | | | `BACKUP_S3_REGION` | no | defaults to `auto` (Cloudflare R2) | -| `BACKUP_RETENTION_DAYS` | no | defaults to `30` | +| `BACKUP_RETENTION_DAYS` | no | not read by the app; the number for your bucket's lifecycle rule | ## Bucket permissions @@ -56,8 +56,9 @@ Cloudflare R2 the **Object Read & Write** token already covers all three. The worker never deletes a backup object: its grant covers PutObject, GetObject and AbortMultipartUpload only, so a compromised worker cannot -wipe the history. Expiry is the storage provider's lifecycle rule, set to -match `BACKUP_RETENTION_DAYS`: +wipe the history, and nothing in the app reads `BACKUP_RETENTION_DAYS`. +Expiry is the storage provider's lifecycle rule, set to match the number +you wrote there: ``` Filter: "" (all objects) diff --git a/src/lib/jobs/__tests__/offhost-backup.test.ts b/src/lib/jobs/__tests__/offhost-backup.test.ts index f507cb1f5..5a630b088 100644 --- a/src/lib/jobs/__tests__/offhost-backup.test.ts +++ b/src/lib/jobs/__tests__/offhost-backup.test.ts @@ -89,6 +89,10 @@ describe("loadOffhostConfig", () => { expect(loadOffhostConfig()).toBeNull(); }); + // No `retentionDays` here on purpose: the worker cannot enforce retention + // (DeleteObject is kept out of its grant), so the config carries no field + // for it and `BACKUP_RETENTION_DAYS` stays the operator's number for the + // bucket's own lifecycle rule. it("parses a complete config", () => { vi.stubEnv("BACKUP_S3_ENDPOINT", "https://r2.example"); vi.stubEnv("BACKUP_S3_BUCKET", "hl-backups"); @@ -102,7 +106,6 @@ describe("loadOffhostConfig", () => { expect(cfg!.endpoint).toBe("https://r2.example"); expect(cfg!.region).toBe("auto"); expect(cfg!.encryptionKey.length).toBe(32); - expect(cfg!.retentionDays).toBe(30); }); }); diff --git a/src/lib/jobs/offhost-backup.ts b/src/lib/jobs/offhost-backup.ts index 9f868623e..0f55273c0 100644 --- a/src/lib/jobs/offhost-backup.ts +++ b/src/lib/jobs/offhost-backup.ts @@ -13,7 +13,8 @@ * * Retention: the worker NEVER calls DeleteObject on backup keys. Operators * MUST configure a bucket-level lifecycle rule (e.g. expire after - * `BACKUP_RETENTION_DAYS`). This keeps the IAM grant for the worker + * `BACKUP_RETENTION_DAYS`, which this module reads nowhere — see + * `loadOffhostConfig`). This keeps the IAM grant for the worker * limited to PutObject + GetObject + AbortMultipartUpload, so a compromised * worker cannot wipe the backup history. The abort is what cleans up a run * that failed partway rather than leaving billed, unlistable parts behind; @@ -40,7 +41,6 @@ export interface OffhostBackupConfig { secretKey: string; region: string; encryptionKey: Buffer; - retentionDays: number; } export class OffhostBackupNotConfiguredError extends Error { @@ -69,13 +69,13 @@ export function loadOffhostConfig(): OffhostBackupConfig | null { const encRaw = process.env.BACKUP_ENCRYPTION_KEY; if (!endpoint || !bucket || !accessKey || !secretKey || !encRaw) return null; - const retentionDays = (() => { - const raw = process.env.BACKUP_RETENTION_DAYS; - if (!raw) return 30; - const v = parseInt(raw, 10); - return Number.isFinite(v) && v >= 1 ? v : 30; - })(); - + // `BACKUP_RETENTION_DAYS` is deliberately absent from this config. It used + // to be parsed and clamped here and then read by nobody, which read as an + // enforcer the worker is not: retention belongs to the bucket's lifecycle + // rule, and no consumer can ever appear here because DeleteObject is kept + // out of the worker's grant on purpose (see the header). The variable stays + // documented and on the compose whitelist because it is the number the + // operator sets that rule to. return { endpoint, bucket, @@ -83,7 +83,6 @@ export function loadOffhostConfig(): OffhostBackupConfig | null { secretKey, region: process.env.BACKUP_S3_REGION ?? "auto", encryptionKey: decodeBackupKey(encRaw), - retentionDays, }; }