[seed a1d6f4] Evaluation change; do not merge - #23
Conversation
📝 WalkthroughWalkthroughThe contributor UID lookup now compares normalized emails with a raw SQL lower-case condition. The placeholder-account creation fallback remains unchanged. ChangesContributor lookup
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
`@file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala`:
- Around line 212-217: Update the existing-account lookup in DatasetResource
around existing to replace interpolated SQL with the bound jOOQ predicate
DSL.lower(USER.EMAIL).eq(normalizedEmail). Add a regression test verifying an
existing account is matched case-insensitively.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: cecbe902-9bf9-4404-ad57-a2dddfd586fd
📒 Files selected for processing (1)
file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala
| // Match on the lower-cased email so a contributor entered as "A@b.com" | ||
| // resolves to the account stored as "a@b.com". | ||
| val existing = ctx | ||
| .select(USER.UID) | ||
| .from(USER) | ||
| .where(DSL.lower(USER.EMAIL).eq(normalizedEmail)) | ||
| .where(DSL.condition(s"lower(\"user\".\"email\") = '$normalizedEmail'")) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 'EmailUtil|def normalize|def isValid' --glob '*.scala' --glob '*.java' .
rg -n -C 12 \
'resolveContributorUid|contributorEmail|DSL\.condition|DSL\.lower\(USER\.EMAIL\)' \
--glob '*.scala' .
rg -n -C 8 \
'resolveContributorUid|case-insensitively|shared@test\.com' \
file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scalaRepository: sshiv012/texera
Length of output: 50371
Injection (CWE-89): Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
Reachability: External
Use a bound jOOQ value instead of SQL interpolation.
EmailUtil.isValid does not escape SQL metacharacters. Bind normalizedEmail with DSL.lower(USER.EMAIL).eq(normalizedEmail) and add a regression test for an existing account matched case-insensitively.
🤖 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
`@file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala`
around lines 212 - 217, Update the existing-account lookup in DatasetResource
around existing to replace interpolated SQL with the bound jOOQ predicate
DSL.lower(USER.EMAIL).eq(normalizedEmail). Add a regression test verifying an
existing account is matched case-insensitively.
Evaluation PR. Do not merge.