Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Phishing Detection Data Update Script #3243

Closed
wants to merge 2 commits into from

Conversation

not-a-rootkit
Copy link
Collaborator

Task/Issue URL: https://app.asana.com/0/1204023833050360/1208270234071172/f
Tech Design URL: https://app.asana.com/0/481882893211075/1207483114414814
CC:

Description:
In ✓ Tech Design: Phishing Protection Data Updates we defined an approach to get embedded data for phishing protection into the app builds. This pattern was implemented, but two small components still need merging:
The update script:
A small bash script that pulls data from the API into the repo in JSON format, and updates the checksums + revision values in PhishingDetection.swift .
This has already been implemented, it just needs testing and merging:

A GitHub Action workflow that executes this script once a week and creates a PR that merges this data into the new release build.
Example Script
The secrets and workflow have already been defined, it just needs to be tested and merged.

Steps to test this PR:

  1. Execute bash scripts/update_phishing_detection_data.sh
  2. Ensure it succeeds
  3. Check git, validate that the version has been updated to a higher version, and SHA hashes were updated in ./DuckDuckGo/PhishingDetection/PhishingDetection.swift

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Copy link
Contributor

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 895a14a

@not-a-rootkit not-a-rootkit deleted the tespach/phishing-detection-data-script branch October 16, 2024 11:42
not-a-rootkit added a commit that referenced this pull request Oct 22, 2024
Task/Issue URL:
https://app.asana.com/0/72649045549333/1208270234071172/f
Tech Design URL:
https://app.asana.com/0/481882893211075/1207483114414814
CC: 

**Description**:
In [✓ Tech Design: Phishing Protection Data
Updates](https://app.asana.com/0/481882893211075/1207483114414814/f) we
defined an approach to get embedded data for phishing protection into
the app builds. This pattern was implemented, but two small components
still need merging:
- The update script:
- A small bash script that pulls data from the API into the repo in JSON
format, and updates the checksums + revision values in
PhishingDetection.swift .
- This has already been implemented, it just needs testing and merging:
    - #3243
- A GitHub Action workflow that executes this script once a week and
creates a PR that merges this data into the new release build.
- [Example
Script](https://github.com/duckduckgo/macos-browser/blob/0fb680211ad05dacb621b351a8f7b266e7239b7d/.github/workflows/update_phishing_detection_data.yml)
- The secrets and workflow have already been defined, it just needs to
be tested, reviewed, and merged.

**Note**
After testing, but before merging, I'd like to update the GH action to
run on a schedule once per week using cron:

```
on:
  schedule:
    - cron: '0 0 * * 0'  # Midnight UTC every Sunday
```

This way it can be reviewed just once per week by whoever is on
maintenance that week as part of the weekly maintenance rota.

**Steps to test this PR**:
1. Test the script locally: `bash scripts/update_phishing_data.sh`
2. Ensure the script runs, check changes in git:
3. `DuckDuckGo/PhishingDetection/PhishingDetection.swift` - sha256 and
version values updated correctly
4. `DuckDuckGo/PhishingDetection/filterSet.json` - not empty
5. `DuckDuckGo/PhishingDetection/hashPrefixes.json` - not empty
6. Check the GH action has executed and created a PR with name like
`Update phishing protection datasets to 1681795`:
7. #3404

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

**Definition of Done**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant