-
Notifications
You must be signed in to change notification settings - Fork 0
hardcode some limits (for now) #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds hardcoded resource limits to prevent unbounded stack creation in the system. It introduces a per-user limit of 3 stacks and a global limit of 100 stacks across all users.
Changes:
- Added limit constants and validation functions to check user and global stack counts
- Refactored
create_stackfunctions to validate limits before insertion - Added error handling in the stack controller for limit exceeded scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/lib/ethui/stacks.ex | Adds limit constants, count queries, validation functions, and integrates limit checks into stack creation logic |
| server/lib/ethui_web/controllers/api/stack_controller.ex | Adds HTTP error responses (403/503) for user and global limit exceeded scenarios |
Comments suppressed due to low confidence (2)
server/lib/ethui/stacks.ex:159
- The limit checking has a race condition vulnerability. The count is checked separately from the insert operation, which means multiple concurrent requests could pass the limit check before any insertions complete. This could allow the global limit to be exceeded.
Consider wrapping the count check and insert in a database transaction, or using a database constraint to enforce the limit atomically.
def get_user_stack_by_slug(nil, slug) do
Repo.get_by(Stack, slug: slug)
|> Repo.preload(:api_key)
end
server/lib/ethui/stacks.ex:164
- The limit checking has a race condition vulnerability. The count is checked separately from the insert operation, which means multiple concurrent requests could pass the limit check before any insertions complete. This could allow users to exceed both the per-user limit and global limit.
Consider wrapping the count check and insert in a database transaction, or using a database constraint to enforce the limits atomically. For example, you could add the count check as part of the Ecto.Multi transaction.
get_user_stack_by_slug(user_id, slug)
end
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {:error, :user_limit_exceeded} -> | ||
| conn | ||
| |> put_status(403) | ||
| |> json(%{ | ||
| status: "error", | ||
| error: "User stack limit reached (maximum 3 stacks per user)" | ||
| }) | ||
|
|
||
| {:error, :global_limit_exceeded} -> | ||
| conn | ||
| |> put_status(503) | ||
| |> json(%{ | ||
| status: "error", | ||
| error: "Global stack limit reached (maximum 100 stacks)" | ||
| }) |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new limit enforcement behavior lacks test coverage. Given that the repository has comprehensive test coverage for the stack controller (as seen in stack_controller_test.exs), tests should be added to verify:
- That creating a stack succeeds when under the user limit
- That creating a 4th stack for the same user returns a 403 error with the appropriate message
- That creating a stack when the global limit is reached returns a 503 error with the appropriate message
- That different users can each create up to 3 stacks
| {:error, :user_limit_exceeded} -> | ||
| conn | ||
| |> put_status(403) | ||
| |> json(%{ | ||
| status: "error", | ||
| error: "User stack limit reached (maximum 3 stacks per user)" | ||
| }) | ||
|
|
||
| {:error, :global_limit_exceeded} -> | ||
| conn | ||
| |> put_status(503) | ||
| |> json(%{ | ||
| status: "error", | ||
| error: "Global stack limit reached (maximum 100 stacks)" | ||
| }) |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded limit values in the error messages create a maintainability issue. If the module constants @max_stacks_per_user or @max_total_stacks are changed, these error messages will become inaccurate.
Consider using string interpolation to reference the module constants directly, e.g., "User stack limit reached (maximum #{@max_stacks_per_user} stacks per user)" and "Global stack limit reached (maximum #{@max_total_stacks} stacks)".
No description provided.