Skip to content

fix(vuln): PR #64 리뷰 후속 — per-repo 격리 강화 + expanduser + 타입힌트#65

Merged
pureliture merged 1 commit into
mainfrom
claude/vuln-rollout-followup
Jun 21, 2026
Merged

fix(vuln): PR #64 리뷰 후속 — per-repo 격리 강화 + expanduser + 타입힌트#65
pureliture merged 1 commit into
mainfrom
claude/vuln-rollout-followup

Conversation

@pureliture

Copy link
Copy Markdown
Contributor

PR #64 gemini-code-assist 리뷰 3건 후속.

  • (HIGH) runtime/vulnerability_scan.py_scan_one_catalog_repoSemgrepExecutionError 외 예외(no-clobber ValueError, import/SARIF 오류 등)를 잡지 못해 전체 카탈로그 루프가 중단될 수 있던 문제. clobber 체크를 try 안으로 옮기고 일반 Exception을 per-repo scan-error로 격리 → "a single repo's failure never aborts the whole pass" 설계 의도 충족.
  • (MED) resolve_semgrep_binary~ 경로를 Path(binary).expanduser()로 처리.
  • (MED) cli/commands/vulnerability.py_append_freshness_notification resultVulnFreshnessResult 타입힌트 + import.

tests: 중복 scan_run_id / 일반 예외가 raise 대신 격리된 scan-error로 보고되는지 검증으로 갱신 + 추가.

proof: uv run pytest 1356 passed / 4 skipped; ruff clean.

Refs #64.

@gemini-code-assist gemini-code-assist 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.

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.

Comment on lines +209 to 211
candidate = Path(binary).expanduser()
if candidate.is_absolute() and candidate.exists():
return str(candidate)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

바이너리 경로가 디렉토리가 아닌 실제 파일인지 확인하기 위해 candidate.exists() 대신 candidate.is_file()을 사용하는 것이 더 안전합니다. 만약 사용자가 실수로 디렉토리 경로를 지정했을 경우, 존재 여부만 체크하면 디렉토리 경로를 그대로 반환하여 이후 실행 단계에서 권한 오류(PermissionError)나 실행 불가 오류가 발생할 수 있습니다.

Suggested change
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)

Comment on lines +335 to +337
def _append_freshness_notification(
log_path: str, result: VulnFreshnessResult
) -> None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

일반 규칙에 따르면, 주 트랜잭션에 영향을 주지 않는 베스트 에포트(best-effort) 성격의 비트랜잭션 보조 작업(예: 알림 파일 추가, 로그 기록 등)은 개별적으로 인라인 try-except 블록을 반복하기보다, 일시적인 실패를 잡아내고 exc_info와 함께 로그를 남긴 후 메인 프로세스를 계속 진행시키는 중앙 집중식 헬퍼 함수를 통해 라우팅해야 합니다. _append_freshness_notification 작업을 이러한 공통 헬퍼 함수를 사용하도록 수정하는 것을 권장합니다.

References
  1. 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>
@pureliture pureliture force-pushed the claude/vuln-rollout-followup branch from ac33263 to 292e4ff Compare June 21, 2026 23:55
@pureliture pureliture merged commit 76acc79 into main Jun 21, 2026
9 checks passed
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