Skip to content

fix(web): normalize upload basenames across path styles - #397

Merged
seonghobae merged 4 commits into
mainfrom
sentinel-zipslip-backslash-fix-6471144421105359756
Aug 11, 2026
Merged

fix(web): normalize upload basenames across path styles#397
seonghobae merged 4 commits into
mainfrom
sentinel-zipslip-backslash-fix-6471144421105359756

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • normalize both / and \ client path separators before extracting upload basenames
  • keep persisted converter source names and batch-manifest filenames consistent on POSIX and Windows
  • close ZIP handles and remove temporary test workspaces deterministically
  • correct the security record and changelog so they do not claim a demonstrated Zip Slip or filesystem-traversal exploit

Boundary

Uploads remain inside a trusted temporary workspace, and batch archive entry names are generated outputs. This is a cross-platform correctness and defense-in-depth change; the reviewed code does not demonstrate arbitrary filesystem writes or archive-entry escape.

Verification

  • regression test covers a Windows-style client filename
  • assertions cover the persisted source basename and manifest metadata
  • adjacent fallback test now closes the archive and cleans its workspace

PR created automatically by Jules for task 6471144421105359756 started by @seonghobae

@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c76b186-cfbd-4928-b469-d25f4a82c666

📥 Commits

Reviewing files that changed from the base of the PR and between 29e26c0 and a5fada7.

📒 Files selected for processing (1)
  • tests/test_saas_web.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_saas_web.py

📝 Walkthrough

Walkthrough

업로드 파일명의 백슬래시를 경로 구분자로 정규화한 뒤 basename을 추출합니다. 단일 및 배치 업로드에 적용하고, Windows 경로 순회 입력에 대한 회귀 테스트와 변경 기록을 추가합니다.

Changes

업로드 파일명 보안 처리

Layer / File(s) Summary
업로드 파일명 정규화
saas_web.py
단일 및 배치 업로드에서 백슬래시를 슬래시로 변환한 뒤 basename을 추출합니다.
경로 순회 회귀 검증
tests/test_saas_web.py, .jules/sentinel.md, CHANGELOG.md
Windows 경로 입력이 windows.ini로 변환되고 변환기 입력 파일명과 manifest 파일명이 일치하는지 검증합니다. 취약점 수정 사항을 기록합니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 업로드 파일명의 경로 형식 정규화라는 주요 변경 사항을 간결하고 명확하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sentinel-zipslip-backslash-fix-6471144421105359756

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@tests/test_saas_web.py`:
- Around line 567-585: Update the test around shrink_media_batch to close the
archive with a with statement and ensure the temporary workspace is removed in a
finally block by calling cleanup_temp_dir with Path(response.path).parent. Keep
the existing manifest and conversion assertions intact.
🪄 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: a41459ba-ace1-416b-96c7-497e02302c60

📥 Commits

Reviewing files that changed from the base of the PR and between b2bc83a and 29e26c0.

📒 Files selected for processing (4)
  • .jules/sentinel.md
  • CHANGELOG.md
  • saas_web.py
  • tests/test_saas_web.py

Comment thread tests/test_saas_web.py
@seonghobae seonghobae changed the title 🛡️ Sentinel: [HIGH] 업로드된 파일명 내 백슬래시 기반 경로 탐색 및 Zip Slip 취약점 수정 fix(web): normalize upload basenames across path styles Aug 11, 2026
@seonghobae
seonghobae merged commit 7c7017a into main Aug 11, 2026
34 checks passed
@seonghobae
seonghobae deleted the sentinel-zipslip-backslash-fix-6471144421105359756 branch August 11, 2026 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant