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

Bug Fix: Phishing Detection Dataset Discrepancies #1032

Merged
merged 8 commits into from
Nov 1, 2024

Conversation

not-a-rootkit
Copy link
Collaborator

@not-a-rootkit not-a-rootkit commented Oct 22, 2024

Please review the release process for BrowserServicesKit here.

Required:

Task/Issue URL: https://app.asana.com/0/1204023833050360/1208567121137949/f
iOS PR: duckduckgo/iOS#3469
macOS PR: duckduckgo/macos-browser#3440
What kind of version bump will this require?: Patch

Optional:

Tech Design URL:
CC:

Description:
In Implement desktop integration efficacy tests - 5-7 days it was discovered that Swift's client-side caching results in out-of-date datasets and significant dataset discrepancies between different clients. For example, it's very common for the same request to return different results from the backend, resulting in a client believing they are updating to a newer revision than they are. Over time, this compounds and results in disparate versions of the same dataset across different clients, putting users at risk of landing on newer phishing pages.

Fix:

  • Remove Client Side Caching in PhishingDetectionClient.swift
  • Ensure embedded dataset is used to replace the on-disk dataset when the revision of the embedded dataset > on disk dataset

Steps to test this PR:

  1. Check unit tests
  2. Change on-disk revision:
  3. echo "1650000" > "/System/Volumes/Data/Users/<user>/Library/Application Support/com.duckduckgo.macos.browser.debug/revision.txt"
  4. Build the browser
  5. Visit https://privacy-test-pages.site/security/badware/phishing.html
  6. Ensure blocked
  7. Check on-disk revision:
  8. cat "/System/Volumes/Data/Users/<user>/Library/Application Support/com.duckduckgo.macos.browser.debug/revision.txt"
  9. Should be > 1650000

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16
  • macOS 10.15
  • macOS 11
  • macOS 12

Internal references:

Software Engineering Expectations
Technical Design Template

@not-a-rootkit not-a-rootkit self-assigned this Oct 22, 2024
@not-a-rootkit not-a-rootkit marked this pull request as ready for review October 22, 2024 16:30
Copy link
Contributor

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Steps work as described

I think we should keep the caching and if dos not work as it should means we are getting wrong values from the server… we should therefore update the server instead.

Copy link
Contributor

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
There are some linting issues

@not-a-rootkit not-a-rootkit merged commit d39d04c into main Nov 1, 2024
7 checks passed
@not-a-rootkit not-a-rootkit deleted the tespach/fix-outdated-phishing-datasets branch November 1, 2024 11:02
samsymons added a commit that referenced this pull request Nov 1, 2024
…the-os

* main:
  Bump github.com/duckduckgo/privacy-dashboard from 5.3.0 to 7.1.1 (#1046)
  Bump github.com/duckduckgo/sync_crypto from 0.2.0 to 0.3.0 (#1048)
  Bump github.com/1024jp/gzipswift from 6.0.1 to 6.1.0 (#1050)
  Update to subscription cookie (#1053)
  Bug Fix: Phishing Detection Dataset Discrepancies (#1032)
  Include www.youtube.com as Youtube host (#1028)
samsymons added a commit that referenced this pull request Nov 4, 2024
* main:
  Validate VPN errors before re-throwing them (#1054)
  Allowing users to delete suggestions (#1027)
  Revert "Bump github.com/1024jp/gzipswift from 6.0.1 to 6.1.0" (#1055)
  Bump github.com/duckduckgo/privacy-dashboard from 5.3.0 to 7.1.1 (#1046)
  Bump github.com/duckduckgo/sync_crypto from 0.2.0 to 0.3.0 (#1048)
  Bump github.com/1024jp/gzipswift from 6.0.1 to 6.1.0 (#1050)
  Update to subscription cookie (#1053)
  Bug Fix: Phishing Detection Dataset Discrepancies (#1032)
  Include www.youtube.com as Youtube host (#1028)
  Ignore GRDB (#1052)
  Connect refactored update flow to the new release notes page (#1025)
  Add to Dock - Add extra custom view to Contextual Dialog Content (#1043)
  Dependabot for BSK (#1045)
  Update autofill to 15.1.0 (#1044)
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.

2 participants