Skip to content

Add Redactable - #9347

Draft
garethrees wants to merge 13 commits into
developfrom
8823-redactable-2
Draft

Add Redactable#9347
garethrees wants to merge 13 commits into
developfrom
8823-redactable-2

Conversation

@garethrees

Copy link
Copy Markdown
Member

Relevant issue(s)

What does this do?

Adds explicit interfaces for getting redacted/unredacted versions of content

Why was this needed?

  • Make it easier to get different versions of content
  • Reduce number of methods / method overrides that currently do the above
  • Protects against information leaks by forcing developers to explicitly permit unredacted access to data

Implementation notes

Here's how this will work:

class User < ApplicationRecord
  include Redactable
  redactable :from_name
end

user.censor_rules.create!(
  text: 'Bob Smith',
  replacement: '[name removed]',
  last_edit_editor: 'x',
  last_edit_comment: 'x'
)

msg = user.info_requests.first.outgoing_messages.first

msg.from_name
# => "[name removed]"

msg.unredacted.from_name
# => "Bob Smith"

Notes to reviewer

While this does force explicit unredacted permission, it still requires us to remember to add attributes to redactable. There's an alternative version of this where we by default assume attributes will hold PII unless explicitly told they won't, but that feels like it would get a bit too annoying?

At the moment this applies both censor rules and text masks. We may want to split these up so that we can do e.g. (record.censored.unmasked.foo).


Have you updated the changelog? If this is not necessary, put square brackets around this: [skip changelog]

@garethrees
garethrees force-pushed the 8823-redactable-2 branch from 4ec9a88 to 08ffbb4 Compare July 3, 2026 15:46
* Allow records to set redactable attributes
* Redact redactable attributes by default
* Add an #unredacted method to get the unredacted version of a redacted
  attribute
* Allow classes to define an `apply_masks_to_{{attr}}` method for custom
  mask behaviour for given records. This is most likely to be necessary
  when applying masks to a non text/plain content type
* Reduce complexity of fallback
* Redactions are now handled by OutgoingMessage so
  don't need to pass the censor rules through
Issues with title/slug interaction; title isn't redacted at the moment
anyway
Removed redactable from InfoRequest for now
The original body method has important self-healing logic: if the
attachment record is stale (hexdigest doesn't match), it calls
FoiAttachment::MaskJob to reparse the raw email and rebuild the
attachment. But since redactable prepends a module that intercepts every
call to #body, apply_masks_to_body now runs instead of the real method —
and it re-derives content from mail_attributes directly, with no
rebuild/retry logic at all.

So when an attachment's hexdigest is stale, mail_attributes raises
FoiAttachment::MissingError immediately. That bubbles up to
IncomingMessage#get_main_body_text_internal, which does rescue
FoiAttachment::MissingError; retry — expecting the rebuild to have
already happened inside body. It never does, so it retries forever,
re-parsing the raw email each iteration, until memory/disk usage
explodes and Docker OOM-kills the container.

apply_masks_to_body now sources from unredacted.body instead of
unmasked_body, so it routes through the real body method's rebuild logic
instead of bypassing it.
When attributes are genuinely nil we still want to return nil rather
than an empty string, even though we're now passing these attributes
through the redactable pipeline.
Need to explicitly check the unredacted email
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.

1 participant