From 8959b02e398178a7136e98a6b40822070c321a5a Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 11 Dec 2024 11:53:22 +0000 Subject: [PATCH 1/4] CDL: Improve performance by simplifying location identification We can assume that the path in the cds.json is relative to the repository root, as that is the directory the CDS compiler is run from. --- .../advanced_security/javascript/frameworks/cap/CDL.qll | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll index f4811c9ff..e1cb64076 100644 --- a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll +++ b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll @@ -14,11 +14,12 @@ abstract class CdlObject extends JsonObject { exists(Location loc, JsonValue locValue | loc = this.getLocation() and locValue = this.getPropValue("$location") and + // The path in the cds.json file is relative to the working directory used when running + // the cds compile command. In our extractor, that's always the root of the repository, + // so we can identify the entry in the `File` table by its relative path. path = - any(File f | - f.getAbsolutePath() - .matches("%" + locValue.getPropValue("file").getStringValue() + ".json") - ).getAbsolutePath().regexpReplaceAll("\\.json$", "") and + any(File f | f.getRelativePath() = locValue.getPropValue("file").getStringValue()) + .getAbsolutePath() and if not exists(locValue.getPropValue("line")) and not exists(locValue.getPropValue("col")) From 704047361d4645c17dc4bec09082585d2cd38c1e Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 17 Dec 2024 23:09:59 +0000 Subject: [PATCH 2/4] Update to use the sourceLocationPrefix In our testing framework we don't always have the .cds file indexed so we can't guarantee the presence of the File entry. Instead, compose the path using the sourceLocationPrefix, which will be robust to whether the .cds file was indexed or not. --- .../javascript/frameworks/cap/CDL.qll | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll index e1cb64076..5582c6c61 100644 --- a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll +++ b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll @@ -16,10 +16,14 @@ abstract class CdlObject extends JsonObject { locValue = this.getPropValue("$location") and // The path in the cds.json file is relative to the working directory used when running // the cds compile command. In our extractor, that's always the root of the repository, - // so we can identify the entry in the `File` table by its relative path. - path = - any(File f | f.getRelativePath() = locValue.getPropValue("file").getStringValue()) - .getAbsolutePath() and + // so we can identify the sourceLocationPrefix to find the path of the root of the repo + // then append the relative path + exists(string sourceLocationPrefix | + sourceLocationPrefix(sourceLocationPrefix) and + path = + sourceLocationPrefix.regexpReplaceAll("/$", "") + "/" + + locValue.getPropValue("file").getStringValue() + ) and if not exists(locValue.getPropValue("line")) and not exists(locValue.getPropValue("col")) From cf269f718064313a5df60165bf19906ca8e5b948 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 18 Dec 2024 09:28:28 +0000 Subject: [PATCH 3/4] Index the cds files at the appropriate path during tests - Index the .cds files in addition to the .cds.json files - Run the cds compiler at the appropriate path --- .../run-codeql-unit-tests-javascript.yml | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/run-codeql-unit-tests-javascript.yml b/.github/workflows/run-codeql-unit-tests-javascript.yml index 77fdd3efd..8a10d70b6 100644 --- a/.github/workflows/run-codeql-unit-tests-javascript.yml +++ b/.github/workflows/run-codeql-unit-tests-javascript.yml @@ -87,13 +87,20 @@ jobs: # Compile .cds files to .cds.json files. - name: Compile CAP CDS files run: | - for cds_file in $(find . -type f \( -iname '*.cds' \) -print) + for test_dir in $(find . -type f -name '*.expected' -exec dirname {} \;); do - echo "I am compiling $cds_file" - cds compile $cds_file \ - -2 json \ - -o "$cds_file.json" \ - --locations + # The CDS compiler produces locations relative to the working directory + # so we switch to the test directory before running the compiler. + pushd $test_dir + for cds_file in $(find . -type f \( -iname '*.cds' \) -print) + do + echo "I am compiling $cds_file" + cds compile $cds_file \ + -2 json \ + -o "$cds_file.json" \ + --locations + done + popd done - name: Run test suites @@ -105,7 +112,7 @@ jobs: CODEQL_STDLIB_IDENT: ${{matrix.codeql_standard_library_ident}} RUNNER_TMP: ${{ runner.temp }} LGTM_INDEX_XML_MODE: all - LGTM_INDEX_FILETYPES: ".json:JSON" + LGTM_INDEX_FILETYPES: ".json:JSON\n.cds:JSON" shell: bash run: > From 237b4b5fbc68a5e0960c9a03f56a09e8d62d0452 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 18 Dec 2024 23:09:48 +0000 Subject: [PATCH 4/4] Upgrade to CodeQL CLI 2.19.4. --- qlt.conf.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qlt.conf.json b/qlt.conf.json index bf1693768..c59aec4fd 100644 --- a/qlt.conf.json +++ b/qlt.conf.json @@ -1,5 +1,5 @@ { - "CodeQLCLI": "2.19.0", - "CodeQLStandardLibrary": "codeql-cli/v2.19.0", - "CodeQLCLIBundle": "codeql-bundle-v2.19.0" + "CodeQLCLI": "2.19.4", + "CodeQLStandardLibrary": "codeql-cli/v2.19.4", + "CodeQLCLIBundle": "codeql-bundle-v2.19.4" } \ No newline at end of file