π‘οΈ Sentinel: [CRITICAL] Admin APIμ κΆν μ μ΄ μ°ν λ° λ°μ΄ν° μ μΆ μ·¨μ½μ ν¨μΉ - #359
π‘οΈ Sentinel: [CRITICAL] Admin APIμ κΆν μ μ΄ μ°ν λ° λ°μ΄ν° μ μΆ μ·¨μ½μ ν¨μΉ#359seonghobae wants to merge 1 commit into
Conversation
AdminControllerμ λͺ¨λ μλν¬μΈνΈμ μΈμ¦ λ° κΆν μ μ΄κ° λλ½λμ΄ μμ΄, 곡격μκ° κΆν μμ΄ λͺ¨λ ν λνΈμ μμ μ μ‘°ννκ³ μ‘°μν μ μλ μ¬κ°ν μ·¨μ½μ μ΄ μ‘΄μ¬νμ΅λλ€. μ΄μ TenantAccessServiceλ₯Ό μ£Όμ νμ¬ μΈμ¦ λ° κ΄λ¦¬μ κΆνμ κ°μ νκ³ , ν λνΈ μλ³μλ₯Ό ν΅ν λ°μ΄ν° 격리(job.belongsToTenant)λ₯Ό μννλλ‘ μ½λλ₯Ό μμ νμ΅λλ€. λν μ¬μλ μμ μ νλΌμ΄λ²μ 보νΈλ₯Ό μν΄ operatorIdλ₯Ό SHA-256μΌλ‘ κ°λͺ ν μ²λ¦¬νμμ΅λλ€.
|
π Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a π emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
π WalkthroughWalkthroughκ΄λ¦¬μ μμ APIμ ν λνΈ μΈμ¦, κΆν κ²μ¬, μμ μμ κΆ κ²μ¦μ μΆκ°νμ΅λλ€. μμ λͺ©λ‘μ ν λνΈ λ²μλ‘ μ ννμ΅λλ€. μμ μ μ¬μλ μμ²μ κΆνμ νμΈνλ©°, μ¬μλ 주체 IDλ SHA-256μΌλ‘ κ°λͺ νν©λλ€. Changesκ΄λ¦¬μ μμ μ κ·Ό μ μ΄
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AdminController
participant TenantAccessService
participant DocumentConversionService
Client->>AdminController: ν
λνΈΒ·κΆν ν€λμ μμ
μμ² μ λ¬
AdminController->>TenantAccessService: κΆν λ° μμ
μμ κΆ κ²μ¦
TenantAccessService-->>AdminController: κ²μ¦ κ²°κ³Ό λ°ν
AdminController->>DocumentConversionService: μ ν¨ν μμ λλ ν΄μλ 주체μ μ¬μλ μμ²
DocumentConversionService-->>AdminController: μμ
κ²°κ³Ό λ°ν
AdminController-->>Client: μμ
λͺ©λ‘ λλ HTTP μν λ°ν
Possibly related PRs
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
β¨ Finishing Touches π‘ 1π οΈ Fix failing CI checks π‘
π 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
π€ 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 `@src/main/java/com/clearfolio/viewer/controller/AdminController.java`:
- Around line 61-67: Update the JavaDoc for the conversion-job retrieval method
near the shown documentation to state that it returns only jobs belonging to
context.tenantId(), not all conversion jobs. Also revise the `@return` description
to specify the list of jobs for the current tenant, while leaving the
implementation unchanged.
In `@src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java`:
- Around line 135-143: Update retryDeadLetteredReturnsAcceptedWhenAccepted so
the conversionService.retryDeadLettered verification matches the exact
precomputed SHA-256 hexadecimal digest of "user-1" instead of anyString(). Keep
the job setup and accepted-result behavior unchanged, and ensure the assertion
proves hashSubjectId supplies that exact value.
πͺ 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: adfa9e74-efaa-486a-8664-20c0ad4b185b
π Files selected for processing (3)
.jules/sentinel.mdsrc/main/java/com/clearfolio/viewer/controller/AdminController.javasrc/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java
| /** | ||
| * Retrieves all conversion jobs, optionally filtered by dead-letter status. | ||
| * | ||
| * @param headers request headers carrying tenant claims | ||
| * @param deadLettered optional filter for dead-lettered jobs | ||
| * @return list of conversion jobs | ||
| */ |
There was a problem hiding this comment.
π Maintainability & Code Quality | π‘ Minor | β‘ Quick win
ν λνΈ λ²μλ₯Ό JavaDocμ λ°μνμμμ€.
Line 62μ βall conversion jobsβ μ€λͺ
μ ꡬνκ³Ό λ€λ¦
λλ€. μ΄ λ©μλλ context.tenantId()μ μν μμ
λ§ λ°νν©λλ€. @return μ€λͺ
λ νμ¬ ν
λνΈμ μμ
λͺ©λ‘μΌλ‘ λ³κ²½νμμμ€.
π€ 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 `@src/main/java/com/clearfolio/viewer/controller/AdminController.java` around
lines 61 - 67, Update the JavaDoc for the conversion-job retrieval method near
the shown documentation to state that it returns only jobs belonging to
context.tenantId(), not all conversion jobs. Also revise the `@return` description
to specify the list of jobs for the current tenant, while leaving the
implementation unchanged.
| @Test | ||
| void retryDeadLetteredReturnsAcceptedWhenAccepted() { | ||
| mockTenantContext(TenantPermissions.JOB_RETRY); | ||
| UUID jobId = UUID.randomUUID(); | ||
| when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.ACCEPTED); | ||
| ConversionJob job = new ConversionJob(jobId, "tenant-a", "user-1", | ||
| "a.pdf", "application/pdf", "hash", 100L, 3); | ||
| when(conversionService.getJob(jobId)).thenReturn(Optional.of(job)); | ||
| when(conversionService.retryDeadLettered(eq(jobId), anyString())) | ||
| .thenReturn(RetryDeadLetterResult.ACCEPTED); |
There was a problem hiding this comment.
π Security & Privacy | π Major | β‘ Quick win
μ¬μλ 주체 κ°λͺ ν κ°μ μ νν κ²μ¦νμμμ€.
Line 142μ anyString()μ μλ³Έ "user-1" λλ μμμ λ¬Έμμ΄λ νμ©ν©λλ€. hashSubjectIdκ° μ κ±°λκ±°λ μλͺ»λ κ°μ λ°νν΄λ μ΄ ν
μ€νΈλ ν΅κ³Όν©λλ€. retryDeadLettered νΈμΆμ΄ "user-1"μ μ νν SHA-256 hexadecimal κ°λ§ λ°λμ§ κ²μ¦νμμμ€.
As per coding guidelines, com.clearfolio.viewer.*μ μμ° λ³κ²½μ 100% JaCoCo line and branch coverageλ₯Ό μ μ§ν΄μΌ ν©λλ€.
π€ 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 `@src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java`
around lines 135 - 143, Update retryDeadLetteredReturnsAcceptedWhenAccepted so
the conversionService.retryDeadLettered verification matches the exact
precomputed SHA-256 hexadecimal digest of "user-1" instead of anyString(). Keep
the job setup and accepted-result behavior unchanged, and ensure the assertion
proves hashSubjectId supplies that exact value.
Source: Coding guidelines
1 participant
AdminControllerμTenantAccessServiceλ₯Ό μ£Όμ νμ¬ λͺ μμ μΈ κ΄λ¦¬μ κΆν(JOB_READ,JOB_DELETE,JOB_RETRY) λ° ν λνΈ μμ κΆμ νμΈνλλ‘ μμ νκ³ , μ΄μμ ID κΈ°λ‘ μ κ°λͺ ν μ²λ¦¬ μΆκ°.PR created automatically by Jules for task 17983354558215036824 started by @seonghobae
Summary by CodeRabbit