Hey @dmaza, @TheresNoTime et al -

Just to set expectations here:

1. Per our Application Security Review SOP, we typically determine the reviews the Security-Team will complete at the beginning of each quarter which, this time around, happened the first week of July 2022. We typically do not accept new reviews for completion until the following quarter.
2. Has the GCP dependency been brought to the attention of WMF-Legal, Privacy Engineering and/or SRE? There is precedent for such external dependencies - see CX MT - but I believe there were many considerations and agreements which were put into place, principally to manage user privacy concerns.

Are we okay to T314294: Deploy Phonos to beta cluster (currently not loaded) prior to the security review being completed?

Hey @TheresNoTime - we softened the language a little bit in this documentation in regards to security reviews and beta deployments (item # 4). I'd likely rate a beta deployment for this code as medium risk, which in accordance with the WMF's risk management framework, requires manager-level oversight and risk acceptance.

In T314296#8167270, @sbassett wrote:

Hey @TheresNoTime - we softened the language a little bit in this documentation in regards to security reviews and beta deployments (item # 4). I'd likely rate a beta deployment for this code as medium risk, which in accordance with the WMF's risk management framework, requires manager-level oversight and risk acceptance.

ack, thank you — T314294 won't be progressed until that's done. I note we need to create a treatment plan within 30 days of said risk being accepted.

In T314296#8167794, @TheresNoTime wrote:

ack, thank you — T314294 won't be progressed until that's done. I note we need to create a treatment plan within 30 days of said risk being accepted.

In this case - for the beta deployment piece - it probably makes sense to just have a manager accept the risk. I'm not sure there's a mitigation plan that could be created to reduce this specific risk, other than specifying the risk of deploying new and potentially risky code to beta prior to a security review will only happen for a limited time and that we plan to perform a security review fairly quickly thereafter. Does that make sense? I know the risk management policy is a bit obtuse and doesn't gracefully accommodate every edge case like this, so I think we're happy to come up with reasonable workarounds when necessary.

In T314296#8169890, @sbassett wrote:

In T314296#8167794, @TheresNoTime wrote:

ack, thank you — T314294 won't be progressed until that's done. I note we need to create a treatment plan within 30 days of said risk being accepted.

In this case - for the beta deployment piece - it probably makes sense to just have a manager accept the risk. I'm not sure there's a mitigation plan that could be created to reduce this specific risk, other than specifying the risk of deploying new and potentially risky code to beta prior to a security review will only happen for a limited time and that we plan to perform a security review fairly quickly thereafter. Does that make sense? I know the risk management policy is a bit obtuse and doesn't gracefully accommodate every edge case like this, so I think we're happy to come up with reasonable workarounds when necessary.

Forgot to note that yes this makes sense, thank you 🙂

@TheresNoTime - No problem. Once T316020 is actioned upon, I'll add an entry to our risk register with the outcome. Thanks.

Security Review Summary - T314296 - 2022-11-24
Last commit reviewed: 14ac4b6

Summary

Overall, the current status of Phonos looks really good; there are only a few outdated dependencies which are not even reported because they are all indirect dependencies.
The overall risk rating is: low.

Vulnerable Packages - Production

npm audit returned no results. low risk
snyk returned no results. low risk
auditjs returned no results. low risk
osv-detector returned no results. low risk
composer-security-checker returned no results. low risk

none

Vulnerable Packages - Development

Outdated Packages

npm autdated returned no results. low risk

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

Static Analysis Findings

semgrep returned no results. low risk
gitleaks returned false positives. low risk
whispers returned false positives. low risk
git secrets returned no results. low risk
snyk code returned this no risk issue:

Use of Password Hash With Insufficient Computational EffortPath: includes/Engine/Engine.php, line 235
        Info: SHA1 hash (used in sha1) is insecure. Consider changing it to a secure hashing algorithm.

Note that the flagged "Password Hash with Insufficient Computation Effort" issue is a false positive; in this case, the SHA1 hash is being used as a hash rather than as a password, so there are no security ramifications.

As there is nothing actionable for CommTech developers here, I am going to resolve the ticket.

In T314296#8439201, @JMcLeod_WMF wrote:

Note that the flagged "Password Hash with Insufficient Computation Effort" issue is a false positive; in this case, the SHA1 hash is being used as a hash rather than as a password, so there are no security ramifications.

Hi @JMcLeod_WMF - You are right, this is why I marked this issue as: no risk issue.

Let us know if you need additional information.

Thank you.