From 514233020b43ff6a15e6dfc5ff8b16caa930ef83 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 24 May 2024 08:20:45 +0100
Subject: [PATCH 1/6] [ExcelAnalyzer] add tests for current implementation

---
 spec/excel_analyzer_spec.rb | 106 ++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 spec/excel_analyzer_spec.rb

diff --git a/spec/excel_analyzer_spec.rb b/spec/excel_analyzer_spec.rb
new file mode 100644
index 00000000..c5014980
--- /dev/null
+++ b/spec/excel_analyzer_spec.rb
@@ -0,0 +1,106 @@
+require_relative 'spec_helper'
+
+RSpec.describe 'ExcelAnalyzer on_hidden_metadata hook' do
+  let(:message) { FactoryBot.create(:incoming_message, sent_at: Time.now) }
+  let(:attachment) { message.foi_attachments.first }
+  let(:blob) { attachment.file_blob }
+
+  around do |example|
+    to = ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL']
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = 'excel@localhost'
+    example.call
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = to
+  end
+
+  it 'hides the attachment with prominence reason' do
+    expect(attachment.prominence).to eq('normal')
+    expect(attachment.prominence_reason).to be_nil
+
+    ExcelAnalyzer.on_hidden_metadata.call(blob, blob.metadata)
+    attachment.reload
+
+    expect(attachment.prominence).to eq('hidden')
+    expect(attachment.prominence_reason).to eq(<<~TXT.squish)
+      We've found a problem with this file, so it's been hidden while we review
+      it. We might not be able to give more details until then.
+    TXT
+  end
+
+  it 'sents report email' do
+    deliveries = ActionMailer::Base.deliveries
+    expect(deliveries.size).to eq(0)
+
+    expect(ExcelAnalyzerNotifier).to receive(:report).
+      with(attachment, blob.metadata).
+      and_call_original
+    ExcelAnalyzer.on_hidden_metadata.call(blob, blob.metadata)
+    expect(deliveries.size).to eq(1)
+
+    deliveries.clear
+  end
+
+  context 'when message was sent more than 1 day ago' do
+    let(:message) do
+      FactoryBot.create(:incoming_message, sent_at: 24.hours.ago - 1.minute)
+    end
+
+    it 'does not hide the attachment' do
+      expect { ExcelAnalyzer.on_hidden_metadata.call(blob, blob.metadata) }.
+        to_not change(attachment, :prominence)
+    end
+
+    it 'sents no email' do
+      deliveries = ActionMailer::Base.deliveries
+      expect(deliveries.size).to eq(0)
+      ExcelAnalyzer.on_hidden_metadata.call(blob, blob.metadata)
+      expect(deliveries.size).to eq(0)
+    end
+  end
+end
+
+RSpec.describe 'ExcelAnalyzerNotifier report mail' do
+  let(:message) { FactoryBot.create(:incoming_message, sent_at: Time.now) }
+  let(:attachment) { message.foi_attachments.first }
+  let(:blob) { attachment.file_blob }
+
+  let(:mail) do
+    ExcelAnalyzerNotifier.report(attachment, blob.metadata).deliver_now
+    ActionMailer::Base.deliveries[-1]
+  end
+
+  around do |example|
+    to = ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL']
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = 'excel@localhost'
+    example.call
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = to
+  end
+
+  after { ActionMailer::Base.deliveries.clear }
+
+  it 'has custom mail headers' do
+    expect(mail['X-WDTK-Contact'].value).to eq('wdtk-excel-analyzer-report')
+    expect(mail['X-WDTK-CaseRef'].value).to eq(attachment.to_param)
+  end
+
+  it 'sents mail from and to the correct addresses' do
+    expect(mail.from).to include(blackhole_email)
+    expect(mail.to).to include(ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'])
+  end
+
+  it 'has attachment ID in the subject' do
+    expect(mail.subject).
+      to eq("ExcelAnalyzer: hidden data detected [#{attachment.id}]")
+  end
+
+  it 'includes the admin URL in the body' do
+    expect(mail.body).
+      to include("http://test.host/admin/requests/#{message.info_request_id}")
+    expect(mail.body).
+      to include("http://test.host/admin/attachments/#{attachment.id}/edit")
+  end
+
+  it 'includes the metadata in the body' do
+    allow(blob).to receive(:metadata).and_return(foo: 'bar')
+    expect(mail.body).to include("foo: bar")
+  end
+end

From e6ee44165dc2457985779459187cbcc424e6c521 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 21 May 2024 11:27:26 +0100
Subject: [PATCH 2/6] [ExcelAnalyzer] update autoloading

Fixes issues in development mode where we were seeing:
> superclass mismatch for class ExcelAnalyzerNotifier

Fixes #1869
---
 lib/excel_analyzer.rb                         | 37 ++++----------
 lib/excel_analyzer/mailers/notifier_mailer.rb | 26 ++++++++++
 .../notifier_mailer}/report.text.erb          |  0
 .../hook_spec.rb}                             | 51 +------------------
 .../mailers/notifier_mailer_spec.rb           | 48 +++++++++++++++++
 5 files changed, 85 insertions(+), 77 deletions(-)
 create mode 100644 lib/excel_analyzer/mailers/notifier_mailer.rb
 rename lib/views/{excel_analyzer_notifier => excel_analyzer/notifier_mailer}/report.text.erb (100%)
 rename spec/{excel_analyzer_spec.rb => excel_analyzer/hook_spec.rb} (52%)
 create mode 100644 spec/excel_analyzer/mailers/notifier_mailer_spec.rb

diff --git a/lib/excel_analyzer.rb b/lib/excel_analyzer.rb
index 2bee4f42..4ffb5903 100644
--- a/lib/excel_analyzer.rb
+++ b/lib/excel_analyzer.rb
@@ -1,3 +1,11 @@
+Rails.application.config.before_initialize do
+  loader = Rails.autoloaders.main
+
+  Dir[File.join(File.dirname(__FILE__), 'excel_analyzer', '**/')].each do
+    loader.push_dir(_1, namespace: ExcelAnalyzer)
+  end
+end
+
 ExcelAnalyzer.on_hidden_metadata = ->(attachment_blob, metadata) do
   foi_attachment = FoiAttachment.joins(:file_blob).
     find_by(active_storage_blobs: { id: attachment_blob })
@@ -16,32 +24,5 @@
     }
   )
 
-  ExcelAnalyzerNotifier.report(foi_attachment, metadata).deliver_now
-end
-
-Rails.configuration.to_prepare do
-  class ExcelAnalyzerNotifier < ApplicationMailer
-    include Rails.application.routes.url_helpers
-    default_url_options[:host] = AlaveteliConfiguration.domain
-
-    def report(foi_attachment, metadata)
-      @foi_attachment = foi_attachment
-      @incoming_message = foi_attachment.incoming_message
-      @metadata = metadata
-
-      from = email_address_with_name(
-        blackhole_email, 'WhatDoTheyKnow.com Excel Analyzer report'
-      )
-
-      headers['X-WDTK-Contact'] = 'wdtk-excel-analyzer-report'
-      headers['X-WDTK-CaseRef'] = @foi_attachment.id
-
-      mail(
-        from: from,
-        to: ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'],
-        subject: _('ExcelAnalyzer: hidden data detected [{{reference}}]',
-                   reference: @foi_attachment.id)
-      )
-    end
-  end
+  ExcelAnalyzer::NotifierMailer.report(foi_attachment, metadata).deliver_now
 end
diff --git a/lib/excel_analyzer/mailers/notifier_mailer.rb b/lib/excel_analyzer/mailers/notifier_mailer.rb
new file mode 100644
index 00000000..a09bd9c7
--- /dev/null
+++ b/lib/excel_analyzer/mailers/notifier_mailer.rb
@@ -0,0 +1,26 @@
+module ExcelAnalyzer
+  class NotifierMailer < ApplicationMailer
+    include Rails.application.routes.url_helpers
+    default_url_options[:host] = AlaveteliConfiguration.domain
+
+    def report(foi_attachment, metadata)
+      @foi_attachment = foi_attachment
+      @incoming_message = foi_attachment.incoming_message
+      @metadata = metadata
+
+      from = email_address_with_name(
+        blackhole_email, 'WhatDoTheyKnow.com Excel Analyzer report'
+      )
+
+      headers['X-WDTK-Contact'] = 'wdtk-excel-analyzer-report'
+      headers['X-WDTK-CaseRef'] = @foi_attachment.id
+
+      mail(
+        from: from,
+        to: ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'],
+        subject: _('ExcelAnalyzer: hidden data detected [{{reference}}]',
+                   reference: @foi_attachment.id)
+      )
+    end
+  end
+end
diff --git a/lib/views/excel_analyzer_notifier/report.text.erb b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
similarity index 100%
rename from lib/views/excel_analyzer_notifier/report.text.erb
rename to lib/views/excel_analyzer/notifier_mailer/report.text.erb
diff --git a/spec/excel_analyzer_spec.rb b/spec/excel_analyzer/hook_spec.rb
similarity index 52%
rename from spec/excel_analyzer_spec.rb
rename to spec/excel_analyzer/hook_spec.rb
index c5014980..a67b05df 100644
--- a/spec/excel_analyzer_spec.rb
+++ b/spec/excel_analyzer/hook_spec.rb
@@ -1,4 +1,4 @@
-require_relative 'spec_helper'
+require_relative '../spec_helper'
 
 RSpec.describe 'ExcelAnalyzer on_hidden_metadata hook' do
   let(:message) { FactoryBot.create(:incoming_message, sent_at: Time.now) }
@@ -30,7 +30,7 @@
     deliveries = ActionMailer::Base.deliveries
     expect(deliveries.size).to eq(0)
 
-    expect(ExcelAnalyzerNotifier).to receive(:report).
+    expect(ExcelAnalyzer::NotifierMailer).to receive(:report).
       with(attachment, blob.metadata).
       and_call_original
     ExcelAnalyzer.on_hidden_metadata.call(blob, blob.metadata)
@@ -57,50 +57,3 @@
     end
   end
 end
-
-RSpec.describe 'ExcelAnalyzerNotifier report mail' do
-  let(:message) { FactoryBot.create(:incoming_message, sent_at: Time.now) }
-  let(:attachment) { message.foi_attachments.first }
-  let(:blob) { attachment.file_blob }
-
-  let(:mail) do
-    ExcelAnalyzerNotifier.report(attachment, blob.metadata).deliver_now
-    ActionMailer::Base.deliveries[-1]
-  end
-
-  around do |example|
-    to = ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL']
-    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = 'excel@localhost'
-    example.call
-    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = to
-  end
-
-  after { ActionMailer::Base.deliveries.clear }
-
-  it 'has custom mail headers' do
-    expect(mail['X-WDTK-Contact'].value).to eq('wdtk-excel-analyzer-report')
-    expect(mail['X-WDTK-CaseRef'].value).to eq(attachment.to_param)
-  end
-
-  it 'sents mail from and to the correct addresses' do
-    expect(mail.from).to include(blackhole_email)
-    expect(mail.to).to include(ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'])
-  end
-
-  it 'has attachment ID in the subject' do
-    expect(mail.subject).
-      to eq("ExcelAnalyzer: hidden data detected [#{attachment.id}]")
-  end
-
-  it 'includes the admin URL in the body' do
-    expect(mail.body).
-      to include("http://test.host/admin/requests/#{message.info_request_id}")
-    expect(mail.body).
-      to include("http://test.host/admin/attachments/#{attachment.id}/edit")
-  end
-
-  it 'includes the metadata in the body' do
-    allow(blob).to receive(:metadata).and_return(foo: 'bar')
-    expect(mail.body).to include("foo: bar")
-  end
-end
diff --git a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
new file mode 100644
index 00000000..ec47d80f
--- /dev/null
+++ b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
@@ -0,0 +1,48 @@
+require_relative '../../spec_helper'
+
+RSpec.describe ExcelAnalyzer::NotifierMailer do
+  let(:message) { FactoryBot.create(:incoming_message) }
+  let(:attachment) { message.foi_attachments.first }
+  let(:blob) { attachment.file_blob }
+
+  let(:mail) do
+    described_class.report(attachment, blob.metadata).deliver_now
+    ActionMailer::Base.deliveries[-1]
+  end
+
+  around do |example|
+    to = ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL']
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = 'excel@localhost'
+    example.call
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = to
+  end
+
+  after { ActionMailer::Base.deliveries.clear }
+
+  it 'has custom mail headers' do
+    expect(mail['X-WDTK-Contact'].value).to eq('wdtk-excel-analyzer-report')
+    expect(mail['X-WDTK-CaseRef'].value).to eq(attachment.to_param)
+  end
+
+  it 'sents mail from and to the correct addresses' do
+    expect(mail.from).to include(blackhole_email)
+    expect(mail.to).to include(ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'])
+  end
+
+  it 'has attachment ID in the subject' do
+    expect(mail.subject).
+      to eq("ExcelAnalyzer: hidden data detected [#{attachment.id}]")
+  end
+
+  it 'includes the admin URL in the body' do
+    expect(mail.body).
+      to include("http://test.host/admin/requests/#{message.info_request_id}")
+    expect(mail.body).
+      to include("http://test.host/admin/attachments/#{attachment.id}/edit")
+  end
+
+  it 'includes the metadata in the body' do
+    allow(blob).to receive(:metadata).and_return(foo: 'bar')
+    expect(mail.body).to include('foo: bar')
+  end
+end

From 38e398493686c1d1298e1b094f76cf3a29207f50 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Thu, 23 May 2024 16:10:27 +0100
Subject: [PATCH 3/6] [ExcelAnalyzer] add job to do extra PII checks

---
 lib/excel_analyzer.rb                         |  6 ++-
 lib/excel_analyzer/jobs/pii_badger_job.rb     | 33 ++++++++++++
 spec/excel_analyzer/hook_spec.rb              | 12 +----
 .../jobs/pii_badger_job_spec.rb               | 52 +++++++++++++++++++
 4 files changed, 91 insertions(+), 12 deletions(-)
 create mode 100644 lib/excel_analyzer/jobs/pii_badger_job.rb
 create mode 100644 spec/excel_analyzer/jobs/pii_badger_job_spec.rb

diff --git a/lib/excel_analyzer.rb b/lib/excel_analyzer.rb
index 4ffb5903..248d2451 100644
--- a/lib/excel_analyzer.rb
+++ b/lib/excel_analyzer.rb
@@ -4,9 +4,11 @@
   Dir[File.join(File.dirname(__FILE__), 'excel_analyzer', '**/')].each do
     loader.push_dir(_1, namespace: ExcelAnalyzer)
   end
+
+  loader.inflector.inflect("pii_badger_job" => "PIIBadgerJob")
 end
 
-ExcelAnalyzer.on_hidden_metadata = ->(attachment_blob, metadata) do
+ExcelAnalyzer.on_hidden_metadata = ->(attachment_blob, _) do
   foi_attachment = FoiAttachment.joins(:file_blob).
     find_by(active_storage_blobs: { id: attachment_blob })
 
@@ -24,5 +26,5 @@
     }
   )
 
-  ExcelAnalyzer::NotifierMailer.report(foi_attachment, metadata).deliver_now
+  ExcelAnalyzer::PIIBadgerJob.perform_later(attachment_blob)
 end
diff --git a/lib/excel_analyzer/jobs/pii_badger_job.rb b/lib/excel_analyzer/jobs/pii_badger_job.rb
new file mode 100644
index 00000000..23b667b3
--- /dev/null
+++ b/lib/excel_analyzer/jobs/pii_badger_job.rb
@@ -0,0 +1,33 @@
+##
+# Job to run additional Personally Identifiable Information (PII) checks on
+# files stored as ActiveStorage::Blob
+#
+# Examples:
+#   ExcelAnalyzer::PIIBadgerJob.perform(ActiveStorage::Blob.first)
+#
+module ExcelAnalyzer
+  class PIIBadgerJob < ApplicationJob
+    queue_as :excel_analyzer
+
+    def perform(attachment_blob)
+      foi_attachment = FoiAttachment.joins(:file_blob).
+        find_by(active_storage_blobs: { id: attachment_blob })
+
+      attachment_blob.open(tmpdir: ENV['EXCEL_ANALYZER_TMP_DIR']) do |file|
+        cmd = [
+          ENV['EXCEL_ANALYZER_PII_BADGER_COMMAND'], '--file', file.path
+        ].join(' ')
+
+        pii_badger_metadata = IO.popen(cmd) { JSON.parse(_1.read) }
+
+        attachment_blob.update(metadata: attachment_blob.metadata.merge(
+          pii_badger: pii_badger_metadata
+        ))
+      end
+
+      ExcelAnalyzer::NotifierMailer.report(
+        foi_attachment, attachment_blob.metadata[:excel]
+      ).deliver_now
+    end
+  end
+end
diff --git a/spec/excel_analyzer/hook_spec.rb b/spec/excel_analyzer/hook_spec.rb
index a67b05df..e7fd3bc4 100644
--- a/spec/excel_analyzer/hook_spec.rb
+++ b/spec/excel_analyzer/hook_spec.rb
@@ -26,17 +26,9 @@
     TXT
   end
 
-  it 'sents report email' do
-    deliveries = ActionMailer::Base.deliveries
-    expect(deliveries.size).to eq(0)
-
-    expect(ExcelAnalyzer::NotifierMailer).to receive(:report).
-      with(attachment, blob.metadata).
-      and_call_original
+  it 'queues PII Badger job' do
+    expect(ExcelAnalyzer::PIIBadgerJob).to receive(:perform_later).with(blob)
     ExcelAnalyzer.on_hidden_metadata.call(blob, blob.metadata)
-    expect(deliveries.size).to eq(1)
-
-    deliveries.clear
   end
 
   context 'when message was sent more than 1 day ago' do
diff --git a/spec/excel_analyzer/jobs/pii_badger_job_spec.rb b/spec/excel_analyzer/jobs/pii_badger_job_spec.rb
new file mode 100644
index 00000000..33e2d7ee
--- /dev/null
+++ b/spec/excel_analyzer/jobs/pii_badger_job_spec.rb
@@ -0,0 +1,52 @@
+require_relative '../../spec_helper'
+
+RSpec.describe ExcelAnalyzer::PIIBadgerJob, type: :job do
+  let(:message) { FactoryBot.create(:incoming_message) }
+  let(:attachment) { message.foi_attachments.first }
+  let(:blob) { attachment.file_blob }
+
+  let(:excel_metadata) { { foo: 'baz' } }
+  let(:pii_metadata) { { bar: 'baz' } }
+
+  around do |example|
+    to = ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL']
+    cmd = ENV['EXCEL_ANALYZER_PII_BADGER_COMMAND']
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = 'excel@localhost'
+    ENV['EXCEL_ANALYZER_PII_BADGER_COMMAND'] = '/usr/bin/pii_badger.sh'
+    example.call
+    ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = to
+    ENV['EXCEL_ANALYZER_PII_BADGER_COMMAND'] = cmd
+  end
+
+  before do
+    blob.update(metadata: blob.metadata.merge(excel: excel_metadata))
+    allow(IO).to receive(:popen).and_return(pii_metadata)
+  end
+
+  def perform
+    described_class.new.perform(blob)
+  end
+
+  it 'calls external command' do
+    expect(IO).to receive(:popen).with(%r(^/usr/bin/pii_badger.sh --file /.*$))
+    perform
+  end
+
+  it 'updates the blob metadata' do
+    expect { perform }.to change(blob, :metadata)
+    expect(blob.metadata).to include(pii_badger: pii_metadata)
+  end
+
+  it 'sents report email' do
+    deliveries = ActionMailer::Base.deliveries
+    expect(deliveries.size).to eq(0)
+
+    expect(ExcelAnalyzer::NotifierMailer).to receive(:report).
+      with(attachment, blob.metadata[:excel]).
+      and_call_original
+    perform
+    expect(deliveries.size).to eq(1)
+
+    deliveries.clear
+  end
+end

From 06a0e93e142a834d18b21ef0a83a9eaa56da93aa Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 21 May 2024 17:02:30 +0100
Subject: [PATCH 4/6] [ExcelAnalyzer] update notifier mailer

Pass blob to the report action. Will allow us to store additional
metadata which can be reported in the template.
---
 lib/excel_analyzer/jobs/pii_badger_job.rb                | 7 +------
 lib/excel_analyzer/mailers/notifier_mailer.rb            | 9 +++++----
 lib/views/excel_analyzer/notifier_mailer/report.text.erb | 2 +-
 spec/excel_analyzer/jobs/pii_badger_job_spec.rb          | 3 +--
 spec/excel_analyzer/mailers/notifier_mailer_spec.rb      | 8 ++++++--
 5 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/lib/excel_analyzer/jobs/pii_badger_job.rb b/lib/excel_analyzer/jobs/pii_badger_job.rb
index 23b667b3..e222110d 100644
--- a/lib/excel_analyzer/jobs/pii_badger_job.rb
+++ b/lib/excel_analyzer/jobs/pii_badger_job.rb
@@ -10,9 +10,6 @@ class PIIBadgerJob < ApplicationJob
     queue_as :excel_analyzer
 
     def perform(attachment_blob)
-      foi_attachment = FoiAttachment.joins(:file_blob).
-        find_by(active_storage_blobs: { id: attachment_blob })
-
       attachment_blob.open(tmpdir: ENV['EXCEL_ANALYZER_TMP_DIR']) do |file|
         cmd = [
           ENV['EXCEL_ANALYZER_PII_BADGER_COMMAND'], '--file', file.path
@@ -25,9 +22,7 @@ def perform(attachment_blob)
         ))
       end
 
-      ExcelAnalyzer::NotifierMailer.report(
-        foi_attachment, attachment_blob.metadata[:excel]
-      ).deliver_now
+      ExcelAnalyzer::NotifierMailer.report(attachment_blob).deliver_now
     end
   end
 end
diff --git a/lib/excel_analyzer/mailers/notifier_mailer.rb b/lib/excel_analyzer/mailers/notifier_mailer.rb
index a09bd9c7..e8d20765 100644
--- a/lib/excel_analyzer/mailers/notifier_mailer.rb
+++ b/lib/excel_analyzer/mailers/notifier_mailer.rb
@@ -3,10 +3,11 @@ class NotifierMailer < ApplicationMailer
     include Rails.application.routes.url_helpers
     default_url_options[:host] = AlaveteliConfiguration.domain
 
-    def report(foi_attachment, metadata)
-      @foi_attachment = foi_attachment
-      @incoming_message = foi_attachment.incoming_message
-      @metadata = metadata
+    def report(attachment_blob)
+      @foi_attachment = FoiAttachment.joins(:file_blob).
+        find_by(active_storage_blobs: { id: attachment_blob })
+      @incoming_message = @foi_attachment.incoming_message
+      @metadata = attachment_blob.metadata
 
       from = email_address_with_name(
         blackhole_email, 'WhatDoTheyKnow.com Excel Analyzer report'
diff --git a/lib/views/excel_analyzer/notifier_mailer/report.text.erb b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
index 00534933..b506e523 100644
--- a/lib/views/excel_analyzer/notifier_mailer/report.text.erb
+++ b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
@@ -5,7 +5,7 @@ Admin request URL: <%= admin_request_url(@incoming_message.info_request_id) %>
 Admin attachment URL: <%= edit_admin_foi_attachment_url(@foi_attachment) %>
 
 The following was detected:
-<% @metadata.each do |key, value| %>
+<% @metadata[:excel].each do |key, value| %>
   <%= key %>: <%= value %>
 <% end %>
 
diff --git a/spec/excel_analyzer/jobs/pii_badger_job_spec.rb b/spec/excel_analyzer/jobs/pii_badger_job_spec.rb
index 33e2d7ee..cd251e8a 100644
--- a/spec/excel_analyzer/jobs/pii_badger_job_spec.rb
+++ b/spec/excel_analyzer/jobs/pii_badger_job_spec.rb
@@ -41,8 +41,7 @@ def perform
     deliveries = ActionMailer::Base.deliveries
     expect(deliveries.size).to eq(0)
 
-    expect(ExcelAnalyzer::NotifierMailer).to receive(:report).
-      with(attachment, blob.metadata[:excel]).
+    expect(ExcelAnalyzer::NotifierMailer).to receive(:report).with(blob).
       and_call_original
     perform
     expect(deliveries.size).to eq(1)
diff --git a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
index ec47d80f..fa19519b 100644
--- a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
+++ b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
@@ -6,7 +6,7 @@
   let(:blob) { attachment.file_blob }
 
   let(:mail) do
-    described_class.report(attachment, blob.metadata).deliver_now
+    described_class.report(blob).deliver_now
     ActionMailer::Base.deliveries[-1]
   end
 
@@ -17,6 +17,10 @@
     ENV['EXCEL_ANALYZER_NOTIFICATION_EMAIL'] = to
   end
 
+  before do
+    allow(blob).to receive(:metadata).and_return(excel: {})
+  end
+
   after { ActionMailer::Base.deliveries.clear }
 
   it 'has custom mail headers' do
@@ -42,7 +46,7 @@
   end
 
   it 'includes the metadata in the body' do
-    allow(blob).to receive(:metadata).and_return(foo: 'bar')
+    allow(blob).to receive(:metadata).and_return(excel: { foo: 'bar' })
     expect(mail.body).to include('foo: bar')
   end
 end

From b64239d1d11f2311c151fff32ee54c16351acbe0 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Thu, 23 May 2024 16:15:42 +0100
Subject: [PATCH 5/6] [ExcelAnalyzer] refactor metadata rendering

---
 lib/views/excel_analyzer/notifier_mailer/report.text.erb | 5 +----
 spec/excel_analyzer/mailers/notifier_mailer_spec.rb      | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/lib/views/excel_analyzer/notifier_mailer/report.text.erb b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
index b506e523..d1840b8f 100644
--- a/lib/views/excel_analyzer/notifier_mailer/report.text.erb
+++ b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
@@ -4,9 +4,6 @@ spreadsheet due to the detection of potentially suspect hidden data.
 Admin request URL: <%= admin_request_url(@incoming_message.info_request_id) %>
 Admin attachment URL: <%= edit_admin_foi_attachment_url(@foi_attachment) %>
 
-The following was detected:
-<% @metadata[:excel].each do |key, value| %>
-  <%= key %>: <%= value %>
-<% end %>
+Excel Analyzer metadata: <%= JSON.pretty_generate(@metadata[:excel]) %>
 
 Please review the file carefully.
diff --git a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
index fa19519b..4a23f4b0 100644
--- a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
+++ b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
@@ -47,6 +47,6 @@
 
   it 'includes the metadata in the body' do
     allow(blob).to receive(:metadata).and_return(excel: { foo: 'bar' })
-    expect(mail.body).to include('foo: bar')
+    expect(mail.body).to include('"foo": "bar"')
   end
 end

From abe7b7736814dfa58002b19ed0fa02c0de8de88a Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Thu, 23 May 2024 16:30:45 +0100
Subject: [PATCH 6/6] [ExcelAnalyzer] add PII output to report email

---
 lib/views/excel_analyzer/notifier_mailer/report.text.erb | 2 ++
 spec/excel_analyzer/mailers/notifier_mailer_spec.rb      | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/views/excel_analyzer/notifier_mailer/report.text.erb b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
index d1840b8f..076895de 100644
--- a/lib/views/excel_analyzer/notifier_mailer/report.text.erb
+++ b/lib/views/excel_analyzer/notifier_mailer/report.text.erb
@@ -6,4 +6,6 @@ Admin attachment URL: <%= edit_admin_foi_attachment_url(@foi_attachment) %>
 
 Excel Analyzer metadata: <%= JSON.pretty_generate(@metadata[:excel]) %>
 
+PII Badger metadata: <%= JSON.pretty_generate(@metadata[:pii_badger]) %>
+
 Please review the file carefully.
diff --git a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
index 4a23f4b0..eafa3ad6 100644
--- a/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
+++ b/spec/excel_analyzer/mailers/notifier_mailer_spec.rb
@@ -46,7 +46,10 @@
   end
 
   it 'includes the metadata in the body' do
-    allow(blob).to receive(:metadata).and_return(excel: { foo: 'bar' })
+    allow(blob).to receive(:metadata).and_return(
+      excel: { foo: 'bar' }, pii_badger: { baz: 'qux' }
+    )
     expect(mail.body).to include('"foo": "bar"')
+    expect(mail.body).to include('"baz": "qux"')
   end
 end