diff --git a/.jules/bolt.md b/.jules/bolt.md index a86b7aaf..ca26add1 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -43,3 +43,9 @@ ## 2026-07-09 - Avoid N+1 API blocking in SBOM aggregator **Learning:** The `collect_inventories` function in `scripts/ci/sbom_inventory_aggregator.py` was fetching SBOMs from the GitHub dependency graph synchronously for every repository in the organization. For large organizations (up to 500 repos), this N+1 network/CLI bottleneck significantly stalled the aggregation workflow. **Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch SBOMs concurrently when multiple repositories are provided, bounded by a `max_workers` limit (e.g., 10) to avoid overwhelming the CLI/API, while preserving the fast serial path for single-item inputs. +## 2026-07-22 - Avoid N+1 API blocking in LLM review context gathering +**Learning:** The `changed_file_context` function in `scripts/ci/noema_review_gate.py` was fetching changed file contents sequentially using synchronous GitHub API calls (via `fetch_head_file_content`). This caused N+1 network request bottlenecks proportional to the number of files (up to `MAX_CONTEXT_FILES`), significantly increasing the execution time. +**Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch file contents concurrently when building bounded context from external APIs in PR gates, preserving the order using `executor.map`. +## 2026-07-22 - LLM 리뷰 컨텍스트 수집 시 N+1 API 차단 방지 +**Learning:** `scripts/ci/noema_review_gate.py`의 `changed_file_context` 함수는 `fetch_head_file_content`를 통해 동기식 GitHub API 호출을 사용하여 순차적으로 변경된 파일의 내용을 가져왔습니다. 이는 파일 수(최대 `MAX_CONTEXT_FILES`)에 비례하여 N+1 네트워크 요청 병목 현상을 일으켜 전체 실행 시간을 크게 증가시켰습니다. +**Action:** PR 게이트의 외부 API에서 바운디드 컨텍스트를 구성할 때 `concurrent.futures.ThreadPoolExecutor`를 사용하여 파일 내용을 동시에 가져오고, `executor.map`을 사용하여 순서를 유지하도록 최적화해야 합니다. diff --git a/.jules/sentinel.md b/.jules/sentinel.md index be2dfa4b..09d60be2 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -35,3 +35,7 @@ **Vulnerability:** Command Injection **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. +## 2026-07-22 - Update pyasn1 to patch denial-of-service vulnerabilities +**Vulnerability:** The CI dependencies (specifically `requirements-strix-ci-hashes.txt`) pinned `pyasn1` to version `0.6.3`, which contained three quadratic/exponential parsing vulnerabilities (CVE-2026-59884, CVE-2026-59885, CVE-2026-59886) that can lead to Denial-of-Service when processing attacker-controlled payload arcs/exponents. +**Learning:** Hard-pinned dependencies in CI scripts are prone to silently rotting and accumulating severe vulnerabilities, surfacing only when an explicit dependency audit runs and blocks the pipeline. +**Prevention:** Regularly run `pip-audit` to detect CVEs proactively before CI enforces an abrupt block. When `pip-audit` catches a vulnerability, ensure both the version and the expected SHA256 hashes are simultaneously updated. diff --git a/requirements-strix-ci-hashes.txt b/requirements-strix-ci-hashes.txt index 4cf45203..1aa159c1 100644 --- a/requirements-strix-ci-hashes.txt +++ b/requirements-strix-ci-hashes.txt @@ -1501,9 +1501,9 @@ protobuf==6.33.6 \ # grpc-google-iam-v1 # grpcio-status # proto-plus -pyasn1==0.6.3 \ - --hash=sha256:697a8ecd6d98891189184ca1fa05d1bb00e2f84b5977c481452050549c8a72cf \ - --hash=sha256:a80184d120f0864a52a073acc6fc642847d0be408e7c7252f31390c0f4eadcde +pyasn1==0.6.4 \ + --hash=sha256:deda9277cfd454080ec40b207fb6df82206a3a2688735233cdcd8d3d565f088b \ + --hash=sha256:9c447d8431c947fe4c8febc4ed9e760bc29011a5b01e5c74b67025bd9fb8ce81 # via pyasn1-modules pyasn1-modules==0.4.2 \ --hash=sha256:29253a9207ce32b64c3ac6600edc75368f98473906e8fd1043bd6b5b1de2c14a \ diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5b024c84..f896bf9f 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -5,6 +5,7 @@ import argparse import base64 +import concurrent.futures import ipaddress import json import os @@ -38,6 +39,7 @@ RUNNING_STATES = {"QUEUED", "IN_PROGRESS", "PENDING", "REQUESTED", "WAITING", "EXPECTED"} MAX_DIFF_CHARS = 60000 MAX_CONTEXT_FILES = 12 +MAX_CONTEXT_WORKERS = 6 MAX_FILE_CONTEXT_CHARS = 4000 MAX_REVIEW_CONTEXT_CHARS = 24000 MAX_THREAD_BODY_CHARS = 1200 @@ -338,17 +340,24 @@ def changed_file_context(repo: str, number: int, head_sha: str) -> str: if not paths: return "Changed file context unavailable: PR reported no changed files." sections: list[str] = [] - for path in paths[:MAX_CONTEXT_FILES]: + def _fetch(path: str) -> str: try: content = fetch_head_file_content(repo, path, head_sha) except RuntimeError as exc: reason = scrub_sensitive_data(str(exc)) or "unknown error" - sections.append(f"### {path}\nUnavailable from head content API: {reason}") - continue + return f"### {path}\nUnavailable from head content API: {reason}" if not content: - sections.append(f"### {path}\nNo UTF-8 text content available from head content API.") - continue - sections.append(f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}") + return f"### {path}\nNo UTF-8 text content available from head content API." + return f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}" + + bounded_paths = paths[:MAX_CONTEXT_FILES] + if len(bounded_paths) == 1: + sections.append(_fetch(bounded_paths[0])) + else: + max_workers = min(MAX_CONTEXT_WORKERS, len(bounded_paths)) + with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: + for result in executor.map(_fetch, bounded_paths): + sections.append(result) if len(paths) > MAX_CONTEXT_FILES: sections.append(f"[{len(paths) - MAX_CONTEXT_FILES} changed files omitted from context budget]") return "\n\n".join(sections) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 75d40dbe..08dfaa88 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -542,3 +542,70 @@ def test_parse_args_and_main(monkeypatch): with pytest.raises(SystemExit, match="--pr-number must be positive"): noema.main(["--repo", "owner/repo", "--pr-number", "0"]) + + +def test_changed_file_context_uses_serial_path_for_one_file(monkeypatch): + """One changed file must not pay the cost of creating a worker pool.""" + calls = [] + monkeypatch.setattr( + noema, + "fetch_changed_file_paths", + lambda repo, number: ["src/only.py"], + ) + monkeypatch.setattr( + noema, + "fetch_head_file_content", + lambda repo, path, head_sha: calls.append((repo, path, head_sha)) + or "print('only')\n", + ) + monkeypatch.setattr( + noema.concurrent.futures, + "ThreadPoolExecutor", + lambda *args, **kwargs: pytest.fail("single-file context created a worker pool"), + ) + + context = noema.changed_file_context("owner/repo", 7, "head") + + assert "### src/only.py" in context + assert "print('only')" in context + assert calls == [("owner/repo", "src/only.py", "head")] + + +def test_changed_file_context_caps_workers_and_preserves_path_order(monkeypatch): + """Multiple files use a bounded pool whose map output keeps path order.""" + paths = [f"src/file-{index}.py" for index in range(noema.MAX_CONTEXT_WORKERS + 2)] + executor_calls = [] + original_executor = noema.concurrent.futures.ThreadPoolExecutor + + class RecordingExecutor: + def __init__(self, *, max_workers): + executor_calls.append(max_workers) + self._executor = original_executor(max_workers=max_workers) + + def __enter__(self): + self._executor.__enter__() + return self + + def __exit__(self, *args): + return self._executor.__exit__(*args) + + def map(self, function, iterable): + return self._executor.map(function, iterable) + + monkeypatch.setattr( + noema, "fetch_changed_file_paths", lambda repo, number: paths + ) + monkeypatch.setattr( + noema, + "fetch_head_file_content", + lambda repo, path, head_sha: f"content for {path}", + ) + monkeypatch.setattr( + noema.concurrent.futures, "ThreadPoolExecutor", RecordingExecutor + ) + + context = noema.changed_file_context("owner/repo", 7, "head") + heading_positions = [context.index(f"### {path}") for path in paths] + + assert executor_calls == [noema.MAX_CONTEXT_WORKERS] + assert heading_positions == sorted(heading_positions)