Skip to content

fix: constrain analysis interpreter discovery to trusted roots#59

Merged
seonghobae merged 6 commits into
developfrom
codex/fix-insecure-interpreter-search-path
Mar 25, 2026
Merged

fix: constrain analysis interpreter discovery to trusted roots#59
seonghobae merged 6 commits into
developfrom
codex/fix-insecure-interpreter-search-path

Conversation

@seonghobae

@seonghobae seonghobae commented Mar 12, 2026

Copy link
Copy Markdown
Collaborator

Motivation

  • The previous interpreter discovery walked ancestors of current_exe and current_dir, which allowed local binary planting (untrusted, writable directories could supply a malicious analysis-engine/.venv/bin/python).
  • The launcher also fell back to running uv from PATH, which is unpinned and increases attack surface.
  • This change eliminates discovery across user-controlled ancestor paths and closes the unpinned uv fallback to reduce arbitrary code execution risk.

Description

  • Restricted runtime_search_roots() to only include the executable parent and packaged resource locations instead of walking current_dir ancestors. (apps/desktop/src-tauri/src/main.rs)
  • Tightened candidate validation from exists() to is_file() so only regular files are accepted as interpreters. (analysis_command)
  • Removed the unpinned "uv" PATH fallback; when no trusted interpreter is found the function now returns a sentinel program and empty args so the existing spawn/error handling reports engine-unavailable instead of executing arbitrary PATH binaries.
  • Small behavior change: when no trusted interpreter is present the launcher now fails closed rather than attempting to run an external tool from PATH.

Testing

  • Ran cargo fmt -- --check, which succeeded.
  • Attempted cargo test / cargo build under CI-like environment, but network access to crates.io is blocked (CONNECT tunnel 403) so tests/build that fetch dependencies could not complete.

Codex Task

📝 Walkthrough

개요

main.rs 파일의 런타임 검색 경로 및 분석 엔진 위치 지정 로직을 수정했습니다. 검색 범위를 좁혔고 폴백 동작을 단순화했습니다.

변경 사항

코호트 / 파일 요약
런타임 및 분석 엔진 로직
apps/desktop/src-tauri/src/main.rs
runtime_search_roots 함수: 실행 파일 부모의 모든 상위 경로 반복 제거, 부모 디렉토리와 관련 리소스 경로만 추가. analysis_command 함수: 파이썬 분석 엔진 탐색 시 존재 확인을 is_file() 확인으로 변경. 폴백 경로에서 복잡한 "uv" 실행 명령을 "__bandscope_missing_analysis_python__" 자리 표시자와 빈 인수를 사용하는 최소 폴백으로 대체.

예상 코드 리뷰 노력

🎯 2 (Simple) | ⏱️ ~12분

🐰 경로를 단순하게, 검색을 좁혀서,
빈 공간을 채우는 자리 표시자와 함께,
논리의 소음을 걷어내고,
더 깔끔한 흐름으로 나아가네요.

@coderabbitai

coderabbitai Bot commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7406bc38-cd4c-4782-ac49-b2f194bbe420

📥 Commits

Reviewing files that changed from the base of the PR and between 460611d and 0688725.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/main.rs

Cache: Disabled due to Reviews > Disable Cache setting

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.


📝 Walkthrough

Summary by CodeRabbit

  • 버그 수정

    • 분석 엔진이 없을 때 즉시 실패 처리해 명확한 오류 메시지 반환 및 불필요한 프로세스 생성을 방지
    • 인터프리터 검사 강화로 잘못된 실행 파일 선택 가능성 감소
    • 런타임 탐색 범위를 축소해 불필요한 경로 검사 제거 및 탐색 시간 단축
  • 동작 변경

    • 분석 엔진 대체 동작이 보다 예측 가능하고 일관되게 동작하도록 조정

Walkthrough

실행 파일 인접 경로만 런타임 검색 대상으로 제한하고, 분석 엔진 파이썬 후보를 exists()에서 is_file()로 엄격히 검사하도록 변경했습니다. 분석 엔진이 발견되지 않으면 MISSING_ANALYSIS_PYTHON 자리표시자를 반환해 run_analysis_engine()에서 즉시 EngineUnavailable으로 실패하도록 분기했습니다.

Changes

Cohort / File(s) Summary
런타임 및 분석 엔진 로직
apps/desktop/src-tauri/src/main.rs
runtime_search_roots()가 실행파일의 부모 디렉터리와 resources/../Resources만 반환하도록 축소. analysis_command()는 후보 검사 exists()is_file()로 강화하고, 미발견 시 MISSING_ANALYSIS_PYTHON(프로그램 이름)과 빈 args 반환. run_analysis_engine()는 해당 자리표시자 감지 시 프로세스 생성을 피하고 즉시 EngineUnavailable으로 실패 처리 추가.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🐰 실행파일 곁에 귀 기울이며,
먼 조상길은 조용히 닫았네.
찾지 못하면 이름만 남기고,
빈 인수로 돌아서며 숨을 고르네.
엔진 없을 땐 조용히 실패하네.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목이 변경 사항의 주요 목적을 명확하게 나타내고 있습니다. 신뢰할 수 있는 경로로의 제약(constrain)과 분석 인터프리터 발견(analysis interpreter discovery)이라는 두 가지 핵심 요소를 포함하고 있습니다.
Description check ✅ Passed PR 설명이 변경 사항과 밀접하게 관련되어 있습니다. 동기, 상세한 설명, 테스트 내용 등이 포함되어 있으며 모두 runtime_search_roots, analysis_command, 경로 제약에 관한 내용입니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 codex/fix-insecure-interpreter-search-path

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 the current code and only fix it if needed.

Inline comments:
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 272-275: Before calling Command::new in run_analysis_engine,
detect the sentinel program string "__bandscope_missing_analysis_python__" and
avoid spawning it to prevent PATH lookup/exec of a malicious binary;
specifically, locate where Command::new(...) is invoked (the tuple with
"__bandscope_missing_analysis_python__") and add a guard: if the program string
== "__bandscope_missing_analysis_python__" then skip spawn/return an appropriate
error/result instead of calling Command::new or spawn.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro

Run ID: 7e24540d-e3b4-445a-82df-e72b6f5394c5

📥 Commits

Reviewing files that changed from the base of the PR and between 169f05a and 7426026.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/main.rs

Comment thread apps/desktop/src-tauri/src/main.rs
@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai resolve

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

@seonghobae seonghobae enabled auto-merge (squash) March 25, 2026 13:00
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 the current code and only fix it if needed.

Inline comments:
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 273-275: Extract the magic string
"__bandscope_missing_analysis_python__" into a single constant (e.g., const
MISSING_ANALYSIS_PYTHON: &str = "__bandscope_missing_analysis_python__") and
replace the hardcoded occurrences (the
"__bandscope_missing_analysis_python__".into() used alongside
std::env::current_dir() and the other occurrence around lines 484-485) with that
constant converted as needed (e.g., .into() or
PathBuf::from(MISSING_ANALYSIS_PYTHON) where appropriate); ensure all
comparisons/usages reference the new MISSING_ANALYSIS_PYTHON constant so the
sentinel is maintained from one source.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro

Run ID: 28170648-2780-4a29-a42e-c71c1bee8169

📥 Commits

Reviewing files that changed from the base of the PR and between 7426026 and 460611d.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/main.rs

Comment thread apps/desktop/src-tauri/src/main.rs
@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae seonghobae dismissed coderabbitai[bot]’s stale review March 25, 2026 14:31

Stale review — HEAD 0688725 already implements all requested changes: MISSING_ANALYSIS_PYTHON constant at line 35, used at line 275, guard at line 485

@seonghobae seonghobae merged commit 30d84a3 into develop Mar 25, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant