Skip to content

fix(server): hide isFavorite from partner asset sync stream#28035

Open
timonrieger wants to merge 3 commits intomainfrom
fix/partner-asset-sync-favorite-leak
Open

fix(server): hide isFavorite from partner asset sync stream#28035
timonrieger wants to merge 3 commits intomainfrom
fix/partner-asset-sync-favorite-leak

Conversation

@timonrieger
Copy link
Copy Markdown
Collaborator

Description

The /sync/stream endpoint sends raw isFavorite values for partner assets, leaking whether a partner has
favorited their photos to users they've shared with.

The /timeline/bucket endpoint already filters this (returns false when ownerId !== auth.user.id), so
the old timeline was unaffected. But the new drift-based timeline reads directly from the local DB
(populated via the sync stream), so partner favorites were visible.

This PR fixes the sync stream by always sending false for isFavorite on partner assets in both
getBackfill and getUpserts of PartnerAssetsSync.

Fixes #21968
Follows up on client-side fix in #24651

How Has This Been Tested?

  • Sync stream verified to send isFavorite: false for partner assets
  • Own asset sync unaffected (still sends real isFavorite)
  • Existing sync tests pass

Checklist:

  • I have carefully read CONTRIBUTING.md
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • All code in src/services/ uses repositories implementations for database calls, filesystem
    operations, etc.
  • All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic
    (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

LLM (Claude) was used to identify the affected query methods and draft the implementation.

@timonrieger timonrieger force-pushed the fix/partner-asset-sync-favorite-leak branch from 381a0f5 to cdd47f3 Compare April 22, 2026 21:55
Copy link
Copy Markdown
Member

@mertalev mertalev left a comment

Choose a reason for hiding this comment

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

This supersedes #24651, yes? We don't need a client-side fix if the data isn't leaked to begin with.

Comment thread server/src/repositories/sync.repository.ts Outdated
@timonrieger
Copy link
Copy Markdown
Collaborator Author

This supersedes #24651, yes? We don't need a client-side fix if the data isn't leaked to begin with.

yes that's right.

I pushed up the change for the new column

Copy link
Copy Markdown
Member

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

This is great, but it doesn't do anything to fix the data that we've already synced. Can you add a migration that resets the session_checkpoint table for the relevant types?

Related, I wonder if we also have this same issue for album assets, which are shared. Do you mind checking if we need to do something for that case as well?

return this.upsertQuery('asset', options)
.select(columns.syncAsset)
.select(columns.syncPartnerAsset)
.select(sql<boolean>`false`.as('isFavorite'))
Copy link
Copy Markdown
Member

@jrasm91 jrasm91 Apr 28, 2026

Choose a reason for hiding this comment

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

Suggested change
.select(sql<boolean>`false`.as('isFavorite'))
.select(sql.val(false).as('isFavorite'))

Does this work instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Partner's "Favorite" icon is shown in timeline

3 participants