feat(apps): inject code to posthog-js #12003

mariusandra · 2022-09-27T17:53:54Z

Problem

We are discussing giving apps the capability to inject code onto websites via posthog-js

Changes

If you add a file called "web.ts" to your plugin, we will transpile it and inject it on your website via the /decide endpoint.

Here's an example app "Pineapple Mode":

function makeItRain() {
    const div = document.createElement('div')
    Object.assign(div.style, {
        position: 'absolute',
        left: `${window.innerWidth * Math.random()}px`,
        top: '-10px',
        fontSize: '24px',
        zIndex: 99999999,
        pointerEvents: 'none',
    })
    div.innerHTML = '🍍'
    window.document.body.appendChild(div)
    const duration = 1000 + Math.random() * 3000
    div.animate([{ top: '-10px' }, { top: `${window.innerHeight + 20}px` }], {
        duration,
        iterations: 1,
    })
    window.setTimeout(() => {
        div.remove()
    }, duration)
}

export function inject(payload) {
    for (let i = 0; i < 10; i++) {
        makeItRain()
    }
    window.setInterval(makeItRain, 200)
}

This is a simple proof of concept. Other app ideas this enables are:

Notification banners
Feedback popup widget
Google Tag Manager
Custom web forms (send data to posthog, use a frontend app to visualise responses)
etc

Security concerns

Injecting arbitrary code on someone else's website sounds bad. However there are some things that prevent people using this as a blatant security hole:

On cloud you can still only install pre-approved apps. No customer can write a custom web.ts file to inject any random code.
Unless we create a blatant "posthog tag manager" app that lets you inject random HTML,
We are planning to implement 2FA to make logging in to posthog more secure
There is the apps audit log, which clearly states what apps were installed by who and when, and with what config.

On self hosted, where plugins are enabled for everyone, this is more concerning. To get around that:

The web.ts file is not included by default if you make a new plugin, you need to explicitly add it.
We can add a flag to org settings make these types of apps strictly opt-in.

Additional notes

Still missing or to consider developing:

Passing a payload to the injected code block
Lazy-loading large code blocks (not inlined in /decide, but imported separately)

The corresponding PR on posthog-js: PostHog/posthog-js#453

Should it be web.ts or inject.ts or ... ?

How did you test this code?

In the browser, see screencast. Tests can and will be added if we take this further.

macobo · 2022-09-28T08:01:28Z

posthog/api/decide.py

@@ -159,5 +159,19 @@ def get_decide(request: HttpRequest):
                on_permitted_recording_domain(team, request) or not team.recording_domains
            ):
                response["sessionRecording"] = {"endpoint": "/s/"}
+
+            source_files = PluginSourceFile.objects.filter(


Note: decide endpoint is really sensitive to lag - e.g. 200 extra milliseconds to it can mean thousands of events dropped.

I believe we should make sure we have the appropriate indexes and heavily cache this as a result - both in-memory and e.g. via redis? The joins seem scary wrt performance here.

posthog/api/decide.py

mariusandra · 2022-09-29T00:02:37Z

I made a bunch of changes, yet still some to do:

re-transpiling code after editing
make it possible to choose the emoji
lazy loading if block > 1kb
"Timeout waiting for app 10 to reload." (it thinks it's a frontend app)
use some kind of hash in the lazy loading url
avoid an extra query per /decide if we're sure no code will be injected

Custom emojis via config (opt-in fields for web):

Feedback form widget:

// web.ts
const style = ` 
.form, .button, .thanks {
  position: fixed;
  bottom: 20px;
  right: 20px;
  background: #f9bd2a;
  color: black;
  font-weight: normal;
  font-family: -apple-system, BlinkMacSystemFont, "Inter", "Segoe UI", "Roboto", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol";
  text-align: left;
}
.button { 
  width: 64px;
  height: 64px;
  border-radius: 100%;
  text-align: center;
  line-height: 60px;
  font-size: 32px;
  border: none;
  cursor: pointer;
}
.button:hover {
  background: #b88505;
}
.form {
  display: none;
  padding: 20px;
  flex-direction: column;
}
.form textarea {
  margin-bottom: 20px;
  background: white;
  color: black;
  border: none;
}
.form button {
  color: white;
  background: black;
}
.thanks {
  display:none;
  padding: 20px;
}
`

const form = `
  <textarea name='feedback' rows=6></textarea>
  <button class='form-submit' type='submit'></button>
`

export function inject({ config, posthog }) {
    const shadow = createShadow()
    const buttonElement = Object.assign(document.createElement('button'), {
        className: 'button',
        innerText: config.buttonText || '?',
        onclick: function () {
            Object.assign(buttonElement.style, { display: 'none' })
            Object.assign(formElement.style, { display: 'flex' })
            const submit: HTMLElement | undefined = formElement.querySelector('.form-submit')
            if (submit) {
                submit.innerText = config.sendButtonText
            }
        },
    })
    shadow.appendChild(buttonElement)

    const formElement = Object.assign(document.createElement('form'), {
        className: 'form',
        innerHTML: form,
        onsubmit: function (e) {
            e.preventDefault()
            posthog.capture('Feedback Sent', { feedback: this.feedback.value })
            Object.assign(formElement.style, { display: 'none' })
            Object.assign(thanksElement.style, { display: 'flex' })
            window.setTimeout(() => {
                Object.assign(thanksElement.style, { display: 'none' })
            }, 3000)
        },
    })
    shadow.appendChild(formElement)

    const thanksElement = Object.assign(document.createElement('div'), {
        className: 'thanks',
        innerHTML: config.thanksText || 'Thank you! Closing in 3 seconds...',
    })
    shadow.appendChild(thanksElement)
}

function createShadow(): ShadowRoot {
    const div = document.createElement('div')
    const shadow = div.attachShadow({ mode: 'open' })
    if (style) {
        const styleElement = Object.assign(document.createElement('style'), { innerText: style })
        shadow.appendChild(styleElement)
    }
    document.body.appendChild(div)
    return shadow
}

Twixes

This seems to work but it's complex enough to warrant some questions :)

Twixes · 2022-10-04T12:03:04Z

frontend/src/scenes/plugins/source/PluginSourceTabs.tsx


 export function PluginSourceTabs({ logic }: { logic: BuiltLogic<pluginSourceLogicType> }): JSX.Element {
-    const { setCurrentFile } = useActions(logic)
+    const { setCurrentFile, addFilePrompt } = useActions(logic)


How can I remove a file once it's added? This bit is confusing.

The file addition prompt asks for an arbitrary TypeScript file name, but it makes zero sense to allow adding "foobar.tsx" as things stand. The only file that makes sense is web.ts… so why not just expose that by default, same as frontend.tsx?

Bug here: I can also "add" existing files, which just erases the content of that existing file

I added a "remove" button. It's not the greatest 🍞 in the world, but does what it says.

Initially I wanted to just hide web.ts if not present, and show the other 2-3 files (plugin.json, index.ts, plus frontend.ts if the flag is enabled) at all times. I now changed it to only show the files that are actually present. I think this is cleaner, as I had the worry I might accidentally add a blank "index.ts" with something before. As for adding arbitrary files, I think we should support that sooner or later. However ok, limited it to only the files we support for now.

Fixed

This is kind of a regression in terms of the experience:

At the very least this should be a dropdown with available files instead of a freeform prompt(). The idea with arbitrary files is very far out at the moment, so optimizing for it is very premature.
Then also this placement of the "Delete" button doesn't seem super intuitive. The button appearing and disappearing moves the "Add new file" button around a lot, and there's a difference in scope between "Add some new file" and "Delete this open file" that doesn't sit well with me for some reason…

I'd still argue for keeping this simpler and using the same mechanism as with frontend.tsx – users with the feature flag on will see web.ts, others won't. This should still scale well here.

frontend/src/scenes/plugins/source/PluginSourceTabs.tsx

Twixes · 2022-10-04T12:29:29Z

plugin-server/src/worker/frontend/transpile.ts

+        code: true,
+        babelrc: false,
+        configFile: false,
+        filename: 'web.ts',


I'm not a fan of the web.ts name… but also I don't have a better idea right now.
Both this and frontend.tsx technically touch the frontend, though of different things.

inject.ts ?

Twixes · 2022-10-04T12:32:26Z

plugin-server/src/worker/plugins/loadPlugin.ts

@@ -71,6 +71,35 @@ export async function loadPlugin(hub: Hub, pluginConfig: PluginConfig): Promise<
            }
        }

+        // transpile "web" app if needed


This block seems awfully similar to the // transpile "frontend" app if needed' one. I'm not super strict about DRY but this is so complex that I'd be more at peace if the two blocks were refactored into one function.

hmm.. I made it just a bit more abstract, though I would be vary of prematurely optimising this too much.

Twixes · 2022-10-04T12:54:50Z

posthog/models/plugin.py

+        indexes = [
+            models.Index(fields=["web_token"]),
+            models.Index(fields=["enabled"]),
+        ]


If we know our queries will use id, web_token AND enabled, Postgres will be able to optimize much better if there is one composite index on the three columns.

Disclaimer: This is the general theory, the practice needs to be tested with EXPLAIN ANALYZE

Luckily postgres indexes are one of the few things we can add and remove as needed :). I went for simple indices since I didn't know what shape the queries will take, and didn't want to overanalyse before this was a problem. Many simple indices also take up less space than multiple combined ones, so I considered this a fair tradeoff 🤷

Already the queries for /decide and /web_js/id/token/ are quite different in what indices they use. Currently: id & web_token & enabled ... vs ... team_id & enabled.

posthog/models/team/team.py

posthog/plugins/web.py

Twixes · 2022-10-04T13:05:38Z

posthog/models/plugin.py

@@ -206,6 +213,8 @@ class PluginConfig(models.Model):
    # - e.g: "undefined is not a function on index.js line 23"
    # - error = { message: "Exception in processEvent()", time: "iso-string", ...meta }
    error: models.JSONField = models.JSONField(default=None, null=True)
+    # Used to access web.ts from a public URL


I probably missed something but I don't really get what's this web token for. Can you expand on this comment?

Web apps will be injected from /web_js/:id/:token/. I don't want somebody to id++ in a loop and download all possible apps.

Sidenote: this definitely should go behind some CDN/cache, but in v0.2.

For me the primary distinction between token and ID is:

a token serves authentication purposes and can change throughout its entity's life

an ID serves unique identification purposes and cannot change

I don't see how authentication would work for a posthog-js feature, so currently I'd say the only real job of this code is identifying the app to fetch code for. And if the only reason for not using id is preventing enumeration, could a plugin UUID (UUIDT) – which would be more general than web_token – work here?

Good points! Generally agree, though we might want cache invalidation as well. Tokens due to their refreshability serve well for this purpose.

mariusandra

@Twixes I addressed most of the feedback. Can you give it another look and mark out what's still blocking, and what can be in v0.0.2?

Twixes · 2022-10-05T12:15:00Z

frontend/src/scenes/plugins/source/PluginSourceTabs.tsx


 export function PluginSourceTabs({ logic }: { logic: BuiltLogic<pluginSourceLogicType> }): JSX.Element {
-    const { setCurrentFile } = useActions(logic)
+    const { setCurrentFile, addFilePrompt } = useActions(logic)


This is kind of a regression in terms of the experience:

At the very least this should be a dropdown with available files instead of a freeform prompt(). The idea with arbitrary files is very far out at the moment, so optimizing for it is very premature.
Then also this placement of the "Delete" button doesn't seem super intuitive. The button appearing and disappearing moves the "Add new file" button around a lot, and there's a difference in scope between "Add some new file" and "Delete this open file" that doesn't sit well with me for some reason…

I'd still argue for keeping this simpler and using the same mechanism as with frontend.tsx – users with the feature flag on will see web.ts, others won't. This should still scale well here.

mariusandra · 2022-10-05T12:51:37Z

Something like this?

Twixes

Looks good to me!

mariusandra added 8 commits September 27, 2022 17:35

support adding custom files

602386a

compile web.ts

f670ee9

add injected code into /decide

df6892a

transpile better

f579e28

add id and payload stub to injected code

8bca704

skip backend if web only

6c1425d

more web source

0fb53ae

add payload to default demo

0e20e40

macobo reviewed Sep 28, 2022

View reviewed changes

posthog/api/decide.py Outdated Show resolved Hide resolved

macobo reviewed Sep 28, 2022

View reviewed changes

posthog/api/decide.py Outdated Show resolved Hide resolved

macobo reviewed Sep 28, 2022

View reviewed changes

posthog/api/decide.py Outdated Show resolved Hide resolved

mariusandra added 10 commits September 28, 2022 10:48

values list and inline for

93c0af8

revert

4e5487f

simpler query

7addedf

reload plugin if source changed

582edac

pass on config that has "web":true

3a6d4f7

add a bootloader for scripts larger than 1kb

a508ede

rename payload to config

a3366fb

access posthog also in injected script

470bd84

cleanup

e54fc59

pass meta object to inject

f5af3a9

mariusandra mentioned this pull request Sep 29, 2022

feat(apps): inject javascript from decide PostHog/posthog-js#453

Merged

2 tasks

mariusandra added 6 commits September 29, 2022 12:34

split web_js code

ee9d001

List

83ce1bc

Merge branch 'master' into webinject

afe4d24

add a web token for plugin configs

707c1fa

Merge branch 'master' into webinject

685beac

test for web.ts

8aa6c2d

ivanagas mentioned this pull request Oct 4, 2022

Blog: All the cool things we built at our Rome hackathon PostHog/posthog.com#4339

Closed

Merge branch 'master' into webinject

0e5815d

Twixes requested changes Oct 4, 2022

View reviewed changes

mariusandra added 18 commits October 4, 2022 15:44

url dataclass

4065bf9

comment

e4e8b34

add and remove files

41e50eb

not users

d5bc437

only show button to add files if there are files to add

68d5c2b

update in bulk

9d970cd

TYPE_CHECKING

1e52ce5

abstract transpilation just a bit

78f389b

feedback and bug

ef7cd51

we're down under

74ea418

errors if failed

c07c7c7

raise on 404

93bf0d0

clean up more

e46a30a

refactor

5950537

add updated_at to list

275d560

no need to check code equality

a15a137

remove australia ip override

97c4289

test

fdbf0d8

mariusandra commented Oct 4, 2022

View reviewed changes

mariusandra requested a review from Twixes October 4, 2022 21:37

Merge branch 'master' into webinject

4c70a84

Twixes requested changes Oct 5, 2022

View reviewed changes

remove support for random files

0d86b9e

Twixes approved these changes Oct 5, 2022

View reviewed changes

mariusandra merged commit c42c2c1 into master Oct 5, 2022

mariusandra deleted the webinject branch October 5, 2022 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apps): inject code to posthog-js #12003

feat(apps): inject code to posthog-js #12003

mariusandra commented Sep 27, 2022 •

edited

Loading

macobo Sep 28, 2022 •

edited

Loading

mariusandra commented Sep 29, 2022 •

edited

Loading

Twixes left a comment

Twixes Oct 4, 2022

mariusandra Oct 4, 2022

Twixes Oct 5, 2022 •

edited

Loading

Twixes Oct 4, 2022

mariusandra Oct 4, 2022

Twixes Oct 4, 2022

mariusandra Oct 4, 2022

Twixes Oct 4, 2022

Twixes Oct 4, 2022

mariusandra Oct 4, 2022

Twixes Oct 4, 2022

mariusandra Oct 4, 2022

Twixes Oct 4, 2022

mariusandra Oct 4, 2022

mariusandra left a comment

Twixes Oct 5, 2022 •

edited

Loading

mariusandra commented Oct 5, 2022

Twixes left a comment

feat(apps): inject code to posthog-js #12003

feat(apps): inject code to posthog-js #12003

Conversation

mariusandra commented Sep 27, 2022 • edited Loading

Problem

Changes

Security concerns

Additional notes

How did you test this code?

macobo Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

mariusandra commented Sep 29, 2022 • edited Loading

Twixes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Twixes Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariusandra left a comment

Choose a reason for hiding this comment

Twixes Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

mariusandra commented Oct 5, 2022

Twixes left a comment

Choose a reason for hiding this comment

mariusandra commented Sep 27, 2022 •

edited

Loading

macobo Sep 28, 2022 •

edited

Loading

mariusandra commented Sep 29, 2022 •

edited

Loading

Twixes Oct 5, 2022 •

edited

Loading

Twixes Oct 5, 2022 •

edited

Loading