Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial version of platform-independent CDS extractor #169

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

Initial version of platform-independent CDS extractor #169

Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
31927be
WIP make the CDS extractor platform independent
data-douser Jan 23, 2025
c50380c
Merge branch 'advanced-security:main' into data-douser/extractor-dev-2
data-douser Jan 23, 2025
db64e01
Merge branch 'advanced-security:main' into data-douser/extractor-dev-2
data-douser Jan 23, 2025
54938b6
Improve extractor script comments and file paths
data-douser Jan 28, 2025
ea229d1
Add requested comment to extractor.js responsFiles
data-douser Jan 28, 2025
aed1fc6
WIP fixes for CDS extractor rewrite
data-douser Jan 29, 2025
d02234e
Fix CDS extractor JS undefined var
data-douser Jan 30, 2025
a17b8c2
Set cwd for CDS JS autobuild process
data-douser Jan 30, 2025
0b6994c
Another attempted fix for index-files.js cwd
data-douser Feb 4, 2025
2673f53
Document index-files change for grandfathered package.json
data-douser Feb 5, 2025
212820a
Remove shell quote from index-files logging
data-douser Feb 5, 2025
cc99914
index-files.js must compiles cds to file (not dir)
data-douser Feb 5, 2025
5d16df0
Attempted fix for missing CDS SARIF results
data-douser Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 49 additions & 36 deletions extractors/cds/tools/index-files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const { execFileSync, spawnSync } = require('child_process');
const { existsSync, readFileSync, statSync } = require('fs');
const { existsSync, readFileSync, statSync, writeFileSync } = require('fs');
const { arch, platform } = require('os');
const { dirname, join, resolve } = require('path');
const { basename, dirname, join, resolve } = require('path');
const { quote } = require('shell-quote');

// Terminate early if this script is not invoked with the required arguments.
Expand All @@ -23,7 +23,7 @@ const osPlatform = platform();
const osPlatformArch = arch();
console.log(`Detected OS platform=${osPlatform} : arch=${osPlatformArch}`);
const codeqlExe = osPlatform === 'win32' ? 'codeql.exe' : 'codeql';
const codeqlExePath = join(quote([process.env.CODEQL_DIST]), codeqlExe);
const codeqlExePath = resolve(join(quote([process.env.CODEQL_DIST]), codeqlExe));

if (!existsSync(sourceRoot)) {
console.warn(`'${codeqlExe} database index-files --language cds' terminated early due to internal error: could not find project root directory '${sourceRoot}'.`);
Expand Down Expand Up @@ -61,9 +61,9 @@ if (!CODEQL_EXTRACTOR_JAVASCRIPT_ROOT) {
}

const autobuildScriptName = osPlatform === 'win32' ? 'autobuild.cmd' : 'autobuild.sh';
const autobuildScriptPath = join(
const autobuildScriptPath = resolve(join(
CODEQL_EXTRACTOR_JAVASCRIPT_ROOT, 'tools', autobuildScriptName
);
));

/**
* Terminate early if:
Expand Down Expand Up @@ -104,7 +104,7 @@ let cdsCommand = 'cds';
try {
execFileSync('cds', ['--version'], { stdio: 'ignore' });
} catch {
console.log('Pre-installing cds compiler');
console.log('Pre-installing cds compiler ...');

// Use a JS `Set` to avoid duplicate processing of the same directory.
const packageJsonDirs = new Set();
Expand All @@ -119,9 +119,12 @@ try {
* Nested package.json files simply cause the package to be installed in the parent
* node_modules directory.
*
* TODO : fix implementation or change ^comment^ to reflect the actual implementation.
*
* We also ensure we skip node_modules, as we can end up in a recursive loop.
*
* NOTE: The original (sh-based) implementation of this extractor would also capture
* "grandfathered" package.json files, which are package.json files that exist in a
* parent directory of the first package.json file found. This (js-based) implementation
* removes this behavior as it seems unnecessary and potentially problematic.
*/
responseFiles.forEach(file => {
let dir = dirname(quote([file]));
Expand Down Expand Up @@ -153,18 +156,18 @@ try {
// Sanity check that we found at least one package.json directory from which the CDS
// compiler dependencies may be installed.
if (packageJsonDirs.size === 0) {
console.warn('WARN: failed to detect any package.json directories for cds compiler installation.');
console.warn('WARN: failed to detect any package.json directories for cds compiler installation.');
exit(0);
}

packageJsonDirs.forEach((dir) => {
console.log(`Installing '@sap/cds-dk' into ${dir} to enable CDS compilation.`);
console.log(`Installing '@sap/cds-dk' into ${dir} to enable CDS compilation ...`);
execFileSync(
'npm',
['install', '--quiet', '--no-audit', '--no-fund', '@sap/cds-dk'],
['install', '--quiet', '--no-audit', '--no-fund', '--no-save', '@sap/cds-dk'],
{ cwd: dir, stdio: 'inherit' }
);
console.log(`Installing node packages into ${dir} to enable CDS compilation.`);
console.log(`Installing node packages into ${dir} to enable CDS compilation ...`);
execFileSync(
'npm',
['install', '--quiet', '--no-audit', '--no-fund'],
Expand All @@ -180,7 +183,7 @@ try {
cdsCommand = 'npx -y --package @sap/cds-dk cds';
}

console.log('Processing CDS files to JSON');
console.log('Processing CDS files to JSON ...');

/**
* Run the cds compile command on each file in the response files list, outputting the
Expand All @@ -189,34 +192,44 @@ console.log('Processing CDS files to JSON');
responseFiles.forEach(rawCdsFilePath => {
const cdsFilePath = quote([rawCdsFilePath]);
const cdsJsonFilePath = `${cdsFilePath}.json`;
console.log(`Processing CDS file ${cdsFilePath} to: ${cdsJsonFilePath}`);
console.log(`Processing CDS file ${cdsFilePath} to ${cdsJsonFilePath} ...`);
const result = spawnSync(
cdsCommand,
['compile', cdsFilePath, '-2', 'json', '-o', cdsJsonFilePath, '--locations'],
{ shell: true }
[
'compile', cdsFilePath,
'-2', 'json',
'--locations',
'--log-level', 'warn'
],
{ cwd: dirname(cdsFilePath), shell: true, stdio: 'pipe' }
);
if (result.error || result.status !== 0) {
const stderrTruncated = quote(
result.stderr.toString().split('\n').filter(line => line.startsWith('[ERROR]')).slice(-4).join('\n'));
const errorMessage = `Could not compile the file ${cdsFilePath}.\nReported error(s):\n\`\`\`\n${stderrTruncated}\n\`\`\``;
if (result.error || result.status !== 0 || !result.stdout) {
const errorMessage = `Could not compile the file ${cdsFilePath}.\nReported error(s):\n\`\`\`\n${result.stderr.toString()}\n\`\`\``;
console.log(errorMessage);
execFileSync(
codeqlExePath,
[
'database',
'add-diagnostic',
'--extractor-name=cds',
'--ready-for-status-page',
'--source-id=cds/compilation-failure',
'--source-name="Failure to compile one or more SAP CAP CDS files"',
'--severity=error',
`--markdown-message="${errorMessage}"`,
`--file-path="${cdsFilePath}"`,
'--',
`${process.env.CODEQL_EXTRACTOR_CDS_WIP_DATABASE}`
],
);
try {
execFileSync(
codeqlExePath,
[
'database',
'add-diagnostic',
'--extractor-name=cds',
'--ready-for-status-page',
'--source-id=cds/compilation-failure',
'--source-name="Failure to compile one or more SAP CAP CDS files"',
'--severity=error',
`--markdown-message="${errorMessage}"`,
`--file-path="${cdsFilePath}"`,
'--',
`${process.env.CODEQL_EXTRACTOR_CDS_WIP_DATABASE}`
],
);
console.log(`Added error diagnostic for source file: ${cdsFilePath}`);
} catch (err) {
console.error(`Failed to add error diagnostic for source file=${cdsFilePath} : ${err}`);
}
}
// Write the compiled JSON result to cdsJsonFilePath.
writeFileSync(cdsJsonFilePath, result.stdout);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of interest, why do we no longer use the -o flag to the cds compiler?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to cds compiler -h, the -o (or --dest) option now Writes output to the given folder instead of stdout.
This seems like a change in behavior and it is causing problems for our existing (sh-based) extractor.

For an example CDS file called service1.cds, using -2 json -o service1.cds.json will actually create service1.cds.json/service1.json. This path mismatch causes many SARIF diffs.

I probably need to submit a fix for the sh-based extractor, simply because I need a working reference to compare against.

});

let excludeFilters = '';
Expand Down
Loading