Skip to content

fix(hwid): serialize device registration across workers - #859

Open
dr-hoseyn wants to merge 2 commits into
PasarGuard:devfrom
dr-hoseyn:codex/fix-atomic-hwid-registration
Open

dr-hoseyn wants to merge 2 commits into
PasarGuard:devfrom
dr-hoseyn:codex/fix-atomic-hwid-registration

Conversation

@dr-hoseyn

@dr-hoseyn dr-hoseyn commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Concurrent subscription requests with different HWIDs can both pass the device-count check and consume the same final slot. Move the check into registration and serialize it with a database lock, so only one new device is admitted while repeat requests from an existing device remain allowed.

PostgreSQL/MySQL lock the parent user row; SQLite acquires a write lock before reading the HWID set. End the validation read transaction first so MySQL's default REPEATABLE READ cannot reuse an older snapshot after waiting for the lock.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Refactor / cleanup
  • Documentation
  • Tests / CI

Checklist

  • I tested the change locally or explained why it cannot be tested.
  • I added or updated tests for behavior changes.
  • I updated documentation, translations, or examples if needed. (Not needed.)
  • I checked database migrations when models or schema changed. (No schema changes.)
  • I did not include secrets, tokens, private keys, or unrelated changes.

Testing

  • Reproduced the bug on the unchanged backend: both different-device requests returned 200 when only one slot remained, for limits of 1 and 3.
  • python -m pytest tests/api/test_hwid.py tests/api/test_user.py -k hwid -q --tb=short: 13 passed, 83 deselected on SQLite.
  • The concurrency tests use separate database connections and pre-registration reads, and cover both different HWIDs competing for the last slot and duplicate requests for one HWID.
  • Existing-device refresh at capacity still preserves its metadata and updates last_used_at; rejected registrations release the lock.
  • Ruff check/format and git diff --check passed.
  • Local database validation used SQLite. The new tests also run in the existing PostgreSQL, TimescaleDB, MySQL, and MariaDB CI jobs.

Screenshots

Not applicable.

Notes for reviewers

The registration helper still owns its commit and now returns whether the device was accepted. The operation maps rejection to the existing 403 response. Existing HWID policy resolution, unlimited registration, and subscription response behavior are unchanged.

Summary by CodeRabbit

  • Bug Fixes
    • Improved device registration to enforce account HWID limits reliably, including during simultaneous registration attempts.
    • Prevented users from exceeding their configured device limit.
    • Re-registering an existing device continues to work without consuming an additional slot.
    • Registration requests that exceed the device limit now return a clear 403 error.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: d323202b-d015-4f19-9956-e76915a95be0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 7fa0406c-1668-438a-968c-204c92f51a19

📥 Commits

Reviewing files that changed from the base of the PR and between aa7054a and 01e4745.

📒 Files selected for processing (3)
  • app/db/crud/hwid.py
  • app/operation/subscription.py
  • tests/api/test_hwid.py

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


Walkthrough

Changes

HWID registration flow

Layer / File(s) Summary
Atomic registration and locking
app/db/crud/hwid.py
register_user_hwid accepts an optional keyword-only limit, locks database state, enforces capacity, commits rejected operations, and returns True or False.
Subscription flow integration
app/operation/subscription.py
validate_and_register_hwid delegates registration checks to register_user_hwid and raises HTTP 403 when the limit is reached.
Concurrency and upsert validation
tests/api/test_hwid.py
Tests cover concurrent last-slot registration, duplicate devices, over-limit devices, and existing-row refreshes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 01e47

HWID registration now atomically enforces device limits while retaining existing-device refresh behavior. No current merge-blocking risk is identified.

Suggested reviewers: m03ed, x0sina

Sequence Diagram(s)

sequenceDiagram
  participant RequestA
  participant RequestB
  participant SubscriptionOperation
  participant register_user_hwid
  participant Database
  RequestA->>SubscriptionOperation: validate HWID
  RequestB->>SubscriptionOperation: validate HWID
  SubscriptionOperation->>register_user_hwid: register with limit
  SubscriptionOperation->>register_user_hwid: register with limit
  register_user_hwid->>Database: acquire database lock
  Database-->>register_user_hwid: serialize registration
  register_user_hwid->>Database: insert or refresh HWID
  register_user_hwid-->>SubscriptionOperation: True or False
  SubscriptionOperation-->>RequestA: 200 or 403
  SubscriptionOperation-->>RequestB: 200 or 403
Loading

Poem

A rabbit guards the device gate
Locks the row and checks the slate
One new token claims the final seat
A twin refreshes, neat and sweet
Full means false, success means true
Tests hop through the flow anew

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: serializing HWID device registration across workers to prevent concurrent limit violations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@dr-hoseyn

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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