Skip to content

Commit 6e244a9

Browse files
riglarclaude
andauthored
feat(upload): dedup encrypted binaries on the plaintext hash (#101)
`--encrypt` uses a fresh random DEK per upload, so identical plaintext produces different ciphertext every time. Since the dedup key was the hash of whatever gets uploaded, encryption silently disabled dedup entirely: every run re-uploaded and re-stored the full binary. Verified on dev — two `--encrypt` runs of a byte-identical APK produced two distinct binaries, while the plaintext run deduped. Hash the plaintext first, dedup on that, and only encrypt on a miss. Because the check now happens before encryption, a hit skips the encryption work too, not just the upload — an encrypted re-run is now faster than the old plaintext path rather than merely matching it. `binaries.sha` is untouched and remains the ciphertext hash: the hosts that verify it (B2, Supabase Storage, storage-cache, Mac LRU) hold ciphertext and no DEK, so they can only hash the bytes they actually have. The plaintext hash rides alongside as `shaPlain` and is sent only when encrypting. A dedup hit is honoured only if the server confirms the matched binary is itself encrypted. This matters: a previously-uploaded *plaintext* copy of the same app has the same plaintext hash, so a lookup blind to encryption state would hand back an unencrypted binary to someone who asked for encryption. The API applies the same predicate (devicecloud-dev/dcd#1168); this is the client refusing to depend on that, so an older or misbehaving deployment degrades into a redundant upload instead of a silent loss of encryption. Requires the API side of dcd#1168 for the encrypted path to dedup; against an older deployment the check finds nothing and behaviour is exactly as it is today. The unencrypted path is unchanged on the wire. One cost worth naming: an encrypted upload that misses now hashes twice (once plaintext, once ciphertext). Hashing is cheap next to encrypt + upload, and only the miss path pays it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a34d9d4 commit 6e244a9

4 files changed

Lines changed: 485 additions & 29 deletions

File tree

src/config/flags/binary.flags.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,6 @@ export const binaryFlags = {
2727
encrypt: {
2828
type: 'boolean',
2929
description:
30-
'Encrypt the app binary, flow zip, and env vars before upload (client-side envelope encryption; each gets its own key). Can also be enabled with DCD_ENCRYPT=1.',
30+
'Encrypt the app binary, flow zip, and env vars before upload (client-side envelope encryption; each gets its own key). The binary is still deduplicated across runs, so an unchanged app is not re-uploaded. Can also be enabled with DCD_ENCRYPT=1.',
3131
},
3232
} as const satisfies ArgsDef;

src/gateways/api-gateway.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,14 +257,32 @@ export const ApiGateway = {
257257
return true;
258258
},
259259

260+
/**
261+
* Look for an already-uploaded binary to skip re-uploading.
262+
*
263+
* Two lookup keys, because encryption changes what is stable (dcd#1168).
264+
* Unencrypted uploads pass `sha` (the hash of exactly what gets stored).
265+
* Encrypted uploads pass `shaPlain` plus `encrypted: true`: their ciphertext is
266+
* freshly keyed on every upload, so its hash never matches, and the plaintext
267+
* hash is the only stable key. `sha` is deliberately not sent in that case —
268+
* see the caller in methods.ts for why sending it would be unsafe.
269+
*
270+
* `encrypted` in the response reports whether the *matched* binary is stored
271+
* encrypted, so the caller can verify the invariant it asked for instead of
272+
* trusting the server to have applied the right predicate.
273+
*/
260274
async checkForExistingUpload(
261275
baseUrl: string,
262276
auth: AuthContext,
263-
sha: string,
277+
lookup: { encrypted?: boolean; sha?: string; shaPlain?: string } | string,
264278
) {
279+
// Historically this took a bare sha string; keep that shape working.
280+
const body =
281+
typeof lookup === 'string' ? { sha: lookup } : { ...lookup };
282+
265283
try {
266284
const res = await fetch(`${baseUrl}/uploads/checkForExistingUpload`, {
267-
body: JSON.stringify({ sha }),
285+
body: JSON.stringify(body),
268286
headers: {
269287
'content-type': 'application/json',
270288
...auth.headers,
@@ -277,7 +295,9 @@ export const ApiGateway = {
277295
}
278296

279297
return await parseJsonResponse<
280-
paths['/uploads/checkForExistingUpload']['post']['responses']['201']['content']['application/json']
298+
paths['/uploads/checkForExistingUpload']['post']['responses']['201']['content']['application/json'] & {
299+
encrypted?: boolean;
300+
}
281301
>(res, 'Failed to check for existing upload');
282302
} catch (error) {
283303
// Handle network-level errors (DNS, connection refused, timeout, etc.)
@@ -342,9 +362,15 @@ export const ApiGateway = {
342362
metadata: TAppMetadata;
343363
path: string;
344364
sha?: string;
365+
/**
366+
* Hash of the PLAINTEXT, sent only for encrypted uploads (dcd#1168). Stored
367+
* as `binaries.sha_plain` so later encrypted uploads of the same input can
368+
* dedup; `sha` remains the ciphertext hash.
369+
*/
370+
shaPlain?: string;
345371
supabaseSuccess: boolean;
346372
}) {
347-
const { baseUrl, auth, id, metadata, path, sha, supabaseSuccess, backblazeSuccess, bytes } = config;
373+
const { baseUrl, auth, id, metadata, path, sha, shaPlain, supabaseSuccess, backblazeSuccess, bytes } = config;
348374
try {
349375
const res = await fetch(`${baseUrl}/uploads/finaliseUpload`, {
350376
body: JSON.stringify({
@@ -354,6 +380,7 @@ export const ApiGateway = {
354380
metadata,
355381
path, // This is tempPath for TUS uploads
356382
...(sha ? { sha } : {}),
383+
...(shaPlain ? { shaPlain } : {}),
357384
supabaseSuccess,
358385
}),
359386
headers: {

src/methods.ts

Lines changed: 78 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -173,23 +173,21 @@ export const uploadBinary = async (config: UploadBinaryConfig) => {
173173
// Prepare file for upload
174174
source = await prepareFileForUpload(filePath, debug, startTime);
175175

176-
// Encrypt before hashing/upload so the SHA, dedup check, and both uploaders
177-
// all operate on ciphertext (binaries.sha = ciphertext hash, per #1138).
178-
if (encrypt) {
179-
const encrypted = await encryptUploadSource(source, apiUrl, debug);
180-
enc = encrypted.enc;
181-
encCleanupDir = encrypted.cleanupDir;
182-
if (log) {
183-
ux.info(colors.dim(`Encrypting binary before upload (KEK v${enc.kek})`));
184-
}
185-
}
186-
187-
// Calculate SHA hash
188-
const sha = await calculateFileHash(source, debug, log);
189-
190-
// Check for existing upload with same SHA
191-
if (!ignoreShaCheck && sha) {
192-
const { exists, binaryId } = await checkExistingUpload(apiUrl, auth, sha, debug);
176+
// Hash the PLAINTEXT first, before any encryption (dcd#1168). Encryption
177+
// uses a fresh random DEK per upload, so the ciphertext hash differs every
178+
// time and cannot dedup — the plaintext hash is the only stable key. Doing it
179+
// in this order also means a dedup hit skips the encryption work entirely,
180+
// not just the upload.
181+
const shaPlain = await calculateFileHash(source, debug, log);
182+
183+
// Check for an existing upload before spending anything on encryption.
184+
if (!ignoreShaCheck && shaPlain) {
185+
const { exists, binaryId } = await checkExistingUpload(
186+
apiUrl,
187+
auth,
188+
encrypt ? { encrypted: true, shaPlain } : { sha: shaPlain },
189+
debug,
190+
);
193191

194192
if (exists && binaryId) {
195193
if (log) {
@@ -203,8 +201,35 @@ export const uploadBinary = async (config: UploadBinaryConfig) => {
203201
}
204202
}
205203

204+
// Encrypt after the dedup check, so the SHA sent at finalise, and both
205+
// uploaders, operate on ciphertext (binaries.sha = ciphertext hash, #1138).
206+
if (encrypt) {
207+
const encrypted = await encryptUploadSource(source, apiUrl, debug);
208+
enc = encrypted.enc;
209+
encCleanupDir = encrypted.cleanupDir;
210+
if (log) {
211+
ux.info(colors.dim(`Encrypting binary before upload (KEK v${enc.kek})`));
212+
}
213+
}
214+
215+
// Re-hash once encrypted: what lands in storage is the ciphertext, and every
216+
// downstream verifySha hashes the bytes it actually holds (no DEK required).
217+
const sha = encrypt ? await calculateFileHash(source, debug, false) : shaPlain;
218+
206219
// Perform the upload
207-
const uploadId = await performUpload({ auth, apiUrl, debug, enc, filePath, sha, source, startTime });
220+
const uploadId = await performUpload({
221+
auth,
222+
apiUrl,
223+
debug,
224+
enc,
225+
filePath,
226+
sha,
227+
// Only encrypted uploads record a plaintext hash; it is what makes the
228+
// next encrypted run of this binary dedupable.
229+
shaPlain: encrypt ? shaPlain : undefined,
230+
source,
231+
startTime,
232+
});
208233

209234
if (log) {
210235
ux.action.stop(colors.success('\n✓ Binary uploaded with ID: ') + formatId(uploadId));
@@ -425,30 +450,43 @@ async function calculateFileHash(
425450
}
426451

427452
/**
428-
* Checks if an upload with the same SHA already exists
453+
* Checks whether a matching binary has already been uploaded.
454+
*
455+
* `lookup` is `{ sha }` for a plaintext upload, or `{ shaPlain, encrypted: true }`
456+
* for an encrypted one (dcd#1168) — see {@link ApiGateway.checkForExistingUpload}.
457+
*
458+
* When asking as an encrypting client, a hit is only honoured if the server
459+
* confirms the matched binary is itself encrypted. Any binary is a *plausible*
460+
* match on plaintext hash, including a previously-uploaded plaintext copy of the
461+
* same app, and reusing that would hand back an unencrypted binary while the user
462+
* had asked for encryption. The server applies the same predicate; this is the
463+
* client refusing to depend on that, so an older or misbehaving deployment
464+
* degrades into a redundant upload rather than a silent loss of encryption.
465+
*
429466
* @param apiUrl API base URL
430467
* @param auth AuthContext carrying request headers
431-
* @param sha SHA-256 hash to check
468+
* @param lookup Dedup key — plaintext hash for encrypted uploads, else the sha
432469
* @param debug Whether debug logging is enabled
433470
* @returns Promise resolving to object with exists flag and optional binaryId
434471
*/
435472
async function checkExistingUpload(
436473
apiUrl: string,
437474
auth: AuthContext,
438-
sha: string,
475+
lookup: { encrypted?: boolean; sha?: string; shaPlain?: string },
439476
debug: boolean,
440477
): Promise<{ binaryId?: string; exists: boolean }> {
441478
try {
442479
if (debug) {
443480
console.log('[DEBUG] Checking for existing upload with matching SHA...');
481+
console.log(`[DEBUG] Lookup: ${JSON.stringify(lookup)}`);
444482
console.log(`[DEBUG] Target endpoint: ${apiUrl}/uploads/checkForExistingUpload`);
445483
}
446484

447485
const shaCheckStartTime = Date.now();
448-
const { appBinaryId, exists } = await ApiGateway.checkForExistingUpload(
486+
const { appBinaryId, encrypted, exists } = await ApiGateway.checkForExistingUpload(
449487
apiUrl,
450488
auth,
451-
sha as string,
489+
lookup,
452490
);
453491

454492
if (debug) {
@@ -459,6 +497,16 @@ async function checkExistingUpload(
459497
}
460498
}
461499

500+
if (exists && lookup.encrypted && encrypted !== true) {
501+
if (debug) {
502+
console.log(
503+
'[DEBUG] Ignoring dedup hit: encryption was requested but the matched binary is not encrypted',
504+
);
505+
}
506+
507+
return { exists: false };
508+
}
509+
462510
return { binaryId: appBinaryId, exists };
463511
} catch (error) {
464512
// Invalid credentials will fail every subsequent request — surface now
@@ -493,6 +541,11 @@ interface PerformUploadConfig {
493541
enc?: BinaryEnvelope;
494542
filePath: string;
495543
sha: string | undefined;
544+
/**
545+
* Hash of the plaintext, set only for encrypted uploads (#1168). Persisted as
546+
* `binaries.sha_plain` so the next encrypted upload of this binary can dedup.
547+
*/
548+
shaPlain?: string;
496549
source: UploadSource;
497550
startTime: number;
498551
}
@@ -769,7 +822,7 @@ function validateUploadResults(
769822
* @returns Promise resolving to upload ID
770823
*/
771824
async function performUpload(config: PerformUploadConfig): Promise<string> {
772-
const { filePath, apiUrl, auth, enc, source, sha, debug, startTime } = config;
825+
const { filePath, apiUrl, auth, enc, source, sha, shaPlain, debug, startTime } = config;
773826

774827
// Request upload URL and paths
775828
const { id, tempPath, finalPath, b2 } = await requestUploadPaths(apiUrl, auth, filePath, source.size, debug);
@@ -832,6 +885,7 @@ async function performUpload(config: PerformUploadConfig): Promise<string> {
832885
path: tempPath,
833886
// sha is undefined when hash calculation failed — omit it explicitly
834887
...(sha ? { sha } : {}),
888+
...(shaPlain ? { shaPlain } : {}),
835889
supabaseSuccess: supabaseResult.success,
836890
});
837891

0 commit comments

Comments
 (0)