fix(vuln): PR #64 리뷰 후속 — per-repo 격리 강화 + expanduser + 타입힌트#65
Conversation
There was a problem hiding this comment.
Code Review
This pull request improves the robustness of the vulnerability scanner by isolating unexpected scan failures per repository, preventing a single failure from aborting the entire catalog scan. It also resolves semgrep binary paths using expanduser() and adds corresponding unit tests. The review feedback suggests using is_file() instead of exists() to verify the resolved binary path, and recommends routing best-effort secondary operations like freshness notifications through a centralized helper function to handle transient failures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| candidate = Path(binary).expanduser() | ||
| if candidate.is_absolute() and candidate.exists(): | ||
| return str(candidate) |
There was a problem hiding this comment.
바이너리 경로가 디렉토리가 아닌 실제 파일인지 확인하기 위해 candidate.exists() 대신 candidate.is_file()을 사용하는 것이 더 안전합니다. 만약 사용자가 실수로 디렉토리 경로를 지정했을 경우, 존재 여부만 체크하면 디렉토리 경로를 그대로 반환하여 이후 실행 단계에서 권한 오류(PermissionError)나 실행 불가 오류가 발생할 수 있습니다.
| candidate = Path(binary).expanduser() | |
| if candidate.is_absolute() and candidate.exists(): | |
| return str(candidate) | |
| candidate = Path(binary).expanduser() | |
| if candidate.is_absolute() and candidate.is_file(): | |
| return str(candidate) |
| def _append_freshness_notification( | ||
| log_path: str, result: VulnFreshnessResult | ||
| ) -> None: |
There was a problem hiding this comment.
일반 규칙에 따르면, 주 트랜잭션에 영향을 주지 않는 베스트 에포트(best-effort) 성격의 비트랜잭션 보조 작업(예: 알림 파일 추가, 로그 기록 등)은 개별적으로 인라인 try-except 블록을 반복하기보다, 일시적인 실패를 잡아내고 exc_info와 함께 로그를 남긴 후 메인 프로세스를 계속 진행시키는 중앙 집중식 헬퍼 함수를 통해 라우팅해야 합니다. _append_freshness_notification 작업을 이러한 공통 헬퍼 함수를 사용하도록 수정하는 것을 권장합니다.
References
- Route best-effort, non-transactional secondary operations (such as heartbeat writes) through a centralized helper function that catches transient failures, logs them with exc_info, and allows the main process to continue, rather than repeating inline try-except blocks or letting failures abort the main transaction.
gemini-code-assist 리뷰 3건 반영.
- runtime/vulnerability_scan.py(HIGH): _scan_one_catalog_repo가 SemgrepExecutionError
외 예외(clobber ValueError, import/SARIF 오류 등)를 잡지 못해 전체 카탈로그 루프가
중단될 수 있던 문제 수정. clobber 체크를 try 안으로 옮기고 일반 Exception을
per-repo scan-error로 격리("a single repo's failure never aborts the whole pass" 충족).
- runtime/vulnerability_scan.py(MED): resolve_semgrep_binary가 ~ 경로를 처리하도록
Path(binary).expanduser() 적용.
- cli/commands/vulnerability.py(MED): _append_freshness_notification result 인자에
VulnFreshnessResult 타입힌트 + import 추가.
- tests: no-clobber/일반예외가 raise 대신 격리된 scan-error로 보고되는지 검증으로 갱신·추가.
proof: uv run pytest 1356 passed/4 skipped; ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ac33263 to
292e4ff
Compare
PR #64
gemini-code-assist리뷰 3건 후속.runtime/vulnerability_scan.py—_scan_one_catalog_repo가SemgrepExecutionError외 예외(no-clobberValueError, import/SARIF 오류 등)를 잡지 못해 전체 카탈로그 루프가 중단될 수 있던 문제. clobber 체크를 try 안으로 옮기고 일반Exception을 per-reposcan-error로 격리 → "a single repo's failure never aborts the whole pass" 설계 의도 충족.resolve_semgrep_binary—~경로를Path(binary).expanduser()로 처리.cli/commands/vulnerability.py—_append_freshness_notificationresult에VulnFreshnessResult타입힌트 + import.tests: 중복 scan_run_id / 일반 예외가 raise 대신 격리된
scan-error로 보고되는지 검증으로 갱신 + 추가.proof:
uv run pytest1356 passed / 4 skipped; ruff clean.Refs #64.