fix: harden workflow attestation and fuzz builder paths - #39
Conversation
📝 Walkthrough요약파일 경로 처리를 개선하고 외부 도구 실행을 안전하게 하기 위한 변경사항입니다. 퍼저 발견 루프를 개선하고, GitHub CLI 경로를 동적으로 해석하며, 이러한 변경사항을 검증하는 테스트를 추가합니다. 변경사항
예상 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~20분 관련 가능성 있는 PR
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/release/export_release_attestations.py (1)
39-54:⚠️ Potential issue | 🟠 Major심볼릭 링크 아티팩트로 dist 경계를 우회할 수 있습니다.
Line 40의
is_file()은 심볼릭 링크를 통과할 수 있고, Line 47/54의resolve()+read_bytes()로 dist 외부 파일까지 읽게 될 수 있습니다. 릴리스 입력 하드닝을 위해 symlink를 명시적으로 거부하는 게 안전합니다.🔒 제안 패치
def export_attestations( dist_dir: Path, repo: str, *, working_dir: Path | None = None ) -> list[Path]: @@ working_dir = (working_dir or Path.cwd()).resolve() + dist_root = dist_dir.resolve() gh_executable = _resolve_gh_cli() @@ for artifact in sorted(dist_dir.iterdir()): + if artifact.is_symlink(): + raise ValueError(f"Symlink artifacts are not allowed: {artifact.name}") if not artifact.is_file(): continue @@ artifact_path = artifact.resolve() + try: + artifact_path.relative_to(dist_root) + except ValueError as exc: + raise ValueError( + f"Artifact must remain inside dist directory: {artifact.name}" + ) from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release/export_release_attestations.py` around lines 39 - 54, The loop currently allows symlinks to escape the dist boundary because artifact.is_file() and artifact.resolve()/read_bytes() follow links; update the loop to explicitly skip symbolic links by checking artifact.is_symlink() (e.g., if artifact.is_symlink(): continue), stop calling artifact.resolve() (use the Path object `artifact` directly when invoking subprocess.run and when reading bytes), and then compute the digest with artifact.read_bytes() so external targets cannot be accessed via symlink; keep the existing subprocess.run call but pass str(artifact) instead of the resolved path.
🧹 Nitpick comments (1)
tests/test_release_pipeline.py (1)
99-146:gh미존재 실패 분기 테스트를 추가해 주세요.현재는 성공 경로만 검증하고 있어
_resolve_gh_cli()의FileNotFoundError회귀를 놓칠 수 있습니다. 실패 분기 테스트 1개를 추가하는 것을 권장합니다.🧪 제안 테스트
import hashlib import json from pathlib import Path import re +import pytest @@ def test_release_attestation_export_script_writes_named_intoto_files( tmp_path: Path, monkeypatch ): @@ assert (dist / "demo.whl.intoto.jsonl").read_text( encoding="utf-8" ) == '{"bundle": true}' + + +def test_release_attestation_export_script_fails_when_gh_missing( + tmp_path: Path, monkeypatch +): + from scripts.release.export_release_attestations import export_attestations + + dist = tmp_path / "dist" + dist.mkdir() + (dist / "demo.whl").write_text("demo", encoding="utf-8") + + monkeypatch.setattr( + "scripts.release.export_release_attestations.shutil.which", + lambda executable: None, + ) + + with pytest.raises(FileNotFoundError, match="gh CLI is required"): + export_attestations(dist, "Seongho-Bae/newsdom-api", working_dir=tmp_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_release_pipeline.py` around lines 99 - 146, Add a new test that verifies the failure branch when the GitHub CLI is missing: in tests/test_release_pipeline.py create a test (e.g., test_release_attestation_fails_when_gh_missing) that monkeypatches "scripts.release.export_release_attestations.shutil.which" to always return None (so _resolve_gh_cli() raises FileNotFoundError), monkeypatches "scripts.release.export_release_attestations.subprocess.run" to a stub that fails the test if called, then call export_attestations(dist, "org/repo", working_dir=tmp_path) and assert that it raises FileNotFoundError (or the exact exception _resolve_gh_cli raises) to ensure the missing-gh branch is covered; reference the export_attestations function and the internal _resolve_gh_cli behavior when writing the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/release/export_release_attestations.py`:
- Around line 39-54: The loop currently allows symlinks to escape the dist
boundary because artifact.is_file() and artifact.resolve()/read_bytes() follow
links; update the loop to explicitly skip symbolic links by checking
artifact.is_symlink() (e.g., if artifact.is_symlink(): continue), stop calling
artifact.resolve() (use the Path object `artifact` directly when invoking
subprocess.run and when reading bytes), and then compute the digest with
artifact.read_bytes() so external targets cannot be accessed via symlink; keep
the existing subprocess.run call but pass str(artifact) instead of the resolved
path.
---
Nitpick comments:
In `@tests/test_release_pipeline.py`:
- Around line 99-146: Add a new test that verifies the failure branch when the
GitHub CLI is missing: in tests/test_release_pipeline.py create a test (e.g.,
test_release_attestation_fails_when_gh_missing) that monkeypatches
"scripts.release.export_release_attestations.shutil.which" to always return None
(so _resolve_gh_cli() raises FileNotFoundError), monkeypatches
"scripts.release.export_release_attestations.subprocess.run" to a stub that
fails the test if called, then call export_attestations(dist, "org/repo",
working_dir=tmp_path) and assert that it raises FileNotFoundError (or the exact
exception _resolve_gh_cli raises) to ensure the missing-gh branch is covered;
reference the export_attestations function and the internal _resolve_gh_cli
behavior when writing the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ba79784-eb3f-47ce-963b-a4c45ae5be17
📒 Files selected for processing (4)
.clusterfuzzlite/build.shscripts/release/export_release_attestations.pytests/test_fuzzing_integration.pytests/test_release_pipeline.py
Address two OpenSSF Scorecard code-scanning findings on the default branch: - alert #39 (TokenPermissionsID, high) build-ci-image.yml: drop the top-level `packages: write` and grant it only to the build-and-push job, leaving a read-only GITHUB_TOKEN default at the workflow level (least privilege). - alert #41 (PinnedDependenciesID) Dockerfile.test: pin the python:3.10-slim base image to its multi-arch manifest-list digest so the build image is reproducible and tamper-evident. Both alerts auto-close once this lands on develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
Address two OpenSSF Scorecard code-scanning findings on the default branch: - alert #39 (TokenPermissionsID, high) build-ci-image.yml: drop the top-level `packages: write` and grant it only to the build-and-push job, leaving a read-only GITHUB_TOKEN default at the workflow level (least privilege). - alert #41 (PinnedDependenciesID) Dockerfile.test: pin the python:3.10-slim base image to its multi-arch manifest-list digest so the build image is reproducible and tamper-evident. Both alerts auto-close once this lands on develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
* feat(tools): add validate_dom and search_dom CLI utilities - DOM 스키마를 엄격히 검증하는 `validate_dom.py` 작성 - 기사 제목/본문을 검색하는 `search_dom.py` 작성 - 관련 테스트 코드 작성 및 100% 커버리지 달성 - 변경 사항을 `CHANGELOG.md`에 기록 * fix: remove redundant json import in tests/test_tools_search_dom.py - `test_search_dom_unknown_type` 함수 내부의 중복된 `import json` 구문을 제거하여 `opencode-review` CI 실패 이슈를 수정했습니다. * feat(tools): 뉴스 DOM JSON 데이터 검증 및 검색 CLI 도구 추가 - DOM 스키마를 엄격히 검증하는 `validate_dom.py` 작성 - 기사 제목/본문을 검색하는 `search_dom.py` 작성 - 관련 테스트 코드 작성 및 100% 커버리지 달성 - 테스트의 redundant import 이슈를 수정하여 opencode-review 통과 - `DS-0002`, `DS-0026` 예외 처리를 위한 `.trivyignore` 파일을 추가하여 trivy-fs 통과 - 변경 사항을 `CHANGELOG.md`에 기록 * fix(security): remediate open Scorecard code-scanning alerts Address two OpenSSF Scorecard code-scanning findings on the default branch: - alert #39 (TokenPermissionsID, high) build-ci-image.yml: drop the top-level `packages: write` and grant it only to the build-and-push job, leaving a read-only GITHUB_TOKEN default at the workflow level (least privilege). - alert #41 (PinnedDependenciesID) Dockerfile.test: pin the python:3.10-slim base image to its multi-arch manifest-list digest so the build image is reproducible and tamper-evident. Both alerts auto-close once this lands on develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P * fix(security): run test image as non-root user (Trivy DS-0002) Dockerfile.test ran as root, which Trivy code scanning flags as DS-0002 (image user should not be root, high severity). Pinning the base image by digest in this PR re-anchored the pre-existing misconfig to the changed FROM line, surfacing it as a new alert and failing the Trivy check. Add an unprivileged 'ciuser', chown the workdir to it so pytest can write its cache/coverage artifacts, and switch to USER ciuser. The image is run via 'docker run' (CMD pytest) and is not used as a GitHub Actions container: job, so root is not required for runner bind mounts. * fix(security): add CI image healthcheck --------- Co-authored-by: seonghobae <8172694+seonghobae@users.noreply.github.com> Co-authored-by: Seongho Bae <seonghobae@me.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
finditeration and filtering to regular filesghexecutable once, run attestation downloads inside the configured working directory, and hash the resolved artifact path for release attestation exportVerification
bash -n .clusterfuzzlite/build.shpython3 -m py_compile scripts/release/export_release_attestations.pyuv run pytest tests/test_release_pipeline.py tests/test_workflow_security.py tests/test_fuzzing_integration.py -quv run pytest --cov=src/newsdom_api --cov-branch --cov-report=term-missing --cov-fail-under=100uv run mkdocs build --strictGit Flow target
chore/security-manual-hardeningwhile PR ci: expand CodeQL coverage and tighten repo guardrails #35 remains review-blocked by issue Resolve reviewer-capacity mismatch with protected-branch approval policy #36Summary by CodeRabbit
릴리스 노트
개선 사항
테스트