Severity: P0 (CRITICAL — cross-tenant data leak)
The CoreSync HTTP endpoints registered by SentenceStudio.Api currently expose every user's per-row data to any authenticated user. This was identified by the API security audit (finding C1) and is the only audit finding deferred from the quick-win remediation series (commits 479b0d71 … 9a53bf81 on main).
Root cause
SharedSyncRegistration.ConfigureSyncTables on the Api side registers user-owned tables (e.g. Conversation, Challenge, DailyPlan, LearningResource ownership rows, etc.) without any selectIncrementalQuery / customSnapshotQuery filter. CoreSync therefore generates default SELECT * FROM "Table" WHERE __CT > @since — no per-user predicate.CoreSync.Http.Server.SyncAgentController.GetBulkChangeSetAsync calls:
_syncProvider.GetChangesAsync(storeId, SyncDirection.DownloadOnly, syncFilterParameters: Array.Empty<SyncFilterParameter>());
WHERE "UserProfileId" = @UserProfileId to the registration, the controller never binds a value for @UserProfileId and the query either errors or — depending on provider — returns all rows.ApplyChangesAsync (upload path) does not validate that incoming SyncItem's UserProfileId column matches the caller's claim, so a malicious client can upload rows with another user's UserProfileId.ISyncProvider is registered as a singleton, which makes it impossible to flow per-request identity through standard DI.
Remediation design
(Lifted from the in-session plan; the audit timestamp is in checkpoint 005.)
- Add filtered queries to
SharedSyncRegistration.ConfigureSyncTables for every user-owned table in the PostgreSQL builder:
tableBuilder
.WithFilter("UserProfileId", DbType.Guid)
.WithSelectIncrementalQuery("SELECT * FROM \"Conversation\" WHERE \"UserProfileId\" = @UserProfileId AND __CT > @since")
.WithCustomSnapshotQuery("SELECT * FROM \"Conversation\" WHERE \"UserProfileId\" = @UserProfileId");
- Lock pure-shared tables (
LearningResource, VocabularyWord, PhraseConstituent, ResourceVocabularyMapping) to SyncDirection.DownloadOnly and skip the user filter — these are catalog tables every user is allowed to download.
- Three sync tables currently lack any ownership column (flagged in
docs/database-schema.md with ⚠️): Challenge, Conversation, VocabularyList. Add UserProfileId columns with EF migrations before the wrapper lands, otherwise the filter has nothing to bind to.
- Implement
TenantScopedSyncProvider : ISyncProvider as a thin decorator that:
- Resolves
IHttpContextAccessor for the current UserProfileId claim,
- Delegates to
PostgreSQLSyncProvider, injecting syncFilterParameters: new[] { new SyncFilterParameter("UserProfileId", currentUserProfileId) } on every GetChangesAsync / GetIncrementalChangesAsync call,
- In
ApplyChangesAsync, rejects (or overwrites) any SyncItem whose UserProfileId column ≠ caller's claim.
- Re-register
ISyncProvider as Scoped. Provisioning (ApplyProvisionAsync) is one-shot and runs at startup with a separate provider instance constructed manually, so it does not need the scoped lifetime.
- Integration tests: two authenticated users sharing the same store id; assert
- User B's
GET /sync/changes does not return rows owned by User A,
- User B's
POST /sync/apply with a payload spoofing User A's UserProfileId is rejected.
Migration risk
- Mobile clients have already downloaded cross-tenant rows in their local SQLite stores. After the wrapper lands, the server's first
GetChangesAsync response will not re-issue deletes for those rows; clients will retain stale data from other users until they reset their local store.
- Recommendation: gate the wrapper behind a feature flag, ship a mobile-side "reset sync" affordance, then flip the flag in production. Coordinate with checkpoint 005's existing audit-deploy plan.
Pre-conditions before code lands
Tracking
This issue replaces five in-session todos that were marked blocked pending external scope:
coresync-sql-filters — add WHERE UserProfileId = @UserProfileId to all user-owned tables.coresync-wrapper — TenantScopedSyncProvider decorator.coresync-di — switch ISyncProvider registration to scoped + reconcile startup provisioning.coresync-test — two-user integration tests for upload + download isolation.rubber-duck-design — circulate this design with rubber-duck before implementation.
References
docs/database-schema.md — sync table registration matrix and ownership-column gaps.- Audit checkpoint:
005-api-security-audit-schema-mark.md.
- Quick-win remediation commits already on
main: d6dd4edc..9a53bf81 (13 commits).
Severity: P0 (CRITICAL — cross-tenant data leak)
The CoreSync HTTP endpoints registered by
SentenceStudio.Apicurrently expose every user's per-row data to any authenticated user. This was identified by the API security audit (finding C1) and is the only audit finding deferred from the quick-win remediation series (commits479b0d71…9a53bf81onmain).Root cause
SharedSyncRegistration.ConfigureSyncTableson the Api side registers user-owned tables (e.g.Conversation,Challenge,DailyPlan,LearningResourceownership rows, etc.) without anyselectIncrementalQuery/customSnapshotQueryfilter. CoreSync therefore generates defaultSELECT * FROM "Table" WHERE __CT > @since— no per-user predicate.CoreSync.Http.Server.SyncAgentController.GetBulkChangeSetAsynccalls:WHERE "UserProfileId" = @UserProfileIdto the registration, the controller never binds a value for@UserProfileIdand the query either errors or — depending on provider — returns all rows.ApplyChangesAsync(upload path) does not validate that incomingSyncItem'sUserProfileIdcolumn matches the caller's claim, so a malicious client can upload rows with another user's UserProfileId.ISyncProvideris registered as a singleton, which makes it impossible to flow per-request identity through standard DI.Remediation design
(Lifted from the in-session plan; the audit timestamp is in checkpoint 005.)
SharedSyncRegistration.ConfigureSyncTablesfor every user-owned table in the PostgreSQL builder:LearningResource,VocabularyWord,PhraseConstituent,ResourceVocabularyMapping) toSyncDirection.DownloadOnlyand skip the user filter — these are catalog tables every user is allowed to download.docs/database-schema.mdwithChallenge,Conversation,VocabularyList. AddUserProfileIdcolumns with EF migrations before the wrapper lands, otherwise the filter has nothing to bind to.TenantScopedSyncProvider : ISyncProvideras a thin decorator that:IHttpContextAccessorfor the currentUserProfileIdclaim,PostgreSQLSyncProvider, injectingsyncFilterParameters: new[] { new SyncFilterParameter("UserProfileId", currentUserProfileId) }on everyGetChangesAsync/GetIncrementalChangesAsynccall,ApplyChangesAsync, rejects (or overwrites) anySyncItemwhoseUserProfileIdcolumn ≠ caller's claim.ISyncProviderasScoped. Provisioning (ApplyProvisionAsync) is one-shot and runs at startup with a separate provider instance constructed manually, so it does not need the scoped lifetime.GET /sync/changesdoes not return rows owned by User A,POST /sync/applywith a payload spoofing User A'sUserProfileIdis rejected.Migration risk
GetChangesAsyncresponse will not re-issue deletes for those rows; clients will retain stale data from other users until they reset their local store.Pre-conditions before code lands
UserProfileIdtoChallenge,Conversation,VocabularyList, backfill existing rows fromOwnerUserId/ first-creator if any, add FK + index.CoreSync.PostgreSQL.PostgreSQLSyncProvideractually honorsselectIncrementalQueryparameters when called viaGetChangesAsync(... SyncFilterParameter[])— if not, escalate upstream toCoreSync.Program.cs.Tracking
This issue replaces five in-session todos that were marked
blockedpending external scope:coresync-sql-filters— addWHERE UserProfileId = @UserProfileIdto all user-owned tables.coresync-wrapper—TenantScopedSyncProviderdecorator.coresync-di— switchISyncProviderregistration to scoped + reconcile startup provisioning.coresync-test— two-user integration tests for upload + download isolation.rubber-duck-design— circulate this design with rubber-duck before implementation.References
docs/database-schema.md— sync table registration matrix and ownership-column gaps.005-api-security-audit-schema-mark.md.main:d6dd4edc..9a53bf81(13 commits).