Skip to content

Conversation

@TheArcaneBrony
Copy link
Member

Description of changes

NOTE: This PR is a successor of #398457.
Scope change: The package has been merged in #398489! 🎉

Original description:

Draupnir is a hardfork of Mjolnir. Mjolnir package has been unmaintained due to upstream bugs. This package is mostly a drop-in replacement. This package also uses newer methods of handling dependencies.

Repository can be found at:
https://github.com/the-draupnir-project/Draupnir

Note on testing: package and module were (and still are) tested in our production environment, works fine as far as it's been used.
Note on replacing Mjolnir: Not sure whether this is appropriate, due to general usage of the bot changing. You can however fully automatically migrate from mjolnir to draupnir.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 19, 2025
@h7x4 h7x4 added 8.has: module (new) This PR adds a module in `nixos/` 8.has: tests This PR has tests labels Apr 23, 2025
@teutat3s
Copy link
Member

teutat3s commented May 3, 2025

@TheArcaneBrony feel free to cherry-pick my commit to fix the manual build: teutat3s@5b44d46

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label May 17, 2025
@sefidel
Copy link
Member

sefidel commented May 23, 2025

We probably need a way to securely provide the synapseHTTPAntispam.authorization value for configuring some of the antispam protections (some option like accessTokenFile)

@teutat3s
Copy link
Member

I did a first teutat3s@726c08d stab teutat3s@92dcdc4 at it in my fork of this PR.

@mweinelt
Copy link
Member

The changes you applied from me require hooking up the flag within the test. Right now both test cases are the same.

@emilylange emilylange changed the title NixOS/Draupnir: init nixos/draupnir: init May 26, 2025
@emilylange
Copy link
Member

Marking this as draft because it still needs some work (e.g. the tests don't eval for multiple reasons).

@TheArcaneBrony would you be fine if I implement the remaining To-Dos and review feedback by simply pushing the changes to your branch/PR? :)

@emilylange emilylange marked this pull request as draft May 26, 2025 12:17
@TheArcaneBrony
Copy link
Member Author

@emilylange i suppose you have push access, so go for it :) i can sync whenever
TIL the tests dont eval

@ethancedwards8
Copy link
Member

@emilylang @TheArcaneBrony I am a summer of nix 2025 participant and this is one of our intended packages. If it is okay with yall, I would be more than happy to take over and start working on it (and I would provide credit to you both). Let me know what you think.

@emilylange
Copy link
Member

@ethancedwards8 hi, thanks for asking.
I think we are good as-is.
I already worked on this over the past few days and made some calls like dropping the pantalaimon options entirely.
There is no much left to do.
But feel free to review the changes once I pushed my local changes today or over the next few days.

@ethancedwards8
Copy link
Member

Sounds good. Thank you! We thought we would offer :). I would be more than happy to help review.

@github-actions github-actions bot added the 8.has: changelog This PR adds or changes release notes label Jun 5, 2025
@emilylange emilylange changed the title nixos/draupnir: init nixos/draupnir: init, nixosTests.draupnir: init Jun 5, 2025
Copy link
Member

@emilylange emilylange left a comment

Choose a reason for hiding this comment

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

I believe this is ready to review now :)

Comment on lines +127 to +130
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Previously this had serviceConfig.StartLimitBurst= "infinity" which was ignored because it should have been in unitConfig.

I am not sure if I agree on having draupnir.service restart indefinitely, but I suppose there is a case where one does not want one's moderation bot be down indefinitely because something caused it to crash.

Either way, this fixes the implementation of those endless restarts.
Open to discuss.

Copy link
Member Author

Choose a reason for hiding this comment

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

Keep in mind Draupnir hardcrashes by design on seemingly benign errors, like being unable to sync due to slow server startup etc.

Comment on lines 39 to 51
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in #400194 (comment), I removed the Pantalaimon "shortcut" options.
Initially they were simply copied over from nixos/mjolnir.
I have fairly strong opinions on this. Pantalaimon is deprecated, marked as insecure in nixpkgs and should no longer be used.

Upstream is also very vocal about not recommending using Pantalaimon with Draupnir:

Those that for some reason want to use Pantalaimon with Draupnir anyway can configure services.pantalaimon-headless.instances on their own.
I do not believe carrying over the options from services.mjolnir.pantalaimon is worth the complexity and a VM test for it would not be run by hydra.nixos.org anyway because olm is marked as insecure and thus skipped.

Comment on lines +27 to +40
Copy link
Member

Choose a reason for hiding this comment

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

RFC 42 for those coming from services.mjolnir.

Comment on lines 53 to 76
Copy link
Member

Choose a reason for hiding this comment

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

I kept it at top-level.
Should be easier to spot this way and I don't think there are even remotely enough secrets to justify making this truly generic.
I am open to move those into services.draupnir.secrets.accessToken and services.draupnir.secrets.pantalaimon.password nonetheless.
Just not free-form.

https://search.nixos.org/options?channel=unstable&show=services.forgejo.secrets for reference by what I mean by "truly generic".

Copy link
Member

Choose a reason for hiding this comment

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

With the addition of third secret for --http-antispam-authorization-path/web.synapseHTTPAntispam.authorization via #400194 (comment) I decided to move the secrets from top-level to services.draupnir.secrets, matching the structure of services.draupnir.settings:

  • cfg.secrets.accessToken for cfg.settings.accessToken
  • cfg.secrets.pantalaimon.password for cfg.settings.pantalaimon.password
  • cfg.secrets.web.synapseHTTPAntispam.authorization for cfg.settings.web.synapseHTTPAntispam.authorization

Copy link
Member

Choose a reason for hiding this comment

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

I suggest deferring sandboxing to some future PR, if someone is interested in this.
I removed some options that are already implied by DynamicUser=true.

@emilylange emilylange marked this pull request as ready for review June 5, 2025 16:19
@emilylange
Copy link
Member

git range-diff can be used to compare the changes in a useful way, since GitHub does not provide UI for that:

# git fetch origin master
# git fetch origin 596cb4db3f6531f637d116c7a5f8566a91ae6cd3
# git fetch origin pull/400194/head
# git range-diff --creation-factor=100 origin/master 596cb4db3f6531f637d116c7a5f8566a91ae6cd3 FETCH_HEAD

At least until 596cb4db3f6531f637d116c7a5f8566a91ae6cd3 (the rev before I pushed my changes) gets garbage collected by GitHub, that is.

Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

Thanks, that makeover looks awesome!

@teutat3s
Copy link
Member

teutat3s commented Jun 5, 2025

Thank you for pushing this closer to the finish line.
I would love to see an option for the module secret similar to #400194 (comment). Not blocking, can be follow up PR if desired.

@emilylange
Copy link
Member

@teutat3s I implemented --http-antispam-authorization-path as per your request. See #400194 (comment).

Note that I did not test this. It should just work though.
If you have the means to test this and report back, please do, that would be awesome :)

@mweinelt mweinelt added the backport release-25.05 Backport PR automatically label Jun 5, 2025
Copy link
Member

@teutat3s teutat3s left a comment

Choose a reason for hiding this comment

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

I tested this PR on a staging server and it works fine. One thing I forgot to mention earlier, we might want to set a default for rawHomeserverUrl or expose it via settings as well - we already expose it via settings.
The settings.web functionality depends on it being correctly set and it defaults to http://localhost:8008, which does not work with e.g. synapse workers.
Maybe just default it to the same value as homeserverUrl and mention it?

TheArcaneBrony and others added 2 commits June 6, 2025 15:35
Co-authored-by: emilylange <git@emilylange.de>
Co-authored-by: Martin Weinelt <hexa@darmstadt.ccc.de>
Co-authored-by: teutat3s <10206665+teutat3s@users.noreply.github.com>
Co-authored-by: emilylange <git@emilylange.de>
Co-authored-by: Martin Weinelt <hexa@darmstadt.ccc.de>
Co-authored-by: teutat3s <10206665+teutat3s@users.noreply.github.com>
@emilylange emilylange requested a review from teutat3s June 6, 2025 13:37
Copy link
Member

@teutat3s teutat3s left a comment

Choose a reason for hiding this comment

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

Thank you so much, this works. Tested with:

  • accessToken
  • web.synapseHTTPAntispam.authorization
  • services.draupnir.settings.web.enabled = true;
  • services.draupnir.settings.web.synapseHTTPAntispam.enabled = true;

Example config:

{
  services.matrix-synapse = {
    ...
    plugins = with config.services.matrix-synapse.package.plugins; [
        synapse-http-antispam
    ];
    modules = [
      {
        # https://the-draupnir-project.github.io/draupnir-documentation/bot/synapse-http-antispam
        # https://github.com/maunium/synapse-http-antispam
        module = "synapse_http_antispam.HTTPAntispam";
        config = {
          base_url = "http://${config.services.draupnir.settings.web.address}:${toString config.services.draupnir.settings.web.port}/api/1/spam_check";
          authorization_path = "<http-antispam-authorization-file>";
          enabled_callbacks = [
            "check_event_for_spam"
            "user_may_invite"
            "user_may_join_room"
          ];
          fail_open = {
            check_event_for_spam = true;
            user_may_invite = true;
            user_may_join_room = true;
          };
          async = {
            check_event_for_spam = true;
          };
        };
      }
    ];
  };

  services.draupnir = {
    enable = true;
    secrets = {
      accessToken = "<access-token-file>";
      web.synapseHTTPAntispam.authorization = "<http-antispam-authorization-file>";
    };
    settings = {
      # https://github.com/the-draupnir-project/Draupnir/blob/main/config/default.yaml
      homeserverUrl = "<homeserver-url>";
      managementRoom = "#moderators:example.org";
      protectAllJoinedRooms = true;
      recordIgnoredInvites = true;
      automaticallyRedactForReasons = [
        "*spam"
        "advertising"
      ];
      web = {
        enabled = true;
        port = 8080;
        address = "127.0.200.101";
        abuseReporting.enabled = true;
        synapseHTTPAntispam.enabled = true;
      };
    };
  };
}

@teutat3s teutat3s added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Jun 6, 2025
@mweinelt mweinelt merged commit c80ea7a into NixOS:master Jun 6, 2025
16 of 19 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jun 6, 2025

Backport failed for release-25.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.05
git worktree add -d .worktree/backport-400194-to-release-25.05 origin/release-25.05
cd .worktree/backport-400194-to-release-25.05
git switch --create backport-400194-to-release-25.05
git cherry-pick -x 4b153aad5d363b9d88ca4b47b4628f8032d05863 d6413ba43682273de9595d4193f64209ff8d770e

@teutat3s
Copy link
Member

teutat3s commented Jun 6, 2025

Just wanted to again thank everyone involved in the past two years for getting this from #222939 to merged.

@mdaniels5757 mdaniels5757 added the 8.has: port to stable This PR already has a backport to the stable release. label Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: port to stable This PR already has a backport to the stable release. 8.has: tests This PR has tests 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. backport release-25.05 Backport PR automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants