Hey @Sgs -

Thanks for the request. We can probably accommodate this review this quarter (which begins next Monday) though not on the timeline you've stated (2 - 3 weeks after a beta deployment). Per our SOP, we schedule application security reviews at the beginning of each quarter to complete during a given quarter. We do not block on beta deployments but would ask that a manager/director accept at least a medium risk for the beta deployment prior to the completion of an application security review.

In T318854#8274199, @sbassett wrote:

Hey @Sgs -

Thanks for the request. We can probably accommodate this review this quarter (which begins next Monday) though not on the timeline you've stated (2 - 3 weeks after a beta deployment). Per our SOP, we schedule application security reviews at the beginning of each quarter to complete during a given quarter. We do not block on beta deployments but would ask that a manager/director accept at least a medium risk for the beta deployment prior to the completion of an application security review.

Alright, thanks for the anwser. cc'ing @DMburugu @kostajh

In T318854#8274199, @sbassett wrote:

Hey @Sgs -

Thanks for the request. We can probably accommodate this review this quarter (which begins next Monday) though not on the timeline you've stated (2 - 3 weeks after a beta deployment). Per our SOP, we schedule application security reviews at the beginning of each quarter to complete during a given quarter. We do not block on beta deployments but would ask that a manager/director accept at least a medium risk for the beta deployment prior to the completion of an application security review.

@sbassett thanks for your comment. Given that d3.js is already in production with Graph (and has been since 2016), is it acceptable to the security team for us to ship code which uses d3 in production, and not just in beta, assuming that @DMburugu can sign off on this from a risk perspective?

@sbassett I'm okay with a medium risk assessment on beta deployments using this library, given that we want the feature to go live in the next weeks.

@Reedy We're trying to restrict d3 to beta cluster with some kind of toggling, so far only hiding the UI (gerrit 836842). Do you have any suggestion on how to do this so we don't load d3.js in production?

In T318854#8321165, @Sgs wrote:

@Reedy We're trying to restrict d3 to beta cluster with some kind of toggling, so far only hiding the UI (gerrit 836842). Do you have any suggestion on how to do this so we don't load d3.js in production?

AIUI, since d3.js is already in production with the Graph extension, then loading itself as part of ResourceLoader should not be problematic. What we could do is avoid calling methods on the library in our code, using some feature flagging, until security review is finished.

In T318854#8321253, @kostajh wrote:

AIUI, since d3.js is already in production with the Graph extension, then loading itself as part of ResourceLoader should not be problematic. What we could do is avoid calling methods on the library in our code, using some feature flagging, until security review is finished.

Yes and no. I'm not seeing any recent, formal security review for d3js, so we'd certainly need to perform some kind of review, especially if we're looking at d3js's main branch, as this request seems to possibly imply? That being said, we can't perform a line-by-line audit of d3js - that just isn't feasible for our team. So what would likely be performed is a vendor review with some additional layers of high-level pentesting and similar assessments. Some recent examples of these types of reviews can be found at T288768#7551991 and T262963#6668136.

Yes and no. I'm not seeing any recent, formal security review for d3js, so we'd certainly need to perform some kind of review, especially if we're looking at d3js's main branch, as this request seems to possibly imply? That being said, we can't perform a line-by-line audit of d3js - that just isn't feasible for our team. So what would likely be performed is a vendor review with some additional layers of high-level pentesting and similar assessments. Some recent examples of these types of reviews can be found at T288768#7551991 and T262963#6668136.

@sbassett Would it be possible to know where we are in the review Queue given the discovery that there was no formal security review for d3js ? We'd like to launch this in the next couple of weeks(before Mid December) and want to pace our expectations.

In T318854#8322187, @DMburugu wrote:

@sbassett Would it be possible to know where we are in the review Queue given the discovery that there was no formal security review for d3js ? We'd like to launch this in the next couple of weeks(before Mid December) and want to pace our expectations.

This review has been assigned to @Reedy to complete this quarter. If you would like to launch this sooner than the review is completed, a high risk can be accepted and added our risk register. Per our current risk management framework, a high risk can be accepted by a c-level.

Change 850098 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[operations/mediawiki-config@master] [labs] GrowthExperiments: Use d3.js with new impact module

https://gerrit.wikimedia.org/r/850098

Change 850098 merged by jenkins-bot:

[operations/mediawiki-config@master] [labs] GrowthExperiments: Use d3.js with new impact module

https://gerrit.wikimedia.org/r/850098

Hey @Reedy, is it possible to get an estimate on which week in the quarter the review will be done?

In T318854#8376406, @DMburugu wrote:

Hey @Reedy, is it possible to get an estimate on which week in the quarter the review will be done?

Note that T321215: Use d3 subpackages instead of the full bundle reworked our code to use a subset of d3 packages. It might be easier to review that patch which adds the relevant d3 packages to devDependencies in the GrowthExperiments package.json.

Per Slack conversation with @DMburugu - I've added a medium risk to our application security risk register for the early (pre security review) production deployment of d3js this Monday, November 28th.

(though technically d3js is already within Wikimedia production for a separate use-case)

In T318854#8411246, @sbassett wrote:

Per Slack conversation with @DMburugu - I've added a medium risk to our application security risk register for the early (pre security review) production deployment of d3js this Monday, November 28th.

(though technically d3js is already within Wikimedia production for a separate use-case)

Thanks @sbassett. If I understand correctly, that means we are OK to go ahead with using d3.js in production as early as November 28th. (Realistically, it is more likely to be December 5 when we start putting this in front of users.)

Change 860841 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] NewImpact: Remove d3 feature-flag

https://gerrit.wikimedia.org/r/860841

Change 860918 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] NewImpact: unskip NewImpact vue test

https://gerrit.wikimedia.org/r/860918

Change 860918 abandoned by Kosta Harlan:

[mediawiki/extensions/GrowthExperiments@master] NewImpact: unskip NewImpact vue test

Reason:

Squashed, added Sergio as co-author on parent patch. Thanks!

https://gerrit.wikimedia.org/r/860918

Change 860841 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] NewImpact: Remove d3 feature-flag

https://gerrit.wikimedia.org/r/860841

In T318854#8421099, @kostajh wrote:

Thanks @sbassett. If I understand correctly, that means we are OK to go ahead with using d3.js in production as early as November 28th. (Realistically, it is more likely to be December 5 when we start putting this in front of users.)

Yes, with the understanding that your team and @DMburugu own the medium risk for that deployment,

In T318854#8425799, @sbassett wrote:

In T318854#8421099, @kostajh wrote:

Thanks @sbassett. If I understand correctly, that means we are OK to go ahead with using d3.js in production as early as November 28th. (Realistically, it is more likely to be December 5 when we start putting this in front of users.)

Yes, with the understanding that your team and @DMburugu own the medium risk for that deployment,

Thanks. What does owning that risk mean, exactly?

Based on https://office.wikimedia.org/wiki/Security/Policy/Risk_Management, are we now supposed to create a risk treatment plan? (And if so, what should that contain -- are there examples?)

In T318854#8425952, @kostajh wrote:

Thanks. What does owning that risk mean, exactly?

In general, it means that the decision to put d3js into production in this fashion (prior to any security review) is a decision completely owned by your team and @DMburugu. Including any potential negative consequences.

Based on https://office.wikimedia.org/wiki/Security/Policy/Risk_Management, are we now supposed to create a risk treatment plan? (And if so, what should that contain -- are there examples?)

That's definitely the best practice - to create a risk mitigation plan. Though for situations like this that doesn't always make sense. Theoretically, the risk mitigation plan would be having a security review performed and mitigating any resultant risk prior to deployment. But given timelines and resource constraints that isn't really an option in this case, and so a default medium risk has been applied and accepted.

Change 861466 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] Revert "NewImpact: Remove d3 feature-flag"

https://gerrit.wikimedia.org/r/861466

Change 861466 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Revert "NewImpact: Remove d3 feature-flag"

https://gerrit.wikimedia.org/r/861466

Change 861506 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] [no-op] GrowthExperiments: Enable D3 in production

https://gerrit.wikimedia.org/r/861506

In T318854#8426250, @sbassett wrote:

In T318854#8425952, @kostajh wrote:

Thanks. What does owning that risk mean, exactly?

In general, it means that the decision to put d3js into production in this fashion (prior to any security review) is a decision completely owned by your team and @DMburugu. Including any potential negative consequences.

Based on https://office.wikimedia.org/wiki/Security/Policy/Risk_Management, are we now supposed to create a risk treatment plan? (And if so, what should that contain -- are there examples?)

That's definitely the best practice - to create a risk mitigation plan. Though for situations like this that doesn't always make sense. Theoretically, the risk mitigation plan would be having a security review performed and mitigating any resultant risk prior to deployment. But given timelines and resource constraints that isn't really an option in this case, and so a default medium risk has been applied and accepted.

We retained the code (T318854#8426821) that would allow us to turn of d3 integration with a config switch (set GENewImpactD3Enabled to false in InitialiseSettings.php), in case the security review comes up with something concerning.

We look forward to seeing the security review. Thanks!

Change 861506 merged by jenkins-bot:

[operations/mediawiki-config@master] [no-op] GrowthExperiments: Enable D3 in production

https://gerrit.wikimedia.org/r/861506

Mentioned in SAL (#wikimedia-operations) [2022-12-01T14:30:13Z] <kharlan@deploy1002> Started scap: Backport for [[gerrit:861506|[no-op] GrowthExperiments: Enable D3 in production (T318854)]]

Mentioned in SAL (#wikimedia-operations) [2022-12-01T14:31:19Z] <kharlan@deploy1002> kharlan and tgr: Backport for [[gerrit:861506|[no-op] GrowthExperiments: Enable D3 in production (T318854)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2022-12-01T14:36:17Z] <kharlan@deploy1002> Finished scap: Backport for [[gerrit:861506|[no-op] GrowthExperiments: Enable D3 in production (T318854)]] (duration: 06m 04s)

Security Review Summary - T318854 - 2023-01-03

Overall, the current vendor code under consideration (specifically the sub components as defined here) appear to be in good shape per various security analyses performed below. These components of the d3js package can be considered to have an overall risk rating of low.

General Security Information

Vulnerable Packages
none

Outdated Packages
As reported via npm outdated:
(no explicit vulnerabilities reported, simply noting for completeness' sake.)

Scorecard score
4.2 / 10 medium risk
(see raw output: P42747)

Potential HTTP and protocol Leaks
none

Static Analysis Findings

1. lockfile-lint returned no results. low risk
2. gitleaks returned no results. low risk
3. whispers returned no results. low risk
4. scan scan:latest returned no results. low risk
5. semgrep 1.2.1 was run with several popular and custom policies and rule-sets. Some lightly-curated results are found within P42748. All of these appear to be "potential" ReDoS issues, but none appear to be legitimate vulnerabilities. low risk

Change 877206 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] NewImpact: Remove d3 feature-flag

https://gerrit.wikimedia.org/r/877206