Security: allow-list the model identifier in Limitations::remove_limitations#1375
Security: allow-list the model identifier in Limitations::remove_limitations#1375vuckro wants to merge 2 commits into
Conversation
…_limitations
remove_limitations() interpolated its $slug argument straight into the meta
table name and the id column name of a DELETE query. $slug originates from
the request ('model') in handle_confirm_limitations_reset(), and wu_request()
does not make a value identifier-safe, so a crafted model value could inject
SQL through the table/column identifier.
Validate $slug against a fixed allow-list of the models that actually carry
limitations (membership, product, customer, site) before it is used to build
any SQL.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 22 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Stuck-merge detector: PR has been merge-eligible but unmerged past the thresholdThe pulse merge pass has classified PR #1375 as Failing checks on PR #1375
Worker guidance for the next attempt
Why you're seeing thisEvery pulse cycle (~120s) the deterministic merge pass re-evaluates open PRs. PRs that pass APPROVED + MERGEABLE but fail required checks have historically been re-evaluated silently every cycle until a human noticed. The stuck-merge detector (t3193) surfaces them after Posted automatically by aidevops.sh v3.20.57 automated scan. |
Summary
Limitations::remove_limitations()interpolated its$slugargument straightinto the meta table name and the id column name of a
DELETEquery.$slugoriginates from request input (model) inhandle_confirm_limitations_reset(), andwu_request()does not make a valueidentifier-safe, so a crafted value could inject SQL through the identifier
(the bound
%did is safe; the identifier is not).Changes
Validate
$slugagainst a fixed allow-list of the models that actually carrylimitations (
membership,product,customer,site) before it is used tobuild any SQL.
Compatibility
All legitimate callers pass one of the allow-listed models, so the reset flow is
unchanged.
Part of a small series of focused security hardening PRs. Full technical detail
is available privately to the maintainers on request (coordinated disclosure).