Skip to content

Commit

Permalink
feat: improve gradle plugin accuracy
Browse files Browse the repository at this point in the history 
This patch set improves the accuracy of the gradle plugin.

Newer version of Gradle added two attributes to configs for controlling
the resolution process: `canBeResolved` and `canBeConsumed` (see
https://docs.gradle.org/current/userguide/declaring_dependencies.html#sec:resolvable-consumable-configs
for details). A recent change to the plugin brought it in line with
Gradle best practices, i.e. prefer `canBeResolved=true` and
`canBeConsumed=false`. If no configs match the criteria, it falls back
to configs with both attributes set to `true`. While this is a good
approach, the implementation is a bit too strict: if _any_ `true`-
`false` configs are found, the `true`-`true` ones won't be considered.
This misses some configs, and hence some dependencies that users expect
to see.

The first change is to relax the strategy, and accept all the configs
that have `canBeResolved=true`. This is equivalent to taking the union
of `true`-`false` and `true`-`false` configs. Since this will
(correctly) result in more dependencies, I had to update
`test/system/kotlin.test.ts`.

The second change fixes a bug in the plugin where sub-projects with the
same name at different levels of a multi-project hierarchy would
overwrite each other. To patch now uses the full path of the sub-project
to disambiguate between sub-projects with the same name.

In addition to the above changes, the patch refactors the plugin code.
  • Loading branch information
@muscar
muscar committed Oct 28, 2021
1 parent d545c85 commit 3043f0b
Show file tree
Hide file tree
Showing 17 changed files with 770 additions and 186 deletions.
113 changes: 75 additions & 38 deletions lib/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ export interface GradleInspectOptions {
reachableVulns?: boolean;
callGraphBuilderTimeout?: number;
initScript?: string;

// Best practices for recent version of Gradle are to avoid configs that are
// both resolvable _and_ consumable, i.e. canBeResolved == true &&
// canBeConsumed == true. Legacy projects will have these, so CLI scan might
// report less dependencies than the users expect. This flag allows the users
// to relax the configuration filtering strategy. This runs the risk that
// there will be configuration variant conflicts, but the user should know
// what they get themselves into.
// See: https://docs.gradle.org/current/userguide/declaring_dependencies.html#sec:resolvable-consumable-configs
gradleAcceptLegacyConfigRoles?: boolean;
}

type Options = api.InspectOptions & GradleInspectOptions;
Expand Down Expand Up @@ -404,8 +414,14 @@ async function printIfEcho(line: string) {
}
}

async function getInjectedScriptPath(): Promise<{
injectedScripPath: string;
function getPluginFileName(options: Options): string {
return options.gradleAcceptLegacyConfigRoles
? 'init.gradle'
: 'legacy-init.gradle';
}

async function injectedPlugin(scriptName: string): Promise<{
injectedPluginFilePath: string;
cleanupCallback?: () => void;
}> {
let initGradleAsset: string | null = null;
Expand All @@ -414,22 +430,22 @@ async function getInjectedScriptPath(): Promise<{
// running from ./dist
// path.join call has to be exactly in this format, needed by "pkg" to build a standalone Snyk CLI binary:
// https://www.npmjs.com/package/pkg#detecting-assets-in-source-code
initGradleAsset = path.join(__dirname, '../lib/init.gradle');
initGradleAsset = path.join(__dirname, '../lib', scriptName);
} else if (/index.ts$/.test(__filename)) {
// running from ./lib
initGradleAsset = path.join(__dirname, 'init.gradle');
initGradleAsset = path.join(__dirname, scriptName);
} else {
throw new Error('Cannot locate Snyk init.gradle script');
throw new Error(`Cannot locate Snyk ${scriptName} script`);
}

// We could be running from a bundled CLI generated by `pkg`.
// The Node filesystem in that case is not real: https://github.com/zeit/pkg#snapshot-filesystem
// Copying the injectable script into a temp file.
try {
const tmpInitGradle = tmp.fileSync({ postfix: '-init.gradle' });
const tmpInitGradle = tmp.fileSync({ postfix: `-${scriptName}` });
fs.writeSync(tmpInitGradle.fd, fs.readFileSync(initGradleAsset));
return {
injectedScripPath: tmpInitGradle.name,
injectedPluginFilePath: tmpInitGradle.name,
cleanupCallback: tmpInitGradle.removeCallback,
};
} catch (error) {
Expand Down Expand Up @@ -488,12 +504,10 @@ function getVersionBuildInfo(
}
}

async function getAllDeps(
async function getGradleVersion(
root: string,
targetFile: string,
options: Options,
): Promise<JsonDepsScriptResult> {
const command = getCommand(root, targetFile);
command: string,
): Promise<string> {
debugLog('`gradle -v` command run: ' + command);
let gradleVersionOutput = '[COULD NOT RUN gradle -v]';
try {
Expand All @@ -503,39 +517,62 @@ async function getAllDeps(
} catch (_) {
// intentionally empty
}
return gradleVersionOutput;
}

if (gradleVersionOutput.match(/Gradle 1/)) {
throw new Error('Gradle 1.x is not supported');
}

const { injectedScripPath, cleanupCallback } = await getInjectedScriptPath();
const args = buildArgs(root, targetFile, injectedScripPath, options);
async function getAllDepsWithPlugin(
root: string,
targetFile: string,
options: Options,
): Promise<JsonDepsScriptResult> {
const command = getCommand(root, targetFile);
const pluginFileName = getPluginFileName(options);
const { injectedPluginFilePath, cleanupCallback } = await injectedPlugin(
pluginFileName,
);
const args = buildArgs(root, targetFile, injectedPluginFilePath, options);

const fullCommandText = 'gradle command: ' + command + ' ' + args.join(' ');
debugLog('Executing ' + fullCommandText);

const stdoutText = await subProcess.execute(
command,
args,
{ cwd: root },
printIfEcho,
);
if (cleanupCallback) {
cleanupCallback();
}
const extractedJSON = extractJsonFromScriptOutput(stdoutText);
const jsonAttrsPretty = getGradleAttributesPretty(stdoutText);
logger(
`The following attributes and their possible values were found in your configurations: ${jsonAttrsPretty}`,
);

return extractedJSON;
}

async function getAllDeps(
root: string,
targetFile: string,
options: Options,
): Promise<JsonDepsScriptResult> {
const command = getCommand(root, targetFile);
const gradleVersion = await getGradleVersion(root, command);
if (gradleVersion.match(/Gradle 1/)) {
throw new Error('Gradle 1.x is not supported');
}

try {
const stdoutText = await subProcess.execute(
command,
args,
{ cwd: root },
printIfEcho,
);
if (cleanupCallback) {
cleanupCallback();
}
const extractedJSON = extractJsonFromScriptOutput(stdoutText);
const jsonAttrsPretty = getGradleAttributesPretty(stdoutText);
logger(
`The following attributes and their possible values were found in your configurations: ${jsonAttrsPretty}`,
);
const versionBuildInfo = getVersionBuildInfo(gradleVersionOutput);
const extractedJSON = await getAllDepsWithPlugin(root, targetFile, options);
const versionBuildInfo = getVersionBuildInfo(gradleVersion);
if (versionBuildInfo) {
extractedJSON.versionBuildInfo = versionBuildInfo;
}

return await processProjectsInExtractedJSON(root, extractedJSON);
} catch (error0) {
const error: Error = error0;
} catch (err) {
const error: Error = err;
const gradleErrorMarkers = /^\s*>\s.*$/;
const gradleErrorEssence = error.message
.split('\n')
Expand All @@ -544,7 +581,7 @@ async function getAllDeps(

const orange = chalk.rgb(255, 128, 0);
const blackOnYellow = chalk.bgYellowBright.black;
gradleVersionOutput = orange(gradleVersionOutput);
const gradleVersionOutput = orange(gradleVersion);
let mainErrorMessage = `Error running Gradle dependency analysis.
Please ensure you are calling the \`snyk\` command with correct arguments.
Expand Down Expand Up @@ -607,7 +644,6 @@ to
)}
${blackOnYellow('===== DEBUG INFORMATION START =====')}
${orange(fullCommandText)}
${orange(gradleVersionOutput)}
${orange(error.message)}
${blackOnYellow('===== DEBUG INFORMATION END =====')}
Expand Down Expand Up @@ -803,4 +839,5 @@ export const exportsForTests = {
getVersionBuildInfo,
toCamelCase,
getGradleAttributesPretty,
getPluginFileName,
};
Loading

0 comments on commit 3043f0b

Please sign in to comment.