From 21b8bc8408e80d2d7ae6b7dee9e3576d3ed992e3 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 23 Sep 2024 11:56:17 -0400 Subject: [PATCH 1/9] implement chmod method in fs --- packages/core/src/shared/fs/fs.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/core/src/shared/fs/fs.ts b/packages/core/src/shared/fs/fs.ts index 9622d41d99e..eee95b46f12 100644 --- a/packages/core/src/shared/fs/fs.ts +++ b/packages/core/src/shared/fs/fs.ts @@ -5,6 +5,7 @@ import vscode from 'vscode' import os from 'os' import { promises as nodefs, constants as nodeConstants, WriteFileOptions } from 'fs' +import { chmod } from 'fs/promises' import { isCloud9 } from '../extensionUtilities' import _path from 'path' import { @@ -356,6 +357,20 @@ export class FileSystem { return await vfs.stat(path) } + /** + * Change permissions on file. + * @param uri file whose permissions should be set. + * @param mode new permissions in octal notation. + * More info: https://nodejs.org/api/fs.html#fspromiseschmodpath-mode + * Examples: https://www.geeksforgeeks.org/node-js-fspromises-chmod-method/ + */ + async chmod(uri: vscode.Uri | string, mode: number): Promise { + if (!this.isWeb) { + const path = this.#toUri(uri) + await chmod(path.fsPath, mode) + } + } + /** * Deletes a file or directory. It is not an error if the file/directory does not exist, unless * its parent directory is not listable (executable). From 3df44f9a1506ef3ded1d49b68ca3c5059a629645 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 23 Sep 2024 12:27:29 -0400 Subject: [PATCH 2/9] add test cases for chmod implementation --- packages/core/src/shared/fs/fs.ts | 2 +- packages/core/src/test/shared/fs/fs.test.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/core/src/shared/fs/fs.ts b/packages/core/src/shared/fs/fs.ts index eee95b46f12..f77c0015353 100644 --- a/packages/core/src/shared/fs/fs.ts +++ b/packages/core/src/shared/fs/fs.ts @@ -358,7 +358,7 @@ export class FileSystem { } /** - * Change permissions on file. + * Change permissions on file. Note that this will do nothing on browser. * @param uri file whose permissions should be set. * @param mode new permissions in octal notation. * More info: https://nodejs.org/api/fs.html#fspromiseschmodpath-mode diff --git a/packages/core/src/test/shared/fs/fs.test.ts b/packages/core/src/test/shared/fs/fs.test.ts index 827c90ee9f2..265ea34926b 100644 --- a/packages/core/src/test/shared/fs/fs.test.ts +++ b/packages/core/src/test/shared/fs/fs.test.ts @@ -8,6 +8,7 @@ import vscode from 'vscode' import * as path from 'path' import * as utils from 'util' import { existsSync, mkdirSync, promises as nodefs, readFileSync, rmSync } from 'fs' +import { stat } from 'fs/promises' import nodeFs from 'fs' import { FakeExtensionContext } from '../../fakeExtensionContext' import fs, { FileSystem } from '../../../shared/fs/fs' @@ -380,6 +381,22 @@ describe('FileSystem', function () { }) }) + describe('chmod()', async function () { + it('changes permissions when not on web', async function () { + const filePath = await makeFile('test.txt', 'hello world', { mode: 0o777 }) + await fs.chmod(filePath, 0o644) + if (!globals.isWeb) { + const result = await stat(filePath) + assert.strictEqual(result.mode & 0o777, 0o644) + } + }) + + it('throws if no file exists', async function () { + const filePath = createTestPath('thisDoesNotExist.txt') + await assert.rejects(() => fs.chmod(filePath, 0o644)) + }) + }) + describe('rename()', async () => { it('renames a file', async () => { const oldPath = await makeFile('oldFile.txt', 'hello world') From a2f183ccc08a6c8b709b64310f92161db809fa31 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 23 Sep 2024 12:30:27 -0400 Subject: [PATCH 3/9] update test message --- packages/core/src/test/shared/fs/fs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/test/shared/fs/fs.test.ts b/packages/core/src/test/shared/fs/fs.test.ts index 265ea34926b..792e486f998 100644 --- a/packages/core/src/test/shared/fs/fs.test.ts +++ b/packages/core/src/test/shared/fs/fs.test.ts @@ -382,7 +382,7 @@ describe('FileSystem', function () { }) describe('chmod()', async function () { - it('changes permissions when not on web', async function () { + it('changes permissions when not on web, otherwise does not throw', async function () { const filePath = await makeFile('test.txt', 'hello world', { mode: 0o777 }) await fs.chmod(filePath, 0o644) if (!globals.isWeb) { From f445047a8ced03aaaf00e926f956a30f81009a6c Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 23 Sep 2024 13:12:27 -0400 Subject: [PATCH 4/9] remove test on windows --- packages/core/src/test/shared/fs/fs.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/test/shared/fs/fs.test.ts b/packages/core/src/test/shared/fs/fs.test.ts index 792e486f998..cab4c5335bf 100644 --- a/packages/core/src/test/shared/fs/fs.test.ts +++ b/packages/core/src/test/shared/fs/fs.test.ts @@ -385,7 +385,8 @@ describe('FileSystem', function () { it('changes permissions when not on web, otherwise does not throw', async function () { const filePath = await makeFile('test.txt', 'hello world', { mode: 0o777 }) await fs.chmod(filePath, 0o644) - if (!globals.isWeb) { + // chmod doesn't exist on windows, non-unix permission system. + if (!globals.isWeb && os.platform() !== 'win32') { const result = await stat(filePath) assert.strictEqual(result.mode & 0o777, 0o644) } From 940c23f4ac19f28db784d4ce1d13455c17731f81 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 23 Sep 2024 13:31:17 -0400 Subject: [PATCH 5/9] add back test case for permissions --- packages/core/src/awsService/ec2/sshKeyPair.ts | 3 +-- .../core/src/test/awsService/ec2/sshKeyPair.test.ts | 11 ++++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/core/src/awsService/ec2/sshKeyPair.ts b/packages/core/src/awsService/ec2/sshKeyPair.ts index 5cf2ca18d97..9b64ab8ae5e 100644 --- a/packages/core/src/awsService/ec2/sshKeyPair.ts +++ b/packages/core/src/awsService/ec2/sshKeyPair.ts @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ import { fs } from '../../shared' -import { chmodSync } from 'fs' import { ToolkitError } from '../../shared/errors' import { ChildProcess } from '../../shared/utilities/childProcess' import { Timeout } from '../../shared/utilities/timeoutUtils' @@ -40,7 +39,7 @@ export class SshKeyPair { if (result.exitCode !== 0) { throw new ToolkitError('ec2: Failed to generate ssh key', { details: { stdout: result.stdout } }) } - chmodSync(keyPath, 0o600) + await fs.chmod(keyPath, 0o600) } public getPublicKeyPath(): string { diff --git a/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts b/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts index 84c0fd75ac8..d6673954627 100644 --- a/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts +++ b/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts @@ -6,11 +6,13 @@ import * as vscode from 'vscode' import assert from 'assert' import * as sinon from 'sinon' import * as path from 'path' +import * as os from 'os' +import { stat } from 'fs/promises' import { SshKeyPair } from '../../../awsService/ec2/sshKeyPair' import { createTestWorkspaceFolder, installFakeClock } from '../../testUtil' import { InstalledClock } from '@sinonjs/fake-timers' import { ChildProcess } from '../../../shared/utilities/childProcess' -import { fs } from '../../../shared' +import { fs, globals } from '../../../shared' describe('SshKeyUtility', async function () { let temporaryDirectory: string @@ -57,6 +59,13 @@ describe('SshKeyUtility', async function () { // Check private key header for algorithm name assert.strictEqual(result.stdout.includes('[ED25519 256]'), true) }) + + it('sets permission of the file to read/write owner', async function () { + if (!globals.isWeb && os.platform() !== 'win32') { + const result = await stat(keyPair.getPrivateKeyPath()) + assert.strictEqual(result.mode & 0o777, 0o600) + } + }) }) it('properly names the public key', function () { From 8ff9d42928a1d62a41e426231dc09fdd352b6790 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 24 Sep 2024 19:06:28 -0400 Subject: [PATCH 6/9] replace usages of chmod with new fs.chmod --- .../core/src/amazonq/lsp/lspController.ts | 3 ++- .../src/shared/sam/debugger/csharpSamDebug.ts | 5 ++-- .../src/shared/sam/debugger/goSamDebug.ts | 2 +- .../shared/utilities/childProcess.test.ts | 26 +++++++++++-------- packages/core/src/test/testUtil.ts | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index 966eee8190e..44fdaf45d5d 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -24,6 +24,7 @@ import { AuthUtil } from '../../codewhisperer' import { isWeb } from '../../shared/extensionGlobals' import { getUserAgent } from '../../shared/telemetry/util' import { isAmazonInternalOs } from '../../shared/vscode/env' +import { fs as fs2 } from '../../shared/fs/fs' function getProjectPaths() { const workspaceFolders = vscode.workspace.workspaceFolders @@ -266,7 +267,7 @@ export class LspController { if (!downloadNodeOk) { return false } - fs.chmodSync(nodeRuntimeTempPath, 0o755) + await fs2.chmod(nodeRuntimeTempPath, 0o755) fs.moveSync(nodeRuntimeTempPath, nodeRuntimePath) return true } catch (e) { diff --git a/packages/core/src/shared/sam/debugger/csharpSamDebug.ts b/packages/core/src/shared/sam/debugger/csharpSamDebug.ts index 3d83247ee49..850f1a241d4 100644 --- a/packages/core/src/shared/sam/debugger/csharpSamDebug.ts +++ b/packages/core/src/shared/sam/debugger/csharpSamDebug.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { chmod, ensureDir, writeFile } from 'fs-extra' +import { ensureDir, writeFile } from 'fs-extra' import * as os from 'os' import * as path from 'path' import { @@ -24,6 +24,7 @@ import { getLogger } from '../../logger' import * as vscode from 'vscode' import * as nls from 'vscode-nls' import globals from '../../extensionGlobals' +import fs from '../../fs/fs' const localize = nls.loadMessageBundle() /** @@ -203,7 +204,7 @@ async function downloadInstallScript(debuggerPath: string): Promise { } await writeFile(installScriptPath, installScript, 'utf8') - await chmod(installScriptPath, 0o700) + await fs.chmod(installScriptPath, 0o700) return installScriptPath } diff --git a/packages/core/src/shared/sam/debugger/goSamDebug.ts b/packages/core/src/shared/sam/debugger/goSamDebug.ts index c4bd8e3997f..96263310dac 100644 --- a/packages/core/src/shared/sam/debugger/goSamDebug.ts +++ b/packages/core/src/shared/sam/debugger/goSamDebug.ts @@ -224,7 +224,7 @@ async function makeInstallScript(debuggerPath: string, isWindows: boolean): Prom script += `go build -o "${delvePath}" "${delveRepo}/cmd/dlv"\n` await fs.writeFile(installScriptPath, script, 'utf8') - await fs.chmod(installScriptPath, 0o755) + await fs2.chmod(installScriptPath, 0o755) return { path: installScriptPath, options: installOptions } } diff --git a/packages/core/src/test/shared/utilities/childProcess.test.ts b/packages/core/src/test/shared/utilities/childProcess.test.ts index a507c3fabdf..9960ccab213 100644 --- a/packages/core/src/test/shared/utilities/childProcess.test.ts +++ b/packages/core/src/test/shared/utilities/childProcess.test.ts @@ -5,6 +5,7 @@ import assert from 'assert' import * as fs from 'fs-extra' +import { fs as fs2 } from '../../../shared' import * as os from 'os' import * as path from 'path' import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../../shared/filesystemUtilities' @@ -80,7 +81,7 @@ describe('ChildProcess', async function () { if (process.platform !== 'win32') { it('runs and captures stdout - unix', async function () { const scriptFile = path.join(tempFolder, 'test-script.sh') - writeShellFile(scriptFile) + await writeShellFile(scriptFile) const childProcess = new ChildProcess(scriptFile) @@ -89,7 +90,7 @@ describe('ChildProcess', async function () { it('errs when starting twice - unix', async function () { const scriptFile = path.join(tempFolder, 'test-script.sh') - writeShellFile(scriptFile) + await writeShellFile(scriptFile) const childProcess = new ChildProcess(scriptFile) @@ -120,7 +121,7 @@ describe('ChildProcess', async function () { writeBatchFile(command) } else { command = path.join(subfolder, 'test script.sh') - writeShellFile(command) + await writeShellFile(command) } const childProcess = new ChildProcess(command) @@ -140,14 +141,17 @@ describe('ChildProcess', async function () { describe('Extra options', function () { let childProcess: ChildProcess - beforeEach(function () { + beforeEach(async function () { const isWindows = process.platform === 'win32' const command = path.join(tempFolder, `test-script.${isWindows ? 'bat' : 'sh'}`) if (isWindows) { writeBatchFile(command, ['@echo %1', '@echo %2', '@echo "%3"', 'SLEEP 20', 'exit 1'].join(os.EOL)) } else { - writeShellFile(command, ['echo $1', 'echo $2', 'echo "$3"', 'sleep 20', 'exit 1'].join(os.EOL)) + await writeShellFile( + command, + ['echo $1', 'echo $2', 'echo "$3"', 'sleep 20', 'exit 1'].join(os.EOL) + ) } childProcess = new ChildProcess(command, ['1', '2'], { collect: false }) @@ -277,7 +281,7 @@ describe('ChildProcess', async function () { if (process.platform !== 'win32') { it('detects running processes and successfully stops a running process - Unix', async function () { const scriptFile = path.join(tempFolder, 'test-script.sh') - writeShellFileWithDelays(scriptFile) + await writeShellFileWithDelays(scriptFile) const childProcess = new ChildProcess('sh', [scriptFile]) const result = childProcess.run() @@ -291,7 +295,7 @@ describe('ChildProcess', async function () { it('can stop() previously stopped processes - Unix', async function () { const scriptFile = path.join(tempFolder, 'test-script.sh') - writeShellFileWithDelays(scriptFile) + await writeShellFileWithDelays(scriptFile) const childProcess = new ChildProcess(scriptFile) @@ -331,16 +335,16 @@ describe('ChildProcess', async function () { fs.writeFileSync(filename, `@echo OFF${os.EOL}echo hi`) } - function writeShellFile(filename: string, contents = 'echo hi'): void { + async function writeShellFile(filename: string, contents = 'echo hi'): Promise { fs.writeFileSync(filename, `#!/bin/sh\n${contents}`) - fs.chmodSync(filename, 0o744) + await fs2.chmod(filename, 0o744) } - function writeShellFileWithDelays(filename: string): void { + async function writeShellFileWithDelays(filename: string): Promise { const file = ` echo hi sleep 20 echo bye` - writeShellFile(filename, file) + await writeShellFile(filename, file) } }) diff --git a/packages/core/src/test/testUtil.ts b/packages/core/src/test/testUtil.ts index 2f51a56cf9c..0e6ec4e839b 100644 --- a/packages/core/src/test/testUtil.ts +++ b/packages/core/src/test/testUtil.ts @@ -177,7 +177,7 @@ export async function createExecutableFile(filepath: string, contents: string): await fs2.writeFile(filepath, `@echo OFF$\r\n${contents}\r\n`) } else { await fs2.writeFile(filepath, `#!/bin/sh\n${contents}`) - nodefs.chmodSync(filepath, 0o744) + await fs2.chmod(filepath, 0o744) } } From 8b281aeb9a5d2f0e7b82909acff55ffdcc7421d4 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 30 Sep 2024 14:15:59 -0400 Subject: [PATCH 7/9] fix remaining conflict --- packages/core/src/shared/fs/fs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/shared/fs/fs.ts b/packages/core/src/shared/fs/fs.ts index 17691ee9e76..63a552781c9 100644 --- a/packages/core/src/shared/fs/fs.ts +++ b/packages/core/src/shared/fs/fs.ts @@ -367,7 +367,7 @@ export class FileSystem { */ async chmod(uri: vscode.Uri | string, mode: number): Promise { if (!this.isWeb) { - const path = this.#toUri(uri) + const path = toUri(uri) await chmod(path.fsPath, mode) } } From 0b050256a0cbf9c20980d326364ed8ecacc1d309 Mon Sep 17 00:00:00 2001 From: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:05:18 -0400 Subject: [PATCH 8/9] Update packages/core/src/test/awsService/ec2/sshKeyPair.test.ts Co-authored-by: Justin M. Keyes --- packages/core/src/test/awsService/ec2/sshKeyPair.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts b/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts index 86fbe772487..3730e9969e7 100644 --- a/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts +++ b/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts @@ -54,7 +54,7 @@ describe('SshKeyUtility', async function () { it('sets permission of the file to read/write owner', async function () { if (!globals.isWeb && os.platform() !== 'win32') { - const result = await stat(keyPair.getPrivateKeyPath()) + const result = await nodefs.stat(keyPair.getPrivateKeyPath()) assert.strictEqual(result.mode & 0o777, 0o600) } }) From 7dd34dafa3befbe9b1ddef08c1c5b8d3011c00bc Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Oct 2024 09:14:20 -0400 Subject: [PATCH 9/9] redo imports --- packages/core/src/test/awsService/ec2/sshKeyPair.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts b/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts index 840c7a249a7..d34abf64566 100644 --- a/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts +++ b/packages/core/src/test/awsService/ec2/sshKeyPair.test.ts @@ -4,10 +4,10 @@ */ import * as vscode from 'vscode' import assert from 'assert' +import nodefs from 'fs' import * as sinon from 'sinon' import * as path from 'path' import * as os from 'os' -import { stat } from 'fs/promises' import { SshKeyPair } from '../../../awsService/ec2/sshKeyPair' import { createTestWorkspaceFolder, installFakeClock } from '../../testUtil' import { InstalledClock } from '@sinonjs/fake-timers' @@ -54,7 +54,7 @@ describe('SshKeyUtility', async function () { it('sets permission of the file to read/write owner', async function () { if (!globals.isWeb && os.platform() !== 'win32') { - const result = await nodefs.stat(keyPair.getPrivateKeyPath()) + const result = nodefs.statSync(keyPair.getPrivateKeyPath()) assert.strictEqual(result.mode & 0o777, 0o600) } })