From 8818b78a7bf94cfbde959956d93bb60521682745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cho=20Garc=C3=ADa?= Date: Mon, 12 Oct 2026 14:56:31 +0200 Subject: [PATCH] upload: send a v6 row-bound name from create-upload-session The CLI picks the row's UUIDv7 id (clientUploadId) and seals the name to it at create, so no row-unbound v4 name is ever stored, even for an upload that fails before finalize. A server that ignores clientUploadId is refused and its pending row cancelled. --- src/upload.mjs | 60 ++++++++++++++++++++++++---------- test/upload.flow.test.mjs | 16 +++++++-- test/upload.hardening.test.mjs | 36 +++++++++++++++++--- 3 files changed, 88 insertions(+), 24 deletions(-) diff --git a/src/upload.mjs b/src/upload.mjs index 16141e9..5ddb3c5 100644 --- a/src/upload.mjs +++ b/src/upload.mjs @@ -19,6 +19,7 @@ import { constants as fsConstants } from 'node:fs' import { open } from 'node:fs/promises' +import { webcrypto } from 'node:crypto' import { encryptNameV6 } from '@shieldfive/crypto/vault' @@ -28,7 +29,6 @@ import { CIPHER_VERSION_PQ, computeV3EmptyUploadProof, computeV3UploadProof, - encryptMetadataV4, encryptPartsV3, generateRandomKeyB64, getCiphertextHashFromParts, @@ -97,18 +97,36 @@ export async function* readFileChunks(path, chunkSize) { } } +// A fresh UUIDv7 (RFC 9562): 48-bit ms timestamp, version 7, variant 10. +// The web app's newRowBoundId (utils/fileRowBinding.ts) builds the same shape; +// the server's create-upload-session takes it as clientUploadId. +export function newRowBoundId(now = Date.now()) { + const bytes = webcrypto.getRandomValues(new Uint8Array(16)) + let ts = BigInt(now) + for (let i = 5; i >= 0; i -= 1) { + bytes[i] = Number(ts & 0xffn) + ts >>= 8n + } + bytes[6] = (bytes[6] & 0x0f) | 0x70 + bytes[8] = (bytes[8] & 0x3f) | 0x80 + const hex = Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join('') + return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` +} + export async function createUploadSession({ apiBaseUrl, accessToken, rootKey, name, size, + rowId, }) { const rootKeyB64 = bytesToBase64(rootKey) - // The row id does not exist yet, so the first name is the row-unbound v4 - // envelope (as on the web). finalizeUpload replaces it with a v6 envelope - // bound to the row id. - const nameEnvelope = await encryptMetadataV4(name, rootKeyB64) + // The client picks the row id (UUIDv7, sent as clientUploadId), so the very + // first stored name is already the v6 envelope bound to that row. No v4 + // (row-unbound) name is ever written, even for an upload that fails before + // finalize. + const nameEnvelopeV6 = await encryptNameV6({ name, folderKey: rootKey, rowId }) // For suite 0x03 this is the classical envelope key; csk_wrapped holds it // under the parent key (the root key: sf uploads to the vault root). const csk = generateRandomKeyB64() @@ -123,7 +141,8 @@ export async function createUploadSession({ // v1 wire format: no cipher_nonce_prefix and no chunkSize (the server // rejects both; they live in the header). body: JSON.stringify({ - name_encrypted: JSON.stringify(nameEnvelope), + clientUploadId: rowId, + name_encrypted: JSON.stringify(nameEnvelopeV6), nameHash: hashMetadataV4(name.toLowerCase(), rootKeyB64), csk_wrapped: cskEnvelope.wrapped, csk_iv: cskEnvelope.iv, @@ -141,7 +160,7 @@ export async function createUploadSession({ `create-upload-session failed (HTTP ${res.status}): ${JSON.stringify(body)}`, ) } - return { session: body, csk } + return { session: body, csk, nameEncryptedV6: JSON.stringify(nameEnvelopeV6) } } // Direct upload: a presigned S3 PUT (web b52b4c8 / PR #726), not b2_upload_file. @@ -521,25 +540,36 @@ async function serverAcceptsEmptyFiles({ apiBaseUrl, accessToken }) { // order, from the same read that feeds encryption. The agent hashes those // chunks to record what was actually uploaded; hashing the file separately, // before or after, cannot see a change made and undone while it was being read. -export async function uploadFile({ apiBaseUrl, accessToken, rootKey, name, path, size, onPlaintextChunk }) { +// `rowId` is for tests only; callers let it default to a fresh UUIDv7. +export async function uploadFile({ apiBaseUrl, accessToken, rootKey, name, path, size, onPlaintextChunk, rowId = newRowBoundId() }) { // Derived before anything is sent, so a vault whose identity cannot be // derived fails before a session (and a pending row) exists. const recipientPublicKey = await vaultMlKemPublicKey(rootKey) if (size === 0 && !(await serverAcceptsEmptyFiles({ apiBaseUrl, accessToken }))) { throw new EmptyFileUnsupportedError(name) } - const { session, csk } = await createUploadSession({ + const { session, csk, nameEncryptedV6 } = await createUploadSession({ apiBaseUrl, accessToken, rootKey, name, size, + rowId, }) // From here on a pending row exists. Any failure before finalize answers // cancels it (see cancelUploadSession). const envelopeKey = base64ToBytes(csk) try { - await sendUpload({ apiBaseUrl, accessToken, name, path, size, onPlaintextChunk, session, envelopeKey, rootKey, recipientPublicKey }) + // The name and the file header are bound to rowId. A server that ignored + // clientUploadId would hand back another id; refuse it (and cancel the + // row it made) rather than upload under a mismatched binding. + if (session?.fileId !== rowId) { + throw new Error( + `create-upload-session returned file id ${session?.fileId} for requested ${rowId}; ` + + 'this ShieldFive server is too old for this client.', + ) + } + await sendUpload({ apiBaseUrl, accessToken, name, path, size, onPlaintextChunk, session, envelopeKey, recipientPublicKey, nameEncryptedV6 }) } catch (err) { if (!err?.finalizeOutcomeUnknown && session?.fileId) { await cancelUploadSession({ apiBaseUrl, accessToken, fileId: session.fileId }) @@ -552,7 +582,7 @@ export async function uploadFile({ apiBaseUrl, accessToken, rootKey, name, path, return { fileId: session.fileId } } -async function sendUpload({ apiBaseUrl, accessToken, name, path, size, onPlaintextChunk, session, envelopeKey, rootKey, recipientPublicKey }) { +async function sendUpload({ apiBaseUrl, accessToken, name, path, size, onPlaintextChunk, session, envelopeKey, recipientPublicKey, nameEncryptedV6 }) { if (!session.proofKey) throw new Error('Upload session is missing its proof key.') // The server's v1 chunk size (5 MiB): each encrypted chunk is one storage part. @@ -571,12 +601,8 @@ async function sendUpload({ apiBaseUrl, accessToken, name, path, size, onPlainte fileId: uuidToBytes(session.fileId), recipientPublicKey, }) - // The name, re-sealed with the row id as AAD (v6), replaces the v4 one at - // finalize, so the backend cannot relabel this row with another file's name. - const nameEncryptedV6 = JSON.stringify( - await encryptNameV6({ name, folderKey: rootKey, rowId: session.fileId }), - ) - + // The same v6 name sent at create (bound to the row id) is sent again at + // finalize, which still expects it. if (session.uploadKind === 'direct') { // No authToken and no storagePath check: since web b52b4c8 (PR #726) the // direct path is a presigned S3 PUT, so the signature inside uploadUrl is diff --git a/test/upload.flow.test.mjs b/test/upload.flow.test.mjs index 7601c10..5877512 100644 --- a/test/upload.flow.test.mjs +++ b/test/upload.flow.test.mjs @@ -39,8 +39,8 @@ function jsonResponse(status, body) { } // Row ids are UUIDs: the header's file_id is the row id's 16 bytes. -const FILE_ID = '3f1c2a9e-7b4d-4c1e-9a2f-0d6e8b5c4a71' -const DIRECT_FILE_ID = '9e2b7c4d-1a3f-4e8b-8c6d-5f0a2b9e7d13' +const FILE_ID = '01928c3a-1b2c-7d3e-8f40-123456789abc' +const DIRECT_FILE_ID = '01928c3a-1b2c-7d3e-9f40-0a0b0c0d0e0f' async function withTempFile(content, fn) { const dir = await mkdtemp(join(tmpdir(), 'sf-cli-flow-')) @@ -128,6 +128,7 @@ test('multipart push: parts, nonce sequencing, first-chunk proof, ciphertextHash name: 'Secret Q3.pdf', path, size: content.length, + rowId: FILE_ID, }), ) } finally { @@ -138,6 +139,14 @@ test('multipart push: parts, nonce sequencing, first-chunk proof, ciphertextHash // Session was requested with the true byte size. assert.equal(createBody.sizeBytes, 20) + // Row binding from the first write: the client picks the UUIDv7 row id and + // the name sent at create is already the v6 envelope sealed to it. + assert.equal(createBody.clientUploadId, FILE_ID) + assert.equal(JSON.parse(createBody.name_encrypted).v, 6) + assert.equal( + await decryptName({ envelope: JSON.parse(createBody.name_encrypted), folderKey: rootKey, rowId: FILE_ID }), + 'Secret Q3.pdf', + ) // Exactly three parts landed, numbered 1..3 in order. assert.deepEqual( @@ -282,6 +291,7 @@ test('multipart push: parts completing OUT OF ORDER still finalize in chunk orde name: 'Big Secret.pdf', path, size: content.length, + rowId: FILE_ID, }), ) } finally { @@ -404,6 +414,7 @@ test('multipart push: a 5xx on a part is retried with a fresh URL and succeeds', name: 'Secret.pdf', path, size: content.length, + rowId: FILE_ID, }), ) } finally { @@ -476,6 +487,7 @@ test('direct push: presigned PUT, no Authorization, no b2FileId at finalize', as name: 'small.txt', path, size: content.length, + rowId: DIRECT_FILE_ID, }), ) } finally { diff --git a/test/upload.hardening.test.mjs b/test/upload.hardening.test.mjs index de6abac..041fb88 100644 --- a/test/upload.hardening.test.mjs +++ b/test/upload.hardening.test.mjs @@ -18,7 +18,7 @@ import { saveManifest, scanFiles } from '../src/sync.mjs' const API = 'https://api.test' const PROOF_KEY = 'cd'.repeat(32) -const FILE_ID = '5a1c2a9e-7b4d-4c1e-9a2f-0d6e8b5c4a72' +const FILE_ID = '01928c3a-1b2c-7d3e-af40-123456789abd' function jsonResponse(status, body) { return { ok: status >= 200 && status < 300, status, json: async () => body, text: async () => JSON.stringify(body) } @@ -64,7 +64,7 @@ test('a failed upload cancels its pending session', async () => { }, () => assert.rejects( - uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'a.bin', path, size: 5 }), + uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'a.bin', path, size: 5, rowId: FILE_ID }), /Direct upload failed \(HTTP 500\)/, ), ) @@ -90,7 +90,7 @@ test('a lost finalize answer does NOT cancel (the file may already be in the vau } throw new Error(`unexpected fetch: ${url}`) }, - () => assert.rejects(uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'a.bin', path, size: 5 })), + () => assert.rejects(uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'a.bin', path, size: 5, rowId: FILE_ID })), ) }) assert.equal(deleted, false) @@ -109,7 +109,7 @@ test('0-byte file: refused before any session when the server lacks the capabili }, () => assert.rejects( - uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'empty', path, size: 0 }), + uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'empty', path, size: 0, rowId: FILE_ID }), (err) => err.code === 'empty_file_unsupported', ), ) @@ -141,7 +141,7 @@ test('0-byte file: header-only part and the server empty-file proof', async () = } throw new Error(`unexpected fetch: ${url}`) }, - () => uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'empty', path, size: 0 }), + () => uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'empty', path, size: 0, rowId: FILE_ID }), ) }) assert.equal(createBody.sizeBytes, 0) @@ -194,3 +194,29 @@ test('push/encrypt listing skips dotfiles and symlinks', async () => { assert.doesNotMatch(stdout, /\.env|alias\.txt/) }) }) + +test('a server that ignores clientUploadId is refused and its row cancelled', async () => { + const deletes = [] + await tempDir(async (dir) => { + const path = join(dir, 'a.bin') + await writeFile(path, 'hello') + await withFetch( + (url, init) => { + if (url === `${API}/api/files/create-upload-session`) { + return jsonResponse(200, { uploadKind: 'direct', fileId: '957a1070-c734-4d7b-9430-27df8de29d58', uploadUrl: 'https://s3.test/put', proofKey: PROOF_KEY }) + } + if (url === `${API}/api/files/delete`) { + deletes.push(JSON.parse(init.body)) + return jsonResponse(200, { ok: true }) + } + throw new Error(`unexpected fetch ${url}`) + }, + () => + assert.rejects( + uploadFile({ apiBaseUrl: API, accessToken: 'tok', rootKey: rootKey(), name: 'a.bin', path, size: 5, rowId: FILE_ID }), + /too old/, + ), + ) + }) + assert.deepEqual(deletes, [{ ids: ['957a1070-c734-4d7b-9430-27df8de29d58'] }]) +})