Skip to content

Commit

Permalink
fix(cli): ENOENT during asset publishing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rix0rrr committed Jun 6, 2023
1 parent cbe9fe5 commit 2ab38ae
Showing 1 changed file with 20 additions and 7 deletions.
27 changes: 20 additions & 7 deletions packages/cdk-assets/lib/private/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,19 @@ function writeZipFile(directory: string, outputFile: string): Promise<void> {
}

/**
* 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) {
async function moveIntoPlace(temporarySource: string, target: string, logger: Logger) {
let delay = 100;
let attempts = 5;
while (true) {
try {
if (await pathExists(target)) {
await fs.unlink(target);
}
await fs.rename(source, target);
// 'rename' is guaranteed to overwrite an existing target, as long as it is a file (not a directory)
await fs.rename(temporarySource, target);
return;
} catch (e: any) {
if (e.code !== 'EPERM' || attempts-- <= 0) {
Expand All @@ -82,6 +83,18 @@ async function moveIntoPlace(source: string, target: string, logger: Logger) {
}
}

async function tryUnlink(target: string): Promise<boolean> {
try {
await fs.unlink(target);
return true;
} catch (e: any) {
if (e.code === 'ENOENT') {
return false;
}
throw e;
}
}

function sleep(ms: number) {
return new Promise(ok => setTimeout(ok, ms));
}
Expand Down

0 comments on commit 2ab38ae

Please sign in to comment.