π‘οΈ Sentinel: [HIGH] νμΌ μ λ‘λ μ λ λ°μ΄νΈ μ½μ μ·¨μ½μ μμ #68
π‘οΈ Sentinel: [HIGH] νμΌ μ
λ‘λ μ λ λ°μ΄νΈ μ½μ
μ·¨μ½μ μμ #68seonghobae wants to merge 1 commit into
Conversation
νμΌ μ λ‘λ μ μλ³Έ νμΌλͺ μ λ λ°μ΄νΈ(`\u0000`)κ° ν¬ν¨λ κ²½μ° μ΄λ₯Ό λͺ μμ μΌλ‘ μ°¨λ¨νλ λ‘μ§μ `DefaultDocumentValidationService`μ μΆκ°νμ΅λλ€. μ΄λ₯Ό ν΅ν΄ νμ₯μ μ ν¨μ± κ²μ¬ μ°ν λ±μ νμΌλͺ κ΄λ ¨ μ·¨μ½μ μ λ°©μ΄ν©λλ€. κ΄λ ¨ λ¨μ ν μ€νΈλ ν¨κ» μΆκ°νμ¬ μ»€λ²λ¦¬μ§ 100%λ₯Ό λ¬μ±νμ΅λλ€.
|
π 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
Hardens the document upload validation layer against null byte (\u0000) injection in filenames, preventing extension-validation bypass during multipart upload handling.
Changes:
- Rejects uploads whose
originalFilenamecontains a null byte by throwingIllegalArgumentExceptionearly invalidateOrThrow. - Adds a unit test ensuring filenames like
contract\u0000.pdfare rejected with the expected error message. - Updates the Jules Sentinel note to document the null byte injection prevention learning.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java | Adds early filename null-byte validation and rejects invalid input before extension parsing. |
| src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java | Adds a regression test covering null byte injection in filenames. |
| .jules/sentinel.md | Documents the null byte injection prevention guidance. |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode exhausted the configured model pool without a usable current-head review conclusion. This is not approval evidence, so the PR is blocked until a source-backed review can establish approval sufficiency or identify concrete fixes.
Findings
1. HIGH .jules/sentinel.md:1 - OpenCode could not establish approval sufficiency
- Problem: every configured model path failed to produce a usable current-head control block.
- Root cause: model execution, timeout, export, normalization, or approval-gate validation did not complete after exponential retry across the configured model pool.
- Impact: approving from deterministic check state alone would miss PR-intent mismatches, missing files, edge-case bugs, robustness gaps, UX/DX regressions, security issues, and CodeGraph-backed base/head flow changes.
- Fix: rerun OpenCode after model availability recovers, or update the PR with the missing files, tests, docs, generated artifacts, and verification evidence needed for a source-backed review conclusion.
- Regression test: keep the approval gate posting REQUEST_CHANGES, not APPROVE or check-only failure, when no model produces a valid current-head review.
Summary
- Result: REQUEST_CHANGES
- Reason: coverage-evidence passed and peer GitHub Checks completed without failures, but no model produced a valid review control block.
- Deterministic evidence checked but not used for approval: current-head changed-file evidence (.jules/sentinel.md, src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java, src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java); coverage-evidence result success; peer checks from statusCheckRollup excluding this OpenCode check.
- Model outcome: model_pool=exhausted; selected_model=none.
- Head SHA:
86232024bac52e3cffaa4acd04747f1cadf654e0 - Workflow run: 28547653612
- Workflow attempt: 1
No PR approval was posted because model-output failure is not evidence that the PR has no blockers.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
| **Vulnerability:** Untrusted paths from API responses were directly assigned to `a.href` and used in `iframe` generation, which allows execution of malicious URIs like `javascript:` or `data:`. | ||
| **Learning:** Even when avoiding `innerHTML`, directly setting URL-like strings to DOM attributes without protocol validation introduces XSS vectors. The payload can be executed when the link is clicked or the iframe is loaded. | ||
| **Prevention:** Implement an `isSafeUrl` verification function to ensure the protocol is strictly `http:` or `https:` (using `new URL()`) before assigning untrusted inputs to DOM attributes like `href` or `src`. | ||
| ## 2026-07-01 - [Null Byte Injection Prevention] |
There was a problem hiding this comment.
HIGH OpenCode could not establish approval sufficiency
- Problem: the model pool exhausted without a valid current-head review control block, so this changed line cannot be approved from deterministic check state alone.
- Impact: PR-intent mismatches, missing files, robustness bugs, UX/DX regressions, and CodeGraph-backed flow changes could be missed.
- Fix: rerun OpenCode after model availability recovers, or add the missing source/test/docs/generated verification evidence needed for a source-backed approval.
- Verification: rerun the OpenCode Review workflow and confirm it emits APPROVE or source-backed REQUEST_CHANGES for this head SHA.
OpenCode Review Overview
Pull request overviewOpenCode exhausted the configured model pool without a usable current-head review conclusion. This is not approval evidence, so the PR is blocked until a source-backed review can establish approval sufficiency or identify concrete fixes. Findings1. HIGH .jules/sentinel.md:1 - OpenCode could not establish approval sufficiency
Summary
No PR approval was posted because model-output failure is not evidence that the PR has no blockers. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
|
|
μ€λ³΅ μ 리: μ΄ PRμ 'νμΌ νμ₯μ/null-byte κ²μ¦' ν΄λ¬μ€ν°μ μ€λ³΅μ λλ€. μ λ³ΈμΌλ‘ #76 (APPROVED + CI green + MERGEABLE)μ μ μ§ν©λλ€. λμΌ νμΌμ μμ ν΄ λμ λ³ν©μ΄ λΆκ°λ₯νλ―λ‘ μ 리ν©λλ€. κ³ μ νκ² νμν λ³κ²½μ΄ μμΌλ©΄ #76μ λ°μνκ±°λ μ¬μ€ννμΈμ. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
π¨ Severity: HIGH
π‘ Vulnerability: νμΌ μ λ‘λ κ²μ¦ λ‘μ§μμ νμΌ μ΄λ¦μ λ λ°μ΄νΈ(
\u0000) μ½μ 곡격μ λν λ°©μ΄κ° λλ½λμ΄ μμ΄, νμ₯μ κ²μ¦ λ±μ μ°νν μ μλ μ·¨μ½μ μ΄ μ‘΄μ¬νμ΅λλ€.π― Impact: 곡격μκ° μ μμ μΌλ‘ μ‘°μλ νμΌ(μ:
contract\u0000.pdf)μ μ λ‘λνμ¬ νμ₯μ νν°λ§μ μ°ννκ³ μλ²μμ μμμΉ λͺ»ν μ½λκ° μ€νλκ² νκ±°λ νμ©λμ§ μμ νμΌ νμ μ μ λ‘λν μ μμ΅λλ€.π§ Fix:
DefaultDocumentValidationService.javaλ΄μvalidateOrThrowλ©μλ μ΄κΈ°μ νμΌ μ΄λ¦μ λ λ°μ΄νΈκ° ν¬ν¨λ κ²½μ° μ¦κ°μ μΌλ‘IllegalArgumentExceptionμ λ°μμμΌ μ λ ₯μ κ±°λΆνλλ‘ λ‘μ§μ μμ νμ΅λλ€.β Verification:
DefaultDocumentValidationServiceTestμrejectsFilenameWithNullByteν μ€νΈ μΌμ΄μ€λ₯Ό μΆκ°νμ¬ ν΄λΉ λ‘μ§μ΄ μ μ μλν¨μ κ²μ¦νμ΅λλ€ (mvn test).PR created automatically by Jules for task 6086060004679117683 started by @seonghobae