AkkomaGang/akkoma

Fork 112

Incomplete status visiblity fixes in PR 1014 #1035

New issue

Closed

opened 2025-12-23 16:46:34 +00:00 by phnt · 3 comments

phnt commented

2025-12-23 16:46:34 +00:00

What I think should be complete fixes along with new tests can be found in the Pleroma MR linked in your issue:
https://git.pleroma.social/pleroma/pleroma/-/merge_requests/4400

I tried to make the commits as concise as possible to ease porting, but considering I had to modify Akkoma commits to merge them cleanly, likely the same will happen here in reverse.

Note: I also changed the returned status code in MastoAPI to 404 to be more in line with what Mastodon does.

@Oneric Fixes in !1014 are incomplete and it is still possible to leak contents of private messages or otherwise interact with invisible posts through other CommonAPI/PleromaAPI routes like unfavorite and report. What I think should be complete fixes along with new tests can be found in the Pleroma MR linked in your issue: https://git.pleroma.social/pleroma/pleroma/-/merge_requests/4400 I tried to make the commits as concise as possible to ease porting, but considering I had to modify Akkoma commits to merge them cleanly, likely the same will happen here in reverse. Note: I also changed the returned status code in MastoAPI to 404 to be more in line with what Mastodon does.

Oneric commented

2025-12-23 19:06:54 +00:00

Owner

Going over the notes and commit titles+messages, after filtering out all C2S-write-actions (Akkoma does allow reading AP objects with an authorised OAuth token, just as if it was an S2S requet with am authorised signature)* and other Pleroma-specific stuff, i believe the following is the state of things; can you confirm I didn’t miss anything?:

already fixed by !1014 or safeguards preexisting
- plain status GETs
- status updates
- replying to a status
- quoting a status
- all poll interactions
- status context queries
- fav + unfav + index of existing favs
- reblog + unreblog + index of existing reblogs
- emoji react + unreact + index of existing reacts
TODO: missing bits addressed in Pleroma!4400
(since Masto API also returns the (updated) object which was interacted with)
- muting + unmuting a status(’s context)
- pinning + unpinning a status (your notes in the PR claim I partially added tests for this, but i don’t remember adding anything wrt pins?) [only the unpinning part didn’t have a safeguard yet and apparently still went though even if the status was never pinned to begin with]
- maybe leaking?: un- and bookmarking a status
  In Pleroma already had safeguards, but gracelessly failing with a 500 Internal serve error and no tests
- un-liking a post one could previously see and had liked, but now can no longer see
  This is a bit of an edge case, might in principle happen if one gets forcibly unfollowed, but the local client still has posts loaded. On the one hand, one should always be able to retract a previous like, on the other hand the post might have been edited in the meantime and one is not and never was allowed to see this updated state, yet Masto API demands the post be returned in the unfav response.
  Unlikely to have much practical relevance, but i guess preventing retraction of the fav is the safer albeit somewhat unsatisfying option
  (this was also hidden in the "unify response" commit)
- NON-LEAKING: prevent filing reports against user profiles, using non-visible posts as examples
  (this is a "non-leaking", since the API response does not contain full status objects, only their ids, thus while nonsensical it didn’t leak anything)
- NON-LEAKING: something about user Updates maybe being broken by the fixes

Going over the notes and commit titles+messages, after filtering out all C2S-write-actions *(Akkoma does allow **reading** AP objects with an authorised OAuth token, just as if it was an S2S requet with am authorised signature)** and other Pleroma-specific stuff, i believe the following is the state of things; can you confirm I didn’t miss anything?: - already fixed by !1014 or safeguards preexisting - plain status GETs - status updates - replying to a status - quoting a status - all poll interactions - status context queries - fav + unfav + index of existing favs - reblog + unreblog + index of existing reblogs - emoji react + unreact + index of existing reacts - TODO: missing bits addressed in Pleroma!4400 *(since Masto API also returns the (updated) object which was interacted with)* - [x] muting + unmuting a status(’s context) - [x] pinning + unpinning a status *(your notes in the PR claim I partially added tests for this, but i don’t remember adding anything wrt pins?)* [only the unpinning part didn’t have a safeguard yet and apparently still went though even if the status was never pinned to begin with] - [x] *maybe leaking?*: un- and bookmarking a status In Pleroma already had safeguards, but gracelessly failing with a 500 Internal serve error and no tests - [x] un-liking a post one could previously see and had liked, but now can no longer see This is a bit of an edge case, might in principle happen if one gets forcibly unfollowed, but the local client still has posts loaded. On the one hand, one _should_ always be able to retract a previous like, on the other hand the post might have been edited in the meantime and one is not and never was allowed to see this updated state, yet Masto API demands the post be returned in the unfav response. Unlikely to have much practical relevance, but i guess preventing retraction of the fav is the safer albeit somewhat unsatisfying option *(this was also hidden in the "unify response" commit)* - [x] NON-LEAKING: prevent filing reports against user profiles, using non-visible posts as examples *(this is a "non-leaking", since the [API response](https://docs.joinmastodon.org/methods/reports/#200-ok) does not contain full status objects, only their ids, thus while nonsensical it didn’t leak anything)* - [x] NON-LEAKING: something about user `Update`s maybe being broken by the fixes

phnt commented

2025-12-23 21:54:33 +00:00

Author

I started taking the notes when things started to get out of and hard to remember for me, they were my todo list as I was going along, so that's why they are a bit off. They aren't a complete list of changes I made, hence the name. Don't trust them much.

already fixed by !1014 or safeguards preexisting
- fav + unfav + index of existing favs
  - unfavs were left open (fixed in fe7108cbc28581cf7242c73867a06a68a96ac14b with an unhelpful commit title and revised later with a different error atom in 49a5630c75b4eb9f9ec764dab0acc5e66c370609)
TODO: missing bits addressed in Pleroma!4400
- pinning + unpinning a status (your notes in the PR claim I partially added tests for this, but i don’t remember adding anything wrt pins?) [only the unpinning part didn’t have a safeguard yet and apparently still went though even if the status was never pinned to begin with]
  - I probably misinterpreted the /pin 400 (now 422) test to be yours. Similarly with the EmojiReact note, by "partially added" I probably meant adding the C2S one, since I can't find other new EmojiReact tests made by me.
  - Pinning a status failed with ownership error instead of visibility error, because the status wasn't owned by the user attempting to pin it. Unpinning did essentially nothing, but still returned a response with the contents of the post, leaking it.
- bookmarking/unbookmarking a status
  - wasn't an issue, I only added error messages and tests
- NON-LEAKING: something about user Updates maybe being broken by the fixes
  - your Transmogrifier change (raise on Update activities in prepare_outgoing) I think prevents outgoing profile updates from federating, lain also added a test for that, so you can check if Akkoma is affected.

EDIT: Never mind, didn't see the edit until I posted it.

I don't think you missed anything (except noted below), but probably at least go through the commits labeled Transmogrifier/MastodonAPI/CommonAPI/PleromaAPI to be sure and since they contain more (updated) tests. I started taking the notes when things started to get out of and hard to remember for me, they were my todo list as I was going along, so that's why they are a bit off. They aren't a complete list of changes I made, hence the name. Don't trust them much. * already fixed by !1014 or safeguards preexisting - fav + unfav + index of existing favs - unfavs were left open (fixed in fe7108cbc28581cf7242c73867a06a68a96ac14b with an unhelpful commit title and revised later with a different error atom in 49a5630c75b4eb9f9ec764dab0acc5e66c370609) * TODO: missing bits addressed in Pleroma!4400 - pinning + unpinning a status (your notes in the PR claim I partially added tests for this, but i don’t remember adding anything wrt pins?) [only the unpinning part didn’t have a safeguard yet and apparently still went though even if the status was never pinned to begin with] - I probably misinterpreted the `/pin 400` (now 422) test to be yours. Similarly with the EmojiReact note, by "partially added" I probably meant adding the C2S one, since I can't find other new EmojiReact tests made by me. - Pinning a status failed with ownership error instead of visibility error, because the status wasn't owned by the user attempting to pin it. Unpinning did essentially nothing, but still returned a response with the contents of the post, leaking it. - bookmarking/unbookmarking a status - wasn't an issue, I only added error messages and tests - NON-LEAKING: something about user Updates maybe being broken by the fixes - your Transmogrifier change (raise on Update activities in `prepare_outgoing`) I think prevents outgoing profile updates from federating, lain also added a test for that, so you can check if Akkoma is affected. EDIT: Never mind, didn't see the edit until I posted it.

Oneric referenced this issue from a pull request that will close it,

2025-12-24 02:01:35 +00:00

Fix more interactions with invisible posts and corresponding data leaks #1036

Oneric commented

2025-12-24 02:04:34 +00:00

Owner

Thank you for the report and response!

I created a !1036 to hopefully fix all leaks and non-sensical interactions. Notably the unfaving and unbookmarking logic differs to allow retractions even after losing access while still preventing leaks.
Further response unifications and cosmetics might be picked up later.

Thank you for the report and response! I created a !1036 to hopefully fix all leaks and non-sensical interactions. Notably the unfaving and unbookmarking logic differs to allow retractions even after losing access while still preventing leaks. Further response unifications and cosmetics might be picked up later.

👍 1

Oneric closed this issue

2025-12-24 02:43:01 +00:00

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

AkkomaGang/akkoma#1035

No description provided.

Rows
Columns