🛡️ Sentinel: [HIGH] 업로드 엔드포인트의 리소스 제어 불능(DoS) 취약점 수정 - #246
Conversation
Moved input validation and filename sanitization earlier in the control flow, immediately validating uploads *before* allocating server resources (`tempfile.mkdtemp()`).
|
👋 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. |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head3360450b8d0bf6d5269a006b9ed7fe79d95ae177. -
Head SHA:
3360450b8d0bf6d5269a006b9ed7fe79d95ae177 -
Workflow run: 29271702993
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: test_saas_web.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_saas_web.py"]
R2 --> V2["targeted test run"]
|
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough업로드 파일명을 저장 전에 정제하도록 호출 경로를 변경했습니다. 배치 업로드는 입력과 파일명을 먼저 검증하고, 유효한 파일만 변환합니다. 오류 manifest와 임시 작업공간 정리도 처리합니다. Changes업로드 자원 제어
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant BatchUploadEndpoint
participant Validator
participant TempWorkspace
participant Transformer
participant BackgroundTask
Client->>BatchUploadEndpoint: 배치 파일 제출
BatchUploadEndpoint->>Validator: 파일명 및 입력 검증
Validator-->>BatchUploadEndpoint: 유효 파일과 오류 manifest 반환
BatchUploadEndpoint->>TempWorkspace: 검증 후 작업공간 생성
BatchUploadEndpoint->>Transformer: 유효 파일 변환
Transformer-->>BatchUploadEndpoint: 변환 결과 반환
BatchUploadEndpoint-->>Client: 결과 ZIP 응답
BatchUploadEndpoint->>BackgroundTask: 응답 후 작업공간 정리 등록
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 @.jules/sentinel.md:
- Around line 63-66: Update the 2025-02-27 “Uncontrolled Resource Consumption in
Upload Endpoints” entry in sentinel.md to include relevant primary academic
literature: add a permissible paper PDF under docs/papers or references, or, if
that is not possible, provide a citation, link, and concise relevance summary in
the entry.
In `@saas_web.py`:
- Around line 613-625: Ensure the batch-processing try/except cleans up the
temporary workspace created by tempfile.mkdtemp when ZipFile creation, manifest
writing, or FileResponse construction fails. Track the workspace path and remove
it from the exception path before returning the existing 500 JSONResponse, while
preserving successful-response cleanup through BackgroundTask. Add a regression
test that forces archive.writestr to fail and verifies the workspace is deleted.
🪄 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: 5184b086-2039-47ce-b1e3-aec512ac140e
📒 Files selected for processing (3)
.jules/sentinel.mdsaas_web.pytests/test_sentinel_upload_resource_control.py
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head3b0812819a2195ccd58afbea6022067b4078f6cf. -
Head SHA:
3b0812819a2195ccd58afbea6022067b4078f6cf -
Workflow run: 31092022896
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: test_sentinel_upload_resource_control.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_sentinel_upload_resource_control.py"]
R2 --> V2["targeted test run"]
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Exact-head review-only request for @coderabbitai review Revalidate the pre-allocation upload validation, safe filename fallback, all-invalid batch manifest behavior, archive-write failure cleanup, optional-FastAPI test isolation, primary research citation, and exact-current-head CI, fuzz, Security Scan, and Semgrep success. Submit a formal non-author |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/codec-carver"
pr="246"
sha="db7eff70356654d8a945305897ed210b52d921d9"
echo '=== PR metadata ==='
gh pr view "$pr" --repo "$repo" --json number,url,state,isDraft,headRefName,headRefOid,baseRefName,author,mergeStateStatus,statusCheckRollup
echo '=== Requested commit ==='
git cat-file -t "$sha"
git show -s --format='commit=%H%nparents=%P%nauthor=%an <%ae>%ndate=%cI%nsubject=%s' "$sha"
echo '=== Changed files at requested commit ==='
git diff-tree --no-commit-id --name-status -r "$sha"
echo '=== Diff at requested commit ==='
git show --format=fuller --find-renames --find-copies --stat --patch "$sha" -- \
saas_web.py tests/test_sentinel_upload_resource_control.py .jules/sentinel.md
echo '=== Relevant source map ==='
ast-grep outline saas_web.py --items all --view expanded | sed -n '1,240p'
echo '=== Test file ==='
cat -n tests/test_sentinel_upload_resource_control.py
echo '=== Citation and upload-related references ==='
rg -n -i -C 3 'CVE|citation|research|doi|upload|persist_upload|sanitize|filename|FileResponse|BackgroundTask|TemporaryDirectory|mkdtemp|zip' \
README.md .jules saas_web.py tests/test_sentinel_upload_resource_control.py 2>/dev/null || true
echo '=== Commit check runs ==='
gh api "repos/$repo/commits/$sha/check-runs?per_page=100" --jq \
'.check_runs[] | [.name,.status,.conclusion,.details_url,.started_at,.completed_at] | `@tsv`' || true
echo '=== Commit statuses ==='
gh api "repos/$repo/commits/$sha/status" --jq \
'.statuses[] | [.context,.state,.target_url,.updated_at] | `@tsv`' || true
echo '=== Base branch protection / rulesets ==='
base="$(gh pr view "$pr" --repo "$repo" --json baseRefName --jq .baseRefName)"
printf 'base=%s\n' "$base"
gh api "repos/$repo/branches/$base/protection" 2>&1 || true
gh api "repos/$repo/rules/branches/$base" 2>&1 || true
gh api "repos/$repo/rulesets?includes_parents=true" 2>&1 || trueLength of output: 84740 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/codec-carver"
pr="246"
sha="db7eff70356654d8a945305897ed210b52d921d9"
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq .baseRefOid)"
echo '=== Full PR commit list ==='
gh pr view "$pr" --repo "$repo" --json commits --jq \
'.commits[] | [.oid,.messageHeadline,.authoredDate] | `@tsv`'
echo '=== Full PR changed files ==='
git diff --name-status "$base...$sha"
echo '=== Full PR diff: implementation and tests ==='
git diff --find-renames --find-copies "$base...$sha" -- \
saas_web.py tests/test_sentinel_upload_resource_control.py .jules/sentinel.md
echo '=== Upload endpoint implementation ranges ==='
sed -n '440,710p' saas_web.py
sed -n '795,900p' saas_web.py
echo '=== Required-workflow definitions ==='
fd -a -t f -e yml -e yaml .github 2>/dev/null | sort | while IFS= read -r file; do
case "$file" in
*close-empty-pr.yml|*opencode-review.yml|*pr-review-merge-scheduler.yml|*security-scan.yml|*strix.yml|*sast-semgrep.yml|*noema-review.yml|*ci.yml|*fuzz*)
echo "--- $file"
cat -n "$file"
;;
esac
done
echo '=== Reviews and review decision ==='
gh pr view "$pr" --repo "$repo" --json reviewDecision,reviews,mergeStateStatus
echo '=== Unresolved review threads ==='
gh api graphql -F owner="${repo%%/*}" -F name="${repo#*/}" -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 10) { nodes { author { login } body url createdAt } }
}
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved | not)
| {path, line, isOutdated, comments: [.comments.nodes[] | {author: .author.login, body, url, createdAt}]}' || true
echo '=== Current check states on exact head ==='
gh api "repos/$repo/commits/$sha/check-runs?per_page=100" --jq \
'.check_runs[] | [.name,.status,(.conclusion // ""),.details_url] | `@tsv`'Length of output: 45459 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/codec-carver"
pr="246"
sha="db7eff70356654d8a945305897ed210b52d921d9"
run_id="31098950692"
echo '=== Exact-head identity and current PR state ==='
gh pr view "$pr" --repo "$repo" --json headRefOid,mergeStateStatus,reviewDecision,isDraft,state
echo '=== Strix workflow and jobs ==='
gh api "repos/$repo/actions/runs/$run_id" --jq \
'{head_sha, status, conclusion, event, run_attempt, html_url, created_at, updated_at}'
gh api "repos/$repo/actions/runs/$run_id/jobs?per_page=100" --jq \
'.jobs[] | [.name,.status,(.conclusion // ""),.html_url] | `@tsv`'
echo '=== Required-workflow paths from active organization rule ==='
gh api "repos/$repo/rules/branches/main" --jq \
'.[] | select(.rules[]?.type == "workflows")
| .rules[] | select(.type == "workflows")
| .parameters.workflows[] | [.path,.ref] | `@tsv`'
echo '=== Exact-head non-success checks ==='
gh api "repos/$repo/commits/$sha/check-runs?per_page=100" --jq \
'.check_runs[]
| select(.status != "completed" or (.conclusion != "success" and .conclusion != "skipped"))
| [.name,.status,(.conclusion // ""),.details_url] | `@tsv`'Length of output: 2296
The code review found no new actionable issue. Confirmed items:
I cannot submit a formal
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headb8bcab5444ff4e4c3c4ee052530a8bc92a5b9c99. -
Head SHA:
b8bcab5444ff4e4c3c4ee052530a8bc92a5b9c99 -
Workflow run: 31111920237
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart 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"]
Evidence --> S2["Test (3 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (3 files)"]
R2 --> V2["targeted test run"]
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head1f8892e24a3feacc164d0e43b231e34f29cf1a9a. -
Head SHA:
1f8892e24a3feacc164d0e43b231e34f29cf1a9a -
Workflow run: 31134231047
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: test_saas_web.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_saas_web.py"]
R2 --> V2["targeted test run"]
|
Closing without merge because this old-base security branch regresses accepted product behavior: it removes the batch preset/preview handlers, drops combined batch-size feedback, removes the batch preview element and controls, and discards current dynamic-limit UX. Its resource-allocation direction is valid, but the branch bundles unrelated UI rollback, duplicated cleanup behavior, and tests that do not preserve the current protected-main contract. Cross-platform filename containment is being rebuilt on #345; credential/result-lifecycle hardening is isolated on #361. Any remaining fail-fast upload-allocation change must be rebuilt from current main as a minimal endpoint-level slice with byte limits, cleanup, accessibility, and full regression evidence intact. |
2 participants
🚨 심각도: HIGH
💡 취약점: 파일 업로드 엔드포인트에서 입력 검증 및 파일 이름 살균 작업을 수행하기 전에 임시 디렉토리를 할당하고 있었습니다.
🎯 영향: 악의적이거나 유효하지 않은 요청이 지속적으로 발생할 경우 디스크 공간과 메모리가 임시로 누수되어 애플리케이션이 리소스 고갈 또는 서비스 거부(DoS) 상태에 빠질 수 있습니다.
🔧 수정: 입력 검증 및 파일 이름 살균 처리 위치를 제어 흐름의 앞단으로 옮겨, 서버 리소스(
tempfile.mkdtemp())를 할당하기 전에 업로드를 즉시 검증하도록 변경했습니다. FastAPI의FileResponse에BackgroundTask를 명시적으로 할당하도록 수정하여 올바르게 백그라운드 작업이 수행되도록 처리했습니다.✅ 검증: 관련 테스트 코드를 성공적으로 실행하여 올바른 동작을 확인했습니다.
PR created automatically by Jules for task 3116578629236057975 started by @seonghobae
Summary by CodeRabbit