From 4b8f80c7304051abc125343bd91630267ab32e77 Mon Sep 17 00:00:00 2001 From: streamich Date: Wed, 26 May 2021 11:47:36 +0200 Subject: [PATCH 01/14] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20add=20"Risk?= =?UTF-8?q?=20Matrix"=20section=20to=20PR=20template?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/PULL_REQUEST_TEMPLATE.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 2a5fc914662b63..57b94b7c5c279a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,6 +2,7 @@ Summarize your PR. If it involves visual changes include a screenshot or gif. + ### Checklist Delete any items that are not applicable to this PR. @@ -15,6 +16,22 @@ Delete any items that are not applicable to this PR. - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) + +### Risk Matrix + +Before closing this PR, invite QA, stakeholders, and other developers to +identify risks that should be tested prior to the change/feature release. + +When forming the risk matrix, consider some of the following and how they may +potentially impact the change: + +| Risk | Probability | Severity | Mitigation/Notes | +|---------------------------|-------------|----------|-------------------------| +| Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | +| Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | +| Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | + + ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) From 0b57013a89e3397bf51d9ce8d9c792338315015b Mon Sep 17 00:00:00 2001 From: streamich Date: Wed, 26 May 2021 13:56:21 +0200 Subject: [PATCH 02/14] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20add=20risk?= =?UTF-8?q?=20examples?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/PULL_REQUEST_TEMPLATE.md | 7 ++-- RISK_MATRIX.md | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 RISK_MATRIX.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 57b94b7c5c279a..38bce4d51fbd5c 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -19,17 +19,20 @@ Delete any items that are not applicable to this PR. ### Risk Matrix +Delete this section if it is not applicable to this PR. + Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. -When forming the risk matrix, consider some of the following and how they may -potentially impact the change: +When forming the risk matrix, consider some of the following examples and how +they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | +| [See more potential risk examples](../RISK_MATRIX.md) | ### For maintainers diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md new file mode 100644 index 00000000000000..1a9602bd86c44e --- /dev/null +++ b/RISK_MATRIX.md @@ -0,0 +1,61 @@ +# Risk consideration + +When merging a new feature of considerable size or modifying an existing one, +consider adding a *Risk Matrix* section to your PR in collaboration with other +developers on your team and the QA team. + +Below are some general themes to consider for the *Risk Matrix*: + + +## General risks + +- What happens when your feature is used in a non-default space or a custom + space? +- What happens when Kibana is run in a cluster mode? +- What happens when plugin you depend on is disabled? +- What happens when feature you depend on is disabled? +- Is your change working correctly regardless of `kibana.yml` configuration or + UI Setting configuration? (For example, does it support both + `state:storeInSessionStorage` UI setting states?) +- What happens when a third party integration you depend on is not responding? +- How is authentication handled with third party services? +- Does the feature work in Elastic Cloud? +- Does the feautre create a setting that needs to be exposed, or configured + differently than the default, on the Elastic Cloud? +- Is there a significant performance impact that may affect Cloud Kibana + instances? +- Does your feature need to be aware of running in a container? +- Does the feature Work with security disabled, or fails gracefully? +- Are there performance risks associated with your feature? Does it access: + (1) many fields; (2) many indices; (3) lots of data; (4) lots of saved + objects; (5) large saved objects. +- Will leaving browser running overnight not have negative impacts ot the page + and server performance? +- Will your feature still work if Kibana is run behind a reverse proxy? +- Does your feature affect other plugins? +- If you write to the file system, what happens if Kibana node goes down? What + happens if Kibana is run in cluster mode? +- Are migrations handled gracefully? Does the feature affect old indices or + saved objects? +- Are you using any technologies, protocols, techniques, conventions, libraries, + NPM modules, etc. that may be new or unprecedented in Kibana? + + +## Security risks + +- XSS attacks—can user inject unescaped HTML on the page? (For example through + React's `dangerouslySetInnerHtml`, `Element.innerHTML`, `Element.outerHTML`). + Is all user input escaped? +- CSRF attacks—are you not using the default Kibana HTTP service to + communicate with the server? If not ensure you configure correctly CSRF header + and talk with security team to review. +- Remote code execution attacks—is your code doing something "highly" + dynamic? Such as: (1) usage of `eval` function; (2) usage of `vm` or + `child_process` Node.js modules; (3) usage of dynamic requires; (4) running + template interpolation such as Handlebars or Lodash `_.template`. +- [Prototype pollution attacks](https://docs.google.com/document/d/19V-d9sb6IF-fbzF4iyiPpAropQNydCnoJApzSX5FdcI/edit?usp=sharing). +- User input validation—are you validating user input beyond the default + HTTP server schema validation? Are you validating complex user input, such + as expression language or KQL? +- Check for accidental reveal of sensitive information. It could be printed to + console through logs or errors. From d8adb92ec06bb67aff733baa0a7fee3684ee22a3 Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:54:21 +0200 Subject: [PATCH 03/14] Update RISK_MATRIX.md Co-authored-by: Tim Sullivan --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index 1a9602bd86c44e..c46361e37d8db3 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -12,7 +12,7 @@ Below are some general themes to consider for the *Risk Matrix*: - What happens when your feature is used in a non-default space or a custom space? - What happens when Kibana is run in a cluster mode? -- What happens when plugin you depend on is disabled? +- What happens when a plugin you depend on is disabled? - What happens when feature you depend on is disabled? - Is your change working correctly regardless of `kibana.yml` configuration or UI Setting configuration? (For example, does it support both From 6ad0bc00e696436e9828b6b32814e7446b4383ae Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:54:31 +0200 Subject: [PATCH 04/14] Update RISK_MATRIX.md Co-authored-by: Tim Sullivan --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index c46361e37d8db3..c2fe18e7264e64 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -13,7 +13,7 @@ Below are some general themes to consider for the *Risk Matrix*: space? - What happens when Kibana is run in a cluster mode? - What happens when a plugin you depend on is disabled? -- What happens when feature you depend on is disabled? +- What happens when a feature you depend on is disabled? - Is your change working correctly regardless of `kibana.yml` configuration or UI Setting configuration? (For example, does it support both `state:storeInSessionStorage` UI setting states?) From 94eaa9f2f0de97fefb62ee369af96a48c0a03250 Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:55:01 +0200 Subject: [PATCH 05/14] Update RISK_MATRIX.md Co-authored-by: Tim Sullivan --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index c2fe18e7264e64..6a0e450c62ff52 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -20,7 +20,7 @@ Below are some general themes to consider for the *Risk Matrix*: - What happens when a third party integration you depend on is not responding? - How is authentication handled with third party services? - Does the feature work in Elastic Cloud? -- Does the feautre create a setting that needs to be exposed, or configured +- Does the feature create a setting that needs to be exposed, or configured differently than the default, on the Elastic Cloud? - Is there a significant performance impact that may affect Cloud Kibana instances? From c08ac21c64ec7c00884b3870d432bef3918e5b7f Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:56:05 +0200 Subject: [PATCH 06/14] Update RISK_MATRIX.md Co-authored-by: Tim Sullivan --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index 6a0e450c62ff52..9678c21859fbce 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -26,7 +26,7 @@ Below are some general themes to consider for the *Risk Matrix*: instances? - Does your feature need to be aware of running in a container? - Does the feature Work with security disabled, or fails gracefully? -- Are there performance risks associated with your feature? Does it access: +- Are there performance risks associated with your feature? Does it potentially access or create: (1) many fields; (2) many indices; (3) lots of data; (4) lots of saved objects; (5) large saved objects. - Will leaving browser running overnight not have negative impacts ot the page From 0ff8ca4367439ed118a2cde3a075db90b35d44a2 Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:56:49 +0200 Subject: [PATCH 07/14] Update RISK_MATRIX.md Co-authored-by: Brandon Kobel --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index 9678c21859fbce..a276aa5c81f4e4 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -11,7 +11,7 @@ Below are some general themes to consider for the *Risk Matrix*: - What happens when your feature is used in a non-default space or a custom space? -- What happens when Kibana is run in a cluster mode? +- What happens when there are multiple Kibana nodes using the same Elasticsearch cluster? - What happens when a plugin you depend on is disabled? - What happens when a feature you depend on is disabled? - Is your change working correctly regardless of `kibana.yml` configuration or From f70d1a1613b15d992b69ab1826469b4e61ee5453 Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:57:15 +0200 Subject: [PATCH 08/14] Update RISK_MATRIX.md Co-authored-by: Brandon Kobel --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index a276aa5c81f4e4..12232c3e213950 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -29,7 +29,7 @@ Below are some general themes to consider for the *Risk Matrix*: - Are there performance risks associated with your feature? Does it potentially access or create: (1) many fields; (2) many indices; (3) lots of data; (4) lots of saved objects; (5) large saved objects. -- Will leaving browser running overnight not have negative impacts ot the page +- Will leaving the browser running overnight have negative impacts on the page and server performance? - Will your feature still work if Kibana is run behind a reverse proxy? - Does your feature affect other plugins? From 9be012301312d5c02c7f2197ed505bba4eb952c7 Mon Sep 17 00:00:00 2001 From: Vadim Dalecky Date: Thu, 27 May 2021 14:57:36 +0200 Subject: [PATCH 09/14] Update RISK_MATRIX.md Co-authored-by: Brandon Kobel --- RISK_MATRIX.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.md b/RISK_MATRIX.md index 12232c3e213950..77235a5d9d6339 100644 --- a/RISK_MATRIX.md +++ b/RISK_MATRIX.md @@ -34,7 +34,7 @@ Below are some general themes to consider for the *Risk Matrix*: - Will your feature still work if Kibana is run behind a reverse proxy? - Does your feature affect other plugins? - If you write to the file system, what happens if Kibana node goes down? What - happens if Kibana is run in cluster mode? + happens if there are multiple Kibana nodes? - Are migrations handled gracefully? Does the feature affect old indices or saved objects? - Are you using any technologies, protocols, techniques, conventions, libraries, From 1d7714c8b9b248fb3675ea7297b652fcd9448b25 Mon Sep 17 00:00:00 2001 From: streamich Date: Tue, 1 Jun 2021 15:36:50 +0200 Subject: [PATCH 10/14] =?UTF-8?q?chore:=20=F0=9F=A4=96=20make=20risk=20mat?= =?UTF-8?q?rix=20.mdx?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- RISK_MATRIX.md => RISK_MATRIX.mdx | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename RISK_MATRIX.md => RISK_MATRIX.mdx (100%) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 38bce4d51fbd5c..336f7e5165d07f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -32,7 +32,7 @@ they may potentially impact the change: | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | -| [See more potential risk examples](../RISK_MATRIX.md) | +| [See more potential risk examples](../RISK_MATRIX.mdx) | ### For maintainers diff --git a/RISK_MATRIX.md b/RISK_MATRIX.mdx similarity index 100% rename from RISK_MATRIX.md rename to RISK_MATRIX.mdx From 8abdf3dc4728d5868e05af255a0857d08cdc65eb Mon Sep 17 00:00:00 2001 From: streamich Date: Tue, 1 Jun 2021 15:39:13 +0200 Subject: [PATCH 11/14] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20improve=20me?= =?UTF-8?q?mory=20leak=20bullet=20point?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- RISK_MATRIX.mdx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/RISK_MATRIX.mdx b/RISK_MATRIX.mdx index 77235a5d9d6339..135aad1b9f6ec1 100644 --- a/RISK_MATRIX.mdx +++ b/RISK_MATRIX.mdx @@ -29,8 +29,7 @@ Below are some general themes to consider for the *Risk Matrix*: - Are there performance risks associated with your feature? Does it potentially access or create: (1) many fields; (2) many indices; (3) lots of data; (4) lots of saved objects; (5) large saved objects. -- Will leaving the browser running overnight have negative impacts on the page - and server performance? +- Could this cause memory to leak in either the browser or server? - Will your feature still work if Kibana is run behind a reverse proxy? - Does your feature affect other plugins? - If you write to the file system, what happens if Kibana node goes down? What From 7c03ef79ba38c62bab631cb4e0711a527d235496 Mon Sep 17 00:00:00 2001 From: streamich Date: Tue, 1 Jun 2021 15:43:07 +0200 Subject: [PATCH 12/14] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20improve=20"S?= =?UTF-8?q?ecurity=20Risks"=20section=20as=20per=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- RISK_MATRIX.mdx | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/RISK_MATRIX.mdx b/RISK_MATRIX.mdx index 135aad1b9f6ec1..39938c825879b0 100644 --- a/RISK_MATRIX.mdx +++ b/RISK_MATRIX.mdx @@ -11,7 +11,8 @@ Below are some general themes to consider for the *Risk Matrix*: - What happens when your feature is used in a non-default space or a custom space? -- What happens when there are multiple Kibana nodes using the same Elasticsearch cluster? +- What happens when there are multiple Kibana nodes using the same Elasticsearch + cluster? - What happens when a plugin you depend on is disabled? - What happens when a feature you depend on is disabled? - Is your change working correctly regardless of `kibana.yml` configuration or @@ -26,9 +27,9 @@ Below are some general themes to consider for the *Risk Matrix*: instances? - Does your feature need to be aware of running in a container? - Does the feature Work with security disabled, or fails gracefully? -- Are there performance risks associated with your feature? Does it potentially access or create: - (1) many fields; (2) many indices; (3) lots of data; (4) lots of saved - objects; (5) large saved objects. +- Are there performance risks associated with your feature? Does it potentially + access or create: (1) many fields; (2) many indices; (3) lots of data; + (4) lots of saved objects; (5) large saved objects. - Could this cause memory to leak in either the browser or server? - Will your feature still work if Kibana is run behind a reverse proxy? - Does your feature affect other plugins? @@ -42,19 +43,20 @@ Below are some general themes to consider for the *Risk Matrix*: ## Security risks -- XSS attacks—can user inject unescaped HTML on the page? (For example through - React's `dangerouslySetInnerHtml`, `Element.innerHTML`, `Element.outerHTML`). - Is all user input escaped? -- CSRF attacks—are you not using the default Kibana HTTP service to - communicate with the server? If not ensure you configure correctly CSRF header - and talk with security team to review. -- Remote code execution attacks—is your code doing something "highly" - dynamic? Such as: (1) usage of `eval` function; (2) usage of `vm` or - `child_process` Node.js modules; (3) usage of dynamic requires; (4) running - template interpolation such as Handlebars or Lodash `_.template`. -- [Prototype pollution attacks](https://docs.google.com/document/d/19V-d9sb6IF-fbzF4iyiPpAropQNydCnoJApzSX5FdcI/edit?usp=sharing). -- User input validation—are you validating user input beyond the default - HTTP server schema validation? Are you validating complex user input, such - as expression language or KQL? -- Check for accidental reveal of sensitive information. It could be printed to - console through logs or errors. +Check to ensure that best practices are used to mitigate common vulnerabilities: + +- Cross-site scripting (XSS) +- Cross-site request forgery (CSRF) +- Remote-code execution (RCE) +- Server-side request forgery (SSRF) +- Prototype pollution +- Information disclosure +- Tabnabbing + +In addition to these risks, in general, server-side input validation should be +implemented as strictly as possible. Extra care should be taken when user input +is used to construct URLs or data structures; this is a common source of +injection attacks and other vulnerabilities. For more information on all of +these topics, see [Security best practices][security-best-practices]. + +[security-best-practices]: (https://www.elastic.co/guide/en/kibana/master/security-best-practices.html#security-best-practices). From 37ab7c58166882e0759f40fa730584dd2f29b0f0 Mon Sep 17 00:00:00 2001 From: streamich Date: Tue, 1 Jun 2021 15:43:59 +0200 Subject: [PATCH 13/14] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20encourage=20?= =?UTF-8?q?to=20add=20to=20the=20risk=20list?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- RISK_MATRIX.mdx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/RISK_MATRIX.mdx b/RISK_MATRIX.mdx index 39938c825879b0..ef7d83c27e3b73 100644 --- a/RISK_MATRIX.mdx +++ b/RISK_MATRIX.mdx @@ -4,7 +4,8 @@ When merging a new feature of considerable size or modifying an existing one, consider adding a *Risk Matrix* section to your PR in collaboration with other developers on your team and the QA team. -Below are some general themes to consider for the *Risk Matrix*: +Below are some general themes to consider for the *Risk Matrix*. (Feel free to +add to this list.) ## General risks From cf9ce73578b603318fa8f67171e620a91339cadd Mon Sep 17 00:00:00 2001 From: streamich Date: Wed, 2 Jun 2021 10:55:31 +0200 Subject: [PATCH 14/14] =?UTF-8?q?docs:=20=E2=9C=8F=EF=B8=8F=20fix=20link?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- RISK_MATRIX.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RISK_MATRIX.mdx b/RISK_MATRIX.mdx index ef7d83c27e3b73..71b0198489934a 100644 --- a/RISK_MATRIX.mdx +++ b/RISK_MATRIX.mdx @@ -60,4 +60,4 @@ is used to construct URLs or data structures; this is a common source of injection attacks and other vulnerabilities. For more information on all of these topics, see [Security best practices][security-best-practices]. -[security-best-practices]: (https://www.elastic.co/guide/en/kibana/master/security-best-practices.html#security-best-practices). +[security-best-practices]: https://www.elastic.co/guide/en/kibana/master/security-best-practices.html