From 99933cc90a3be2532050f7b5894a25da40ef0617 Mon Sep 17 00:00:00 2001 From: Naugtur Date: Tue, 31 Jul 2018 22:38:49 +0200 Subject: [PATCH 1/8] initial notes for audit resolver RFC --- accepted/0003-interactive-audit-resolver.md | 58 +++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 accepted/0003-interactive-audit-resolver.md diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md new file mode 100644 index 000000000..248d3cf98 --- /dev/null +++ b/accepted/0003-interactive-audit-resolver.md @@ -0,0 +1,58 @@ +# Interactive audit resolver + +## Summary + +Add means for a human to resolve issues if they can't be fixed and interactively make decisions about each issue. +Remember the resolutions and allow them to affect `npm audit` behavior. +Decision options include 'ignore', 'remind later', 'fix', 'remove'. + + +## Motivation + +At times, running `npm audit fix` won't fix all audit issues. Fixes might not be available or the user might not want to apply some of them, eg. major version updates to test dependencies. + +It should be possible to make `npm audit` a step in a CI pipeline. Managing security of dependencies should be effective, easy to audit over time and secure in itself. + +For that, the user needs a new tool which makes addressing issues one by one convenient. + +## Detailed Explanation + +TODO: expand bullets below + +interactive CLI tool to speed up making and applying decisions +audit-resolv.json (or a section in package.json) to store the decisions +ability to postpone, ignore, remember fixed - all recorded for specific paths and issues, so the same issue found elsewhere would not get ignored +tools to help out when a fix is not yet known +resolutions and their changes are easy to track in git + +## Rationale and Alternatives + +{{Discuss 2-3 different alternative solutions that were considered. This is required, even if it seems like a stretch. Then explain why this is the best choice out of available ones.}} + +- nsp check used to support ignoring a particular issue in all dependencies, but that's not enough to be in control and remain secure +- ignoring dev dependencies is not enough. sometimes the developer knows a vulnerability can be ignored because a package with a DOS is used in tooling for the project, not production code. +- manually handling all those issues doesn't scale + +## Implementation + +reference implementation https://github.com/npm/cli/pull/10 + +prototype of a working tool (in production use) https://www.npmjs.com/package/npm-audit-resolver + +{{Give a high-level overview of implementaion requirements and concerns. Be specific about areas of code that need to change, and what their potential effects are. Discuss which repositories and sub-components will be affected, and what its overall code effect might be.}} + +{{THIS SECTION IS REQUIRED FOR RATIFICATION -- you can skip it if you don't know the technical details when first submitting the proposal, but it must be there before it's accepted}} + +## Prior Art + +{{This section is optional if there are no actual prior examples in other tools}} + +{{Discuss existing examples of this change in other tools, and how they've addressed various concerns discussed above, and what the effect of those decisions has been}} + +I recall a tool wrapping `nsp check` that gave me the idea to store exact paths of dependencies set to ignore (or any resolution) + +## Unresolved Questions and Bikeshedding + +{{Write about any arbitrary decisions that need to be made (syntax, colors, formatting, minor UX decisions), and any questions for the proposal that have not been answered.}} + +{{THIS SECTION SHOULD BE REMOVED BEFORE RATIFICATION}} From 8eba5d817df13637a344d6cb5580809d8240007e Mon Sep 17 00:00:00 2001 From: Naugtur Date: Sun, 5 Aug 2018 18:23:01 +0200 Subject: [PATCH 2/8] complete initial audit resolve writeup --- accepted/0003-interactive-audit-resolver.md | 82 +++++++++++++++------ 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index 248d3cf98..8890e7f27 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -2,36 +2,76 @@ ## Summary -Add means for a human to resolve issues if they can't be fixed and interactively make decisions about each issue. -Remember the resolutions and allow them to affect `npm audit` behavior. -Decision options include 'ignore', 'remind later', 'fix', 'remove'. +This proposal is for adding means for a human to resolve issues from `npm audit` and interactively make decisions about each issue. Available as `npm audit resolve` command. + +Features: +- Remember the resolutions and allow them to affect `npm audit` behavior. +- Decision options include 'ignore', 'remind later', 'fix', 'remove'. +- Allow tracking who resolved an issue and when using git history. ## Motivation -At times, running `npm audit fix` won't fix all audit issues. Fixes might not be available or the user might not want to apply some of them, eg. major version updates to test dependencies. +At times, running `npm audit fix` won't fix all audit issues. Fixes might not be available or the user might not want to apply some of them, eg. major version updates or build/test dependencies. -It should be possible to make `npm audit` a step in a CI pipeline. Managing security of dependencies should be effective, easy to audit over time and secure in itself. +It should be possible to make `npm audit` a step in a CI pipeline to go red every time a new vulnerability affects the project. Managing security of dependencies should be quick to update, effective, easy to audit over time and secure in itself. -For that, the user needs a new tool which makes addressing issues one by one convenient. +For that, the user needs a new tool which makes addressing issues one by one convenient even across a whole ecosystem of projects. ## Detailed Explanation -TODO: expand bullets below +`npm audit resolve` runs audit and iterates over all actions from the output with a prompt for a decision on each. +If fix is available, it's offered as one of the options to proceed. Other options include: ignore, remind later and delete. + +All decisions are stored in `audit-resolv.json` file as key-value, where key is `${dependency path}|${advisory id}` and value is a structure specific to the resolution. + +`npm audit` reads `audit-resolv.json` and respects the resolution (ignores ignored issues, ignores issues postponed to a date in the future) + +### resolutions +- **fix** runs the fix proposed in audit action and marks the issue as fixed in `audit-resolv.json`. On each subsequent run of `npm audit resolve` if the issue comes up again, the user is also warned that the problem was supposed to be fixed already. +- **ignore** marks the issue as ignored and `npm audit` will no longer fail because of it (but should display a single line warning about the issue being ignored). If a new vulnerability is found in the same dependency, it does not get ignored. If another dependency is installed with the same vulnerability advisory id it is not ignored. If the same package is installed as a dependency to another dependency (different path) it is not ignored. +- **postpone** marks the issue as postponed to a timestamp 24h in the future. Instead of ignoring an issue permanently just to make a build pass, one can postpone it when in rush, but make it show up as a CI faiure on the next working day. +- **remove** runs `npm rm` on the top level dependency containing the issue. It's a convenience option for the user to remove an old package which they no longer intend to use. -interactive CLI tool to speed up making and applying decisions -audit-resolv.json (or a section in package.json) to store the decisions -ability to postpone, ignore, remember fixed - all recorded for specific paths and issues, so the same issue found elsewhere would not get ignored -tools to help out when a fix is not yet known -resolutions and their changes are easy to track in git +(this RFC can't define the full scope of investigate) +- **investigate** when fix is not available, investigate option shows instead. It goes up through the dependencies chain and finds the one that needs their package.json updated with new version specification to enable a fix. +The result can be a call to action to create a PR (patch could be automatically generated) +Patch could also be applied locally or a specific change to package-lock.json could be proposed. + +**skip** and **abort** options should also be provided. ## Rationale and Alternatives -{{Discuss 2-3 different alternative solutions that were considered. This is required, even if it seems like a stretch. Then explain why this is the best choice out of available ones.}} +**Ignoring is necessary but must be under control at all times** +- `nsp check` used to support ignoring a particular issue in all dependencies, but that's not enough to be in control and remain secure. It wasn't uncommon for projects to ignore an advisory id because it was coming up as an issue in code that didn't run in production, thus risking it getting into the production code when updating another dependency to a version with the same issue. +- ignoring just dev dependencies is not enough. Sometimes the developer knows a vulnerability can be ignored because a package with a DOS is used in tooling for the project (not a dev dependency but eg. a migration script), not production code. To be really secure, ignoring must be explicit. +- Editing a file to specify what to ignore will not suffice any more at that level of detail, so automation is necessary. + +**Why audit-resolv.json?** +Resolutions must be recorded in a file and committed to git(or alternative) for edit history and full audit on who decided what. +Resolution format must be readable for a JavaScript developer. + +- `package-lock.json` is not a good option for storing resolutions because even if we overlook the fact that it'd be adding another purpose to a single-purpose file, the community at large is still used to removing and regenerating it often. +- `package.json` is not a good option, because it is already overloaded for so many functionalities. The output of resolutions could be lengthy and clutter the file. User will need the resolution information much less often than they look in the package.json file. ALso, resolution file is not meant to be edited by hand. +- `.npmrc` is not a good option because it doesn't live in the repository and being a dotfile is easy to miss, so a developer could overlook it affecting the audit. Also using it for the purpose of storing resolutions to project specific issues seems counter-intuitive. + +A separate file referred to as `audit-resolv.json` has the benefit of being single purpose, easy to track and audit, easy to version and migrate between versions of `npm` and comfortable for the users to review in git history and pull requests. -- nsp check used to support ignoring a particular issue in all dependencies, but that's not enough to be in control and remain secure -- ignoring dev dependencies is not enough. sometimes the developer knows a vulnerability can be ignored because a package with a DOS is used in tooling for the project, not production code. -- manually handling all those issues doesn't scale +**postpone, remove, investigate** +Other options are helpers for more elaborate actions a developer would take when confronted with an issue they can't or don't want to fix. + +Why is postpone useful at all? It's designed to build a secure development culture where one didn't yet form. Without it, a developer under time pressure would mark an issue as ignored with intention to un-mark it later. +While shipping with a known vulnerability is a bad practice, NPM's mission with the community should be to empower people to build more secure products and trust their skill and understanding of their project's particular needs. We should also aspire to help teams introduce more secure workflows effortlessly, so letting a build pass without risking compromising security long-term is a win. + +Remove is only useful as a convenience. Imagine a developer introducing `npm audit` and having to go through tens of issues. If they notice one of the first issues is caused by a dependency they no longer use, instead of remembering to clean it up later, they can choose this option. + +Investigate option has a lot of potential for the future where NPM could do things ranging from linking to resources on mitigation to generating local fixes or providing a marketplace of companies offering services fixing the issue for customers (business potential for NPM). + +### Alternatives + +Currently the only alternative is to manage the issues manually and keep track of the resolutions using conventions created by a team individually. Ignoring dev dependencies and certain level of severity will be provided for `npm audit` so it lowers the barrier of entry to using it in a CI system. + +Paid alternatives for managing security of a node.js project are available, including Snyk, with focus on providing patches to their customers. ## Implementation @@ -39,20 +79,14 @@ reference implementation https://github.com/npm/cli/pull/10 prototype of a working tool (in production use) https://www.npmjs.com/package/npm-audit-resolver -{{Give a high-level overview of implementaion requirements and concerns. Be specific about areas of code that need to change, and what their potential effects are. Discuss which repositories and sub-components will be affected, and what its overall code effect might be.}} +The implementation is, and should remain, runnable standalone as a separate package with minor wrapping code - useful for testing new features without bundling unfinished work with npm cli versions and therefore node.js -{{THIS SECTION IS REQUIRED FOR RATIFICATION -- you can skip it if you don't know the technical details when first submitting the proposal, but it must be there before it's accepted}} ## Prior Art -{{This section is optional if there are no actual prior examples in other tools}} - -{{Discuss existing examples of this change in other tools, and how they've addressed various concerns discussed above, and what the effect of those decisions has been}} - I recall a tool wrapping `nsp check` that gave me the idea to store exact paths of dependencies set to ignore (or any resolution) +Not aware of other prior art. Didn't find much in the npm packages ecosystem when researching it at the time of release of npm audit. ## Unresolved Questions and Bikeshedding -{{Write about any arbitrary decisions that need to be made (syntax, colors, formatting, minor UX decisions), and any questions for the proposal that have not been answered.}} -{{THIS SECTION SHOULD BE REMOVED BEFORE RATIFICATION}} From ef757fb0b9c0f470776d90f2116baebf17d1a015 Mon Sep 17 00:00:00 2001 From: Naugtur Date: Tue, 4 Dec 2018 23:27:38 +0100 Subject: [PATCH 3/8] document resolve file format --- accepted/0003-interactive-audit-resolver.md | 40 ++++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index 8890e7f27..7afcce82d 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -23,12 +23,12 @@ For that, the user needs a new tool which makes addressing issues one by one con `npm audit resolve` runs audit and iterates over all actions from the output with a prompt for a decision on each. If fix is available, it's offered as one of the options to proceed. Other options include: ignore, remind later and delete. -All decisions are stored in `audit-resolv.json` file as key-value, where key is `${dependency path}|${advisory id}` and value is a structure specific to the resolution. +All decisions are stored in `audit-resolve.json` file as key-value, where key is `${advisory id}|${dependency path}` and value is a structure specific to the resolution. -`npm audit` reads `audit-resolv.json` and respects the resolution (ignores ignored issues, ignores issues postponed to a date in the future) +`npm audit` reads `audit-resolve.json` and respects the resolution (ignores ignored issues, ignores issues postponed to a date in the future) ### resolutions -- **fix** runs the fix proposed in audit action and marks the issue as fixed in `audit-resolv.json`. On each subsequent run of `npm audit resolve` if the issue comes up again, the user is also warned that the problem was supposed to be fixed already. +- **fix** runs the fix proposed in audit action and marks the issue as fixed in `audit-resolve.json`. On each subsequent run of `npm audit resolve` if the issue comes up again, the user is also warned that the problem was supposed to be fixed already. - **ignore** marks the issue as ignored and `npm audit` will no longer fail because of it (but should display a single line warning about the issue being ignored). If a new vulnerability is found in the same dependency, it does not get ignored. If another dependency is installed with the same vulnerability advisory id it is not ignored. If the same package is installed as a dependency to another dependency (different path) it is not ignored. - **postpone** marks the issue as postponed to a timestamp 24h in the future. Instead of ignoring an issue permanently just to make a build pass, one can postpone it when in rush, but make it show up as a CI faiure on the next working day. - **remove** runs `npm rm` on the top level dependency containing the issue. It's a convenience option for the user to remove an old package which they no longer intend to use. @@ -47,7 +47,7 @@ Patch could also be applied locally or a specific change to package-lock.json co - ignoring just dev dependencies is not enough. Sometimes the developer knows a vulnerability can be ignored because a package with a DOS is used in tooling for the project (not a dev dependency but eg. a migration script), not production code. To be really secure, ignoring must be explicit. - Editing a file to specify what to ignore will not suffice any more at that level of detail, so automation is necessary. -**Why audit-resolv.json?** +**Why audit-resolve.json?** Resolutions must be recorded in a file and committed to git(or alternative) for edit history and full audit on who decided what. Resolution format must be readable for a JavaScript developer. @@ -55,7 +55,7 @@ Resolution format must be readable for a JavaScript developer. - `package.json` is not a good option, because it is already overloaded for so many functionalities. The output of resolutions could be lengthy and clutter the file. User will need the resolution information much less often than they look in the package.json file. ALso, resolution file is not meant to be edited by hand. - `.npmrc` is not a good option because it doesn't live in the repository and being a dotfile is easy to miss, so a developer could overlook it affecting the audit. Also using it for the purpose of storing resolutions to project specific issues seems counter-intuitive. -A separate file referred to as `audit-resolv.json` has the benefit of being single purpose, easy to track and audit, easy to version and migrate between versions of `npm` and comfortable for the users to review in git history and pull requests. +A separate file referred to as `audit-resolve.json` has the benefit of being single purpose, easy to track and audit, easy to version and migrate between versions of `npm` and comfortable for the users to review in git history and pull requests. **postpone, remove, investigate** Other options are helpers for more elaborate actions a developer would take when confronted with an issue they can't or don't want to fix. @@ -81,6 +81,36 @@ prototype of a working tool (in production use) https://www.npmjs.com/package/np The implementation is, and should remain, runnable standalone as a separate package with minor wrapping code - useful for testing new features without bundling unfinished work with npm cli versions and therefore node.js +### audit-resolve.json + +Audit resolution file format: +map key-value where key is the advisory number concatenated with package path + +```js +{ + "version": 1, + "ADVISORY_NUMBER|DEPENDENCY_PATH":{ + "resolution": "RESOLUTION_TYPE", + "reason": "Reason provided by the person making the decision (optional)", + "remindTime": timestamp + + } +} +``` + +example + +```js +{ + "717|spawn-shell>merge-options": { + "resolution":"remind", + "remindTime": 1542440172844 + }, + ... +} +``` + +*initially npm-audit-resolver used a different format* ## Prior Art From cf453ab373c336d4cdd332ad0bc78723da4b00ca Mon Sep 17 00:00:00 2001 From: Naugtur Date: Tue, 4 Dec 2018 23:38:21 +0100 Subject: [PATCH 4/8] clarify investigate option --- accepted/0003-interactive-audit-resolver.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index 7afcce82d..340a35eb5 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -35,7 +35,7 @@ All decisions are stored in `audit-resolve.json` file as key-value, where key is (this RFC can't define the full scope of investigate) - **investigate** when fix is not available, investigate option shows instead. It goes up through the dependencies chain and finds the one that needs their package.json updated with new version specification to enable a fix. -The result can be a call to action to create a PR (patch could be automatically generated) +The result can be a call to action to create a PR (patch to package.json could be automatically generated) Patch could also be applied locally or a specific change to package-lock.json could be proposed. **skip** and **abort** options should also be provided. @@ -65,7 +65,8 @@ While shipping with a known vulnerability is a bad practice, NPM's mission with Remove is only useful as a convenience. Imagine a developer introducing `npm audit` and having to go through tens of issues. If they notice one of the first issues is caused by a dependency they no longer use, instead of remembering to clean it up later, they can choose this option. -Investigate option has a lot of potential for the future where NPM could do things ranging from linking to resources on mitigation to generating local fixes or providing a marketplace of companies offering services fixing the issue for customers (business potential for NPM). +Investigate option is there to aid the user and direct them towards fixing the issue upstream. +This RFC doesn't cover potential future usecases where NPM could do things ranging from linking to resources on mitigation to generating local fixes or providing a marketplace of companies offering services fixing the issue for customers (business potential for NPM). ### Alternatives From 20bd7944a598153693dc6b8cef0cbc47b87447eb Mon Sep 17 00:00:00 2001 From: Naugtur Date: Tue, 26 Mar 2019 14:29:30 +0100 Subject: [PATCH 5/8] update audit-resolve format --- accepted/0003-interactive-audit-resolver.md | 64 +++++++++++++++++---- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index 340a35eb5..cc32b527b 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -23,7 +23,14 @@ For that, the user needs a new tool which makes addressing issues one by one con `npm audit resolve` runs audit and iterates over all actions from the output with a prompt for a decision on each. If fix is available, it's offered as one of the options to proceed. Other options include: ignore, remind later and delete. -All decisions are stored in `audit-resolve.json` file as key-value, where key is `${advisory id}|${dependency path}` and value is a structure specific to the resolution. +All decisions are stored in `audit-resolve.json` file as key-value, where key is `${advisory id}|${dependency path}` and value is: + +```js +{ + decision: "fix|ignore|postpone", + madeAt: +} +``` `npm audit` reads `audit-resolve.json` and respects the resolution (ignores ignored issues, ignores issues postponed to a date in the future) @@ -33,10 +40,8 @@ All decisions are stored in `audit-resolve.json` file as key-value, where key is - **postpone** marks the issue as postponed to a timestamp 24h in the future. Instead of ignoring an issue permanently just to make a build pass, one can postpone it when in rush, but make it show up as a CI faiure on the next working day. - **remove** runs `npm rm` on the top level dependency containing the issue. It's a convenience option for the user to remove an old package which they no longer intend to use. -(this RFC can't define the full scope of investigate) - **investigate** when fix is not available, investigate option shows instead. It goes up through the dependencies chain and finds the one that needs their package.json updated with new version specification to enable a fix. The result can be a call to action to create a PR (patch to package.json could be automatically generated) -Patch could also be applied locally or a specific change to package-lock.json could be proposed. **skip** and **abort** options should also be provided. @@ -90,24 +95,61 @@ map key-value where key is the advisory number concatenated with package path ```js { "version": 1, - "ADVISORY_NUMBER|DEPENDENCY_PATH":{ - "resolution": "RESOLUTION_TYPE", + "decisions": { + "ADVISORY_NUMBER|DEPENDENCY_PATH":{ + "decision": "RESOLUTION_TYPE", "reason": "Reason provided by the person making the decision (optional)", - "remindTime": timestamp - + "madeAt": timestamp + } } } ``` +`madeAt` should be generated by all tools using the file but is not mandatory for the sake of manually adding ignore decisions. + example ```js { - "717|spawn-shell>merge-options": { - "resolution":"remind", - "remindTime": 1542440172844 - }, + "version": 1, + "decisions": { + "717|spawn-shell>merge-options": { + "decision":"remind", + "madeAt": 1542440172844 + }, ... + } +} +``` + +JSON schema +```js +{ + properties: { + version: { + 'type': 'number', + 'minimum': 1 + }, + decisions: { + type: 'object', + additionalProperties: { + type: 'object', + required: ['decision'], + properties: { + decision: { + type: 'string', + enum: ['fix','ignore','postpone'] + }, + reason: { type: 'string' }, + madeAt: { type: 'number' } + } + } + } + }, + required: [ + 'version', + 'decisions' + ] } ``` From 8c7196b353cbb3d9d5356dc6c5f3a3cacf3c006b Mon Sep 17 00:00:00 2001 From: naugtur Date: Fri, 17 Jul 2020 10:39:10 +0200 Subject: [PATCH 6/8] Update RFC to cover the support for audit-resolve file only --- accepted/0003-interactive-audit-resolver.md | 96 +++++++++------------ 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index cc32b527b..a7dfcb667 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -1,49 +1,56 @@ -# Interactive audit resolver +# Audit resolutions ## Summary -This proposal is for adding means for a human to resolve issues from `npm audit` and interactively make decisions about each issue. Available as `npm audit resolve` command. +Historically this proposal was titled "Interactive audit resolver". That idea was too wide to introduce in one go and parts of it are better served in userland packages. This is now the core subset of the original proposal containing support for resolutions file. + +This proposal is for adding means for a human to resolve issues from `npm audit` by making and documenting decisions to ignore particular false positives. +The implementation should introduce support for reading and applying decisions from `audit-resolve.json` file Features: -- Remember the resolutions and allow them to affect `npm audit` behavior. -- Decision options include 'ignore', 'remind later', 'fix', 'remove'. +- Let users save tecisions about vulnerabilities and change `npm audit` behavior to accomodate that. +- Decision options include 'ignore', 'remind later', 'fix', 'none'. - Allow tracking who resolved an issue and when using git history. - +- Define a format to be used by userland packages when helping users make and store decisions. ## Motivation At times, running `npm audit fix` won't fix all audit issues. Fixes might not be available or the user might not want to apply some of them, eg. major version updates or build/test dependencies. -It should be possible to make `npm audit` a step in a CI pipeline to go red every time a new vulnerability affects the project. Managing security of dependencies should be quick to update, effective, easy to audit over time and secure in itself. +It should be possible to make `npm audit` a step in a CI pipeline to go red every time a new vulnerability affects the project. In cases when `npm audit` reports a vulnerability that is not affecting the project, user should be able to ignore it. + +examples: +- yargs-parser as a transitive dependency of an API gateway is not something that should break a CI run if vulnerable +- ReDoS vulnerability in a dependency of a commandline tool + +Managing security of dependencies should be quick to update, effective, easy to audit over time and secure in itself. -For that, the user needs a new tool which makes addressing issues one by one convenient even across a whole ecosystem of projects. ## Detailed Explanation -`npm audit resolve` runs audit and iterates over all actions from the output with a prompt for a decision on each. -If fix is available, it's offered as one of the options to proceed. Other options include: ignore, remind later and delete. +`npm audit` consumes decisions about what to ignore or warn about when an issue comes back that was already fixed. All decisions are stored in `audit-resolve.json` file as key-value, where key is `${advisory id}|${dependency path}` and value is: ```js { - decision: "fix|ignore|postpone", + decision: "fix|ignore|postpone|none", madeAt: + expiresAt: } ``` -`npm audit` reads `audit-resolve.json` and respects the resolution (ignores ignored issues, ignores issues postponed to a date in the future) +`npm audit` reads `audit-resolve.json` to get decisions -### resolutions -- **fix** runs the fix proposed in audit action and marks the issue as fixed in `audit-resolve.json`. On each subsequent run of `npm audit resolve` if the issue comes up again, the user is also warned that the problem was supposed to be fixed already. -- **ignore** marks the issue as ignored and `npm audit` will no longer fail because of it (but should display a single line warning about the issue being ignored). If a new vulnerability is found in the same dependency, it does not get ignored. If another dependency is installed with the same vulnerability advisory id it is not ignored. If the same package is installed as a dependency to another dependency (different path) it is not ignored. -- **postpone** marks the issue as postponed to a timestamp 24h in the future. Instead of ignoring an issue permanently just to make a build pass, one can postpone it when in rush, but make it show up as a CI faiure on the next working day. -- **remove** runs `npm rm` on the top level dependency containing the issue. It's a convenience option for the user to remove an old package which they no longer intend to use. +`audit-resolve.json` file could be created manually, but the expected UX of that file would be via a userland package that reads audit output, helps the user decide what to do and saves the decision. This tool will be referenced as *audit resolver* below. -- **investigate** when fix is not available, investigate option shows instead. It goes up through the dependencies chain and finds the one that needs their package.json updated with new version specification to enable a fix. -The result can be a call to action to create a PR (patch to package.json could be automatically generated) -**skip** and **abort** options should also be provided. + +### resolutions +- **fix** - a fix was applied and *audit resolver* marked issue as fixed in `audit-resolve.json`. On each subsequent run of `npm audit` if the issue comes up again, the user is also warned that the problem was supposed to be fixed already. +- **ignore** - *audit resolver* marks the issue as ignored and `npm audit` will no longer fail because of it (but should display a single line warning about the issue being ignored). If a new vulnerability is found in the same dependency, it does not get ignored. If another dependency is installed with the same vulnerability advisory id it is not ignored. If the same package is installed as a dependency to another dependency (different path) it is not ignored. +- **postpone** *audit resolver* marks the issue as postponed to a timestamp 24h in the future. Instead of ignoring an issue permanently just to make a build pass, one can postpone it when in rush, but make it show up as a CI faiure on the next working day. The option is separate to ignore for better visibility of the intention of a person in a rush. This is to build better security culture in a team. +- **none** an entry in decisions list was generated but the decision was not made or was explicitly cancelled in the future ## Rationale and Alternatives @@ -62,16 +69,13 @@ Resolution format must be readable for a JavaScript developer. A separate file referred to as `audit-resolve.json` has the benefit of being single purpose, easy to track and audit, easy to version and migrate between versions of `npm` and comfortable for the users to review in git history and pull requests. -**postpone, remove, investigate** +**postpone, remove** Other options are helpers for more elaborate actions a developer would take when confronted with an issue they can't or don't want to fix. Why is postpone useful at all? It's designed to build a secure development culture where one didn't yet form. Without it, a developer under time pressure would mark an issue as ignored with intention to un-mark it later. While shipping with a known vulnerability is a bad practice, NPM's mission with the community should be to empower people to build more secure products and trust their skill and understanding of their project's particular needs. We should also aspire to help teams introduce more secure workflows effortlessly, so letting a build pass without risking compromising security long-term is a win. -Remove is only useful as a convenience. Imagine a developer introducing `npm audit` and having to go through tens of issues. If they notice one of the first issues is caused by a dependency they no longer use, instead of remembering to clean it up later, they can choose this option. - -Investigate option is there to aid the user and direct them towards fixing the issue upstream. -This RFC doesn't cover potential future usecases where NPM could do things ranging from linking to resources on mitigation to generating local fixes or providing a marketplace of companies offering services fixing the issue for customers (business potential for NPM). +*audit-resolver* could perform more actions, like let the user remove a package entirely or help find a patch in the future. ### Alternatives @@ -81,12 +85,13 @@ Paid alternatives for managing security of a node.js project are available, incl ## Implementation -reference implementation https://github.com/npm/cli/pull/10 +prototype of a working *audit-resolver* (in production use) https://www.npmjs.com/package/npm-audit-resolver -prototype of a working tool (in production use) https://www.npmjs.com/package/npm-audit-resolver +Core capability covering reading, parsing, validating and representing the `audit-resolve.json` file was extracted from npm-audit-resolver into https://www.npmjs.com/package/audit-resolve-core - API of that package to be discussed. Functionality is there. The implementation is, and should remain, runnable standalone as a separate package with minor wrapping code - useful for testing new features without bundling unfinished work with npm cli versions and therefore node.js + ### audit-resolve.json Audit resolution file format: @@ -100,6 +105,7 @@ map key-value where key is the advisory number concatenated with package path "decision": "RESOLUTION_TYPE", "reason": "Reason provided by the person making the decision (optional)", "madeAt": timestamp + "expiresAt": timestamp } } } @@ -123,37 +129,13 @@ example ``` JSON schema -```js -{ - properties: { - version: { - 'type': 'number', - 'minimum': 1 - }, - decisions: { - type: 'object', - additionalProperties: { - type: 'object', - required: ['decision'], - properties: { - decision: { - type: 'string', - enum: ['fix','ignore','postpone'] - }, - reason: { type: 'string' }, - madeAt: { type: 'number' } - } - } - } - }, - required: [ - 'version', - 'decisions' - ] -} -``` -*initially npm-audit-resolver used a different format* +https://github.com/naugtur/audit-resolve-core/blob/master/auditFile/versions/v1.js + +Schema defines `version` and `decisions` fields. +The `rules` field is meant for the *audit-resolver* as instructions how to behave. + +*initially npm-audit-resolver used a different format, audit-resolve-core supports detecting old format and converting it* ## Prior Art @@ -162,4 +144,6 @@ Not aware of other prior art. Didn't find much in the npm packages ecosystem whe ## Unresolved Questions and Bikeshedding +- adding means to fill in the content of `audit-resolve.json` file to npm itself should be a matter of another RFC + From d43b5d9d07e67e5c46d1ccccffbede4480dbec9b Mon Sep 17 00:00:00 2001 From: naugtur Date: Fri, 17 Jul 2020 10:43:31 +0200 Subject: [PATCH 7/8] add explicit reference to implementation --- accepted/0003-interactive-audit-resolver.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index a7dfcb667..601589939 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -52,6 +52,10 @@ All decisions are stored in `audit-resolve.json` file as key-value, where key is - **postpone** *audit resolver* marks the issue as postponed to a timestamp 24h in the future. Instead of ignoring an issue permanently just to make a build pass, one can postpone it when in rush, but make it show up as a CI faiure on the next working day. The option is separate to ignore for better visibility of the intention of a person in a rush. This is to build better security culture in a team. - **none** an entry in decisions list was generated but the decision was not made or was explicitly cancelled in the future + +Proposed npm functionality is implemented in userland here: https://www.npmjs.com/package/npm-audit-resolver +The `check-audit` command from the above package is providing the exact functionality proposed here. + ## Rationale and Alternatives **Ignoring is necessary but must be under control at all times** From f333557af40beecf49d60d222599f02e5f0947fc Mon Sep 17 00:00:00 2001 From: naugtur Date: Fri, 17 Jul 2020 11:05:41 +0200 Subject: [PATCH 8/8] note about homan timestamps --- accepted/0003-interactive-audit-resolver.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/accepted/0003-interactive-audit-resolver.md b/accepted/0003-interactive-audit-resolver.md index 601589939..bb9be3ae5 100644 --- a/accepted/0003-interactive-audit-resolver.md +++ b/accepted/0003-interactive-audit-resolver.md @@ -40,6 +40,8 @@ All decisions are stored in `audit-resolve.json` file as key-value, where key is } ``` +For human-writeability the timestamp could support readable date fromats as well. + `npm audit` reads `audit-resolve.json` to get decisions `audit-resolve.json` file could be created manually, but the expected UX of that file would be via a userland package that reads audit output, helps the user decide what to do and saves the decision. This tool will be referenced as *audit resolver* below.