feat(panel): HWID device limit (subscription, admin API, dashboard)#429
Open
Nemu-x wants to merge 14 commits intoPasarGuard:mainfrom
Open
feat(panel): HWID device limit (subscription, admin API, dashboard)#429Nemu-x wants to merge 14 commits intoPasarGuard:mainfrom
Nemu-x wants to merge 14 commits intoPasarGuard:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds HWID device-limiting: DB schema and models, CRUD and enforcement with HMAC-SHA256 hashing, subscription flow integration returning diagnostic headers on denial, admin HWID APIs and dashboard UI, tests, docs, and a runtime HWID hash salt production guard. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant API as Subscription API
participant SubOp as SubscriptionOp
participant HWIDOp as HWIDOp
participant DB as Database
Client->>API: GET subscription (X-HWID, headers...)
API->>SubOp: user_subscription_with_client_type(hwid,...)
SubOp->>HWIDOp: enforce_subscription_hwid(user, hwid, metadata...)
HWIDOp->>DB: BEGIN TRANSACTION / LOCK user row
DB-->>HWIDOp: locked
HWIDOp->>DB: SELECT hwid_user_devices WHERE user_id AND hwid_hash
DB-->>HWIDOp: existing? (row/none)
alt existing
HWIDOp->>DB: UPDATE last_seen_at, metadata
DB-->>HWIDOp: updated
else not existing
HWIDOp->>DB: SELECT COUNT(*) WHERE user_id
DB-->>HWIDOp: count
alt count < limit
HWIDOp->>DB: INSERT new hwid_user_devices
DB-->>HWIDOp: created
else limit reached
HWIDOp-->>SubOp: HWIDDecision(allowed=false,max_devices_reached=true)
end
end
HWIDOp->>DB: COMMIT
HWIDOp-->>SubOp: HWIDDecision(allowed=true/false)
SubOp-->>API: decision
alt allowed
API-->>Client: 200 subscription (+ active header if enabled)
else denied
API-->>Client: 404 + diagnostic headers (missing_hwid / limit reached)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
app/routers/hwid.py (1)
24-26: AddQueryparameter constraints tooffset/limitin lines 24-26 and line 71 to prevent negative/excessively large values.Import
Queryfrom fastapi and apply bounds:offsetwithge=0,limitwithge=1, le=200. This prevents invalid pagination inputs from reaching the database.🔧 Suggested patch
-from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Query @@ async def get_hwid_devices( - offset: int = 0, - limit: int = 50, + offset: int = Query(default=0, ge=0), + limit: int = Query(default=50, ge=1, le=200), @@ -async def get_top_users(limit: int = 20, db: AsyncSession = Depends(get_db), admin=Depends(get_current)): +async def get_top_users( + limit: int = Query(default=20, ge=1, le=200), db: AsyncSession = Depends(get_db), admin=Depends(get_current) +):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routers/hwid.py` around lines 24 - 26, Add validation to the pagination query params by importing Query from fastapi and changing the offset and limit parameters to use Query defaults and bounds (offset: int = Query(0, ge=0), limit: int = Query(50, ge=1, le=200)) wherever they appear (the handler function parameters that declare offset, limit, user_id around lines 24-26 and the similar parameter set at line 71); ensure you add "from fastapi import Query" to the imports so negative or excessively large values are rejected before reaching the database.app/models/hwid.py (1)
33-49: Add request-field constraints for stronger API contracts (lines 33-49).
user_id,hwid, andhwid_hashlack validation constraints. Adding bounds and length checks here prevents invalid payloads early and aligns with the codebase's existing Field validation patterns (e.g.,data_limit: int = Field(ge=0),name: str = Field(min_length=3, max_length=64)). Field is already imported.🔧 Suggested patch
class HWIDDeleteRequest(BaseModel): - user_id: int - hwid_hash: str + user_id: int = Field(ge=1) + hwid_hash: str = Field(min_length=1, max_length=128) @@ class HWIDDeleteAllRequest(BaseModel): - user_id: int + user_id: int = Field(ge=1) @@ class HWIDAddRequest(BaseModel): - user_id: int - hwid: str - device_os: str | None = None - os_version: str | None = None - device_model: str | None = None - user_agent: str | None = None - request_ip: str | None = None + user_id: int = Field(ge=1) + hwid: str = Field(min_length=1, max_length=512) + device_os: str | None = Field(default=None, max_length=64) + os_version: str | None = Field(default=None, max_length=64) + device_model: str | None = Field(default=None, max_length=128) + user_agent: str | None = Field(default=None, max_length=512) + request_ip: str | None = Field(default=None, max_length=64)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/hwid.py` around lines 33 - 49, Add Pydantic Field validations to strengthen the API contract: on HWIDDeleteRequest and HWIDDeleteAllRequest set user_id: int = Field(ge=1) to require positive IDs; on HWIDDeleteRequest set hwid_hash: str = Field(min_length=8, max_length=256) (or appropriate bounds) and on HWIDAddRequest set user_id: int = Field(ge=1) and hwid: str = Field(min_length=8, max_length=256); also optionally add reasonable max_length constraints for device_os, os_version, device_model, user_agent and request_ip to match project patterns — use the already-imported Field and update the HWIDDeleteRequest, HWIDDeleteAllRequest, and HWIDAddRequest class attribute definitions accordingly.app/operation/subscription.py (2)
390-408: ⚖️ Poor tradeoffDuplicated HWID enforcement logic.
The HWID enforcement block (lines 390-408) is nearly identical to lines 265-285 in
user_subscription. Consider extracting a helper method like_enforce_hwid_or_deny()that returns eitherNone(allowed) or aResponse(denial) to reduce duplication.Example refactor
async def _enforce_hwid_or_deny( self, db: AsyncSession, db_user: User, sub_settings: SubSettings, hwid: str | None, device_os: str | None, os_version: str | None, device_model: str | None, user_agent: str | None, request_ip: str | None, ) -> Response | None: hwid_operator = HWIDOperation(operator_type=self.operator_type) hwid_decision = await hwid_operator.enforce_subscription_hwid( db, user=db_user, subscription_settings=sub_settings, hwid=hwid, device_os=device_os, os_version=os_version, device_model=device_model, user_agent=user_agent, request_ip=request_ip, ) if not hwid_decision.allowed: headers = ( self._HWID_NOT_SUPPORTED_HEADERS if hwid_decision.missing_hwid else self._HWID_LIMIT_REACHED_HEADERS if hwid_decision.max_devices_reached else {} ) return Response(status_code=404, headers=headers) return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/operation/subscription.py` around lines 390 - 408, The HWID enforcement logic is duplicated between the block in subscription.py (current snippet) and the user_subscription block; extract that logic into a private helper method (e.g., _enforce_hwid_or_deny) that accepts (db: AsyncSession, db_user: User, sub_settings: SubSettings, hwid: str | None, device_os: str | None, os_version: str | None, device_model: str | None, user_agent: str | None, request_ip: str | None) and returns Response | None; inside it instantiate HWIDOperation(operator_type=self.operator_type), call enforce_subscription_hwid(...) with the same args, build headers the same way (use self._HWID_NOT_SUPPORTED_HEADERS / self._HWID_LIMIT_REACHED_HEADERS) and return Response(status_code=404, headers=...) when not allowed or None when allowed; then replace the duplicated blocks in both user_subscription and the current function with a single call to _enforce_hwid_or_deny and immediately return that Response if not None.
278-285: 💤 Low valueConsider logging or raising for unexpected denial state.
The
else {}branch (line 282-283) handles a case whereallowed=Falsebut neithermissing_hwidnormax_devices_reachedis true. While defensive, this could silently mask unexpected denial reasons. Consider adding a log statement for debugging unexpected states.Suggested improvement
if not hwid_decision.allowed: headers = ( self._HWID_NOT_SUPPORTED_HEADERS if hwid_decision.missing_hwid else self._HWID_LIMIT_REACHED_HEADERS if hwid_decision.max_devices_reached else {} ) + # Log unexpected denial state for debugging + if not hwid_decision.missing_hwid and not hwid_decision.max_devices_reached: + # Unexpected denial reason - consider logging + pass # Preserve existing safe denial behavior for invalid/missing subscription-like requests. return Response(status_code=404, headers=headers)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/operation/subscription.py` around lines 278 - 285, When hwid_decision.allowed is False but neither hwid_decision.missing_hwid nor hwid_decision.max_devices_reached is true, add a defensive log (e.g. self._logger.warning or error) before returning the 404 so unexpected denial reasons are visible; include the hwid_decision object (or its relevant fields), any request/subscription identifiers available in the surrounding method, and the chosen headers (_HWID_NOT_SUPPORTED_HEADERS/_HWID_LIMIT_REACHED_HEADERS) to aid debugging — optionally consider escalating by raising an exception if this state should never occur.app/db/crud/hwid.py (2)
22-35: 💤 Low valueMinor redundancy:
hash_hwidnormalizes twice.
hash_hwid(hwid)callsnormalize_hwid(hwid)internally (line 33), but callers likeenforce_hwid_device_limit(line 64) andadd_hwid_device(line 158) already pass a normalized HWID. This is harmless but slightly inefficient.Consider removing the internal normalization since callers are responsible for it:
♻️ Suggested simplification
def hash_hwid(hwid: str) -> str: - normalized = normalize_hwid(hwid) - digest = hmac.new(HWID_HASH_SALT.encode("utf-8"), normalized.encode("utf-8"), hashlib.sha256).hexdigest() + digest = hmac.new(HWID_HASH_SALT.encode("utf-8"), hwid.encode("utf-8"), hashlib.sha256).hexdigest() return digest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/db/crud/hwid.py` around lines 22 - 35, hash_hwid currently calls normalize_hwid internally which is redundant because callers like enforce_hwid_device_limit and add_hwid_device already supply a normalized HWID; remove the internal normalization in hash_hwid (replace the call to normalize_hwid(hwid) with using the hwid parameter directly), update hash_hwid's docstring or add a short comment stating it expects a normalized HWID, and verify callers (enforce_hwid_device_limit, add_hwid_device) continue to pass normalized values.
220-229: ⚡ Quick winDirect
commit()is inconsistent with other methods and may cause issues if called within a transaction.
delete_hwid_deviceanddelete_all_hwid_devicescalldb.commit()directly, whereasadd_hwid_deviceandenforce_hwid_device_limituse thebegin_nested()/begin()pattern. If a caller wraps these delete calls in a transaction, the innercommit()would commit the outer transaction prematurely.Consider aligning with the transaction-safe pattern used elsewhere:
♻️ Suggested fix for transaction safety
async def delete_hwid_device(db: AsyncSession, *, user_id: int, hwid_hash: str) -> int: + tx_ctx = db.begin_nested() if db.in_transaction() else db.begin() + async with tx_ctx: - result = await db.execute(delete(HWIDUserDevice).where(HWIDUserDevice.user_id == user_id, HWIDUserDevice.hwid_hash == hwid_hash)) - await db.commit() + result = await db.execute(delete(HWIDUserDevice).where(HWIDUserDevice.user_id == user_id, HWIDUserDevice.hwid_hash == hwid_hash)) return int(result.rowcount or 0) async def delete_all_hwid_devices(db: AsyncSession, *, user_id: int) -> int: + tx_ctx = db.begin_nested() if db.in_transaction() else db.begin() + async with tx_ctx: - result = await db.execute(delete(HWIDUserDevice).where(HWIDUserDevice.user_id == user_id)) - await db.commit() + result = await db.execute(delete(HWIDUserDevice).where(HWIDUserDevice.user_id == user_id)) return int(result.rowcount or 0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/db/crud/hwid.py` around lines 220 - 229, The two delete helpers delete_hwid_device and delete_all_hwid_devices call db.commit() directly which can prematurely commit an outer transaction; change them to use the AsyncSession transaction context like other functions (e.g. async with db.begin() or async with db.begin_nested() as appropriate) and execute the delete inside that context, remove the direct db.commit() call, capture the result from db.execute(...) and return int(result.rowcount or 0) after the context completes so transaction semantics match add_hwid_device/enforce_hwid_device_limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.py`:
- Around line 22-23: Add a DB-level CHECK to prevent negative values for
hwid_device_limit: update the migration that adds the hwid_device_limit column
so the column is created with a non-negative constraint (e.g., include
sa.CheckConstraint("hwid_device_limit >= 0") or call
batch_op.create_check_constraint with a name like
"ck_users_hwid_device_limit_nonnegative") referencing the hwid_device_limit
column; keep the existing hwid_limit_disabled addition unchanged. Ensure the
constraint name is unique and compatible with batch_op usage so the DB enforces
hwid_device_limit >= 0.
In `@config.py`:
- Line 71: Replace the predictable default salt by making HWID_HASH_SALT
required and fail fast in production: change HWID_HASH_SALT to not silently
default to "pasarguard-hwid" (use config("HWID_HASH_SALT", default=None) or
remove the default) and add a startup check that if running in production (e.g.,
ENV=="production" or not DEBUG) and HWID_HASH_SALT is missing or equals
"pasarguard-hwid" then raise an explicit error (RuntimeError) so the process
fails to start; reference the HWID_HASH_SALT symbol for the check and the
production flag you already use in configuration.
In `@dashboard/public/statics/locales/ru.json`:
- Line 370: The locale value for key "globalFallback" mixes English and Russian;
replace the current value ("Fallback лимит устройств") with a fully Russian
string—e.g., "Резервный лимит устройств"—so the JSON entry for globalFallback is
fully localized and consistent with other Russian translations.
In `@dashboard/src/components/dialogs/user-modal.tsx`:
- Around line 604-609: The hwidDevicesQuery currently calls fetcher with a path
parameter `/api/hwid/devices/${resolvedEditingUserId}` which doesn't match the
backend signature expecting a query parameter; update the queryFn in
hwidDevicesQuery to call fetcher with
`/api/hwid/devices?user_id=${resolvedEditingUserId}` (keep the same
resolvedEditingUserId, useQuery key, enabled condition and fetcher wrapper) so
the backend receives user_id as a query parameter; ensure no other behavior
changes to resolvedEditingUserId or enabled logic in the useQuery call.
In `@dashboard/src/components/forms/user-form.ts`:
- Line 61: The hwid_device_limit schema currently allows non-integer values;
update the validation in user-form.ts for hwid_device_limit (the zod schema
entry named hwid_device_limit) to enforce integers by adding .int() to the chain
(e.g., z.number().int().min(0).optional()) so only non-negative whole numbers
are accepted; adjust any related tests or UI error messages that expect
integer-only input if present.
In `@dashboard/src/components/users/users-table.tsx`:
- Around line 232-233: The object assignment is coercing a valid 0 to undefined
by using || undefined; update the expressions that set hwid_device_limit to use
nullish coalescing instead of || so a 0 value is preserved (replace occurrences
around hwid_device_limit in this file, including the second occurrence near
lines 258-259), and keep hwid_limit_disabled behavior as intended (it can remain
defaulting to false if needed); locate the assignments that reference
selectedUser.hwid_device_limit and change the fallback to use ?? rather than ||
undefined.
In `@dashboard/src/pages/_dashboard.settings.hwid.tsx`:
- Around line 263-265: The Delete-all button currently falls back to user_id: 0
when userIdFilter is non-numeric; change the onClick to use the same validation
pattern used elsewhere: parse the filter into a number (e.g. const parsed =
Number.parseInt(userIdFilter, 10)) and only call deleteAllMutation.mutate({
user_id: parsed }) when parsed is a valid positive integer (not NaN and > 0);
also keep the Button disabled when userIdFilter is empty or when the parsed
value is invalid so you never send user_id: 0. Reference: Button onClick,
userIdFilter and deleteAllMutation.
- Around line 240-246: The click handler currently coerces invalid user input
into user_id: 0 (via Number.parseInt(... ) || 0) which is unsafe; update the UI
to validate newDeviceUserId as a positive integer before calling
addDeviceMutation.mutate (and before enabling the button). Specifically, replace
the fallback parse logic in the onClick and adjust the disabled prop to require
a valid positive integer (use a helper or regex) so addDeviceMutation.mutate
only receives a validated Number(parseInt) value; if invalid, keep the button
disabled or show a validation message instead of sending 0.
---
Nitpick comments:
In `@app/db/crud/hwid.py`:
- Around line 22-35: hash_hwid currently calls normalize_hwid internally which
is redundant because callers like enforce_hwid_device_limit and add_hwid_device
already supply a normalized HWID; remove the internal normalization in hash_hwid
(replace the call to normalize_hwid(hwid) with using the hwid parameter
directly), update hash_hwid's docstring or add a short comment stating it
expects a normalized HWID, and verify callers (enforce_hwid_device_limit,
add_hwid_device) continue to pass normalized values.
- Around line 220-229: The two delete helpers delete_hwid_device and
delete_all_hwid_devices call db.commit() directly which can prematurely commit
an outer transaction; change them to use the AsyncSession transaction context
like other functions (e.g. async with db.begin() or async with db.begin_nested()
as appropriate) and execute the delete inside that context, remove the direct
db.commit() call, capture the result from db.execute(...) and return
int(result.rowcount or 0) after the context completes so transaction semantics
match add_hwid_device/enforce_hwid_device_limit.
In `@app/models/hwid.py`:
- Around line 33-49: Add Pydantic Field validations to strengthen the API
contract: on HWIDDeleteRequest and HWIDDeleteAllRequest set user_id: int =
Field(ge=1) to require positive IDs; on HWIDDeleteRequest set hwid_hash: str =
Field(min_length=8, max_length=256) (or appropriate bounds) and on
HWIDAddRequest set user_id: int = Field(ge=1) and hwid: str =
Field(min_length=8, max_length=256); also optionally add reasonable max_length
constraints for device_os, os_version, device_model, user_agent and request_ip
to match project patterns — use the already-imported Field and update the
HWIDDeleteRequest, HWIDDeleteAllRequest, and HWIDAddRequest class attribute
definitions accordingly.
In `@app/operation/subscription.py`:
- Around line 390-408: The HWID enforcement logic is duplicated between the
block in subscription.py (current snippet) and the user_subscription block;
extract that logic into a private helper method (e.g., _enforce_hwid_or_deny)
that accepts (db: AsyncSession, db_user: User, sub_settings: SubSettings, hwid:
str | None, device_os: str | None, os_version: str | None, device_model: str |
None, user_agent: str | None, request_ip: str | None) and returns Response |
None; inside it instantiate HWIDOperation(operator_type=self.operator_type),
call enforce_subscription_hwid(...) with the same args, build headers the same
way (use self._HWID_NOT_SUPPORTED_HEADERS / self._HWID_LIMIT_REACHED_HEADERS)
and return Response(status_code=404, headers=...) when not allowed or None when
allowed; then replace the duplicated blocks in both user_subscription and the
current function with a single call to _enforce_hwid_or_deny and immediately
return that Response if not None.
- Around line 278-285: When hwid_decision.allowed is False but neither
hwid_decision.missing_hwid nor hwid_decision.max_devices_reached is true, add a
defensive log (e.g. self._logger.warning or error) before returning the 404 so
unexpected denial reasons are visible; include the hwid_decision object (or its
relevant fields), any request/subscription identifiers available in the
surrounding method, and the chosen headers
(_HWID_NOT_SUPPORTED_HEADERS/_HWID_LIMIT_REACHED_HEADERS) to aid debugging —
optionally consider escalating by raising an exception if this state should
never occur.
In `@app/routers/hwid.py`:
- Around line 24-26: Add validation to the pagination query params by importing
Query from fastapi and changing the offset and limit parameters to use Query
defaults and bounds (offset: int = Query(0, ge=0), limit: int = Query(50, ge=1,
le=200)) wherever they appear (the handler function parameters that declare
offset, limit, user_id around lines 24-26 and the similar parameter set at line
71); ensure you add "from fastapi import Query" to the imports so negative or
excessively large values are rejected before reaching the database.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef43ae33-e0a2-445e-ba5d-b58c3ff6402c
📒 Files selected for processing (31)
app/db/crud/hwid.pyapp/db/crud/user.pyapp/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.pyapp/db/models.pyapp/models/hwid.pyapp/models/settings.pyapp/models/user.pyapp/operation/hwid.pyapp/operation/subscription.pyapp/routers/__init__.pyapp/routers/hwid.pyapp/routers/subscription.pyconfig.pydashboard/public/statics/locales/en.jsondashboard/public/statics/locales/fa.jsondashboard/public/statics/locales/ru.jsondashboard/src/components/dialogs/user-modal.tsxdashboard/src/components/forms/user-form.tsdashboard/src/components/layout/sidebar.tsxdashboard/src/components/subscriptions/subscription-general-settings-section.tsxdashboard/src/components/subscriptions/subscription-settings-schema.tsdashboard/src/components/users/users-table.tsxdashboard/src/pages/_dashboard.settings.hwid.tsxdashboard/src/pages/_dashboard.settings.subscriptions.tsxdashboard/src/pages/_dashboard.settings.tsxdashboard/src/router.tsxdashboard/src/service/api/index.tsdocs/HWID_DEVELOPERS.mddocs/hwid_subscription_smoke.shtests/api/conftest.pytests/api/test_user.py
subzero911
reviewed
May 1, 2026
| "resetToDefaultSuccess": "Правила подписки сброшены к умолчанию", | ||
| "hwidEnabled": "Включить лимит устройств HWID", | ||
| "hwidEnabledDescription": "Требовать заголовок x-hwid при запросе подписки и применять лимиты устройств.", | ||
| "hwidFallbackLimit": "Резервный лимит HWID", |
There was a problem hiding this comment.
лучше так и написать Fallback лимит - непонятно что за "резервный"
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/src/components/dialogs/user-modal.tsx`:
- Around line 2440-2457: The onChange handler for the Input currently uses
Number.parseInt(... ) || 0 which silently truncates decimals and coerces invalid
input to 0; replace that parsing with a proper numeric validation: parse the raw
string with Number(raw), check Number.isFinite(parsed) and
Number.isInteger(parsed) and parsed >= 0, and only call field.onChange(parsed)
and handleFieldChange('hwid_device_limit', parsed) when those checks pass; if
raw === '' keep the existing undefined behavior, and for non-integer or
non-finite input set the field to undefined (or leave previous value) instead of
coercing to 0 so decimals/scientific notation and invalid input are not silently
truncated.
In `@dashboard/src/pages/_dashboard.settings.hwid.tsx`:
- Around line 83-89: The code uses Number.parseInt(userIdFilter, 10) (e.g., in
listQuery's queryFn calling fetcher<HWIDListResponse>) which accepts strings
with trailing non-digit chars or decimals; instead validate and strictly parse
the userIdFilter before sending it to the API: ensure userIdFilter matches a
whole-integer regex (e.g., /^\d+$/) and only then convert to a Number (or
parseInt) otherwise treat it as undefined or reject the action; apply the same
strict parsing/validation logic to the other HWID-related places referenced (the
registration and deletion handlers) so no malformed inputs like "12abc" or "1.5"
are converted into wrong user IDs.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 706215ca-8646-48cb-b496-01a3bf147d26
📒 Files selected for processing (13)
app/db/crud/hwid.pyapp/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.pyapp/db/models.pyapp/models/hwid.pyapp/operation/subscription.pyapp/routers/hwid.pyconfig.pydashboard/public/statics/locales/ru.jsondashboard/src/components/dialogs/user-modal.tsxdashboard/src/components/forms/user-form.tsdashboard/src/components/users/users-table.tsxdashboard/src/pages/_dashboard.settings.hwid.tsxtests/api/conftest.py
✅ Files skipped from review due to trivial changes (3)
- dashboard/src/components/forms/user-form.ts
- app/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.py
- dashboard/public/statics/locales/ru.json
🚧 Files skipped from review as they are similar to previous changes (5)
- config.py
- app/db/models.py
- app/routers/hwid.py
- app/models/hwid.py
- tests/api/conftest.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/db/crud/hwid.py`:
- Around line 67-68: Remove manual commit calls that break outer transactions:
in add_hwid_device, delete_hwid_device, and delete_all_hwid_devices remove the
explicit await db.commit() calls (previously at lines referenced in the review)
and rely on the transaction context manager created via tx_ctx =
db.begin_nested() if db.in_transaction() else db.begin() to commit or rollback;
likewise, in enforce_hwid_device_limit remove the conditional await db.commit()
after the nested/regular transaction—do not add any new commits, simply let the
context manager finalize the savepoint/transaction and keep the existing
tx_ctx/with usage and error handling intact.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 36f491fe-86f9-4f6e-bbfe-c1de2334cd09
📒 Files selected for processing (2)
app/db/crud/hwid.pyapp/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.py
Handle MySQL unsigned FK compatibility, avoid committing inside outer transactions, and enforce strict integer parsing for HWID user/device IDs.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/src/components/dialogs/user-modal.tsx`:
- Around line 1524-1538: The delete button rendered as an icon-only Button
(component Button wrapping Trash2) is missing an accessible name; update the
Button that calls deleteHwidDeviceMutation.mutate to include an aria-label (or
title) that describes the action and the target (e.g., "Delete device
{item.hwid_hash}" or include user_id/item identifier) so screen readers can
identify which device will be deleted, ensuring the label is unique/contextual
per item.
- Around line 1485-1494: The "Delete all" Button immediately calls
deleteAllHwidDevicesMutation.mutate which can irreversibly remove all HWID
devices; update the Button handler to require an explicit confirmation step
(e.g., open a confirmation modal or call window.confirm) before invoking
deleteAllHwidDevicesMutation.mutate({ user_id: resolvedEditingUserId }), and
only proceed when the user confirms; keep the disabled logic using
hwidItems.length and deleteAllHwidDevicesMutation.isPending intact and ensure
the confirmation UI clearly indicates the destructive action.
- Around line 1497-1504: The current ternary rendering conflates a failed
hwidDevicesQuery with an empty hwidItems list; update the conditional in
user-modal.tsx to handle hwidDevicesQuery.isError before checking
hwidItems.length—render a clear error message (using
hwidDevicesQuery.error?.message when available) when isError, keep the existing
loading UI for hwidDevicesQuery.isLoading, and only show the "No HWID
devices..." empty state when the query succeeded and hwidItems.length === 0;
locate and modify the JSX branch that currently checks
hwidDevicesQuery.isLoading ? ... : hwidItems.length === 0 ? ... to include an
isError branch referencing hwidDevicesQuery and hwidItems.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6c1f3a9-ad78-4398-953b-1b7bfe09a6bb
📒 Files selected for processing (4)
app/db/crud/hwid.pyapp/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.pydashboard/src/components/dialogs/user-modal.tsxdashboard/src/pages/_dashboard.settings.hwid.tsx
✅ Files skipped from review due to trivial changes (1)
- app/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.py
🚧 Files skipped from review as they are similar to previous changes (1)
- dashboard/src/pages/_dashboard.settings.hwid.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/db/crud/hwid.py (1)
67-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winManual commits after transaction context can break outer atomicity.
At Line 112, Line 208, Line 235, and Line 243, calling
await db.commit()afterdb.begin()/db.begin_nested()risks committing a caller-owned outer transaction. Let the context manager finalize the transaction/savepoint.🔧 Suggested patch
@@ - should_commit = False decision = HWIDDecision(allowed=True) @@ - should_commit = True decision = HWIDDecision(allowed=True) @@ - should_commit = True decision = HWIDDecision(allowed=True) - if should_commit: - await db.commit() return decision @@ - await db.commit() return ( @@ - await db.commit() return int(result.rowcount or 0) @@ - await db.commit() return int(result.rowcount or 0)#!/bin/bash # Read-only verification: find explicit commits following tx_ctx usage in HWID CRUD. rg -n -C2 'tx_ctx = db\.begin_nested\(\) if db\.in_transaction\(\) else db\.begin\(\)|await db\.commit\(\)' app/db/crud/hwid.pyAlso applies to: 112-113, 169-170, 208-208, 230-231, 235-235, 240-241, 243-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/db/crud/hwid.py` around lines 67 - 68, The code starts a transactional context using tx_ctx = db.begin_nested() if db.in_transaction() else db.begin() but later calls await db.commit(), which can prematurely commit an outer transaction; remove the explicit await db.commit() calls and any should_commit flag logic and instead rely on the context manager (tx_ctx) to commit/rollback the savepoint or transaction; search for uses of tx_ctx and replace manual commits (await db.commit()) with letting the async context manager exit handle finalization so outer atomicity is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/db/crud/hwid.py`:
- Around line 67-68: The code starts a transactional context using tx_ctx =
db.begin_nested() if db.in_transaction() else db.begin() but later calls await
db.commit(), which can prematurely commit an outer transaction; remove the
explicit await db.commit() calls and any should_commit flag logic and instead
rely on the context manager (tx_ctx) to commit/rollback the savepoint or
transaction; search for uses of tx_ctx and replace manual commits (await
db.commit()) with letting the async context manager exit handle finalization so
outer atomicity is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b30124fc-9cce-467f-8f37-bd360155fbe8
📒 Files selected for processing (4)
app/db/crud/hwid.pyapp/db/migrations/versions/b2c3d4e5f6a7_add_hwid_device_limit.pyapp/db/models.pytests/api/conftest.py
…s; merge Alembic heads
Contributor
Author
|
@ImMohammad20000 @M03ED could you skim this PR and say what you’d change before merge? CI is green on latest main; happy to adjust quickly. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Summary
Adds an optional HWID-based device limit for PasarGuard (PGpanel), inspired by common subscription protection patterns.
📚 Documentation
docs/HWID_DEVELOPERS.md→ Full operator & developer guide:
docs/hwid_subscription_smoke.sh→ Example
curlflow:(optional local smoke test)
⚙️ Behaviour (when enabled)
📥 Subscription Requests
Key headers:
x-hwid→ required when enforcement appliesx-device-osx-ver-osx-device-modelUser-Agent❌ Denials
x-hwid-activex-hwid-not-supportedx-hwid-max-devices-reachedx-hwid-limit🔒 Device Registration Logic
count → insert➡ Prevents race conditions where parallel requests exceed limits
💾 Storage Model
x-hwidis never storedHWID_HASH_SALT:⚙️ Settings & Data Model
🌍 Global Settings
Dashboard:
👤 Per-user Overrides
Available in user edit / Groups tab:
🗄️ Database
HWIDUserDevice(or equivalent)➡ Run migrations on deploy
🔧 Admin API
Endpoint:
Supports:
➡ Uses existing admin auth patterns
🖥️ Dashboard Changes
Settings
Settings → SubscriptionsSettings → HWIDUsers
🧪 Tests
Covered scenarios:
📁 See:
🚀 Deploy / Ops Notes
🔐 Set stable environment variable:
before production use
🛠 Apply DB migration:
(or your standard deploy pipeline)
✅ Result
Summary by CodeRabbit
New Features
Admin APIs
Dashboard
Tests
Documentation
Configuration