Skip to content

Commit

Permalink
feat: return fixed issueIds
Browse files Browse the repository at this point in the history
  • Loading branch information
lili2311 committed Apr 16, 2021
1 parent 2c7d76f commit 7f9c9f6
Show file tree
Hide file tree
Showing 15 changed files with 163 additions and 131 deletions.
Expand Up @@ -29,11 +29,12 @@ export async function showResultsSummary(
resultsByPlugin,
exceptionsByScanType,
);
const fixedIssuesSummary = `${calculateFixedIssues(resultsByPlugin)} fixed issues`;
return `\n${successfulFixesSummary}${
unresolvedSummary ? `\n\n${unresolvedSummary}` : ''
}${
unresolvedCount || changedCount
? `\n\n${overallSummary}\n${vulnsSummary}`
? `\n\n${overallSummary}\n${vulnsSummary}\n${PADDING_SPACE}${fixedIssuesSummary}`
: ''
}`;
}
Expand Down Expand Up @@ -142,11 +143,17 @@ export function calculateFixed(
export function calculateFixedIssues(
resultsByPlugin: FixHandlerResultByPlugin,
): number {
let fixed = 0;
const fixedIssues: string[] = [];
for (const plugin of Object.keys(resultsByPlugin)) {
fixed += resultsByPlugin[plugin].succeeded.map((i) => i.changes).length;
for (const i of resultsByPlugin[plugin].succeeded) {
i.changes
.filter((c) => c.success)
.forEach((c) => {
fixedIssues.push(...c.issueIds);
});
}
}
return fixed;
return fixedIssues.length;
}

export function calculateFailed(
Expand Down
Expand Up @@ -110,6 +110,7 @@ async function fixAll(
{
success: true,
userMessage: `Fixed through ${fixedCache[filePath].fixedIn}`,
issueIds: fixedCache[filePath].issueIds,
},
],
});
Expand All @@ -120,9 +121,16 @@ async function fixAll(
debug('Manifest has not changed!');
throw new NoFixesCouldBeAppliedError();
}

// can't use .flat() or .flatMap() because it's not supported in Node 10
const issueIds: string[] = [];
for (const c of changes) {
issueIds.push(...c.issueIds);
}
Object.keys(fixedMeta).forEach((f) => {
fixedCache[f] = {
fixedIn: targetFile,
issueIds,
};
});
handlerResult.succeeded.push({ original: entity, changes });
Expand All @@ -144,9 +152,9 @@ export async function fixIndividualRequirementsTxt(
parsedRequirements: ParsedRequirements,
options: FixOptions,
directUpgradesOnly: boolean,
): Promise<{ changes: FixChangesSummary[]; appliedRemediation: string[] }> {
): Promise<{ changes: FixChangesSummary[] }> {
const fullFilePath = pathLib.normalize(pathLib.join(dir, fileName));
const { updatedManifest, changes, appliedRemediation } = updateDependencies(
const { updatedManifest, changes } = updateDependencies(
parsedRequirements,
remediation.pin,
directUpgradesOnly,
Expand All @@ -156,7 +164,7 @@ export async function fixIndividualRequirementsTxt(
);

if (!changes.length) {
return { changes, appliedRemediation };
return { changes };
}

if (!options.dryRun) {
Expand All @@ -166,7 +174,7 @@ export async function fixIndividualRequirementsTxt(
debug('Skipping writing changes to file in --dry-run mode');
}

return { changes, appliedRemediation };
return { changes };
}

export async function applyAllFixes(
Expand All @@ -189,7 +197,7 @@ export async function applyAllFixes(
/* Apply all upgrades first across all files that are included */
for (const fileName of Object.keys(provenance)) {
const skipApplyingPins = true;
const { changes, appliedRemediation } = await fixIndividualRequirementsTxt(
const { changes } = await fixIndividualRequirementsTxt(
workspace,
dir,
base,
Expand All @@ -199,15 +207,14 @@ export async function applyAllFixes(
options,
skipApplyingPins,
);
appliedUpgradeRemediation.push(...appliedRemediation);
upgradeChanges.push(...changes);
fixedMeta[pathLib.normalize(pathLib.join(dir, fileName))] = upgradeChanges;
}

/* Apply all left over remediation as pins in the entry targetFile */
const toPin: RemediationChanges = filterOutAppliedUpgrades(
remediation,
appliedUpgradeRemediation,
upgradeChanges,
);
const directUpgradesOnly = false;
const fileForPinning = await selectFileForPinning(entity);
Expand All @@ -227,19 +234,22 @@ export async function applyAllFixes(

function filterOutAppliedUpgrades(
remediation: RemediationChanges,
appliedRemediation: string[],
upgradeChanges: FixChangesSummary[],
): RemediationChanges {
const pinRemediation: RemediationChanges = {
...remediation,
pin: {}, // delete the pin remediation so we can collect un-applied remediation
};
const pins = remediation.pin;
const normalizedAppliedRemediation = appliedRemediation.map(
(packageAtVersion) => {
const [pkgName, versionAndMore] = packageAtVersion.split('@');
return `${standardizePackageName(pkgName)}@${versionAndMore}`;
},
);
const normalizedAppliedRemediation = upgradeChanges
.map((c) => {
if (c.success && c.from) {
const [pkgName, versionAndMore] = c.from?.split('@');
return `${standardizePackageName(pkgName)}@${versionAndMore}`;
}
return false;
})
.filter(Boolean);
for (const pkgAtVersion of Object.keys(pins)) {
const [pkgName, versionAndMore] = pkgAtVersion.split('@');
if (
Expand Down
Expand Up @@ -9,8 +9,8 @@ export function calculateRelevantFixes(
requirements: Requirement[],
updates: DependencyPins,
type: FixesType,
): { [upgradeFrom: string]: string } {
const lowerCasedUpdates: { [upgradeFrom: string]: string } = {};
): DependencyPins {
const lowerCasedUpdates = {};
const topLevelDeps = requirements.map(({ name }) => name).filter(isDefined);

Object.keys(updates).forEach((update) => {
Expand All @@ -21,9 +21,10 @@ export function calculateRelevantFixes(
if (type === 'transitive-pins' ? isTransitive : !isTransitive) {
const [name, newVersion] = upgradeTo.split('@');

lowerCasedUpdates[update] = `${standardizePackageName(
name,
)}@${newVersion}`;
lowerCasedUpdates[update] = {
...updates[update],
upgradeTo: `${standardizePackageName(name)}@${newVersion}`,
};
}
});
return lowerCasedUpdates;
Expand Down
Expand Up @@ -11,7 +11,6 @@ export function generatePins(
): {
pinnedRequirements: string[];
changes: FixChangesSummary[];
appliedRemediation: string[];
} {
// Lowercase the upgrades object. This might be overly defensive, given that
// we control this input internally, but its a low cost guard rail. Outputs a
Expand All @@ -26,34 +25,35 @@ export function generatePins(
return {
pinnedRequirements: [],
changes: [],
appliedRemediation: [],
};
}
const appliedRemediation: string[] = [];
const changes: FixChangesSummary[] = [];
const pinnedRequirements = Object.keys(standardizedPins)
.map((pkgNameAtVersion) => {
const [pkgName, version] = pkgNameAtVersion.split('@');
const newVersion = standardizedPins[pkgNameAtVersion].split('@')[1];
const newVersion = standardizedPins[pkgNameAtVersion].upgradeTo.split(
'@',
)[1];
const newRequirement = `${standardizePackageName(
pkgName,
)}>=${newVersion}`;
changes.push({
from: `${pkgName}@${version}`,
to: `${pkgName}@${newVersion}`,
issueIds: standardizedPins[pkgNameAtVersion].vulns,
success: true,
userMessage: `Pinned ${standardizePackageName(
pkgName,
)} from ${version} to ${newVersion}${
referenceFileInChanges ? ` (pinned in ${referenceFileInChanges})` : ''
}`,
});
appliedRemediation.push(pkgNameAtVersion);
return `${newRequirement} # not directly required, pinned by Snyk to avoid a vulnerability`;
})
.filter(isDefined);

return {
pinnedRequirements,
changes,
appliedRemediation,
};
}
Expand Up @@ -11,25 +11,22 @@ export function generateUpgrades(
): {
updatedRequirements: UpgradedRequirements;
changes: FixChangesSummary[];
appliedRemediation: string[];
} {
// Lowercase the upgrades object. This might be overly defensive, given that
// we control this input internally, but its a low cost guard rail. Outputs a
// mapping of upgrade to -> from, instead of the nested upgradeTo object.
const lowerCasedUpgrades = calculateRelevantFixes(
const normalizedUpgrades = calculateRelevantFixes(
requirements,
updates,
'direct-upgrades',
);
if (Object.keys(lowerCasedUpgrades).length === 0) {
if (Object.keys(normalizedUpgrades).length === 0) {
return {
updatedRequirements: {},
changes: [],
appliedRemediation: [],
};
}

const appliedRemediation: string[] = [];
const changes: FixChangesSummary[] = [];
const updatedRequirements = {};
requirements.map(
Expand All @@ -53,7 +50,7 @@ export function generateUpgrades(

// Check if we have an upgrade; if we do, replace the version string with
// the upgrade, but keep the rest of the content
const upgrade = Object.keys(lowerCasedUpgrades).filter(
const upgrade = Object.keys(normalizedUpgrades).filter(
(packageVersionUpgrade: string) => {
const [pkgName, versionAndMore] = packageVersionUpgrade.split('@');
return `${standardizePackageName(
Expand All @@ -67,9 +64,12 @@ export function generateUpgrades(
if (!upgrade) {
return;
}
const newVersion = lowerCasedUpgrades[upgrade].split('@')[1];
const newVersion = normalizedUpgrades[upgrade].upgradeTo.split('@')[1];
const updatedRequirement = `${originalName}${versionComparator}${newVersion}`;
changes.push({
issueIds: normalizedUpgrades[upgrade].vulns,
from: `${originalName}@${version}`,
to: `${originalName}@${newVersion}`,
success: true,
userMessage: `Upgraded ${originalName} from ${version} to ${newVersion}${
referenceFileInChanges
Expand All @@ -80,13 +80,11 @@ export function generateUpgrades(
updatedRequirements[originalText] = `${updatedRequirement}${
extras ? extras : ''
}`;
appliedRemediation.push(upgrade);
},
);

return {
updatedRequirements,
changes,
appliedRemediation,
};
}
Expand Up @@ -24,7 +24,6 @@ export function updateDependencies(
): {
updatedManifest: string;
changes: FixChangesSummary[];
appliedRemediation: string[];
} {
const {
requirements,
Expand All @@ -38,22 +37,21 @@ export function updateDependencies(
}
debug('Finished parsing manifest');

const {
updatedRequirements,
changes: upgradedChanges,
appliedRemediation,
} = generateUpgrades(requirements, updates, referenceFileInChanges);
const { updatedRequirements, changes: upgradedChanges } = generateUpgrades(
requirements,
updates,
referenceFileInChanges,
);
debug('Finished generating upgrades to apply');

let pinnedRequirements: string[] = [];
let pinChanges: FixChangesSummary[] = [];
let appliedPinsRemediation: string[] = [];
if (!directUpgradesOnly) {
({
pinnedRequirements,
changes: pinChanges,
appliedRemediation: appliedPinsRemediation,
} = generatePins(requirements, updates, referenceFileInChanges));
({ pinnedRequirements, changes: pinChanges } = generatePins(
requirements,
updates,
referenceFileInChanges,
));
debug('Finished generating pins to apply');
}

Expand All @@ -72,6 +70,5 @@ export function updateDependencies(
return {
updatedManifest,
changes: [...pinChanges, ...upgradedChanges],
appliedRemediation: [...appliedPinsRemediation, ...appliedRemediation],
};
}
1 change: 1 addition & 0 deletions packages/snyk-fix/src/plugins/types.ts
Expand Up @@ -24,5 +24,6 @@ export interface FixHandlerResultByPlugin {
export interface FixedCache {
[filePath: string]: {
fixedIn: string;
issueIds: string[];
};
}
6 changes: 6 additions & 0 deletions packages/snyk-fix/src/types.ts
Expand Up @@ -207,13 +207,19 @@ export type FixChangesSummary = FixChangesSuccess | FixChangesError;
interface FixChangesSuccess {
success: true;
userMessage: string;
issueIds: string[];
from?: string;
to?: string;
}

interface FixChangesError {
success: false;
userMessage: string;
reason: string;
tip?: string;
issueIds: string[];
from?: string;
to?: string;
}

export interface ErrorsByEcoSystem {
Expand Down
Expand Up @@ -18,7 +18,8 @@ Summary:
0 items were not fixed
2 items were successfully fixed
6 total issues: 2 High | 2 Medium | 2 Low
6 fixable issues"
6 fixable issues
8 fixed issues"
`;

exports[`fix *req*.txt / *.txt Python projects fixes multiple files via -r with the same name (some were already fixed) 1`] = `
Expand All @@ -41,7 +42,8 @@ Summary:
0 items were not fixed
3 items were successfully fixed
6 total issues: 3 High | 3 Medium
6 fixable issues"
6 fixable issues
9 fixed issues"
`;

exports[`fix *req*.txt / *.txt Python projects retains python markers 1`] = `
Expand Down

0 comments on commit 7f9c9f6

Please sign in to comment.