feat: (experimental) show pinning remediation advice for Python #755

kyegupov · 2019-08-29T08:59:34Z

Ready for review
Follows CONTRIBUTING rules
Reviewed by Snyk internal team

What does this PR do?

Shows Python remediation advice in snyk test, based on pinning.
Pinning means directly specifying the needed version, even for transitive dependencies.

This depends on the Registry changes: https://github.com/snyk/registry/pull/9434
The advice is calculated and shown differently, based on whether the actionableCliRemediation flag is set.

Screenshots:

With actionableCliRemediation:

Without actionableCliRemediation:

dkontorovskyy · 2019-08-29T09:11:30Z

src/lib/feature-flags.ts

@@ -0,0 +1,22 @@
+import request = require('./request');
+import snyk = require('.'); // TODO(kyegupov): fix import


maybe time had come ? :)

no :( that index.js file is absolutely insane.

dkontorovskyy · 2019-08-29T09:12:57Z

src/lib/snyk-test/run-test.ts

@@ -1,5 +1,5 @@
 import * as _ from 'lodash';
-import fs = require('then-fs');
+import * as fs from 'fs';


lili2311 · 2019-08-29T16:43:31Z

src/cli/commands/test/formatters/legacy-format-issue.ts

  let wizardHintText = '';
  if (WIZARD_SUPPORTED_PACKAGE_MANAGERS.includes(packageManager)) {
    wizardHintText = 'Run `snyk wizard` to explore remediation options.';
  }
-
+  if (pinningSupported
+    && (vuln.nearestFixedInVersion || vuln.fixedIn)


nearestFixedInVersion is docker specific I believe

Extracted the field into a Docker-specific type.

I mean surely we should not be checking it here: if (pinningSupported && vuln.fixedIn) is all we need.

yep, also done that :)

lili2311 · 2019-08-29T16:44:32Z

screenshots please

lili2311 · 2019-08-29T16:46:51Z

src/cli/commands/test/formatters/remediation-based-format-issues.ts

    };
  }
  const results = [chalk.bold.white('Remediation advice')];

-  const upgradeTextArray = constructUpgradesText(remediationInfo.upgrade, basicVulnInfo);


I feel like pinning can be it's own function that generates pinning? constructPinOrUpgradesText why not have construct pinning advice?

Is pinning not it's own section that is for packages that have no upgrade paths? It doesn't seem right to prefer pinning to upgrading? Is there some stuff to read on this feature somewhere to gain more context and what Python remediation will do?

With Python, we don't have full ecosystem information, but we can perform pinning.
Thus, we chose to do pinning insteat of upgrade paths.
So, it's either-or.

lili2311 · 2019-08-29T16:49:10Z

src/cli/commands/test/formatters/remediation-based-format-issues.ts

      .join('\n');
    upgradeTextArray.push(upgradeText + thisUpgradeFixes);
  }
  return upgradeTextArray;
 }

+function constructPinOrUpgradesText(


this is huge! Can it not be split up for Pin vs Upgrade, I feel like they are completely different?

Was split, common formatting code was extracted.

lili2311 · 2019-09-03T13:10:59Z

src/cli/commands/test/formatters/legacy-format-issue.ts

@@ -157,7 +170,7 @@ function createRemediationText(vuln, packageManager) {
        const selfUpgradeInfo = (v.upgradePath.length > 0)
          ? ` (triggers upgrades to ${ v.upgradePath.join(' > ')})`
          : '';
-        const testedPackageName = v.upgradePath[0].split('@');
+        const testedPackageName = parsePackageNameVersion(v.upgradePath[0] as string);


why the change?

Because we can have scoped packages. Splitting "@snyk/dep-graph@1.0.0" by "@" is incorrect. The "snyk-module" library, horrible as it is, should deal with that.

lili2311 · 2019-09-03T13:11:42Z

src/cli/commands/test/formatters/remediation-based-format-issues.ts

-  if (upgradeTextArray.length > 0) {
-    results.push(upgradeTextArray.join('\n'));
+  if (remediationInfo.pin && Object.keys(remediationInfo.pin).length) {
+    const upgradesByCulprit: UpgradesByCulprit = {};


upgradesByCulprit is there a nicer name for this? What is it trying to name? Is it transitive vs direct?

Ok, will rename to "byAffectedPackage".

lili2311 · 2019-09-03T13:12:22Z

src/cli/commands/test/formatters/remediation-based-format-issues.ts

-    results.push(upgradeTextArray.join('\n'));
+  if (remediationInfo.pin && Object.keys(remediationInfo.pin).length) {
+    const upgradesByCulprit: UpgradesByCulprit = {};
+    for (const topLvlPkg of Object.keys(remediationInfo.upgrade)) {


topLvlPkg topLevelPackage is nicer, let's avoid shortening as it is harder to read

decided on topLevelPkg. First, saves some characters, second, since package is a reserved word, shortening to pkg is a good habit anyway.

lili2311 · 2019-09-03T13:15:49Z

src/cli/commands/test/formatters/remediation-based-format-issues.ts

+      }
+    }
+    const upgradeTextArray = constructPinText(remediationInfo.pin, upgradesByCulprit, basicVulnInfo);
+    if (upgradeTextArray.length > 0) {


this is repeated twice, can just do it once after this whole if?

lili2311

💯

snyksec · 2019-09-05T21:22:25Z

🎉 This PR is included in version 1.226.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

kyegupov requested a review from a team as a code owner August 29, 2019 08:59

kyegupov self-assigned this Aug 29, 2019

ghost requested review from miiila and orsagie August 29, 2019 08:59

dkontorovskyy changed the title ~~feat: (experimental) shoiw pinning remediation advice for Python~~ feat: (experimental) show pinning remediation advice for Python Aug 29, 2019

dkontorovskyy reviewed Aug 29, 2019

View reviewed changes

kyegupov force-pushed the feat/python-show-pinning-advice branch from 1963934 to f9704b0 Compare August 29, 2019 09:11

dkontorovskyy reviewed Aug 29, 2019

View reviewed changes

kyegupov force-pushed the feat/python-show-pinning-advice branch from f9704b0 to b680181 Compare August 29, 2019 09:45

lili2311 reviewed Aug 29, 2019

View reviewed changes

kyegupov force-pushed the feat/python-show-pinning-advice branch 6 times, most recently from 95e250d to fee14ff Compare September 2, 2019 14:46

miiila removed their request for review September 3, 2019 12:16

lili2311 reviewed Sep 3, 2019

View reviewed changes

kyegupov force-pushed the feat/python-show-pinning-advice branch 2 times, most recently from 1e2befc to 8f441cf Compare September 3, 2019 15:52

lili2311 approved these changes Sep 3, 2019

View reviewed changes

kyegupov force-pushed the feat/python-show-pinning-advice branch 3 times, most recently from 3c0ff38 to faac0f6 Compare September 4, 2019 11:24

kyegupov force-pushed the feat/python-show-pinning-advice branch 3 times, most recently from 7afa20d to 6a69d1f Compare September 5, 2019 16:50

feat: (experimental) shoiw pinning remediation advice for Python

d66c57c

kyegupov force-pushed the feat/python-show-pinning-advice branch from 6a69d1f to d66c57c Compare September 5, 2019 17:54

kyegupov merged commit 2d093d4 into master Sep 5, 2019

kyegupov deleted the feat/python-show-pinning-advice branch September 5, 2019 21:04

snyksec added the released label Sep 5, 2019

snyk-bot mentioned this pull request Mar 9, 2022

[Snyk] Upgrade pino from 5.6.2 to 5.17.0 #2994

Closed

robin-qualityengineer mentioned this pull request May 23, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 robin-qualityengineer/snyk#161

Open

This was referenced May 23, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 aliscco/snyk#361

Open

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 cloudgeek7/snyk#230

Open

froscheimer mentioned this pull request May 24, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 froscheimer/snyk#105

Open

snyk-bot mentioned this pull request May 24, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 EitanGayor/monorepo_demo#416

Open

snyk-bot mentioned this pull request Jun 24, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 aliscco/snyk#413

Open

EitanGayor mentioned this pull request Jun 30, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 EitanGayor/monorepo_demo#471

Open

aliscco mentioned this pull request Aug 5, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 aliscco/snyk#473

Open

snyk-bot mentioned this pull request Aug 31, 2022

[Snyk] Upgrade pino from 5.6.2 to 5.17.0 #3770

Closed

snyk-bot mentioned this pull request Sep 24, 2022

[Snyk] Fix for 10 vulnerabilities aliscco/snyk#539

Open

This was referenced Sep 25, 2022

[Snyk] Fix for 10 vulnerabilities aliscco/snyk#573

Open

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 aliscco/snyk#625

Open

snyk-bot mentioned this pull request Oct 1, 2022

[Snyk] Fix for 10 vulnerabilities aliscco/snyk#646

Open

aliscco mentioned this pull request Oct 2, 2022

[Snyk] Fix for 10 vulnerabilities aliscco/snyk#663

Open

snyk-bot mentioned this pull request Oct 4, 2022

[Snyk-dev] Fix for 10 vulnerabilities #4067

Closed

snyk-bot mentioned this pull request Oct 29, 2022

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 aliscco/snyk#714

Open

snyk-bot mentioned this pull request Feb 19, 2023

[Snyk] Fix for 8 vulnerabilities cloudgeek7/snyk#558

Merged

EitanGayor mentioned this pull request Feb 20, 2023

[Snyk] Upgrade pino from 5.6.2 to 5.17.0 EitanGayor/monorepo_demo#671

Open

snyk-bot mentioned this pull request Feb 21, 2023

[Snyk] Security upgrade @google-cloud/profiler from 2.0.2 to 4.1.3 EitanGayor/monorepo_demo#734

Open

cloudgeek7 mentioned this pull request Mar 15, 2023

[Snyk] Fix for 9 vulnerabilities cloudgeek7/snyk#655

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: (experimental) show pinning remediation advice for Python #755

feat: (experimental) show pinning remediation advice for Python #755

kyegupov commented Aug 29, 2019 •

edited

Loading

dkontorovskyy Aug 29, 2019

kyegupov Aug 29, 2019

dkontorovskyy Aug 29, 2019

lili2311 Aug 29, 2019

kyegupov Sep 2, 2019

lili2311 Sep 2, 2019

kyegupov Sep 2, 2019

lili2311 commented Aug 29, 2019

lili2311 Aug 29, 2019

kyegupov Sep 2, 2019

lili2311 Aug 29, 2019

kyegupov Sep 2, 2019

lili2311 Sep 3, 2019

kyegupov Sep 3, 2019

lili2311 Sep 3, 2019

kyegupov Sep 3, 2019

lili2311 Sep 3, 2019

kyegupov Sep 3, 2019

lili2311 Sep 3, 2019

kyegupov Sep 3, 2019

lili2311 left a comment

snyksec commented Sep 5, 2019

		@@ -0,0 +1,22 @@
		import request = require('./request');
		import snyk = require('.'); // TODO(kyegupov): fix import

feat: (experimental) show pinning remediation advice for Python #755

feat: (experimental) show pinning remediation advice for Python #755

Conversation

kyegupov commented Aug 29, 2019 • edited Loading

What does this PR do?

Screenshots:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lili2311 commented Aug 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lili2311 left a comment

Choose a reason for hiding this comment

snyksec commented Sep 5, 2019

kyegupov commented Aug 29, 2019 •

edited

Loading