Skip to content

Commit

Permalink
Filter out security advisories which do not affected the discovered d…
Browse files Browse the repository at this point in the history
…ependencies during a security-only update
  • Loading branch information
rhyskoedijk committed Oct 22, 2024
1 parent e3987e9 commit 5108b0d
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 18 deletions.
20 changes: 14 additions & 6 deletions extension/tasks/dependabotV2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,24 +111,32 @@ async function run() {
let dependencyNamesToUpdate: string[] = undefined;
const securityUpdatesOnly = update['open-pull-requests-limit'] === 0;
if (securityUpdatesOnly) {
// Run an update job to discover all dependencies
const discoveredDependencyListOutputs = await dependabot.update(
DependabotJobBuilder.newDiscoverDependencyListJob(taskInputs, updateId, update, dependabotConfig.registries),
DependabotJobBuilder.listAllDependenciesJob(taskInputs, updateId, update, dependabotConfig.registries),
dependabotUpdaterOptions,
);
dependencyNamesToUpdate = discoveredDependencyListOutputs

// Get the list of security advisories that apply to the discovered dependencies
const dependenciesToCheckForSecurityAdvisories = discoveredDependencyListOutputs
?.find((x) => x.output.type == 'update_dependency_list')
?.output?.data?.dependencies?.map((d) => d.name);
?.output?.data?.dependencies?.map((d) => {
d.name, d.version;
});
securityAdvisories = await getSecurityAdvisories(
taskInputs.githubAccessToken,
packageEcosystem,
dependencyNamesToUpdate || [],
dependenciesToCheckForSecurityAdvisories || [],
);

// Only update dependencies that have security advisories
dependencyNamesToUpdate = Array.from(new Set(securityAdvisories.map((a) => a['dependency-name'])));
}

// Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating
failedTasks += handleUpdateOperationResults(
await dependabot.update(
DependabotJobBuilder.newUpdateAllJob(
DependabotJobBuilder.updateAllDependenciesJob(
taskInputs,
updateId,
update,
Expand All @@ -148,7 +156,7 @@ async function run() {
for (const pullRequestId in existingPullRequests) {
failedTasks += handleUpdateOperationResults(
await dependabot.update(
DependabotJobBuilder.newUpdatePullRequestJob(
DependabotJobBuilder.updatePullRequestJob(
taskInputs,
pullRequestId,
update,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class DependabotJobBuilder {
* @param registries
* @returns
*/
public static newDiscoverDependencyListJob(
public static listAllDependenciesJob(
taskInputs: ISharedVariables,
id: string,
update: IDependabotUpdate,
Expand Down Expand Up @@ -49,7 +49,7 @@ export class DependabotJobBuilder {
* @param securityAdvisories
* @returns
*/
public static newUpdateAllJob(
public static updateAllDependenciesJob(
taskInputs: ISharedVariables,
id: string,
update: IDependabotUpdate,
Expand All @@ -67,7 +67,6 @@ export class DependabotJobBuilder {
registries,
false,
undefined,
// TODO: To make this more accurate, we should be evaluating the dependency name AND version against the security advisories, but that requires a lot of complex dependabot-core logic to do correctly
securityUpdatesOnly
? dependencyNamesToUpdate?.filter((d) => securityAdvisories?.find((a) => a['dependency-name'] == d))
: dependencyNamesToUpdate,
Expand All @@ -85,7 +84,7 @@ export class DependabotJobBuilder {
* @param pullRequestToUpdate
* @returns
*/
public static newUpdatePullRequestJob(
public static updatePullRequestJob(
taskInputs: ISharedVariables,
id: string,
update: IDependabotUpdate,
Expand Down
47 changes: 39 additions & 8 deletions extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export interface ISecurityAdvisory {
export async function getSecurityAdvisories(
accessToken: string,
packageEcosystem: string,
dependencyNames: string[],
dependencies: { name: string; version?: string }[],
): Promise<ISecurityAdvisory[]> {
const ecosystem = GITHUB_ECOSYSTEM[packageEcosystem];
const query = `
Expand All @@ -62,10 +62,11 @@ export async function getSecurityAdvisories(
`;

// GitHub API doesn't support querying multiple dependencies at once, so we need to make a request for each dependency individually.
// To speed up the process, we can make the requests in parallel. We batch the requests to avoid hitting the rate limit too quickly.
// To speed up the process, we can make the requests in parallel, 100 at a time. We batch the requests to avoid hitting the rate limit too quickly.
// https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api
console.log(`Fetching security advisories for ${dependencyNames.length} dependencies...`);
const securityAdvisories = await batch(100, dependencyNames, async (dependencyName) => {
console.log(`Checking security advisories for ${dependencies.length} dependencies...`);
const dependencyNames = dependencies.map((dependency) => dependency.name);
const securityAdvisories = await batchSecurityAdvisoryQuery(100, dependencyNames, async (dependencyName) => {
const variables = {
ecosystem: ecosystem,
package: dependencyName,
Expand Down Expand Up @@ -107,16 +108,46 @@ export async function getSecurityAdvisories(
});
});

console.log(`Found ${securityAdvisories.length} security advisories`);
return securityAdvisories;
// Filter out advisories that are not relevant the version of the dependency we are using
const affectedAdvisories = securityAdvisories.filter((advisory) => {
const dependency = dependencies.find((d) => d.name === advisory['dependency-name']);
if (!dependency) {
return false;
}
const isAffected =
advisory['affected-versions'].length > 0 &&
advisory['affected-versions'].find((range) => versionIsInRange(dependency.version, range));
const isPatched =
advisory['patched-versions'].length > 0 &&
advisory['patched-versions'].find((range) => versionIsInRange(dependency.version, range));
const isUnaffected =
advisory['unaffected-versions'].length > 0 &&
advisory['unaffected-versions'].find((range) => versionIsInRange(dependency.version, range));
return (isAffected && !isPatched) || isUnaffected;
});

const vulnerableDependencyCount = new Set(affectedAdvisories.map((advisory) => advisory['dependency-name'])).size;
console.log(
`Found ${vulnerableDependencyCount} vulnerable dependencies affected by ${affectedAdvisories.length} security advisories`,
);
return affectedAdvisories;
}

async function batch(batchSize: number, items: string[], callback: (item: string) => Promise<ISecurityAdvisory[]>) {
async function batchSecurityAdvisoryQuery(
batchSize: number,
items: string[],
action: (item: string) => Promise<ISecurityAdvisory[]>,
) {
const results: ISecurityAdvisory[] = [];
for (let i = 0; i < items.length; i += batchSize) {
const batch = items.slice(i, i + batchSize);
const batchResults = await Promise.all(batch.map(callback));
const batchResults = await Promise.all(batch.map(action));
results.push(...batchResults.flat());
}
return results;
}

function versionIsInRange(version: string, range: string): boolean {
// TODO: Parse the major/minor/patch version and do a proper comparison, taking in to considerations version ranges (e.g. ">=1.0.0")
return true;
}

0 comments on commit 5108b0d

Please sign in to comment.