Skip to content
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.

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

Fix d4all protections #284

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions src/protections/ProtectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,6 @@ import { MatrixDataManager, RawSchemedData, SchemaMigration, SCHEMA_VERSION_KEY
import { Permalinks } from "../commands/interface-manager/Permalinks";
import { CommandExceptionKind } from "../commands/interface-manager/CommandException";

const PROTECTIONS: Protection[] = [
new FirstMessageIsImage(),
new BanPropagation(),
new BasicFlooding(),
new WordList(),
new MessageIsVoice(),
new MessageIsMedia(),
new TrustedReporters(),
new DetectFederationLag(),
new JoinWaveShortCircuit(),
];

const ENABLED_PROTECTIONS_EVENT_TYPE = "org.matrix.mjolnir.enabled_protections";
type EnabledProtectionsEvent = RawSchemedData & {
enabled: string[],
Expand All @@ -66,11 +54,7 @@ class EnabledProtectionsManager extends MatrixDataManager<EnabledProtectionsEven
protected readonly schema: SchemaMigration[] = [
async function enableBanPropagationByDefault(input: EnabledProtectionsEvent) {
const enabled = new Set(input.enabled);
const banPropagationProtection = PROTECTIONS.find(p => p.name === 'BanPropagationProtection');
if (banPropagationProtection === undefined) {
throw new TypeError("Couldn't find the ban propagation protection");
}
enabled.add(banPropagationProtection.name)
enabled.add("BanPropagationProtection")
return {
enabled: [...enabled],
[SCHEMA_VERSION_KEY]: 1,
Expand All @@ -81,9 +65,14 @@ class EnabledProtectionsManager extends MatrixDataManager<EnabledProtectionsEven
private readonly enabledProtections = new Set</* protection name */string>();

constructor(
private readonly mjolnir: Mjolnir
private readonly mjolnir: Mjolnir,
private readonly protections: Protection[]
) {
super()
const banPropagationProtection = this.protections.find(p => p.name === 'BanPropagationProtection');
Copy link
Member

Choose a reason for hiding this comment

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

would be cool if we could make 'BanPropagationProtection' a static name field on the BanPropagationProtection class so we don't have to risk typoing it. Which would then remove the need for this check

if (banPropagationProtection === undefined) {
throw new TypeError("Couldn't find the ban propagation protection");
}
}

protected async requestMatrixData(): Promise<unknown> {
Expand Down Expand Up @@ -146,9 +135,21 @@ export class ProtectionManager {
get protections(): Readonly<Map<string /* protection name */, Protection>> {
return this._protections;
}
private readonly protection_classes: Protection[] = [
Copy link
Member

Choose a reason for hiding this comment

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

NIT: Let's stick to the naming convention of protectionClasses please, sorry if that's been butchered in some other place.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm seems like the _protections member is already doing this job...

new FirstMessageIsImage(),
new BanPropagation(),
new BasicFlooding(),
new WordList(),
new MessageIsVoice(),
new MessageIsMedia(),
new TrustedReporters(),
new DetectFederationLag(),
new JoinWaveShortCircuit(),
];


constructor(private readonly mjolnir: Mjolnir) {
this.enabledProtectionsManager = new EnabledProtectionsManager(this.mjolnir);
this.enabledProtectionsManager = new EnabledProtectionsManager(this.mjolnir, this.protection_classes);
}

/*
Expand All @@ -159,7 +160,7 @@ export class ProtectionManager {
await this.enabledProtectionsManager.start();
this.mjolnir.reportManager.on("report.new", this.handleReport.bind(this));
this.mjolnir.matrixEmitter.on("room.event", this.handleEvent.bind(this));
for (const protection of PROTECTIONS) {
for (const protection of this.protection_classes) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could just move the list of protections (formerly) PROTECTIONS to here? and then pass _protections to the EnabledProtectionsManager`?

try {
await this.registerProtection(protection);
} catch (e) {
Expand Down
Loading