Dev#105
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 9 minutes and 35 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
WalkthroughThis release removes the Kener external status integration, optimizes file uploads with asynchronous VirusTotal scanning, threads emailVerified through the auth system, persists storage/Stripe caches across hot-reloads, hardens integration test input validation, and fixes pagination. ChangesKener Status Page Integration Removal
File Upload Async VirusTotal & Optimization
Auth System: emailVerified Field
Storage Provider & Stripe Sync Cache Persistence
Integration Test Input Validation & Error Handling
Minor Fixes & Cleanups
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…ble types' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/files/route.ts (1)
55-73:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix
emailVerifiedshape before assigning squad owner to authenticated user.This is the compile blocker from CI (TS2322 around Line 72). Prisma returns
emailVerifiedasDate | null, but this flow expects a boolean contract. Convert it when building the acting user object (and when passing preloaded values).💡 Suggested fix
- const ownerUser = await prisma.user.findUnique({ + const ownerUserRaw = await prisma.user.findUnique({ where: { id: squad.ownerUserId }, select: { id: true, email: true, name: true, storageUsed: true, storageQuotaMB: true, urlId: true, role: true, randomizeFileUrls: true, preferredUploadDomain: true, emailVerified: true, }, }) - if (!ownerUser) + if (!ownerUserRaw) return apiError('Squad owner not found', HTTP_STATUS.UNAUTHORIZED) - user = ownerUser + user = { + ...ownerUserRaw, + emailVerified: Boolean(ownerUserRaw.emailVerified), + } @@ - { emailVerified: user.emailVerified, role: user.role } + { emailVerified: Boolean(user.emailVerified), role: user.role }Also applies to: 169-174
🤖 Prompt for 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. In `@app/api/files/route.ts` around lines 55 - 73, Prisma's ownerUser.emailVerified is Date|null but the code expects a boolean; when building the acting user set user = ownerUser, convert emailVerified to a boolean (e.g., !!ownerUser.emailVerified or Boolean(ownerUser.emailVerified)) so the user object matches the expected shape, and apply the same conversion to any other preload/assignment sites that pass emailVerified (the other occurrence noted around the later preloaded values) so all usages (prisma.user.findUnique result, assigned user variable, and any preloadedValue assignments) store a boolean rather than a Date|null.Source: Pipeline failures
🤖 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 @.github/README.md:
- Line 5: The README still mentions "Kener" in the feature/stack sections;
remove or update those references to reflect the new status-page approach by
replacing any "Kener" text/links with the new status-page wording or removing
the stale bullet, and update any related badges/links so the feature/stack
entries and examples reference the status-page integration instead of Kener
(search for "Kener" in the README to locate and change each occurrence).
In `@app/api/admin/integrations/test/route.ts`:
- Around line 61-63: Several provider fetches (e.g., the Stripe call that uses
secretKey and the other provider fetches) lack a timeout and can hang; add the
same AbortController timeout pattern used elsewhere: create an AbortController,
start a setTimeout that calls controller.abort() after the desired timeout, and
pass { signal: controller.signal, headers: { ... } } into each fetch options
object (replace plain fetch calls that only set headers). Use the same timeout
duration and clear the timer after the response is received; ensure any existing
variable names like secretKey are preserved and only the fetch options and
timeout wiring are changed.
In `@app/api/files/route.ts`:
- Around line 124-125: The code converts uploadedFile.size to MB via bytesToMB
for quota checks but then persists that MB value into byte-based fields,
corrupting accounting; revert storage writes to bytes: use the original
uploadedFile.size (or convert MB back to bytes with MB * 1024 * 1024 if you only
have the MB value) when assigning file.size and when incrementing storageUsed,
and only use bytesToMB(uploadedFile.size) for validation/display logic
(references: bytesToMB, uploadedFile.size, file.size, storageUsed).
- Around line 275-289: The quarantine branch inside the scanWithVirusTotal
callback currently calls Promise.allSettled([...]) and ignores the results;
update this to await the Promise.allSettled result, inspect each outcome for a
rejected status from storageProvider!.deleteFile(filePath) or
prisma.file.update({ where: { id: fileRecord.id }, ... }), and handle failures
explicitly (e.g., log error with fileId and vtResult, surface or rethrow if
critical, and optionally retry or mark a quarantine-failure flag in the DB).
Ensure the handler preserves the original quarantine behavior on success but
does not silently swallow failures from Promise.allSettled.
In `@CHANGELOG.md`:
- Around line 27-29: The changelog entry inaccurately states the `/api/status`
route was removed; update the wording in the release notes to reflect the
current state by saying the dynamic status integration was removed and the
`/api/status` route remains present but now returns a 404 NOT_FOUND (rather than
being deleted), and indicate that StatusIndicator now links statically to
emberlystat.us and no longer performs external API calls; ensure the terms
`/api/status` and `StatusIndicator` are used so readers can locate the affected
behavior.
In `@packages/components/layout/StatusIndicator.tsx`:
- Around line 13-14: The StatusIndicator component currently renders a static
green dot and the text "System Status" which implies live healthy state; update
the JSX in StatusIndicator to use a neutral indicator and wording since it's
just a link: change the span's className from "bg-emerald-500" to a neutral
color like "bg-gray-400" (or a neutral token used in your design system) and
change the adjacent label text from "System Status" to "View System Status"
(also update any aria-label/title to match) so the UI no longer implies a live
health check.
In `@packages/lib/cache/session-cache.ts`:
- Line 42: When hydrating cached sessions replace the direct cast "return
JSON.parse(data) as CachedUserSession" with code that parses the JSON into a
temporary object, ensures the emailVerified property exists and is coerced to a
boolean (e.g., set obj.emailVerified = Boolean(obj.emailVerified) or default to
false if missing), then return the object typed as CachedUserSession;
specifically target the JSON.parse(...) expression and the CachedUserSession
result so older Redis entries missing emailVerified are backfilled on read.
In `@packages/lib/config/index.ts`:
- Around line 166-237: The config allows passthrough of unknown integration keys
so legacy fields like integrations.kener can survive safeParse/deepMerge and be
returned to ADMIN; update the fix by either: (A) tightening the configSchema for
integrations to remove unknown keys (remove .passthrough() on the integrations
object and provider sub-objects in configSchema) so unknown providers are
dropped during parse; or (B) explicitly strip legacy/unknown keys in
maskSecretsForAdmin() (add logic to remove integrations.kener and/or filter
integrations to a whitelist of known providers) so ADMIN responses never include
stale kener data; ensure changes touch the integrations schema in
config/index.ts (the integrations object and its provider sub-objects) and the
maskSecretsForAdmin() function used by app/api/settings/route.ts so the leak is
closed.
In `@packages/lib/files/filename.ts`:
- Around line 98-103: The sanitized slug (urlSafeName) can become empty after
replacing/trimming, so after the slice(s, e) in the slug creation block check if
urlSafeName === '' and if so assign a safe fallback (e.g. 'file' or 'untitled'
or 'file-' + a short random/hash) before it is used further; update the variable
urlSafeName in that same block (where baseNameWithoutExt and urlSafeName are
used) so downstream path construction that appends the extension never receives
an empty string.
In `@packages/lib/storage/quota.ts`:
- Around line 255-266: The code incorrectly treats a deliberate zero admin
override as "unset" because it uses falsy checks; change both checks that read
if (!baseQuotaMB) (and the subsequent if (!baseQuotaMB && defaultQuotaMB)) to
null/undefined checks so 0 is honored—e.g. use if (baseQuotaMB == null) (or if
(baseQuotaMB === undefined || baseQuotaMB === null)) when deciding to compute
plan-based quota, and use if (baseQuotaMB == null && defaultQuotaMB != null)
when falling back to defaultQuotaMB; keep the existing arithmetic using
planLimits.storageQuotaGB and perkStorageBonusGB unchanged.
- Around line 99-103: After performing the one-time Stripe sync (the block using
stripeSyncCache and STRIPE_SYNC_TTL_MS) do not simply return the most-recent
active subscription; re-apply the existing "storage-bucket-*" precedence logic
before returning limits. Concretely: when determining latestActiveSubscription
after the sync, prefer any active subscription whose product/type startsWith
"storage-bucket-" (and treat it with the bucket/unlimited semantics the original
precedence expects) over other active subscriptions, so you don't accidentally
return capped limits when a storage-bucket subscription exists; update the
selection logic that currently picks the newest active subscription to check for
and prefer storage-bucket-* subscriptions first.
---
Outside diff comments:
In `@app/api/files/route.ts`:
- Around line 55-73: Prisma's ownerUser.emailVerified is Date|null but the code
expects a boolean; when building the acting user set user = ownerUser, convert
emailVerified to a boolean (e.g., !!ownerUser.emailVerified or
Boolean(ownerUser.emailVerified)) so the user object matches the expected shape,
and apply the same conversion to any other preload/assignment sites that pass
emailVerified (the other occurrence noted around the later preloaded values) so
all usages (prisma.user.findUnique result, assigned user variable, and any
preloadedValue assignments) store a boolean rather than a Date|null.
🪄 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: CHILL
Plan: Pro
Run ID: aa365595-4dd3-495e-a667-6d8b113d7cef
📒 Files selected for processing (23)
.github/README.mdCHANGELOG.mdapp/api/admin/integrations/test/route.tsapp/api/files/route.tsapp/api/settings/route.tsapp/api/status/route.tspackage.jsonpackages/components/admin/settings/settings-manager.tsxpackages/components/dashboard/user-list.tsxpackages/components/layout/StatusIndicator.tsxpackages/components/layout/footer.tsxpackages/lib/auth/api-auth.tspackages/lib/cache/session-cache.tspackages/lib/config/index.tspackages/lib/events/handlers/file-expiry.tspackages/lib/files/filename.tspackages/lib/files/security-validation.tspackages/lib/files/upload-validation.tspackages/lib/kener/index.tspackages/lib/storage/index.tspackages/lib/storage/quota.tspackages/lib/storage/sync-buckets.tspackages/lib/utils/index.ts
💤 Files with no reviewable changes (1)
- packages/components/layout/footer.tsx
…ble types' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Merged
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
1 participant
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 by CodeRabbit
Security Improvements
Performance
Removals
Bug Fixes