SNOW-1332628: Fix and test for permission verification (#878)

snowflakedb · Aug 5, 2024 · 9a3f930 · 9a3f930
1 parent d6dd1fe
commit 9a3f930
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,15 +37,13 @@ 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');
     assert.strictEqual(configuration['authenticator'], 'oauth');
   });
 
   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
@@ -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));
+        }
+      });
+    });
+  });
+}