Skip to content

Commit 6d0cbf4

Browse files
authored
perf(platform): index approved OAuth assets by content tuple (#556)
## Issue OAuth upload validation currently scans every approved asset for each requested asset while holding the project lock. At the allowed 5,000-asset batch size, this can require millions of comparisons. ## Fix This reduces validation from O(approved × requested) to expected O(approved + requested). Build a `Set` of approved `(blake3, md5, size)` tuples, using a local `assetApprovalKey()` helper with `JSON.stringify` for both indexing and lookup. Project matching and exact matching of all three fields are preserved. ## Benchmark results 12 paired runs on Apple M2 Pro, Node v24.21.0. Validation timings include Set construction and JSON encoding; database, presigning, and network I/O are excluded. | Approved assets | Requested assets | Before | After | % reduction | | --- | --- | --- | --- | --- | | 100 | 100 | 0.084 ms | 0.051 ms | 39.1% | | 5,000 | 100 | 4.721 ms | 2.082 ms | 55.9% | | 5,000 | 1,000 | 40.969 ms | 2.317 ms | 94.3% | | 5,000 | 5,000 | **227.016 ms** | **3.340 ms** | **98.5%** | | 20,000 | 5,000 | 808.418 ms | 10.352 ms | 98.7% | The 5,000/5,000 case is about **68× faster**. ## Why the tests changed Expanded coverage verifies multiple approved assets, reordered batches, duplicates, mismatched fields, mixed valid/invalid batches, and strict field types (`"3"` versus `3`). **All 19 tests pass against both the original and optimized implementations.** ## Verification Typechecking passed; 4,447 affected tests and 14,496 full-suite tests passed.
1 parent dbbdcd3 commit 6d0cbf4

2 files changed

Lines changed: 33 additions & 13 deletions

File tree

‎platform/packages/api/src/routes/deploy.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,18 +163,19 @@ app.post('/:id/deploy/upload-urls', async (context) => {
163163
context.get('companyIdentity'),
164164
true,
165165
);
166-
const approved = Object.values(approval.data.artifact!.assetManifest);
166+
// JSON preserves tuple boundaries and field types, including malformed
167+
// approval metadata that must not match a valid upload through coercion.
168+
function assetApprovalKey(
169+
asset: Pick<UploadUrlsRequestAsset, 'blake3' | 'md5' | 'size'>,
170+
): string {
171+
return JSON.stringify([asset.blake3, asset.md5, asset.size]);
172+
}
173+
const approved = new Set(
174+
Object.values(approval.data.artifact!.assetManifest).map(assetApprovalKey),
175+
);
167176
if (
168177
approval.data.projectId !== projectId ||
169-
assets.some(
170-
(asset) =>
171-
!approved.some(
172-
(entry) =>
173-
entry.blake3 === asset.blake3 &&
174-
entry.md5 === asset.md5 &&
175-
entry.size === asset.size,
176-
),
177-
)
178+
assets.some((asset) => !approved.has(assetApprovalKey(asset)))
178179
) {
179180
throw new OAuthError(
180181
'access_denied',

‎platform/packages/api/test/integration/oauth.test.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,24 @@ describe('registered OAuth applications', () => {
208208
size: 3,
209209
contentType: 'text/javascript',
210210
};
211+
const otherAsset = {
212+
...asset,
213+
blake3: computeAssetHash(new TextEncoder().encode('page').buffer, '/page.js'),
214+
md5: 'b'.repeat(32),
215+
size: 4,
216+
};
211217
const prepared = createDeployArtifact({
212218
...artifact,
213-
assetManifest: { '/app.js': { ...asset, hash: asset.blake3 } },
219+
assetManifest: {
220+
'/app.js': { ...asset, hash: asset.blake3 },
221+
'/page.js': { ...otherAsset, hash: otherAsset.blake3 },
222+
'/wrong-type.js': {
223+
...asset,
224+
blake3: 'd'.repeat(32),
225+
hash: 'd'.repeat(32),
226+
size: '3' as unknown as number,
227+
},
228+
},
214229
});
215230
const issued = await request('/oauth/preview-deployment-requests', {
216231
method: 'POST',
@@ -222,15 +237,19 @@ describe('registered OAuth applications', () => {
222237
}),
223238
});
224239
const receipt = (await issued.json()) as { access_token: string; approval_id: string };
225-
const upload = (entry: typeof asset) =>
240+
const upload = (...assets: Array<typeof asset>) =>
226241
request('/projects/' + project.project_id + '/deploy/upload-urls', {
227242
method: 'POST',
228243
headers: { ...headers, Authorization: 'Bearer ' + receipt.access_token },
229-
body: JSON.stringify({ assets: [entry] }),
244+
body: JSON.stringify({ assets }),
230245
});
231246
expect((await upload(asset)).status).toBe(200);
247+
expect((await upload(otherAsset, asset, asset)).status).toBe(200);
232248
expect((await upload({ ...asset, size: 4 })).status).toBe(403);
249+
expect((await upload({ ...asset, md5: 'b'.repeat(32) })).status).toBe(403);
233250
expect((await upload({ ...asset, blake3: 'c'.repeat(32) })).status).toBe(403);
251+
expect((await upload(otherAsset, { ...asset, size: 4 })).status).toBe(403);
252+
expect((await upload({ ...asset, blake3: 'd'.repeat(32) })).status).toBe(403);
234253
const [record] = await env.db
235254
.select()
236255
.from(oauthRequests)

0 commit comments

Comments
 (0)