π‘οΈ Sentinel: [HIGH] μ μ± μ°ν μλͺ μ μν μνΈν κ²μ¦ μΆκ°#36
π‘οΈ Sentinel: [HIGH] μ μ±
μ°ν μλͺ
μ μν μνΈν κ²μ¦ μΆκ°#36seonghobae wants to merge 4 commits into
Conversation
- μ μ± μ°ν μ νλμ½λ©λ ν ν° λμ `conversion.policy-override-secret`μ μ¬μ©ν HMAC-SHA256 μνΈν μλͺ κ²μ¦μ λμ νμ΅λλ€. - νμ΄λ° 곡격 λ°©μ§λ₯Ό μν΄ `MessageDigest.isEqual`μ μ¬μ©ν μμ μκ° λΉκ΅λ₯Ό ꡬννμ΅λλ€. - λ³κ²½λ μΈμ¦ λ©μ»€λμ¦μ λ°μνλλ‘ λ¨μ ν μ€νΈ λ° μμ±μ μ λ°μ΄νΈνμ΅λλ€.
|
π 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. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the βpolicy overrideβ path (used to bypass blocked upload extensions) by replacing a hardcoded token check with cryptographic verification, making policy bypass contingent on a valid HMAC-SHA256 signature derived from a configured secret.
Changes:
- Added
conversion.policy-override-secretconfiguration and plumbed it throughConversionPropertiesintoDefaultDocumentValidationService. - Implemented HMAC-SHA256 signature verification for policy overrides and updated failure modes/messages.
- Updated/expanded tests to use signed tokens and cover invalid signature / missing-secret scenarios.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java | Adds HMAC-based verification for policy override requests. |
| src/main/java/com/clearfolio/viewer/config/ConversionProperties.java | Introduces a new policyOverrideSecret configuration property. |
| src/main/resources/application.yml | Wires conversion.policy-override-secret from an environment variable. |
| src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java | Updates tests for signed tokens; adds negative cases for invalid signature and missing secret. |
| src/test/java/com/clearfolio/viewer/service/ServiceInterfaceDefaultMethodsTest.java | Updates override token value in default-method delegation tests. |
| src/test/java/com/clearfolio/viewer/service/PolicyOverrideRequestTest.java | Updates expected approval token values in request construction tests. |
| src/test/java/com/clearfolio/viewer/service/DefaultDocumentConversionServiceTest.java | Updates expected forwarded approval token values. |
| src/test/java/com/clearfolio/viewer/controller/ConversionControllerTest.java | Updates expected forwarded approval token values. |
| src/test/java/com/clearfolio/viewer/controller/ConversionControllerMultipartLimitTest.java | Configures conversion.policy-override-secret for integration tests and updates signed tokens. |
| .jules/sentinel.md | Documents the vulnerability, learning, and prevention for Sentinel tracking. |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- μ μ± μ°ν μ νλμ½λ©λ ν ν° λμ `conversion.policy-override-secret`μ μ¬μ©ν HMAC-SHA256 μνΈν μλͺ κ²μ¦μ λμ νμ΅λλ€. - νμ΄λ° 곡격 λ°©μ§λ₯Ό μν΄ `MessageDigest.isEqual`μ μ¬μ©ν μμ μκ° λΉκ΅λ₯Ό ꡬννμ΅λλ€. - λ³κ²½λ μΈμ¦ λ©μ»€λμ¦μ λ°μνλλ‘ λ¨μ ν μ€νΈ λ° μμ±μ μ λ°μ΄νΈνμ΅λλ€.
OpenCode Review Overview
Change Flow DAGflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (10 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (10 files)"]
R1 --> V1["required checks"]
|
- μ μ± μ°ν μ 곡μ μν¬λ¦Ώ κΈ°λ°μ νλμ½λ©λ ν ν° λμ `conversion.policy-override-public-key`λ₯Ό μ¬μ©ν λΉλμΉν€(RSA-SHA256) μνΈν μλͺ κ²μ¦μ λμ νμ΅λλ€. - `approverId + ":" + extension` νμμ νμ΄λ‘λλ₯Ό μλͺ νμ¬ ν ν° μμ‘°λ₯Ό λ°©μ§νμ΅λλ€. - μλͺ κ²μ¦ λ‘μ§μ΄ μ¬λ°λ₯΄κ² λμνλλ‘ ν μ€νΈ μνΈ λ΄μ ν ν°λ€μ λμ μΌλ‘ μμ±λ μ ν¨ν RSA μλͺ κ°μΌλ‘ κ°±μ νμ΅λλ€.
|
μ€λ³΅ μ 리(2μ°¨): 'μ μ± μλͺ μνΈκ²μ¦' ν΄λ¬μ€ν° μ€λ³΅ β μ λ³Έ #83 μ μ§. λμΌ λͺ©μ /νμΌλ‘ λμ λ³ν© λΆκ°. κ³ μ λ³κ²½μ #83μ λ°μνκ±°λ μ¬μ€ν. |
νμΈνμ΅λλ€. μ΄ μμ μ λ μ΄μ νμνμ§ μμ κ²μΌλ‘ κ°μ£Όνκ³ μ€λ¨νκ² μ΅λλ€. |
π¨ μ¬κ°λ: λμ
π‘ μ·¨μ½μ : μ μ± μ°ν κΈ°λ₯μ΄ μμμ νλμ½λ©λ λ¬Έμμ΄ ν ν°(
approvalToken)μ μμ‘΄νμ¬ μμ€ν μ μνΈν μλͺ κ²μ¦ 보μ νμ€μ μλ°νμ΅λλ€.π― μν₯: μ€μν 보μ μ μ± (μ: μ°¨λ¨λ νμΌ νμ₯μ μ λ‘λ νμ©)μ΄ λ¬΄λ¨μΌλ‘ μ½κ² μ°νλ μ μμ΅λλ€.
π§ μμ μ¬ν:
approvalTokenμ μ΄μ approverId + ":" + extensionνμμ νμ΄λ‘λμ λν HMAC μλͺ κ°μ΄μ΄μΌ ν©λλ€.conversion.policy-override-secretμ κΈ°λ°μΌλ‘ μμ± λ° κ²μ¦λ©λλ€.MessageDigest.isEqualμ μ¬μ©νμ¬ νμ΄λ° 곡격μ λ°©μ§νμ΅λλ€.β κ²μ¦:
mvn testλ₯Ό μ€ννμ¬ μλ‘μ΄ HMAC μλͺ μ ν¨μ± κ²μ¬κ° μ±κ³΅νκ³ 100% ν μ€νΈ 컀λ²λ¦¬μ§κ° μ μ§λλμ§ νμΈνμ΅λλ€. μ ν¨νμ§ μμ μλͺ κ³Ό μ€μ λμ§ μμ μν¬λ¦Ώμ λν μ€ν¨ μ¬λ‘λ μΆκ°λμμ΅λλ€.PR created automatically by Jules for task 9334741202511535821 started by @seonghobae