Skip to content

fix(profiles): enforce household limits during creation - #2091

Closed
fluxis wants to merge 1 commit into
Silo-Server:mainfrom
fluxis:fix/atomic-profile-quota
Closed

fluxis wants to merge 1 commit into
Silo-Server:mainfrom
fluxis:fix/atomic-profile-quota

Conversation

@fluxis

@fluxis fluxis commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Concurrent profile creation can exceed an account's max_profiles limit. A request that passes its initial check can also create a profile after an administrator lowers the limit. This change checks the current limit and household size within the profile write transaction.

Related issue: N/A
Validation tasks: changes #1180 C1; changes #1196 C1. No human revalidation is claimed.

Approach

PostgreSQL profile creation holds the account row lock while checking the limit and inserting the profile. A separate count query sees a preceding creator's commit. The existing native 409 conflict response also covers a quota reached during the transaction. Lowering the limit preserves existing profiles and history and blocks further creation until the household falls below the limit.

Direct store creation now wraps profile and library membership writes in one transaction. Account provisioning and canonical preference synchronization use that same quota check. The database contract registry includes the new native and store tests and rejects missing or skipped execution.

Apple and Android need no wire changes: the quota error already exists. The shared native handler preserves the v1 error code, and profile provisioning uses the same PostgreSQL path. Jellyfin-compatible clients do not create household profiles through a separate store path. No user manual update is needed: the existing account limit is unchanged.

Validation

Current qualification uses native main 6bdbb7d12f5f89cf17435ceec0c60a7cf70d06a6 and head c8c559d31e4b14ac1f859922d05faeb65cb30ee0. The profile quota patch is unchanged. Fresh focused PostgreSQL-store and profile-handler checks passed. The compiled production migration runner initialized a fresh isolated database, and all four added database contracts passed through the strict runner without skips. Removing the transactional quota guard reproduced concurrent overflow and a stale create after a guarded downgrade; restoring it passed both native HTTP cases and preserved household/history data. Exact database, container, volume, and outer test-daemon cleanup passed. Browser captures retain their original build labels. Full local suites were not run.

Native quota mutation checks passed against a fresh database using the rebased branch and the production migrations. Removing the transactional cap guard reproduced both concurrent overflow and creation after a guarded downgrade; restoring it passed both cases and preserved the existing household and history. All four new database contract entries passed through the strict CI runner without skips. The affected PostgreSQL store and profile handler regressions also passed.

Formatting and documentation path checks passed. Vet passed for the native API, profile handler and PostgreSQL store packages, and the DB contract runner unit tests passed. Changed-line lint passed with Go 1.26.4 and golangci-lint 2.12.2 over the four affected packages, with zero issues. The full Go and web suites were not run; verification is limited to affected behavior.

Evidence

Surface: native API with synthetic accounts on base ca186fe3, using the profile quota changes. The before case disables only the transactional quota guard in a private source copy; the after case restores production code. The database was created through the production migration runner.

Before:
POST /api/v2/profiles, two requests with one free slot -> 201, 201
PUT /api/v2/admin/users/{id}, max_profiles=1 -> 204
POST /api/v2/profiles, queued before that downgrade -> 201

After:
POST /api/v2/profiles, two requests with one free slot -> 201, 409
PUT /api/v2/admin/users/{id}, max_profiles=1 -> 204
POST /api/v2/profiles, queued before that downgrade -> 409
POST /api/v2/profiles, after that downgrade -> 409

These are observed native response statuses. The existing profile list and history remain equal after refusal. Desktop (1280 × 900) and mobile web (390 × 844) captures show the same synthetic household on the native profile chooser: Parent and both Guest profiles before, Parent and one Guest profile after. Native login, account, session, profile and library services back the browser; no browser API response is mocked. All four captures were visually checked for private information. Build: d40a302d5ce9d63a6f6a50373c7b3a2b4d5b091a. Publishing to the private evidence site awaits the developer’s evidence CLI login.

Risks

Profile creation holds the account row lock until its transaction commits, serializing creation with account updates. Existing over-limit households keep their data. No migration or deployment is included. Rollback evidence covers the profile and canonical-setting transaction and direct library-membership writes; existing later PIN and forced-subtitle writes remain outside that transaction. The separate access-group inheritance work in #1801 would require this check to resolve an effective group limit if it merges first.

Checklist

  • I read and can explain the complete diff.
  • This pull request addresses one concern.
  • Publish the inspected desktop and phone evidence.

AI Disclosure

  • Harness: OpenAI Codex with multi-agent delegation
  • Tool(s): OpenAI Codex, GitHub CLI, Go toolchain, Playwright, Chromium
  • Model(s): gpt-6.1-sol (configured Codex model; exact runtime identifier is not separately exposed)
  • Involvement: AI-generated; human review pending
  • Adversarial review: A separate Codex agent reviewed the complete PostgreSQL and handler diff, native database tests, and CI registry. It found no blocker on the current schema and confirmed row-lock serialization, fresh quota reads, retained household data, and transactional rollback coverage. The pending nullable group-limit change in feat(access): let access groups set the household profile limit #1801 remains a documented merge-order risk. Fault and mutation tests cover admission, rollback, retry, and fixture cleanup.

Note

Enforce household profile limits under row locks during profile creation

  • Profile creation now runs inside WithPreferenceSettingsTransaction in profiles.go. createProfile locks the account row, reads the current max_profiles, counts existing profiles, and returns the new typed userstore.ProfileLimitError when the household is full. Only the first profile becomes primary.
  • The API handler in profiles.go maps ProfileLimitError to HTTP 409 with the profile_limit_reached code and the current limit in the message.
  • WithPreferenceSettingsTransaction in preference_settings_tx.go now uses explicit Read Committed isolation via BeginTx.
  • Adds PostgreSQL database-contract tests for concurrent creates, post-downgrade stale creates, rollback at both write points, and library-failure rollback, registered in db-contracts.txt.
  • Behavioral Change: profile-limit rejections now return 409 instead of 500; an in-flight create re-reads the quota under lock, so it fails after an admin lowers max_profiles; existing profiles and history in an over-cap household are preserved.

Macroscope summarized d40a302.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@silo-kody

silo-kody Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Silo Kody — review complete

Review finished. Check the inline comments for findings and verify each suggestion against the code and tests.

Reviewing changes in Silo
  • Include the related issue, expected behavior, and validation steps in the PR description.
  • For API changes, describe the effect on Apple and Android clients and Jellyfin compatibility.
  • For plugin changes, identify the affected SDK contract, plugin, and catalog entry.
  • Follow this repository's AGENTS.md and CONTRIBUTING.md.
  • Request another review with @kody start-review in a PR comment.
  • React with 👍 or 👎 to give feedback on individual suggestions.
Review settings
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

@fluxis
fluxis force-pushed the fix/atomic-profile-quota branch from d40a302 to c8c559d Compare October 8, 2026 18:08
@silo-kody

silo-kody Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Silo Kody — review complete

Review finished. Check the inline comments for findings and verify each suggestion against the code and tests.

Reviewing changes in Silo
  • Include the related issue, expected behavior, and validation steps in the PR description.
  • For API changes, describe the effect on Apple and Android clients and Jellyfin compatibility.
  • For plugin changes, identify the affected SDK contract, plugin, and catalog entry.
  • Follow this repository's AGENTS.md and CONTRIBUTING.md.
  • Request another review with @kody start-review in a PR comment.
  • React with 👍 or 👎 to give feedback on individual suggestions.
Review settings
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

@fluxis fluxis closed this Oct 8, 2026
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