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

feat: Use NavigatorUAData and navigator.webdriver to improve bot detection #1359

Merged
merged 13 commits into from
Aug 20, 2024

Conversation

robbie-c
Copy link
Contributor

@robbie-c robbie-c commented Aug 14, 2024

Changes

We weren't detecting some headless browsers in our bot detection, so I've added headless chrome to the list of blocked UAs. See this zendesk ticket https://posthoghelp.zendesk.com/agent/tickets/16802

We weren't using the navigator.webdriver field, so I've added that to the check as well.

I've also added some support for NavigatorUAData, so that we detect headless chrome where the --useragent argument is changed to something else

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

Copy link

vercel bot commented Aug 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Aug 14, 2024 0:58am

Copy link

github-actions bot commented Aug 14, 2024

Size Change: +1.18 kB (+0.1%)

Total Size: 1.17 MB

Filename Size Change
dist/array.full.js 333 kB +294 B (+0.09%)
dist/array.js 154 kB +295 B (+0.19%)
dist/main.js 155 kB +295 B (+0.19%)
dist/module.js 154 kB +295 B (+0.19%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 10.4 kB
dist/recorder-v2.js 110 kB
dist/recorder.js 110 kB
dist/surveys-preview.js 59.8 kB
dist/surveys.js 66 kB
dist/tracing-headers.js 8.26 kB
dist/web-vitals.js 5.79 kB

compressed-size-action

@robbie-c robbie-c added the bump patch Bump patch version when this PR gets merged label Aug 14, 2024
@robbie-c robbie-c requested a review from daibhin August 16, 2024 10:02
Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

looks sound to me 💪

/// <reference types="cypress" />
import { start } from '../support/setup'

describe('User Agent Blocking', () => {
Copy link
Member

Choose a reason for hiding this comment

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

❤️ this is lovely

Comment on lines +2036 to +2038
if (navigator) {
return isLikelyBot(navigator, this.config.custom_blocked_useragents)
} else {
Copy link
Member

Choose a reason for hiding this comment

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

the only thing that gives me pause - and i've helpfully not thought about this at all - is the change from userAgent being required to navigator being required.

my assumption is this is ok because if we're in an environment with no navigator then we also have no userAgent...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - I can see why there would be situations where people might want to pass a userAgent, but we currently get the userAgent by doing navigator.userAgent so any environment without a navigator won't have a userAgent, so this is no change.

@robbie-c robbie-c merged commit 990e323 into main Aug 20, 2024
14 checks passed
@robbie-c robbie-c deleted the feat/detect-headless-browsers branch August 20, 2024 15:11
Phanatic pushed a commit that referenced this pull request Aug 20, 2024
…ction (#1359)

* Use NavigatorUAData and navigator.webdriver to improve bot detection

* Use real values of brands from headless and regular chrome in tests

* Reduce bundle size slightly

* Remove global

* Fix test setup

* Opt out of bot filtering in cypress

* Make _is_likely_bot an instance function on posthog, use it in cypress tests

* Fix cypress imports

* Code golf

* More code golf

* Fix testcafe tests

* Treat cypress as a bot

* Fix cypress chaining logic
@robbie-c robbie-c mentioned this pull request Aug 23, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants