feat(sync-v2): complete native server convergence - #94
Conversation
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCette PR ajoute la synchronisation v2 durable, les sessions web, le bootstrap administrateur, l’administration native et les interfaces web associées. Elle ajoute les routes API v2, les migrations, les tests et la documentation correspondante. ChangesFonctionnalités v2
Signature DCO
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant WebApp
participant HTTP_API
participant DomainServices
participant SyncService
participant SQLite
WebApp->>HTTP_API: Envoie une mutation avec operation_id
HTTP_API->>DomainServices: Transmet le contexte de mutation
DomainServices->>SyncService: Réclame l’opération
SyncService->>SQLite: Enregistre l’opération et l’événement
SyncService-->>HTTP_API: Retourne le reçu
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
type: feat
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
github-actions
Bot
added
type: feat
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
type: feat
github-actions
Bot
added
type: feat
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services.rs`:
- Around line 1799-1805: Dans le flux de création de partage autour de
songs_by_ids_on, définissez ou réutilisez MAX_SHARE_TRACKS et rejetez ids dès
qu’il contient plus de cette limite, après le traitement des rejeux et avant
toute insertion. Ajoutez un test couvrant exactement MAX_SHARE_TRACKS + 1 pistes
et vérifiez l’erreur attendue, en conservant la prise du writer_guard pour toute
mutation de base de données.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1b5b548a-7fbd-4f77-841c-e03c0ca7472e
📒 Files selected for processing (6)
README.mddocs/rfcs/RFC-002-waveflow-server-v2.mdsrc/security.rssrc/services.rstests/v2_foundations.rswebapp/src/pages.tsx
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai review |
|
|
@coderabbitai review |
1 similar comment
|
@coderabbitai review |
✅ Action performedReview finished.
|
github-actions
Bot
added
type: feat
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/services.rs (3)
1867-1872: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftÉvitez la course lors de la lecture d’un partage public.
public_sharelit le partage et ses pistes avant de prendre_writerà Line 1867. Sidelete_share_with_contextsupprime le partage entre ces opérations, l’UPDATE à Line 1868 affecte zéro ligne. Le code ignore ce résultat et renvoie les données obsolètes.Prenez le writer gate avant la lecture, puis exécutez la lecture et l’incrément dans une transaction cohérente. Sinon, vérifiez
rows_affected()et renvoyezServiceError::NotFound.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services.rs` around lines 1867 - 1872, Update public_share to acquire the writer guard before reading the share and its tracks, then perform the read and visit-count increment within one coherent transaction/critical section. Ensure deletion cannot occur between the read and UPDATE; alternatively, validate the UPDATE result via rows_affected() and return ServiceError::NotFound when no row was updated.
1265-1273: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFiltrez les favoris par visibilité.
starred_ids_onsélectionne toutes les lignesuser_starde l’utilisateur. La requête ne vérifie nilibrary_memberni le type d’entité. Après le retrait d’une bibliothèque,sync_snapshotpeut donc renvoyer des identifiants devenus inaccessibles.Ajoutez des prédicats
EXISTSpar type d’entité, comme dansratings_onaux Lines 1287-1292. Conservez l’identifiant utilisateur comme premier bind.As per coding guidelines, la règle exige « Enforce tenancy in repository queries using library_member, not only in HTTP handlers. »
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services.rs` around lines 1265 - 1273, Update starred_ids_on to restrict user_star results by entity visibility using type-specific EXISTS predicates against library_member, following the pattern in ratings_on. Keep user_id as the first bind and preserve the existing ordering, row mapping, and return type.Source: Coding guidelines
2145-2146: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winLibérez le writer gate avant la lecture finale des utilisateurs.
create_subsonic_userconserve_writerpendantself.users(actor_id)après le commit.update_userapplique le même schéma.users()charge tous les comptes et les adhésions. Cette lecture peut donc bloquer toutes les mutations et les scans.Ajoutez
drop(_writer);immédiatement après chaquetx.commit().await?et avantself.users(...).Correction minimale
tx.commit().await?; + drop(_writer); self.users(actor_id)Also applies to: 2203-2208, 2253-2263, 2297-2302
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services.rs` around lines 2145 - 2146, Release the writer guard immediately after each successful transaction commit and before the final user reads. Update create_subsonic_user, update_user, and the other affected mutation paths around their tx.commit().await? calls by adding drop(_writer); before invoking self.users(...) or equivalent user-loading logic, while preserving the existing transaction behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/services.rs`:
- Around line 1867-1872: Update public_share to acquire the writer guard before
reading the share and its tracks, then perform the read and visit-count
increment within one coherent transaction/critical section. Ensure deletion
cannot occur between the read and UPDATE; alternatively, validate the UPDATE
result via rows_affected() and return ServiceError::NotFound when no row was
updated.
- Around line 1265-1273: Update starred_ids_on to restrict user_star results by
entity visibility using type-specific EXISTS predicates against library_member,
following the pattern in ratings_on. Keep user_id as the first bind and preserve
the existing ordering, row mapping, and return type.
- Around line 2145-2146: Release the writer guard immediately after each
successful transaction commit and before the final user reads. Update
create_subsonic_user, update_user, and the other affected mutation paths around
their tx.commit().await? calls by adding drop(_writer); before invoking
self.users(...) or equivalent user-loading logic, while preserving the existing
transaction behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1b03d23d-f37a-4080-9dcb-5ec4bcaab62f
📒 Files selected for processing (2)
src/services.rstests/v2_foundations.rs
starred_ids_on returned every user_star row unfiltered while ratings_on already restricted results to entities the user can still reach. A listener removed from a library kept seeing favourites for tracks, albums and artists outside their scope. Apply the same type-specific EXISTS predicates against library_member, keeping user_id as the first bind and the existing ordering. public_share read the share and its tracks outside the writer gate, so a share revoked in between was still served with an incremented visit count. Let the UPDATE arbitrate instead: no affected row means the share is gone and the visitor gets the same 404 as for an unknown token. Release the writer guard right after tx.commit() in create_subsonic_user and update_user: both then run self.users(), a full account read, while still holding the process-wide gate. The other commit sites only publish a sync receipt in memory, so they were left alone. Claude-Session: https://claude.ai/code/session_01NJBwjsQ17Bx2PgvPbBGNpM Signed-off-by: InstaZDLL <github.105mh@8shield.net>
github-actions
Bot
added
type: feat
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @InstaZDLL. * #94 (comment) The following files were modified: * `src/authentication.rs` * `src/catalog.rs` * `src/database.rs` * `src/http.rs` * `src/lib.rs` * `src/main.rs` * `src/media.rs` * `src/security.rs` * `src/services.rs` * `src/subsonic.rs` * `src/sync.rs` * `webapp/src/api.ts` * `webapp/src/main.tsx` * `webapp/src/pages.tsx` * `webapp/src/player.tsx`
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services.rs`:
- Around line 1877-1887: Rendre la validation d’expiration atomique dans la
méthode contenant la requête UPDATE share en ajoutant le prédicat expires_at à
la clause WHERE, avec le paramètre de temps actuel. Conserver le contrôle de
visited == 0 pour retourner ServiceError::NotFound lorsqu’aucune ligne n’est
modifiée, notamment si le partage a expiré pendant l’attente de writer_guard.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cffb5a62-775c-4aae-88cf-1d9f911a74af
📒 Files selected for processing (2)
src/services.rstests/v2_foundations.rs
public_share filtered on expires_at during the initial SELECT, then waited on the process-wide writer gate before incrementing the visit count. That wait can block behind a scan, so a share could expire in between and still be served once. Carry the expiry predicate into the UPDATE and reuse the same timestamp for last_visited_at, so revocation and expiry are both arbitrated at write time; visited == 0 keeps returning NotFound. Claude-Session: https://claude.ai/code/session_01NJBwjsQ17Bx2PgvPbBGNpM Signed-off-by: InstaZDLL <github.105mh@8shield.net>
github-actions
Bot
added
type: feat
The handoff still described PR #94 as pending and listed the browser-session debt as open, both of which landed. Bring it back in line with main: - M4 is closed: base in 14aec76, server completion in 6716df9. - Document the user-data sync layer and RFC-003, absent until now. - Move the localStorage debt and the Dependabot DCO exemption to a "closed, do not reopen" section, with the evidence for each. - Record the three security fixes found in review after #94 (unfiltered favourites, share visits surviving revocation or expiry, writer gate held across a read) as pitfalls to apply to new code, not just as history. - Note the second FFmpeg CI failure mode: choco exits 0 on a 503, so a green install step can still mean no binary. - Record that @vitejs/plugin-react 6 requires Vite >= 7, and why vite-env.d.ts is load-bearing under TypeScript 7. - Add the dependency state, including the cross-version check proving chacha20poly1305 0.11 still reads secrets sealed by 0.10. Next step is the WaveFlow Desktop integration pass, not M5. Claude-Session: https://claude.ai/code/session_01NJBwjsQ17Bx2PgvPbBGNpM Signed-off-by: InstaZDLL <github.105mh@8shield.net>
2 participants
Résumé
/api/v2pour le setup, le catalogue, les bibliothèques, notes/historique, partages, utilisateurs, identifiants Subsonic et l’état du transcodagelocalStoragepar un access token en mémoire et un refresh rotatif HttpOnly/SameSite avec contrôle d’origine + CSRFLe catalogue reste autoritaire côté serveur et WaveFlow Desktop le consommera comme source distante séparée. Aucun rapprochement avec la bibliothèque locale n’est introduit ici.
Validation
bun run format,bun run lint,bun run test,bun run builddanswebapp/— 21 testscargo fmt --all --checkcargo clippy --all-targets --all-features -- -D warningscargo check --all-targets --all-featurescargo test --all-features— 37 testsbun run buildà la racine — SPA puis binaire release LTOAucun tag de release n’est créé par cette PR.
Summary by CodeRabbit
Nouvelles fonctionnalités
Améliorations de sécurité
Documentation