From 1668dbdc105ad7e79e0ee1ac0b6446b9db4fabf6 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Tue, 6 Jun 2023 19:06:31 +0200 Subject: [PATCH] fix(cli): ENOENT during asset publishing (#25869) There was still a TOCTOU error in the file asset publishing, which could lead to `ENOENT: no such file or directory` when assets were being published in parallel. The whole `if (fileExists) { delete; }` logic was actually not necessary, as `fs.rename` will atomically overwrite existing files already, so we can just call it directly. Closes #25293. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/cdk-assets/lib/private/archive.ts | 23 ++++++---------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/packages/cdk-assets/lib/private/archive.ts b/packages/cdk-assets/lib/private/archive.ts index 36eedbddbf8e8..8e0d9a900b46e 100644 --- a/packages/cdk-assets/lib/private/archive.ts +++ b/packages/cdk-assets/lib/private/archive.ts @@ -58,17 +58,18 @@ function writeZipFile(directory: string, outputFile: string): Promise { } /** - * Rename the file to the target location, taking into account that we may see EPERM on Windows - * while an Antivirus scanner still has the file open, so retry a couple of times. + * Rename the file to the target location, taking into account: + * + * - That we may see EPERM on Windows while an Antivirus scanner still has the + * file open, so retry a couple of times. + * - This same function may be called in parallel and be interrupted at any point. */ async function moveIntoPlace(source: string, target: string, logger: Logger) { let delay = 100; let attempts = 5; while (true) { try { - if (await pathExists(target)) { - await fs.unlink(target); - } + // 'rename' is guaranteed to overwrite an existing target, as long as it is a file (not a directory) await fs.rename(source, target); return; } catch (e: any) { @@ -86,18 +87,6 @@ function sleep(ms: number) { return new Promise(ok => setTimeout(ok, ms)); } -async function pathExists(x: string) { - try { - await fs.stat(x); - return true; - } catch (e: any) { - if (e.code === 'ENOENT') { - return false; - } - throw e; - } -} - function randomString() { return Math.random().toString(36).replace(/[^a-z0-9]+/g, ''); }