Skip to content

Conversation

@robbie-c
Copy link
Member

@robbie-c robbie-c commented May 17, 2025

Important

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Problem

We accidentally broke people's CSP rules with a 3rd party library where were webpack was including an eval()

It'd be good to fix this! I made a PR in the library, and the maintainer was super-responsive. microlinkhq/react-json-view#77

Changes

Let's bump the dependency and add a script to protect against this kind of thing in the future

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

The script fails with the old dep, passes with the new

@github-actions
Copy link
Contributor

Size Change: +4.99 kB (+0.14%)

Total Size: 3.66 MB

Filename Size Change
frontend/dist/toolbar.js 3.66 MB +4.99 kB (+0.14%)

compressed-size-action

@robbie-c robbie-c marked this pull request as ready for review May 17, 2025 13:58
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR addresses a Content Security Policy (CSP) violation caused by eval() usage in a third-party dependency, upgrading @microlink/react-json-view and implementing preventive measures.

  • Added frontend/bin/check-toolbar-csp-eval.mjs to detect potential CSP violations by scanning for eval(), Function constructors, and unsafe timer patterns
  • Updated @microlink/react-json-view from v1.21.3 to v1.26.2 to resolve CSP violation
  • Added new CI step in .github/workflows/ci-frontend.yml to run CSP violation checks
  • Added @babel/parser and @babel/traverse as dev dependencies for AST parsing

3 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

robbie-c and others added 3 commits May 17, 2025 15:02
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 5)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 5)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@robbie-c robbie-c requested review from a team, jabahamondes, lricoy and pauldambra and removed request for a team May 17, 2025 15:26
Copy link
Member

@lricoy lricoy left a comment

Choose a reason for hiding this comment

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

🚀

I wasn't able to run it locally so far but lgtm

# we only care if the toolbar will increase a lot
minimum-change-threshold: 1000

- name: Check toolbar for CSP eval violations
Copy link
Member

Choose a reason for hiding this comment

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

Nice

@robbie-c robbie-c merged commit d1f273e into master May 18, 2025
95 checks passed
@robbie-c robbie-c deleted the ensure-toolbar-doesnt-break-csp-rules branch May 18, 2025 10:18
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.

4 participants