From 9a3f930cc1223030fa1db7a3f12b3fdcfc8cc345 Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Mon, 5 Aug 2024 12:07:52 +0200 Subject: [PATCH] SNOW-1332628: Fix and test for permission verification (#878) --- .github/workflows/build-test.yml | 2 +- lib/file_transfer_agent/file_util.js | 11 ++-- test/integration/connectionOptions.js | 8 +-- .../configuration_parsing_test.js | 11 ++-- .../file_transfer_agent/file_util_test.js | 56 ++++++++++++++++++- 5 files changed, 72 insertions(+), 16 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 66478d7f0..694176221 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -73,7 +73,7 @@ jobs: CLOUD_PROVIDER: ${{ matrix.cloud }} run: /usr/local/bin/bash ./ci/test_mac.sh - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: # without the token code cov may fail because of Github limits https://github.com/codecov/codecov-action/issues/557 token: ${{ secrets.CODE_COV_UPLOAD_TOKEN }} diff --git a/lib/file_transfer_agent/file_util.js b/lib/file_transfer_agent/file_util.js index 6df2b66a9..16a8701cf 100644 --- a/lib/file_transfer_agent/file_util.js +++ b/lib/file_transfer_agent/file_util.js @@ -146,14 +146,17 @@ function getMatchingFilePaths(dir, fileName) { } function validateOnlyUserReadWritePermission(filePath) { + if (os.platform() === 'win32') { + return; + } fs.accessSync(filePath, fs.constants.F_OK); const mode = (fs.statSync(filePath)).mode; - const permission = mode & 0o600; + const permission = (mode & 0o00777 | 0o600); //This should be 600 permission, which means the file permission has not been changed by others. - if (permission.toString(8) === '600') { - Logger.getInstance().debug('Validated that the user has only read and write permission for file: %s', filePath); + if (permission === 0o600) { + Logger.getInstance().debug(`Validated that the user has only read and write permission for file: ${filePath}, Permission: ${permission}`); } else { - throw new Error('File permissions different than read/write for user. File: %s', filePath); + throw new Error(`File permissions different than read/write for user. File: ${filePath}`); } } diff --git a/test/integration/connectionOptions.js b/test/integration/connectionOptions.js index fdba7b096..7c778b8b7 100644 --- a/test/integration/connectionOptions.js +++ b/test/integration/connectionOptions.js @@ -74,18 +74,16 @@ const snowflakeAccount = snowflakeTestAdminUser !== undefined ? const wrongUserName = { - accessUrl: accessUrl, - username: 'node', - password: 'test', + username: snowflakeTestUser, + password: 'testWrongPass', account: snowflakeTestAccount }; const wrongPwd = { - accessUrl: accessUrl, - username: 'nodejs', + username: snowflakeTestUser, password: '', account: snowflakeTestAccount }; diff --git a/test/unit/configuration/configuration_parsing_test.js b/test/unit/configuration/configuration_parsing_test.js index 40e62d5c2..0fdf66442 100644 --- a/test/unit/configuration/configuration_parsing_test.js +++ b/test/unit/configuration/configuration_parsing_test.js @@ -13,13 +13,18 @@ let tempDir = null; describe('should parse toml connection configuration', function () { + beforeEach( async function () { + process.env.SNOWFLAKE_HOME = process.cwd() + '/test'; + const configurationPath = path.join(process.env.SNOWFLAKE_HOME, 'connections.toml'); + await fsPromises.chmod(configurationPath, '600'); + }); + afterEach( function () { delete process.env.SNOWFLAKE_HOME; delete process.env.SNOWFLAKE_DEFAULT_CONNECTION_NAME; }); it('should parse toml with connection configuration: ', async function () { - process.env.SNOWFLAKE_HOME = process.cwd() + '/test'; const configuration = await loadConnectionConfiguration(); assert.strictEqual(configuration['account'], 'snowdriverswarsaw.us-west-2.aws'); assert.strictEqual(configuration['username'], 'test_user'); @@ -32,7 +37,6 @@ describe('should parse toml connection configuration', function () { }); it('should parse toml with connection configuration - oauth', async function () { - process.env.SNOWFLAKE_HOME = process.cwd() + '/test/'; process.env.SNOWFLAKE_DEFAULT_CONNECTION_NAME = 'aws-oauth'; const configuration = await loadConnectionConfiguration(); assert.strictEqual(configuration['token'], 'token_value'); @@ -40,7 +44,6 @@ describe('should parse toml connection configuration', function () { }); it('should throw exception when token file does not exist', async function () { - process.env.SNOWFLAKE_HOME = process.cwd() + '/test/'; process.env.SNOWFLAKE_DEFAULT_CONNECTION_NAME = 'aws-oauth-file'; try { await loadConnectionConfiguration(); @@ -62,8 +65,6 @@ describe('should parse toml connection configuration', function () { it('should throw exception if configuration does not exists', function (done) { process.env.SNOWFLAKE_DEFAULT_CONNECTION_NAME = 'unknown'; - process.env.SNOWFLAKE_HOME = process.cwd() + '/test/'; - try { loadConnectionConfiguration(); assert.fail(); diff --git a/test/unit/file_transfer_agent/file_util_test.js b/test/unit/file_transfer_agent/file_util_test.js index 83368c29c..c95b70419 100644 --- a/test/unit/file_transfer_agent/file_util_test.js +++ b/test/unit/file_transfer_agent/file_util_test.js @@ -7,7 +7,7 @@ const testUtil = require('../../integration/testUtil'); const os = require('os'); const fsPromises = require('fs').promises; const crypto = require('crypto'); -const getMatchingFilePaths = require('../../../lib/file_transfer_agent/file_util').getMatchingFilePaths; +const { getMatchingFilePaths, validateOnlyUserReadWritePermission } = require('../../../lib/file_transfer_agent/file_util'); describe('matching files by wildcard', function () { @@ -50,3 +50,57 @@ describe('matching files by wildcard', function () { }); }); + +if (os.platform() !== 'win32') { + describe('verify only user read/write permission', function () { + let testFilePath; + + before(async function () { + testFilePath = await testUtil.createTempFileAsync(os.tmpdir(), testUtil.createRandomFileName()); + }); + + after(async function () { + await fsPromises.rm(testFilePath); + }); + + [ + { + permission: '600', + expectedResult: true + }, + { + permission: '100600', + expectedResult: true + }, + { + permission: '700', + expectedResult: false + }, + { + permission: '640', + expectedResult: false + }, + { + permission: '100777', + expectedResult: false + }, + { + permission: '444', + expectedResult: false + }, + { + permission: '12477', + expectedResult: false + } + ].forEach(({ permission, expectedResult }) => { + it('verify permission', async function () { + await fsPromises.chmod(testFilePath, permission); + if (!expectedResult) { + assert.throws(() => validateOnlyUserReadWritePermission(testFilePath)); + } else { + assert.doesNotThrow(() => validateOnlyUserReadWritePermission(testFilePath)); + } + }); + }); + }); +}