🛡️ Sentinel: [CRITICAL] 프로젝트 ID 입력의 경로 탐색(Path Traversal) 취약점 수정#494
🛡️ Sentinel: [CRITICAL] 프로젝트 ID 입력의 경로 탐색(Path Traversal) 취약점 수정#494seonghobae wants to merge 3 commits into
Conversation
🚨 Severity: CRITICAL 💡 Vulnerability: `apps/desktop/src-tauri/src/main.rs`의 `app_owned_root` 함수와 페이로드 파싱 단계에서 사용자 입력인 `project_id`에 대한 검증이 부족하여 `../` 등을 통한 경로 탐색이 가능했습니다. 🎯 Impact: 악의적인 `project_id`를 조작하여 의도된 앱 캐시 및 임시 디렉토리 구조를 벗어나 시스템 내의 다른 디렉토리에 접근하거나 파일을 덮어쓸 수 있는 위험이 있었습니다. 🔧 Fix: `project_id`에 `..`, `/`, `\` 문자가 포함되어 있는지 검사하는 로직을 `parse_request_payload`에 추가하여 안전하지 않은 입력을 즉시 거부하도록 변경했습니다. ✅ Verification: 단위 테스트(`parse_request_payload_rejects_path_traversal`)를 추가하여 취약점이 완화되었는지 검증하였으며, 전체 테스트 스위트를 통해 정상 동작을 확인했습니다.
|
👋 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 desktop Tauri IPC surface against path traversal by rejecting unsafe projectId values before they can be used as filesystem path segments, and it documents the security learning in the Jules Sentinel log.
Changes:
- Add defensive validation rejecting
..,/, and\inprojectId(IPC payload parsing andapp_owned_root). - Add unit tests ensuring traversal-like
projectIdpayloads are rejected. - Update the Sentinel security note describing the vulnerability and prevention guidance.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/desktop/src-tauri/src/main.rs | Adds projectId path traversal validation in both request parsing and app-owned path construction; adds tests for rejection cases. |
| .jules/sentinel.md | Records the path traversal security lesson and prevention guidance for future reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if project_id.contains("..") || project_id.contains('/') || project_id.contains('\\') { | ||
| return Err("Invalid project ID: path traversal detected.".to_string()); | ||
| } |
| if project_id.trim().is_empty() | ||
| || project_id.contains("..") | ||
| || project_id.contains('/') | ||
| || project_id.contains('\\') | ||
| { | ||
| return Err("Invalid analysis job request: invalid field 'projectId'".into()); | ||
| } |
| #[test] | ||
| fn parse_request_payload_rejects_path_traversal_forward_slash() { | ||
| let payload = json!({ | ||
| "sourceKind": "local_audio", | ||
| "projectId": "/malicious-project", | ||
| "sourceLabel": "My Song", | ||
| "roleFocus": ["lead-vocal"], | ||
| }); | ||
|
|
||
| let result = parse_request_payload(payload); | ||
|
|
||
| assert!(result.is_err()); | ||
| assert_eq!( | ||
| result.unwrap_err(), | ||
| "Invalid analysis job request: invalid field 'projectId'" | ||
| ); | ||
| } | ||
|
|
| **Vulnerability:** The `project_id` field in the analysis payload is supplied by the user (or remote client) and is used directly in `app_owned_root` (`base_root.join(project_id)`) to determine the directory for project caching, temp space, and data. This allows an attacker to pass `../` in the `project_id` string, causing path traversal and letting them access or overwrite directories outside the designated app boundaries. | ||
| **Learning:** Even internal-looking identifiers (like `project_id`) are attack vectors if they originate from user input or IPC payloads. The codebase's `app-security.md` rule to "Defend against path traversal and parent directory escape" was missed in this initial implementation. | ||
| **Prevention:** Always validate and sanitize all identifier fields that get concatenated into paths. Explicitly reject payloads where the identifier contains path traversal characters like `..`, `/`, or `\`. Add tests specifically validating rejection on paths containing these sequences to maintain test coverage and security posture. |
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Check outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Evidence
Rust coverage tooling (cargo-llvm-cov)
Rust test coverage
Coverage Decision
Changed-File Evidence Mapflowchart 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"]
|
|
현재 head의 중앙 원인:
수정 방향:
참고: 이 실패는 원래 OpenCode가 |
🚨 Severity: CRITICAL 💡 Vulnerability: `apps/desktop/src-tauri/src/main.rs`의 `app_owned_root` 함수와 페이로드 파싱 단계에서 사용자 입력인 `project_id`에 대한 검증이 부족하여 `../` 등을 통한 경로 탐색이 가능했습니다. 🎯 Impact: 악의적인 `project_id`를 조작하여 의도된 앱 캐시 및 임시 디렉토리 구조를 벗어나 시스템 내의 다른 디렉토리에 접근하거나 파일을 덮어쓸 수 있는 위험이 있었습니다. 🔧 Fix: `project_id`에 `..`, `/`, `\` 문자가 포함되어 있는지 검사하는 로직을 `parse_request_payload`에 추가하여 안전하지 않은 입력을 즉시 거부하도록 변경했습니다. ✅ Verification: 단위 테스트(`parse_request_payload_rejects_path_traversal`)를 추가하여 취약점이 완화되었는지 검증하였으며, 전체 테스트 스위트를 통해 정상 동작을 확인했습니다.
|
현재 \ 기준으로 이 PR은 자동 update-branch 대상이 아니라 merge conflict 상태입니다. 임시 clone에서 \로 재현했습니다. 충돌 파일:
권장 처리:
이 상태에서는 OpenCode/auto-merge가 판단으로 해결할 수 없고, 사람이 충돌 내용을 선택해야 합니다. |
|
정정: 바로 위 conflict 안내 댓글은 shell quoting 문제로 inline code 일부가 누락됐을 수 있습니다. 현재 develop 기준 merge simulation 결과는 아래가 정확합니다. 자동 update-branch 대상이 아니라 merge conflict 상태입니다. 임시 clone에서 git merge --no-commit --no-ff origin/develop 로 재현했습니다. 충돌 파일:
권장 처리:
이 상태에서는 OpenCode/auto-merge가 판단으로 해결할 수 없고, 사람이 충돌 내용을 선택해야 합니다. |
|
Superseding this PR with a focused clean branch: #527. Reason: this branch currently has |
|
Closed in favor of #527, which isolates the project-ID path traversal fix on a clean |
이 PR은 경로 탐색(Path Traversal) 취약점을 수정하는 보안 패치를 포함합니다.
project_id필드가 내부 식별자로 쓰이지만 사용자 IPC 페이로드로부터 입력될 수 있다는 점을 고려하여, 해당 필드에..,/,\와 같은 디렉토리 순회 문자가 포함되어 있을 경우 이를 거부하는 방어적 코드를apps/desktop/src-tauri/src/main.rs의parse_request_payload에 추가했습니다. 이를 통해app_owned_root에서 예상하지 못한 디렉토리로의 접근을 원천 차단합니다.모든 변경 사항은 새롭게 추가된 단위 테스트를 포함하여 성공적으로 검증되었습니다.
PR created automatically by Jules for task 215616366698651889 started by @seonghobae