Skip to content

[DO NOT MERGE] Stream web analytics data to Umami - #187

Draft
sjlegg wants to merge 10 commits into
mainfrom
umami
Draft

[DO NOT MERGE] Stream web analytics data to Umami#187
sjlegg wants to merge 10 commits into
mainfrom
umami

Conversation

@sjlegg

@sjlegg sjlegg commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
  • JS snippet in <head> to stream web analytics data to Umami (NHSE-hosted in AWS)
  • Self-host JS snippet to mitigate script injection vulnerability
  • GitHub Action to automate snippet updates (nightly; creates a PR for human review)
  • New privacy policy
  • Add link to privacy policy to footer

@sjlegg
sjlegg requested review from csutter and a balanced review from Copilot August 18, 2026 16:04
@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 18, 2026 16:04 Inactive
Copilot AI reviewed Aug 18, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds Umami web analytics and documents the associated privacy handling.

Changes:

  • Injects the Umami analytics script site-wide.
  • Adds and links a new privacy policy.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
eleventy.config.mjs Adds footer link and analytics transform.
app/privacy-policy.md Documents analytics data processing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/privacy-policy.md Outdated
Comment thread eleventy.config.mjs Outdated
@sjlegg
sjlegg requested a balanced review from Copilot August 18, 2026 16:19

This comment was marked as outdated.

Sign in to view

@sjlegg sjlegg linked an issue Aug 19, 2026 that may be closed by this pull request
Add privacy-friendly analytics #148
Open
@annapowellsmith-nhs
annapowellsmith-nhs marked this pull request as draft August 19, 2026 14:00
@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 15:10 Inactive
@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 15:50 Inactive
@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 15:51 Inactive
@sjlegg
sjlegg requested a balanced review from Copilot August 19, 2026 15:52

This comment was marked as resolved.

Sign in to view

@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 16:13 Inactive
@sjlegg
sjlegg requested a balanced review from Copilot August 19, 2026 16:17
@sjlegg
sjlegg requested a review from jhanrnhs August 19, 2026 16:18

This comment was marked as resolved.

Sign in to view

@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 16:23 Inactive
@sjlegg
sjlegg requested a balanced review from Copilot August 19, 2026 16:24

This comment was marked as resolved.

Sign in to view

@sjlegg
Disable persisted credentials in GitHub Action
eb90a7b 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 16:33 Inactive
@frankieroberto
frankieroberto temporarily deployed to dpsp-website-umami-dvheupcg8z0 August 19, 2026 16:40 Inactive
@sjlegg

sjlegg commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Moving conversation with @annapowellsmith-nhs from Slack:
APS: Is there a reason we rejected the suggestion to add integrity crossorigin attributes but continue with a third-party script? Would be simpler...

SL:

  • The contents of the script changes whenever Umami is updated as part of normal development. Recent change history below. That means that if we did it that way whenever we did an update the snippet would stop working until we got teams to review PRs to update the checksum, resulting in either data gaps or extra work for us going round chasing teams to review and merge PRs quickly after we do an update.
  • This way we usually keep the data stream going across updates - unless the script change with that update happened to be functional and breaking.
  • I don't think we can just say we'll never update the script - it's crossing an engineering red line and more importantly I think we'll want features and improvements that come with new Umami versions.
  • Maybe an alternative approach could be to add something to ECS that copies the script and version controls it, such that the checksum only changes intentionally and people keep using old versions of the scripts? But in that case we'd still need an efficient way to update which version of the script apps are pointing to so I'm not sure it gains much.

Recent Umami script change history (AI summary)
Aug 4, 2026: "Avoid saving same-domain referrers and resolve path-only referrers correctly"
Jul 20, 2026: "Convert tracker build to TypeScript and add type checks" (build-process change, not necessarily behavioural)
May 29, 2026: "fix(tracker): restore link navigation when tracking fails"
May 24, 2026: "Added new headers to tracker"
May 15, 2026: Merge of tracker bug fixes (#4248)
May 7, 2026: "fix(tracker): track clicks on annotated containers and gracefully handle invalid pushState URLs"
May 7, 2026: "perf(tracker): defer INP percentile computation to flush time"
Apr 30, 2026 (×2): SPA pageview suppression fix,data-auto-pageviewattribute added
Apr 11 / Apr 8, 2026: tracker double-init guard changes

So the tracker is actively maintained, with real behavioural and even security-adjacent changes (e.g. referrer leakage fix, double-init fix) landing roughly every 1–4 weeks on average, not just cosmetic tweaks.

Honest opinion - what approach would you take here?

APS: My instinct is to checksum, version number the script url, and only update for major versions - but there may be things I've missed, I'll have a proper look. (We should probably move this discussion to the PR!)
SL: Which might be a bit easier to think about once we've hardened the Umami installation - we'll have a process in place to update Umami anyway, and whatever that is could have versioning the script built in. Teams would also still need a way to regenerate the hash easily (and probably without understanding what's going on very much)
APS: Yes sounds like good plan (maybe link to this discussion on the PR).

I think the checksum approach is lighter weight, means we can control when we need to update umami, we can do the hash and URL part, and just make a PR to update the version - and assuming the repo owners trust us, it's an easy enough PR to merge.

annapowellsmith-nhs
annapowellsmith-nhs reviewed Aug 20, 2026
View reviewed changes
Comment thread .github/workflows/update-umami.yml
@@ -0,0 +1,91 @@
# We keep a reviewed copy of the tracking JavaScript snippet in the repository.
# Loading it from CloudFront would allow its contents to change without a code

@annapowellsmith-nhs annapowellsmith-nhs Aug 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure this is true if we load it from Cloudfront and use the integrity crossorigin attributes to checksum?

@annapowellsmith-nhs

annapowellsmith-nhs commented Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown

@sjlegg thanks for adding a copy of the slack convo!

As per comment, I am still wondering if we can just point the <script> tag at cloudflare cloudfront and use the integrity crossorigin attributes to checksum against the third-party file?

I guess the advantage of keeping the file locally is that, when we come to upgrade, future owners of this repo have total confidence over what they're merging: the disadvantage is that when we come to upgrade, we'd need to sync deployment of the Umami upgrade with the merge here, which makes things more complex.

Overall, keeping the script third-party feels cleaner to me, but happy to hear counterarguments.

:seagull_emoji:

annapowellsmith-nhs
annapowellsmith-nhs reviewed Aug 21, 2026
View reviewed changes
Comment thread app/assets/umami.js

@annapowellsmith-nhs annapowellsmith-nhs Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd put this in a new app/assets/scripts folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@annapowellsmith-nhs annapowellsmith-nhs annapowellsmith-nhs left review comments
@csutter csutter Awaiting requested review from csutter
@jhanrnhs jhanrnhs Awaiting requested review from jhanrnhs

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add privacy-friendly analytics

4 participants

@sjlegg @annapowellsmith-nhs @frankieroberto