fix: enforce hourly frequency restriction for free-tier users#250
fix: enforce hourly frequency restriction for free-tier users#250
Conversation
…sers Add FREQUENCY_TIERS constant to shared/models/auth.ts and checkFrequencyTier validation to the monitor create (main + extension) and update endpoints. Free-tier users sending frequency: "hourly" now receive a 403 with code FREQUENCY_TIER_RESTRICTED. Closes #247 https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds server-side frequency-tier enforcement: a new Changes
Sequence Diagram(s)mermaid Client->>API: POST /api/monitors (body with frequency) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
🚥 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 docstrings
🧪 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 |
The security review identified that /api/v1/monitors POST and PATCH endpoints were missing the checkFrequencyTier guard, allowing free-tier users to bypass the hourly frequency restriction via the public API. https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
Derive the required tier names from FREQUENCY_TIERS constant instead of hardcoding "pro or power". Return INVALID_FREQUENCY with 400 status for unrecognized frequency values, separate from FREQUENCY_TIER_RESTRICTED (403) for known frequencies on insufficient tiers. https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
Note that hourly frequency requires Pro or Power plan in the POST endpoint description, and add FREQUENCY_TIER_RESTRICTED to the error codes table. https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
![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 the current code and only fix it if needed.
Inline comments:
In `@server/services/monitorValidation.ts`:
- Line 50: The check using "tier as any" loses type safety; replace it with a
proper type guard or a safer assertion: ensure the runtime check uses a
predicate like isAllowedTier(tier) or cast from unknown to the specific union
type (e.g., treat tier as unknown then assert to the Tier union) before calling
allowedTiers.includes, and/or implement a function (isTier(value): value is
Tier) and call that instead of "tier as any" so that allowedTiers.includes(tier)
is type-safe; update references to allowedTiers and the tier parameter in
monitorValidation.ts accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b64d4b0b-6df4-4f61-a56f-a73e510fe915
📒 Files selected for processing (8)
client/src/pages/Developer.tsxserver/routes.tsserver/routes/extension.test.tsserver/routes/extension.tsserver/routes/v1.tsserver/services/monitorValidation.test.tsserver/services/monitorValidation.tsshared/models/auth.ts
server/services/monitorValidation.ts
Outdated
| code: "INVALID_FREQUENCY", | ||
| }; | ||
| } | ||
| if (!allowedTiers.includes(tier as any)) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Type assertion could be improved.
The as any cast works but loses type safety. Consider using a type guard or a more precise assertion:
♻️ Optional refactor for type safety
- if (!allowedTiers.includes(tier as any)) {
+ if (!(allowedTiers as readonly string[]).includes(tier)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!allowedTiers.includes(tier as any)) { | |
| if (!(allowedTiers as readonly string[]).includes(tier)) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/monitorValidation.ts` at line 50, The check using "tier as
any" loses type safety; replace it with a proper type guard or a safer
assertion: ensure the runtime check uses a predicate like isAllowedTier(tier) or
cast from unknown to the specific union type (e.g., treat tier as unknown then
assert to the Tier union) before calling allowedTiers.includes, and/or implement
a function (isTier(value): value is Tier) and call that instead of "tier as any"
so that allowedTiers.includes(tier) is type-safe; update references to
allowedTiers and the tier parameter in monitorValidation.ts accordingly.
Replace `tier as any` with `allowedTiers as readonly string[]` cast to preserve type safety while satisfying TypeScript's includes() check. Addresses CodeRabbit review feedback on PR #250. https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
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 `@server/services/monitorValidation.ts`:
- Around line 41-43: The current validation treats falsy values (so "" passes)
and does an unsafe dynamic lookup on FREQUENCY_TIERS, enabling prototype
pollution; change the initial check to only allow undefined as "no frequency"
(use frequency === undefined) and reject empty string inputs with a 400 error,
and before indexing FREQUENCY_TIERS guard the lookup with
Object.prototype.hasOwnProperty.call(FREQUENCY_TIERS, frequency) so inherited
keys (e.g. "__proto__") are not accepted; if the hasOwnProperty check fails
return a structured validation error, otherwise safely read allowedTiers =
FREQUENCY_TIERS[frequency as keyof typeof FREQUENCY_TIERS] and proceed with the
existing .includes() logic.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c76239eb-62cc-4c3f-a3eb-51415fd45996
📒 Files selected for processing (1)
server/services/monitorValidation.ts
bd73-com
dismissed
coderabbitai[bot]’s stale review
Fixed: replaced 'tier as any' with safer cast in commit 4a6e245.
…string Use hasOwnProperty guard before indexing FREQUENCY_TIERS to prevent __proto__ key access. Use strict undefined check instead of falsy check to reject empty-string frequency values with a proper 400 error. https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
bd73-com
dismissed
coderabbitai[bot]’s stale review
Fixed: added hasOwnProperty guard and strict undefined check in commit cb15151.
2 participants
Summary
Adds server-side enforcement preventing free-tier users from using hourly check frequency on monitors. Previously, only the client UI restricted this — a direct API call could bypass it entirely. This closes the authorization gap across all four monitor-creation/update surfaces.
Changes
Core logic
FREQUENCY_TIERSconstant toshared/models/auth.tsdefining which tiers allow each frequencycheckFrequencyTier()validation function toserver/services/monitorValidation.tsFREQUENCY_TIER_RESTRICTED(403) for tier violations,INVALID_FREQUENCY(400) for unknown valuesEndpoint enforcement
POST /api/monitorsandPATCH /api/monitors/:id(SPA routes) — frequency tier check addedPOST /api/extension/monitors(extension route) — frequency tier check addedPOST /api/v1/monitorsandPATCH /api/v1/monitors/:id(public API) — frequency tier check addedTests
checkFrequencyTiercovering all tier/frequency combos, edge cases, error messagesDocumentation
Developer.tsx: Added tier restriction note to POST /monitors docs, addedFREQUENCY_TIER_RESTRICTEDto error codes tableHow to test
/api/monitorswithfrequency: "hourly"— expect 403 withFREQUENCY_TIER_RESTRICTED/api/v1/monitors/:idwithfrequency: "hourly"— expect 403npm run check && npm run test— all 1725 passRelated issues
https://claude.ai/code/session_01GWwVsyBJt7ZjxS9YJZdYpr
Summary by CodeRabbit
New Features
Documentation
Tests