mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
fix(upload): content-address storage keys to stop URL-first overwrites
buildR2Key fell back to a static "uploads" namespace (and "video" slug) when
no torrent info-hash was available, so two distinct URL-first uploads sharing a
filename produced the SAME R2/S3 key — the second silently overwrote the first
object while the first video note still pointed at that URL (data loss).
WebTorrent uploads were already safe because the info-hash namespaces the key.
Add computeStorageContentHash(file) in js/r2.js: when no info-hash is present it
derives a deterministic content-based namespace — full SHA-256 of the bytes for
files ≤512 MB (idempotent: re-uploading the same file reuses the key), and a
SHA-256 fingerprint over name/size/mtime + sampled head/tail bytes above that,
so we never buffer a multi-GB video in memory. Wired into BOTH upload paths
(r2Service + s3UploadService) via a new storageIdentifier kept separate from the
info-hash, so magnet generation is unchanged.
Test: tests/storage-key-collision.test.mjs (distinct same-named uploads get
distinct keys; same content is idempotent; large-file fingerprint path samples
edges without buffering and stays deterministic). Full unit suite green.
test_integrity_note:
change_type: ["new_tests"]
scenarios:
- id: SCN-upload-key-no-collision
given: "a URL-first upload with no torrent info-hash"
when: "the storage key is derived (computeStorageContentHash -> buildR2Key)"
then: "distinct same-named files get distinct keys, same content is idempotent, and no key uses the colliding 'uploads' namespace"
observable_outcomes:
- "returned key strings for real Blob/File content and a fake >512MB file"
determinism_controls:
- "fixed byte content; fake large file with stubbed slice() and fixed lastModified"
anti_cheat_rationale:
prevents:
- "hard-coded return value"
- "over-mocking internal logic"
relaxation:
did_relax_any_assertion: false
Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
0d6af82b30
commit
066d98febb
5 files changed
+180
-5
No files matched your search
@@ -66,7 +66,7 @@ For any nostr related work, please review the nip documentation located in /docs
|
||||
### 4a. Cloudflare/S3 upload-path gotchas (audited 2026-06-16)
|
||||
|
||||
* **Two near-identical upload services exist** — `js/services/r2Service.js` (Cloudflare R2) and `js/services/s3UploadService.js` (generic S3). They share `buildR2Key`, `resolveUploadIdentifier`, and the magnet build but are *copies*. **Any fix to one must be mirrored in the other** or they silently drift.
|
||||
* **Storage-key collisions = data loss.** `buildR2Key` (`js/r2.js`) builds `u/<npub>/<namespace>/<slug>.<ext>` where `namespace` falls back to the literal `"uploads"` and `slug` to `"video"` (e.g. a non-ASCII filename, or no info-hash). WebTorrent uploads are content-addressed (info-hash namespaces the key) and safe, but a **URL-first upload with no info-hash and a duplicate filename overwrites the previous object** while the old note still points at that URL. Include a unique component (e.g. `videoRootId` or a random suffix) when no info-hash is available.
|
||||
* **Storage-key collisions = data loss — FIXED 2026-06-16.** `buildR2Key` (`js/r2.js`) builds `u/<npub>/<namespace>/<slug>.<ext>` where `namespace` used to fall back to the literal `"uploads"` and `slug` to `"video"` (e.g. a non-ASCII filename, or no info-hash) — so a URL-first upload with no info-hash and a duplicate filename overwrote the previous object while the old note still pointed at that URL. WebTorrent uploads were always safe (info-hash namespaces the key = content-addressed). Fix: `computeStorageContentHash(file)` (`js/r2.js`) derives a content-based namespace when no info-hash is available (full SHA-256 for ≤512 MB, sampled metadata+edge fingerprint above that to avoid buffering huge files), wired into **both** `r2Service` and `s3UploadService`. Test: `tests/storage-key-collision.test.mjs`. **If you add a third upload path, reuse `computeStorageContentHash`.**
|
||||
* **Whole-file hash before upload.** When the modal doesn't pre-seed an info-hash, `resolveUploadIdentifier` runs `calculateTorrentInfoHash(file)`, which reads the *entire* file before the upload begins and before any progress is emitted — on multi-GB files this looks like a hang. Emit a "preparing/hashing" status or parallelize.
|
||||
* **Publish can report success unconfirmed.** `publishEventToRelay` (`js/nostrPublish.js`) has an optimistic-success fallthrough when the relay handle exposes no `on()`/`then()` — a note may show "Published" with no relay ACK. Don't tighten blindly (legacy `seen`-only relays rely on it), but log it.
|
||||
* **Silent partial failures.** Thumbnail and `.torrent` upload failures are caught and `warn`-only; the video note still publishes with no user signal. Empty (0-byte) files hit `CompleteMultipartUpload` with `Parts:[]` and surface a cryptic error — guard `file.size === 0`.
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import { userLogger } from "./utils/logger.js";
|
||||
import { userLogger, devLogger } from "./utils/logger.js";
|
||||
import {
|
||||
computeSha256HexFromValue,
|
||||
valueToUint8Array,
|
||||
getSharedTextEncoder,
|
||||
} from "./utils/cryptoUtils.js";
|
||||
const DB_NAME = "bitvidSettings";
|
||||
const DB_VERSION = 1;
|
||||
const STORE_NAME = "kv";
|
||||
@@ -242,4 +247,74 @@ export function buildPublicUrl(baseUrl, key) {
|
||||
return `${sanitizedBase}/${encodedKey}`;
|
||||
}
|
||||
|
||||
// Above this size we fingerprint metadata + sampled edges instead of hashing the
|
||||
// whole file, so we never buffer a multi-GB video in memory (which would freeze
|
||||
// the main thread). Only reached on the rare path where a torrent info-hash is
|
||||
// unavailable.
|
||||
const STORAGE_FULL_HASH_MAX_BYTES = 512 * 1024 * 1024; // 512 MB
|
||||
const STORAGE_FINGERPRINT_SAMPLE_BYTES = 256 * 1024; // 256 KB head + tail
|
||||
|
||||
async function fingerprintLargeFile(file, size) {
|
||||
const encoder = getSharedTextEncoder();
|
||||
const metaString = `${file?.name || ""} ${size} ${
|
||||
Number(file?.lastModified) || 0
|
||||
}`;
|
||||
const meta = encoder ? encoder.encode(metaString) : new Uint8Array();
|
||||
const canSlice = typeof file?.slice === "function";
|
||||
const headBlob = canSlice
|
||||
? file.slice(0, Math.min(STORAGE_FINGERPRINT_SAMPLE_BYTES, size))
|
||||
: null;
|
||||
const tailBlob = canSlice
|
||||
? file.slice(Math.max(0, size - STORAGE_FINGERPRINT_SAMPLE_BYTES))
|
||||
: null;
|
||||
const head = headBlob ? await valueToUint8Array(headBlob) : null;
|
||||
const tail = tailBlob ? await valueToUint8Array(tailBlob) : null;
|
||||
const parts = [meta, head, tail].filter(Boolean);
|
||||
const total = parts.reduce((sum, part) => sum + part.length, 0);
|
||||
const combined = new Uint8Array(total);
|
||||
let offset = 0;
|
||||
for (const part of parts) {
|
||||
combined.set(part, offset);
|
||||
offset += part.length;
|
||||
}
|
||||
return computeSha256HexFromValue(combined);
|
||||
}
|
||||
|
||||
/**
|
||||
* Derives a deterministic, content-based namespace component for a storage key
|
||||
* when no torrent info-hash is available. Prevents distinct URL-first uploads
|
||||
* that share a filename from overwriting each other - the `buildR2Key`
|
||||
* `"uploads"` fallback was a silent data-loss path.
|
||||
*
|
||||
* Small/medium files use a full SHA-256 of the bytes (idempotent + collision
|
||||
* safe). Files larger than `STORAGE_FULL_HASH_MAX_BYTES` use a SHA-256
|
||||
* fingerprint over name/size/mtime + sampled head & tail bytes, so we never
|
||||
* buffer a huge file in memory. Returns "" only if the file can't be read at
|
||||
* all (callers must then supply their own uniqueness).
|
||||
*
|
||||
* @param {Blob|File} file
|
||||
* @returns {Promise<string>}
|
||||
*/
|
||||
export async function computeStorageContentHash(file) {
|
||||
if (!file) {
|
||||
return "";
|
||||
}
|
||||
try {
|
||||
const size = Number(file.size) || 0;
|
||||
if (size > 0 && size <= STORAGE_FULL_HASH_MAX_BYTES) {
|
||||
const hex = await computeSha256HexFromValue(file);
|
||||
if (hex) {
|
||||
return `sha256${hex}`;
|
||||
}
|
||||
}
|
||||
const fingerprint = await fingerprintLargeFile(file, size);
|
||||
if (fingerprint) {
|
||||
return `fp${fingerprint}`;
|
||||
}
|
||||
} catch (error) {
|
||||
devLogger.warn("[r2] Failed to derive content hash for storage key:", error);
|
||||
}
|
||||
return "";
|
||||
}
|
||||
|
||||
export { sanitizeBaseDomain };
|
||||
@@ -19,6 +19,7 @@
|
||||
import {
|
||||
buildR2Key,
|
||||
buildPublicUrl,
|
||||
computeStorageContentHash,
|
||||
} from "../r2.js";
|
||||
import {
|
||||
sanitizeBucketName,
|
||||
@@ -872,6 +873,12 @@ class R2Service {
|
||||
const keyIdentifier = await resolveUploadIdentifier({ infoHash, file });
|
||||
const normalizedInfoHash = normalizeInfoHash(infoHash || keyIdentifier);
|
||||
const hasValidInfoHash = isValidInfoHash(normalizedInfoHash);
|
||||
// When no info-hash is available, derive a content-based namespace so two
|
||||
// distinct URL-first uploads that share a filename can't overwrite each
|
||||
// other. Kept separate from keyIdentifier so magnet generation still keys
|
||||
// off the real info-hash only.
|
||||
const storageIdentifier =
|
||||
keyIdentifier || (file ? await computeStorageContentHash(file) : "");
|
||||
|
||||
let bucketResult = null;
|
||||
try {
|
||||
@@ -909,7 +916,7 @@ class R2Service {
|
||||
this.setCloudflareUploadStatus(statusMessage, "info");
|
||||
|
||||
// Use forced keys if provided, otherwise generate them
|
||||
const key = forcedVideoKey || buildR2Key(npub, file, keyIdentifier);
|
||||
const key = forcedVideoKey || buildR2Key(npub, file, storageIdentifier);
|
||||
const publicUrl = forcedVideoUrl || buildPublicUrl(bucketEntry.publicBaseUrl, key);
|
||||
|
||||
const buildTorrentKey = () => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { ensureS3SdkLoaded, makeS3Client } from "../storage/s3-client.js";
|
||||
import { multipartUpload } from "../storage/s3-multipart.js";
|
||||
import { buildR2Key } from "../r2.js";
|
||||
import { buildR2Key, computeStorageContentHash } from "../r2.js";
|
||||
import {
|
||||
buildS3ObjectUrl,
|
||||
getCorsOrigins,
|
||||
@@ -35,6 +35,7 @@ const defaultDeps = {
|
||||
makeS3Client,
|
||||
multipartUpload,
|
||||
buildR2Key,
|
||||
computeStorageContentHash,
|
||||
buildS3ObjectUrl,
|
||||
getCorsOrigins,
|
||||
prepareS3Connection,
|
||||
@@ -276,6 +277,13 @@ export class S3UploadService {
|
||||
});
|
||||
const normalizedInfoHash = normalizeInfoHash(infoHash || keyIdentifier);
|
||||
const hasValidInfoHash = isValidInfoHash(normalizedInfoHash);
|
||||
// When no info-hash is available, derive a content-based namespace so two
|
||||
// distinct URL-first uploads that share a filename can't overwrite each
|
||||
// other. Kept separate from keyIdentifier so magnet generation still keys
|
||||
// off the real info-hash only.
|
||||
const storageIdentifier =
|
||||
keyIdentifier ||
|
||||
(file ? await this.deps.computeStorageContentHash(file) : "");
|
||||
|
||||
const s3 = this.deps.makeS3Client({
|
||||
endpoint: normalized.endpoint,
|
||||
@@ -287,7 +295,7 @@ export class S3UploadService {
|
||||
|
||||
const key =
|
||||
forcedVideoKey ||
|
||||
this.deps.buildR2Key(npub, file, keyIdentifier);
|
||||
this.deps.buildR2Key(npub, file, storageIdentifier);
|
||||
const publicUrl =
|
||||
forcedVideoUrl ||
|
||||
this.deps.buildS3ObjectUrl({
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
// Scenario (SCN-upload-key-no-collision):
|
||||
// Given a URL-first upload with NO torrent info-hash available,
|
||||
// When the storage key is derived (computeStorageContentHash -> buildR2Key),
|
||||
// Then two DISTINCT files that happen to share a filename must produce
|
||||
// DIFFERENT keys (so neither overwrites the other — the old `buildR2Key`
|
||||
// "uploads" fallback was a silent data-loss path, KNOWN_BUGS upload audit
|
||||
// #1), the SAME file must produce a STABLE key (idempotent re-upload), and
|
||||
// no key may fall back to the colliding "uploads" namespace.
|
||||
//
|
||||
// Anti-cheat: asserts externally observable key strings for real Blob/File
|
||||
// content (small path) AND a fake >512MB file (sampled-fingerprint path, which
|
||||
// must NOT buffer the whole file). No internal call-sequence assertions.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { buildR2Key, computeStorageContentHash } from "../js/r2.js";
|
||||
|
||||
const NPUB =
|
||||
"npub1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqsx5h2j";
|
||||
|
||||
async function keyFor(file) {
|
||||
const identifier = await computeStorageContentHash(file);
|
||||
return buildR2Key(NPUB, file, identifier);
|
||||
}
|
||||
|
||||
test("distinct same-named URL-first uploads get distinct, non-colliding keys", async () => {
|
||||
const a = new File([new Uint8Array([1, 2, 3, 4, 5])], "video.mp4", {
|
||||
type: "video/mp4",
|
||||
});
|
||||
const b = new File([new Uint8Array([5, 4, 3, 2, 1])], "video.mp4", {
|
||||
type: "video/mp4",
|
||||
});
|
||||
|
||||
const ka = await keyFor(a);
|
||||
const kb = await keyFor(b);
|
||||
|
||||
assert.notEqual(ka, kb, "distinct content must not share a storage key");
|
||||
assert.ok(
|
||||
!ka.includes("/uploads/") && !kb.includes("/uploads/"),
|
||||
"must not fall back to the colliding 'uploads' namespace",
|
||||
);
|
||||
});
|
||||
|
||||
test("same content yields a stable key (idempotent re-upload, no churn)", async () => {
|
||||
const bytes = new Uint8Array([7, 7, 7, 9, 9, 9, 1, 2]);
|
||||
const a1 = new File([bytes], "clip.mp4", { type: "video/mp4" });
|
||||
const a2 = new File([bytes], "clip.mp4", { type: "video/mp4" });
|
||||
|
||||
assert.equal(
|
||||
await keyFor(a1),
|
||||
await keyFor(a2),
|
||||
"identical content (even as separate File objects) must map to one key",
|
||||
);
|
||||
});
|
||||
|
||||
test("large files use a deterministic sampled fingerprint without buffering the whole file", async () => {
|
||||
// Fake a >512MB file: slice() returns only small edge blobs, so nothing close
|
||||
// to the reported size is ever allocated. Proves the fingerprint branch is
|
||||
// both bounded-memory and deterministic.
|
||||
const makeBig = (headByte, tailByte) => {
|
||||
let bytesRead = 0;
|
||||
return {
|
||||
name: "big.mp4",
|
||||
size: 600 * 1024 * 1024,
|
||||
lastModified: 111,
|
||||
slice(start) {
|
||||
const byte = start === 0 ? headByte : tailByte;
|
||||
const chunk = new Uint8Array([byte, byte, byte]);
|
||||
bytesRead += chunk.length;
|
||||
// Guard against accidental whole-file reads in the implementation.
|
||||
assert.ok(bytesRead < 1024 * 1024, "must only sample edges, not buffer the file");
|
||||
return new Blob([chunk]);
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
const k1 = await keyFor(makeBig(1, 2));
|
||||
const k1dup = await keyFor(makeBig(1, 2));
|
||||
const k2 = await keyFor(makeBig(1, 9)); // differing tail sample
|
||||
|
||||
assert.equal(k1, k1dup, "same metadata + edge samples => stable fingerprint");
|
||||
assert.notEqual(k1, k2, "differing content edges => distinct key");
|
||||
assert.ok(!k1.includes("/uploads/"), "fingerprint path must not collide on 'uploads'");
|
||||
});
|
||||
Reference in new issue
Block a user