Merge pull request #67 from github/dont-fail-on-symlinks

Ensure config path comparisons don't fail where workspace path is a symlink

lib/config-utils.js

@@ -51,14 +51,16 @@ class Config {
             const localQueryPath = queryUses.slice(2);
             // Resolve the local path against the workspace so that when this is
             // passed to codeql it resolves to exactly the path we expect it to resolve to.
-            const workspacePath = util.getRequiredEnvParam('GITHUB_WORKSPACE');
-            const absoluteQueryPath = path.join(workspacePath, localQueryPath);
+            const workspacePath = fs.realpathSync(util.getRequiredEnvParam('GITHUB_WORKSPACE'));
+            let absoluteQueryPath = path.join(workspacePath, localQueryPath);
             // Check the file exists
             if (!fs.existsSync(absoluteQueryPath)) {
                 throw new Error(getLocalPathDoesNotExist(configFile, localQueryPath));
             }
+            // Call this after checking file exists, because it'll fail if file doesn't exist
+            absoluteQueryPath = fs.realpathSync(absoluteQueryPath);
             // Check the local path doesn't jump outside the repo using '..' or symlinks
-            if (!(fs.realpathSync(absoluteQueryPath) + path.sep).startsWith(workspacePath + path.sep)) {
+            if (!(absoluteQueryPath + path.sep).startsWith(workspacePath + path.sep)) {
                 throw new Error(getLocalPathOutsideOfRepository(configFile, localQueryPath));
             }
             this.additionalQueries.push(absoluteQueryPath);

lib/config-utils.test.js

@@ -95,18 +95,18 @@ ava_1.default("load non-empty input", async (t) => {
         - b
       paths:
         - c/d`;
+        fs.mkdirSync(path.join(tmpDir, 'foo'));
         // And the config we expect it to parse to
         const expectedConfig = new configUtils.Config();
         expectedConfig.name = 'my config';
         expectedConfig.disableDefaultQueries = true;
-        expectedConfig.additionalQueries.push(tmpDir);
-        expectedConfig.additionalQueries.push(path.join(tmpDir, 'foo'));
+        expectedConfig.additionalQueries.push(fs.realpathSync(tmpDir));
+        expectedConfig.additionalQueries.push(fs.realpathSync(path.join(tmpDir, 'foo')));
         expectedConfig.externalQueries = [new configUtils.ExternalQuery('foo/bar', 'dev')];
         expectedConfig.pathsIgnore = ['a', 'b'];
         expectedConfig.paths = ['c/d'];
         fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
         setInput('config-file', 'input');
-        fs.mkdirSync(path.join(tmpDir, 'foo'));
         const actualConfig = await configUtils.loadConfig();
         // Should exactly equal the object we constructed earlier
         t.deepEqual(actualConfig, expectedConfig);

src/config-utils.test.ts

@@ -100,12 +100,14 @@ test("load non-empty input", async t => {
       paths:
         - c/d`;
 
+    fs.mkdirSync(path.join(tmpDir, 'foo'));
+
     // And the config we expect it to parse to
     const expectedConfig = new configUtils.Config();
     expectedConfig.name = 'my config';
     expectedConfig.disableDefaultQueries = true;
-    expectedConfig.additionalQueries.push(tmpDir);
-    expectedConfig.additionalQueries.push(path.join(tmpDir, 'foo'));
+    expectedConfig.additionalQueries.push(fs.realpathSync(tmpDir));
+    expectedConfig.additionalQueries.push(fs.realpathSync(path.join(tmpDir, 'foo')));
     expectedConfig.externalQueries = [new configUtils.ExternalQuery('foo/bar', 'dev')];
     expectedConfig.pathsIgnore = ['a', 'b'];
     expectedConfig.paths = ['c/d'];
@@ -113,8 +115,6 @@ test("load non-empty input", async t => {
     fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
     setInput('config-file', 'input');
 
-    fs.mkdirSync(path.join(tmpDir, 'foo'));
-
     const actualConfig = await configUtils.loadConfig();
 
     // Should exactly equal the object we constructed earlier

src/config-utils.ts

@@ -51,16 +51,19 @@ export class Config {
             const localQueryPath = queryUses.slice(2);
             // Resolve the local path against the workspace so that when this is
             // passed to codeql it resolves to exactly the path we expect it to resolve to.
-            const workspacePath = util.getRequiredEnvParam('GITHUB_WORKSPACE');
-            const absoluteQueryPath = path.join(workspacePath, localQueryPath);
+            const workspacePath = fs.realpathSync(util.getRequiredEnvParam('GITHUB_WORKSPACE'));
+            let absoluteQueryPath = path.join(workspacePath, localQueryPath);
 
             // Check the file exists
             if (!fs.existsSync(absoluteQueryPath)) {
                 throw new Error(getLocalPathDoesNotExist(configFile, localQueryPath));
             }
 
+            // Call this after checking file exists, because it'll fail if file doesn't exist
+            absoluteQueryPath = fs.realpathSync(absoluteQueryPath);
+
             // Check the local path doesn't jump outside the repo using '..' or symlinks
-            if (!(fs.realpathSync(absoluteQueryPath) + path.sep).startsWith(workspacePath + path.sep)) {
+            if (!(absoluteQueryPath + path.sep).startsWith(workspacePath + path.sep)) {
                 throw new Error(getLocalPathOutsideOfRepository(configFile, localQueryPath));
             }