Skip to content

fix: restoring history cannot change an account's privacy - #928

Merged
blaipr merged 1 commit into
mainfrom
fix/restoring-history-cannot-change-an-accounts-privacy
Sep 17, 2026
Merged

blaipr merged 1 commit into
mainfrom
fix/restoring-history-cannot-change-an-accounts-privacy

Conversation

@blaipr

@blaipr blaipr commented Sep 17, 2026

Copy link
Copy Markdown
Member

AccountUseCases::restoreModified() copies isPrivate and isPrivateGroup straight from the
history snapshot, and the repository's restoreModified() excludes userId/userGroupId from the
UPDATE but not those two — so a restore wrote them back unconditionally.

On the edit screen the same two flags are gated: AccountForm::constrainPrivacyToPermission()
allows them only for an application administrator, or for the owner holding isAccPrivate() (the
group holding isAccPrivateGroup()). A history DTO never passes through AccountForm, so nothing
re-applied that rule on the way back in.

SaveEditRestoreController checks only ACCOUNT_EDIT_RESTORE, which AccountPermission buckets
with plain edit access. So anybody the account was shared with for editing could restore an old
version and, with it, set the account private — and AccountAcl tests privacy before the
administrator branch, so a private account disappears for account administrators too. Or strip the
privacy from an account that had it.

The same shape, one field over

This method already refuses to take the owner and the group from the snapshot: restoreModified()
takes explicit $changeOwner / $changeUserGroup flags, computed from userCanChangeOwner() and
userCanChangeGroup(), because restoring used to revert ownership past the ACL. Privacy is the same
question asked about a different pair of columns, and it was still being answered by the snapshot.

The fix puts the decision in the same place, beside those two: constrainPrivacyToPermission()
mirrors the form's rule and zeroes what the caller may not set, before the model is built.

Tests

testRestoreModifiedCannotMakeAnAccountPrivate restores a snapshot with both flags set as an
ordinary user and asserts the model reaching the repository has both at zero. Mutation-verified:
reverting src/ fails it.

The two existing restore tests built their expectation straight from the snapshot, so they now say
what the method actually promises — the privacy flags are decided, not copied.

AccountUseCases::restoreModified() copies isPrivate and isPrivateGroup straight from the
history snapshot, and the repository's restoreModified() excludes userId/userGroupId from
the UPDATE but not those two, so a restore wrote them back unconditionally.

On the edit screen AccountForm::constrainPrivacyToPermission() allows them only for an
application administrator, or the owner holding isAccPrivate() (the group holding
isAccPrivateGroup()). A history DTO never passes through AccountForm, so nothing re-applied
that rule coming back in — and SaveEditRestoreController checks only ACCOUNT_EDIT_RESTORE,
which AccountPermission buckets with plain edit access. So anybody the account was shared
with for editing could restore an old version to mark it private, and AccountAcl tests
privacy before the administrator branch, so a private account disappears for account
administrators too; or strip the privacy from one that had it.

This method already refuses to take the owner and the group from the snapshot, for the same
reason. Privacy is the same question about a different pair of columns, and the decision now
sits beside those two.
@blaipr
blaipr merged commit 15fe84e into main Sep 17, 2026
8 checks passed
@blaipr
blaipr deleted the fix/restoring-history-cannot-change-an-accounts-privacy branch September 17, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant