refactor(npm-db): update author field to be more verbose #314

pxlpnk · 2018-06-15T14:05:42Z

This splits up the author field into 3 fields

name
website
username

"author": {
    "name": "Cal Leeming", // required
    "website": null, // optional, can be null
    "username": null //  optional, can be null
}

The PR comes with a (tools/migrations/transform_author.js) script that changes the existing database.
You can check out the branch and run: node tools/migrations/transform_author.js). Then run npm test afterwards to check the migration.

The migrated files where not yet checked in to keep the PR less noisy. I will supply those changes as soon as the proposed changes are approved. The PR will be failing until those changes are applied.

I updated the files with id 109 and 113 as they had author set to null and replaced it with unknown.

This is the first PR to get #200 moving forward.

pxlpnk · 2018-06-15T15:16:33Z

tools/vuln_valid/index.js

+  author: joi.object().keys(
+    {
+      name: joi.string().required(),
+      username: joi.string().optional().allow(null),


Should username and website be required, but allowed to be null?
I kind of think this makes more sense.

It helps consistency through the reports format so that's ok with me.

pxlpnk · 2018-06-19T08:16:44Z

Rebased and migrated the files

vdeturckheim · 2018-06-19T16:10:47Z

vuln/npm/129.json

@@ -17,4 +21,4 @@
  "cvss_vector": "CVSS:3.0/AV:N/AC:L/PR:L/UI:R/S:U/C:H/I:H/A:N",
  "cvss_score": 7.3,
  "coordinating_vendor": "^Lift Security"
-}
+}


Let's keep the empty line

👍 Updated the PR

vdeturckheim · 2018-06-19T16:11:22Z

vuln/npm_old/98.json

+  "cvss_vector": "CVSS:3.0/AV:N/AC:H/PR:N/UI:R/S:U/C:H/I:H/A:N",
+  "cvss_score": 6.8,
+  "coordinating_vendor": "^Lift Security"
+}


What should we do with this directory?

/me fat-fingered here.

pxlpnk · 2018-06-20T05:38:43Z

I think this one and #316 should be in good shape. WDYTH?

lirantal · 2018-06-20T06:30:46Z

@pxlpnk sounds good but let me also prepare a PR to our automatic report creation script at #234 to accommodate for the changes.

I do also want to get feedback about both of these in the main issue before we merge.

lirantal · 2018-06-24T06:48:07Z

tools/vuln_valid/index.js

@@ -24,7 +24,14 @@ const npmModel = joi.object().keys({
  updated_at: joi.string().regex(/^\d{4}-\d{2}-\d{2}$/).required().isoDate(),
  title: joi.string().required(),
  title: joi.string().max(150).regex(/^[^\n]+$/).required(),
-  author: joi.string().allow(null).required(),
+  author: joi.object().required(),


the first author field is redundant, no?
same goes for the title field above

You where right, I removed the first author: joi.object().required(). As for the title field. I don't know what the intention with this was. But I can clean this up in a separate PR. I think there might be more in this file that could need a touch up.

pxlpnk · 2018-06-27T06:25:26Z

I can't really figure out what the problem is here with travis. @vdeturckheim Any idea?

I rebase, fixed some things and created a messy history.

This splits up the author field into 3 fields - name - website - username ``` "author": { "name": "Cal Leeming", "website": null, "username": null } ```

pxlpnk commented Jun 15, 2018

View reviewed changes

lirantal assigned pxlpnk Jun 15, 2018

lirantal requested review from vdeturckheim and dgonzalez June 15, 2018 21:39

vdeturckheim approved these changes Jun 19, 2018

View reviewed changes

dgonzalez approved these changes Jun 19, 2018

View reviewed changes

pxlpnk requested a review from a team as a code owner June 19, 2018 08:09

pxlpnk force-pushed the fix/npm-db-author branch from 5fbaee2 to f307a73 Compare June 19, 2018 08:13

vdeturckheim reviewed Jun 19, 2018

View reviewed changes

pxlpnk force-pushed the fix/npm-db-author branch from f307a73 to e126212 Compare June 20, 2018 05:31

lirantal requested changes Jun 24, 2018

View reviewed changes

lirantal approved these changes Jun 25, 2018

View reviewed changes

pxlpnk force-pushed the fix/npm-db-author branch from 3f7a144 to fb43ad5 Compare June 27, 2018 06:51

pxlpnk mentioned this pull request Jun 28, 2018

Suggestion for new vulnerability report format #200

Closed

8 tasks

pxlpnk mentioned this pull request Jul 13, 2018

[ANN]: refactor author field to be more verbose, Deadline: 27.07.2018 #345

Closed

pxlpnk added 2 commits August 1, 2018 14:19

refactor(npm-db): update author field to be more verbose

0194c94

This splits up the author field into 3 fields - name - website - username ``` "author": { "name": "Cal Leeming", "website": null, "username": null } ```

fix(npm-vuln): change author with null to "unknown"

dec584d

pxlpnk force-pushed the fix/npm-db-author branch from fb43ad5 to dec584d Compare August 1, 2018 07:20

pxlpnk added 2 commits August 1, 2018 14:20

Merge branch 'master' into fix/npm-db-author

9406835

refactor(npm-db): migrate author field to be more verbose

6a5c2cb

pxlpnk merged commit 9eacac1 into nodejs:master Aug 1, 2018

pxlpnk deleted the fix/npm-db-author branch August 1, 2018 07:28

lirantal mentioned this pull request Aug 3, 2018

feat(nswg-reporter): use rich user object for reports #361

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(npm-db): update author field to be more verbose #314

refactor(npm-db): update author field to be more verbose #314

pxlpnk commented Jun 15, 2018 •

edited

pxlpnk Jun 15, 2018

lirantal Jun 15, 2018

pxlpnk commented Jun 19, 2018

vdeturckheim Jun 19, 2018

pxlpnk Jun 20, 2018

vdeturckheim Jun 19, 2018

pxlpnk Jun 20, 2018

pxlpnk commented Jun 20, 2018

lirantal commented Jun 20, 2018

lirantal Jun 24, 2018

pxlpnk Jun 25, 2018

lirantal Jun 25, 2018

pxlpnk commented Jun 27, 2018 •

edited

refactor(npm-db): update author field to be more verbose #314

refactor(npm-db): update author field to be more verbose #314

Conversation

pxlpnk commented Jun 15, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pxlpnk commented Jun 19, 2018

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

pxlpnk commented Jun 20, 2018

lirantal commented Jun 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pxlpnk commented Jun 27, 2018 • edited

pxlpnk commented Jun 15, 2018 •

edited

pxlpnk commented Jun 27, 2018 •

edited