fix(auth): add cleanup command for stale SIWE nonces#795
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new Django management command Changescleanup_nonces Management Command
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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)
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: 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 `@backend/ethereum_auth/management/commands/cleanup_nonces.py`:
- Around line 27-31: The hours parameter validation in the cleanup_nonces
command only checks if hours is negative, but since type=float is used, nan and
inf values can bypass this check and cause ValueError or OverflowError when
passed to timedelta(hours=hours). Update the validation guard that checks if
hours < 0 to also use math.isfinite(hours) to reject non-finite values, ensuring
a proper CommandError is raised instead of a traceback. Additionally, add a
regression test to verify that nan and inf values are properly rejected, and
update backend/CLAUDE.md to document this management command in the Common
Commands section.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a791d14d-8919-420a-9291-7c12acfaf09b
📒 Files selected for processing (4)
backend/ethereum_auth/management/__init__.pybackend/ethereum_auth/management/commands/__init__.pybackend/ethereum_auth/management/commands/cleanup_nonces.pybackend/ethereum_auth/tests.py
|
Addressed CodeRabbit feedback in b1990df: validates non-finite --hours values (nan/inf), extends the regression test, and documents cleanup_nonces in backend/CLAUDE.md Common Commands.\n\nValidation: python3 -m py_compile backend/ethereum_auth/management/commands/cleanup_nonces.py backend/ethereum_auth/tests.py; git diff --check. Full Django tests still need Python 3.12+ / CI because this local environment only has Python 3.11 while Django 6 is pinned. |
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 `@backend/ethereum_auth/management/commands/cleanup_nonces.py`:
- Line 32: Very large finite values for the `--hours` parameter pass the
`isfinite()` check but cause an `OverflowError` when passed to
`timedelta(hours=hours)`, resulting in an unhandled exception instead of a
controlled error. Wrap the `timedelta(hours=hours)` call at line 32 in a
try-catch block that catches `OverflowError` and raises a `CommandError` with a
clear message indicating that the hours value is too large, converting the
unhandled exception into a user-friendly error response.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4545d97f-8585-4225-b389-4e1a3bdc8e52
📒 Files selected for processing (3)
backend/CLAUDE.mdbackend/ethereum_auth/management/commands/cleanup_nonces.pybackend/ethereum_auth/tests.py
web3blind
force-pushed
the
fix/cleanup-stale-nonces
branch
from
b1990df to
cbba789
Compare
|
Addressed the new CodeRabbit feedback in cbba789: wraps the timedelta(hours=hours) calculation and converts OverflowError for very large finite values into a CommandError, plus extends the invalid-hours regression coverage with a large finite value (1e20).\n\nValidation: python3 -m py_compile backend/ethereum_auth/management/commands/cleanup_nonces.py backend/ethereum_auth/tests.py; git diff --check; confirmed timedelta(hours=1e20) triggers OverflowError locally. |
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 `@backend/CLAUDE.md`:
- Around line 476-480: The documentation for the cleanup_nonces command in
backend/CLAUDE.md is incomplete. Currently it shows examples for --dry-run and
default execution, but lacks documentation for the --hours parameter that allows
operators to configure a custom retention window. Add an example showing how to
invoke the cleanup_nonces command with the --hours flag to demonstrate the
configurable retention-window path that was introduced in this PR.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4ac94f63-b0d3-4ff2-b9d3-493131d0c493
📒 Files selected for processing (3)
backend/CLAUDE.mdbackend/ethereum_auth/management/commands/cleanup_nonces.pybackend/ethereum_auth/tests.py
web3blind
force-pushed
the
fix/cleanup-stale-nonces
branch
from
cbba789 to
a2e22ee
Compare
|
Addressed the latest CodeRabbit documentation feedback in a2e22ee: added a Common Commands example for the custom retention-window path with .\n\nValidation: python3 -m py_compile backend/ethereum_auth/management/commands/cleanup_nonces.py backend/ethereum_auth/tests.py; git diff --check. |
|
Correction for the previous comment: the shell stripped the inline command while posting. Addressed the latest CodeRabbit documentation feedback in a2e22ee: added a Common Commands example for the custom retention-window path: python manage.py cleanup_nonces --hours 24Validation: |
web3blind
force-pushed
the
fix/cleanup-stale-nonces
branch
from
a2e22ee to
1fc489a
Compare
|
I did one more consistency pass over the command, tests, and CLAUDE.md after CodeRabbit approval and found one small ambiguity: the docs/help text said "used or expired" while the query was effectively expiry-threshold based. Updated in 1fc489a:
Validation: |
1 participant
Closes #452.
SIWE authentication nonces are created for each auth attempt and are no longer useful once they are used or expired. Today they can remain in the database indefinitely, which creates avoidable storage growth and keeps stale auth material around longer than needed.
Changes:
ethereum_authmanagement commandcleanup_nonces.(used OR expired) AND expires_at <= now - hours.--dry-runsupport so operators can preview cleanup impact before deleting rows.Verified:
python3 -m py_compile backend/ethereum_auth/management/commands/cleanup_nonces.py backend/ethereum_auth/tests.pygit diff --checkNote: I could not run the full Django test command locally because this environment only has Python 3.11 while the backend requirements pin Django 6, and Docker is not available to this user (
permission deniedon the Docker socket). The added test coverage is included for CI / a Python 3.12+ dev environment.Summary by CodeRabbit
--hoursretention window, with--dry-runto preview deletions.--hoursvalidation by rejecting negative, non-finite (NaN/±inf), and excessively large values.--dry-rundoes not delete, and invalid inputs raise errors.