[#8823] Track when a CensorRule redacts something - #9505
Open
garethrees wants to merge 7 commits into
Open
Conversation
garethrees
force-pushed
the
8823-censor-rule-application-tracking-v3
branch
4 times, most recently
from
September 3, 2026 06:30
80f49ec to
3317871
Compare
garethrees
force-pushed
the
8823-censor-rule-application-tracking-v3
branch
2 times, most recently
from
September 4, 2026 13:48
8055402 to
1003a63
Compare
Within limit is ideal, but like the other allowed patterns, one longer line is clearer than breaking over several in this context.
We'll want to use apply_masks in places where the record is not persisted (e.g. preview before send, etc) so make sure this method can work in both contexts.
garethrees
force-pushed
the
8823-censor-rule-application-tracking-v3
branch
from
September 4, 2026 15:06
1003a63 to
6add5a1
Compare
Most of the time we're applying to plain text so make this the default. Went with an argument with default rather than a kwarg to reduce change at call-sites. Could revisit in future but I think this is fine for now given some of this may change if we manage to implement a more unified redaction interface.
Call apply_masks on the record whose content is being masked, rather than on an ancestor record, so that each record is responsible for masking its own content. IncomingMessage, OutgoingMessage and FoiAttachment all delegate apply_masks to the InfoRequest, so moving the receiver is not a behavioural change in itself. Where we were applying censor rules directly, use the full mask pipeline so that text masks get applied too.
Record a CensorRule::Redaction row per (rule, redactable, attribute) whenever a rule's application to text or binary content actually changes it, so we can later show which records a given rule has affected. This tracks which attribute a redaction came from (body, from_name, filename) rather than just the redactable record, so a row for one attribute can't be deleted by a different attribute's check finding no match on the same object (e.g. FoiAttachment has both filename and body; a no-match on body would otherwise wipe a real filename match). redacted_attribute is required whenever a redactable is given, and CensorRule::Redaction. Don't leave orphan CensorRule::Redaction records if redactable is destroyed. Calling apply_masks on an IncomingMessage requires the redacted_attribute keyword. We can omit that when calling the InfoRequest version, so we've used this for script/redact-raw-emails.rb as in this case we're actually redacting parts of a Mail object rather than an IncomingMessage record.
Show, per request, which records a censor rule has redacted. A "Records redacted" column on the existing censor rule list links through to a new redactions index page listing the records that the rule has actually modified.
garethrees
force-pushed
the
8823-censor-rule-application-tracking-v3
branch
from
September 4, 2026 15:21
6add5a1 to
bcdf084
Compare
garethrees
marked this pull request as ready for review
September 4, 2026 15:21
garethrees
commented
Sep 4, 2026
| module CensorRule::Recordable | ||
| extend ActiveSupport::Concern | ||
|
|
||
| PATTERN_COLUMNS = %w[text regexp case_sensitive ignore_diacritics].freeze |
Member
Author
There was a problem hiding this comment.
couldn't think of a better name – its the attributes where, if changed, may result in different content being redacted so we'd reprocess (but e.g. changing the last edit comment wouldn't make a difference to the redacted content)
| info_request.try(:applicable_censor_rules) or [] | ||
| end | ||
|
|
||
| # FIXME: We appear not to be applying text masks here |
Member
Author
There was a problem hiding this comment.
Not sure why we aren't applying masks here. I haven't looked in to why as it's a bit outside the scope of the PR and this one is a bit more consequential. May revisit before merge but think we can get by without doing so.
|
|
||
| validates_presence_of :redacted_attribute | ||
|
|
||
| scope :for_request, ->(info_request) { |
Member
Author
There was a problem hiding this comment.
Might be a simpler way of writing this – please do suggest improvements!
| @@ -0,0 +1,40 @@ | |||
| # Record when a CensorRule has actually been applied to a Redactable | |||
| module CensorRule::Recordable | |||
Member
Author
There was a problem hiding this comment.
Not sure this is the best name either, but didn't want to clash with Redactable – the concern for including in records that can be redacted.
Member
Author
|
I haven’t fully tested it since making a couple of small tweaks, adding some tests and rebasing but it should be okay to start taking a look through. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issue(s)
Fixes #8823
What does this do?
Record a CensorRule::Redaction row per (rule, redactable, attribute) whenever a rule's application to text or binary content actually changes it, so we can later show which records a given rule has affected.
Why was this needed?
When many censor rules could apply to something, its not easy to determine which ones did, if they did at all.
This is confusing for admins, and also when making erasure decisions in the code (e.g. in #9138)
Implementation notes
I was originally going to store the diff of what was removed by the redaction so that we could use this as a basis for admin search, but that got complicated so for now just adding some basic tracking.
Screenshots
Showing which redactions are actually doing something on the request page:
Clicking through shows which child records the redaction has been applied to:
Notes to reviewer