Skip to content

Ignore received EDUs if origin server in room ACL #18475

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

Merged
MadLittleMods merged 14 commits into from
Apr 25, 2026
Merged

Ignore received EDUs if origin server in room ACL #18475

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7be240d
Ignore received EDUs if origin server in room ACL
devonh May 23, 2025
e268468
Add changelog entry
devonh May 23, 2025
681987a
Update changelog.d/18475.feature
devonh May 23, 2025
cfc03b1
Only remove banned rooms from receipts, not the whole EDU
sandhose Jun 2, 2025
ad9efbe
Update synapse/federation/federation_server.py
devonh Nov 13, 2025
20f395e
Update synapse/federation/federation_server.py
devonh Nov 13, 2025
6f550c2
to_remove_room_ids rename
devonh Nov 13, 2025
2733158
Move logic under top level try
devonh Nov 14, 2025
2d9d14e
Merge branch 'develop' into devon/acl-edus
devonh Nov 14, 2025
eac6538
Merge branch 'develop' into devon/acl-edus
devonh Nov 14, 2025
0ab0e39
Merge branch 'develop' into devon/acl-edus
devonh Dec 17, 2025
745b86e
Merge branch 'develop' into devon/acl-edus
MadLittleMods Apr 24, 2026
3cd171f
Add comment why we copy
MadLittleMods Apr 24, 2026
7b92eda
(empty commit) try CI again after deleting `devon/acl-edus` on the Co…
MadLittleMods Apr 24, 2026
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
1 change: 1 addition & 0 deletions changelog.d/18475.feature
View file
Open in desktop

@MadLittleMods MadLittleMods Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey, I'd be interested in taking over this PR to finish what has to be done. Is this ok for you [...] ? :)

-- @FrenchGithubUser, #18475 (comment)

This PR appears close as-is (just needs one comment added) so I don't think creating a new PR is that useful. Getting the Complement side sorted sounds good as that's the main blocker here. Once the Complement tests are merged, we can just wrap up this PR ⏩

(better to comment on the diff so we can thread the discussion)

@MadLittleMods MadLittleMods Apr 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested to make sure COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh -run TestACLsForEDUs (test from matrix-org/complement#862) passes with this PR and fails on develop as expected

@MadLittleMods MadLittleMods Apr 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Complement failing with panic: BUG: Empty package name encountered.

For some reason, the Complement CI is failing with:

panic: BUG: Empty package name encountered.

goroutine 8 [running]:
github.com/gotesttools/gotestfmt/v2/parser.(*packageTracker).ensurePackage(...)
	/home/runner/go/pkg/mod/github.com/gotesttools/gotestfmt/v2@v2.5.0/parser/parse.go:220
github.com/gotesttools/gotestfmt/v2/parser.(*packageTracker).AddOutput(0xc000198dc0?, {0x0?, 0x2e?}, {0x0?, 0x0?}, {0xc00001f500?, 0x0?, 0x0?})
	/home/runner/go/pkg/mod/github.com/gotesttools/gotestfmt/v2@v2.5.0/parser/parse.go:192 +0x43c
github.com/gotesttools/gotestfmt/v2/parser.parse(0xc00001c380, 0xc00001c3f0, 0xc00001c460, 0xc00001c4d0, 0xc00001c540)
	/home/runner/go/pkg/mod/github.com/gotesttools/gotestfmt/v2@v2.5.0/parser/parse.go:133 +0x69f
created by github.com/gotesttools/gotestfmt/v2/parser.Parse in goroutine 1
	/home/runner/go/pkg/mod/github.com/gotesttools/gotestfmt/v2@v2.5.0/parser/parse.go:26 +0xea
cat: write error: Broken pipe

This is because the first lines of the output don't include "Package":"..." like the rest of the lines and gotestfmt chokes on parsing it.

{"ImportPath":"./tests/msc4222","Action":"build-output","Output":"# ./tests/msc4222\n"}
{"ImportPath":"./tests/msc4222","Action":"build-output","Output":"stat /home/runner/work/synapse/synapse/complement/tests/msc4222: directory not found\n"}
{"ImportPath":"./tests/msc4222","Action":"build-fail"}

Archive: 18475-72962188961-job-log.txt

If you download the archive, you can reproduce this locally by running: sed -n '3379,15707p' ~/Downloads/18475-72962188961-job-log.txt | sed 's/^[^ ]* //' | go run github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@v2.5.0 -hide "successful-downloads,successful-tests,empty-packages"

And if you skip those first 3 lines, things work: sed -n '3382,15707p' ~/Downloads/18475-72962188961-job-log.txt | sed 's/^[^ ]* //' | go run github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@v2.5.0 -hide "successful-downloads,successful-tests,empty-packages"

Why are we seeing /home/runner/work/synapse/synapse/complement/tests/msc4222: directory not found?

Not sure, matrix-org/complement -> tests/msc4222 on main still exists 🤔

Doesn't feel like it's anything related to this PR but it's consistent across runs and we're not seeing the same problem on develop

I bet this is because matrix-org/complement#783 existed with the same branch devon/acl-edus but that Complement branch is outdated and doesn't include tests/msc4222. We probably just need to delete that branch ⏩

This started failing because I merged in the latest develop here which brings in this update to include ./tests/msc4222 as a test package:

synapse/scripts-dev/complement.sh

Line 288 in 22e1643

./tests/msc4222

@MadLittleMods MadLittleMods Apr 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CI passed after deleting the devon/acl-edus branch in the Complement repo ✅

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make ACLs apply to EDUs.
Comment thread
MadLittleMods marked this conversation as resolved.
Outdated
Comment thread
devonh marked this conversation as resolved.
Outdated
24 changes: 24 additions & 0 deletions synapse/federation/federation_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,30 @@ async def _process_edu(edu_dict: JsonDict) -> None:
edu_type=edu_dict["edu_type"],
content=edu_dict["content"],
)

if edu.edu_type == EduTypes.TYPING:
Comment thread
MadLittleMods marked this conversation as resolved.
Outdated
origin_host, _ = parse_server_name(origin)
room_id = edu.content["room_id"]
try:
await self.check_server_matches_acl(origin_host, room_id)
except AuthError:
logger.warning(
"Ignoring typing EDU for room %s from banned server", room_id
Comment thread
devonh marked this conversation as resolved.
Outdated
)
return

if edu.edu_type == EduTypes.RECEIPT:
origin_host, _ = parse_server_name(origin)
for room_id, _ in edu.content.items():
try:
await self.check_server_matches_acl(origin_host, room_id)
except AuthError:
Comment thread
MadLittleMods marked this conversation as resolved.
logger.warning(
"Ignoring receipt EDU containing room %s from banned server",
room_id,
)
return

try:
await self.registry.on_edu(edu.edu_type, origin, edu.content)
except Exception:
Expand Down