Skip to content

Quarantine redacted media #593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 17 commits into from
May 12, 2025
Merged

Quarantine redacted media #593

merged 17 commits into from
May 12, 2025

Conversation

@Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Mar 24, 2025 •
edited
Loading

This adds a new flag --quarantine to some commands that redact.

Issues:

  • This should run at a lower priority to redactions themselves, but currently there isn't a priority queue mechanism.
  • Media beyond the (current) 4 day window won't be caught.
  • All media is treated as unsafe if it gets redacted, even innocents. It might be wise to have a way to undo this change.
  • There may be a light DOS thing here where innocent media could be included in an event (e.g. user avatars) that then becomes quarantined as a result. We now only run when the user requests it to, reducing this.

@Half-Shot Half-Shot marked this pull request as ready for review March 31, 2025 13:29
@Half-Shot Half-Shot requested a review from a team as a code owner March 31, 2025 13:29
H-Shay
H-Shay reviewed Mar 31, 2025
View reviewed changes
@H-Shay
Copy link
Contributor

H-Shay commented Mar 31, 2025

Your points 3 and 4 are slightly concerning to me - I am wondering if it makes sense to have this functionality behind a flag of some sort so it isn't the default, and is rather something that mods can opt in and out of given the circumstances?

@Half-Shot
Copy link
Contributor Author

Your points 3 and 4 are slightly concerning to me - I am wondering if it makes sense to have this functionality behind a flag of some sort so it isn't the default, and is rather something that mods can opt in and out of given the circumstances?

Ack. This now only runs explicitly, except for the NSFW protection which has an optional setting for automatics.

H-Shay
H-Shay reviewed Apr 1, 2025
View reviewed changes
H-Shay
H-Shay reviewed Apr 1, 2025
View reviewed changes
H-Shay
H-Shay reviewed Apr 1, 2025
View reviewed changes
H-Shay
H-Shay reviewed Apr 1, 2025
View reviewed changes
H-Shay
H-Shay reviewed Apr 1, 2025
View reviewed changes
H-Shay
H-Shay requested changes Apr 1, 2025
View reviewed changes
Copy link
Contributor

@H-Shay H-Shay left a comment

Choose a reason for hiding this comment

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

Generally looks great, thanks for doing this! I just noted a few things, and was wondering if you'd be willing to add tests for the new QuarantineMedia command.

@H-Shay
Copy link
Contributor

H-Shay commented Apr 17, 2025

This looks broadly good to me, thanks for taking it on! Looks like one of the new tests is failing though:

1) Test: The redaction command - if admin
2025-04-17T14:21:28.5356933Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run:        Correctly quarantines media after being redacted:
2025-04-17T14:21:28.5358786Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run:      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
2025-04-17T14:21:28.5359975Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: + actual - expected
2025-04-17T14:21:28.5360743Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: 
2025-04-17T14:21:28.5361541Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: + 'aolPCYRRQyQUAihMHRgprtdt'
2025-04-17T14:21:28.5362448Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: - MXCUrl {
2025-04-17T14:21:28.5362962Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: -   domain: 'localhost:9999',
2025-04-17T14:21:28.5363913Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: -   mediaId: 'aolPCYRRQyQUAihMHRgprtdt'
2025-04-17T14:21:28.5364782Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run: - }
2025-04-17T14:21:28.5365735Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run:       at Context.<anonymous> (test/integration/commands/redactCommandTest.ts:241:16)
2025-04-17T14:21:28.5367471Z [2025-04-17T14:21:28Z INFO  mx_tester::exec] run:       at processTicksAndRejections (node:internal/process/task_queues:95:5)

Otherwise I think this is good to go.

@Half-Shot Half-Shot requested a review from H-Shay April 17, 2025 15:59
@H-Shay H-Shay merged commit c0653b1 into main May 12, 2025
5 checks passed
@H-Shay H-Shay deleted the hs/media-quarantine-on-redact branch May 12, 2025 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@H-Shay H-Shay H-Shay approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Half-Shot @H-Shay