Skip to content

fix(codeql): collect complete fail-closed GHAS analysis history - #2555

Merged
seonghobae merged 69 commits into
mainfrom
fix/sdp-ghas-analyses-pagination-20261002
Oct 13, 2026
Merged

seonghobae merged 69 commits into
mainfrom
fix/sdp-ghas-analyses-pagination-20261002

Conversation

@seonghobae

@seonghobae seonghobae commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Current lifecycle

  • status: independently reviewed prerequisite integration; normal hosted checks pending; not merge eligible yet
  • exact head: 3aad65b187897d2afc290cece0582a5c3d47a1e3
  • protected base inspected: 37b10243cec3d160ecc9c1be75c71428b160a703
  • exact reviewed tree: 41c575a8ca04889ebcb0f512d71192a57f59e53a
  • complete 96-file full-index patch SHA256: ce45c17e96aa8d5ee9b6f2c3433b164e1085d1c60c42e9106a4276ce3e867dda

The existing single writer normally integrated the verified #2530 dc54310c prerequisite with #2555's own published 32dd90ce, retained both GHAS pagination and bounded HTTPError read/close safeguards, reused #2558's pypdf6.19.0 security correction, and applied only the retained independently reviewed Maturin real-close test-fixture repair. No peer checkout, original product checkout, dirty6, credentials, production deployment or protected policy was changed. This does not close or supersede the existing owner PRs.

The previous complete source review was negative for the inherited recording-only HTTPError fixture. Its actual RED evidence is preserved: 1 passed text followed by process exit1 from ResourceWarning is not acceptance. The corrected complete 96-file substantive independent re-review is PASS, report SHA256 e6aee06ae0e11fc3c40ded4ed1a8d667876f78cbcc891ab40b349a2541010199. The reviewer independently observed one saved real close call and closed error/body/closer. Local review is not counted GitHub approval.

On the actual clean committed writer, warnings-fatal selected tests passed: Python3.14.6 224 cases, Python3.10.20 172 cases; both process exit0 and JUnit failure/error/skip0. These are overlapping interpreter cohorts, not396 unique tests and not the full central suite. All717 candidate source bytes match the reviewed tree; original dirty6 and untracked reports17 hashes are preserved. Full-index base-to-committed-head diffcheck is clean.

The preceding byte-identical prerequisite locks were fully audited with the actual complete Python Security shell:10 requirements plus project manifest returned no known vulnerabilities, while the existing raw Strix-input override skip remained documented. The corrected tree changes only the test fixture relative to that audit tree, not the audited dependency inputs. The preceding selected runtime-quality10 Verify commands also passed locally; those passes are historical source-bound evidence, not fresh committed-head hosted gate results.

Normal push and fresh Git remote/PR head both verify the exact head above. New hosted Python3.10 compatibility, Python3.14 full quality, Security Scan, Python Security, Semgrep and runtime-quality runs materialized; their terminal results and exact-head qualifying approvals remain separate requirements. No duplicate manual producer or scanner was dispatched. Draft exemptions must not be treated as review success. This prerequisite must satisfy fresh applicable protections before any normal merge; semantic-data-portal#82/#107 merges and the #108 post-merge prerequisite remain unfinished.

Latest supported-interpreter collection correction

Head 3aad65b187897d2afc290cece0582a5c3d47a1e3 adds only a verified two-line test-fixture repair. Python3.10 actual collection failed with exit2 at the unnecessary top-level tomllib import; removing it and using a self-contained loads stub retains the runpy tomli binding assertion without changing production parser consumers, dependencies, policy, permissions or credentials. CodeRabbit automatically marked its exact thread addressed in commit3aad65b; this is issue resolution, not counted approval.

Complete96file substantive source re-review PASS binds the current tree/patch above and reportSHA256 0e8c86e0a25c0facb46c25f2044c791cb45bb9b1536f9c9017b98b922a23f053. The reviewer independently verified fallback binding identity and import order. Actual committed-head affected module passes24cases/JUnit24/failure-error-skip0/process0 under bothPython3.10.20 and3.14.6 with warningsfatal. Related246case tests passed beforecommit. The preceding cumulative224/172 local tests and hosted5427/100%quality/security results belong to predecessor1cda48bf, not fresh successor acceptance. New hosted checks and counted exact-head approvals must converge again before merge.

Historical pagination verification

The original sections below preserve pagination intent and earlier scoped evidence; their historical234-case and219-statement coverage counts must not be relabelled as current whole-composition acceptance.

Summary

  • Collect every GHAS CodeQL analyses page instead of treating the first 100 rows as complete configuration evidence.
  • Keep bearer requests bound to canonical HTTPS GitHub API, the original repository/ref/tool/page-size query and sequential page numbers; verify numeric repository aliases against original repository metadata.
  • Fail closed on malformed/poisoned/cyclic links, incomplete later pages or the documented page budget. Do not return partial evidence.

This is a prerequisite retrieval repair for ContextualWisdomLab/semantic-data-portal#82 and ContextualWisdomLab/semantic-data-portal#107, tracked in #2477. It does NOT resolve historical configuration retirement, enable a lifecycle policy, change identity pairing, upload/delete analyses, synthesize verdicts or authorize merge. Those remain separate verified follow-up work.

Root cause and regression

The old helper ignores the response Link header. The new 201-row/three-page regression proves that an active identity on page three remains visible and still rejects a head that lacks it. Poisoned next-page links are rejected before a second bearer request. API failures do not produce a complete prefix.

Verification

  • RED→GREEN traversal, poisoned Link, incomplete collection/bounds and numeric-alias cases.
  • 234 affected tests pass in both normal and GITHUB_ACTIONS=true modes.
  • Affected production helper: 219 statements / 82 branches, 100% coverage; 100% docstrings.
  • Parent independently reran all 234 tests and coverage/docstrings against the exact final staged source.
  • Independent reviews found terminal-last inconsistency and repeated physical Link header truncation. Separate fixers reproduced RED and corrected both with 32 additional tests. Parent's original multi-header failure now collects both pages. Final independent local full-diff re-review passed with no blocking findings, bound to staged-diff SHA-256 cf5a1b2c643b0a30b4d5476473f6170c5d4305912b6de9c4d33db10aae14f7fc. Local review is not GitHub formal approval.
  • git diff --check passes.

macOS /bin/bash 3.2 causes nine unchanged workflow fixture failures, also reproduced on the exact clean base. The local affected workflow-contract run uses installed Bash 5 through a scratch-only interpreter substitution. This is not a claim that the entire central suite or hosted required gates passed. Independent local review and fresh protected-head hosted security/approval remain required.

Security boundaries

Existing owner dependency/hash-lock and scanner-finding repairs are now included as described above; no new credentials, raw runtime secret reads, cross-origin redirects, deployment changes or gate weakening. Base/head configuration identity policy remains unchanged; obsolete-upload retirement is not part of this PR.

Developer experience:

The existing JSON transport callers retain their return shape. Analyses callers receive complete, validated history or an explicit failure rather than a silently truncated prefix. The pagination delta is stdlib-only. The prerequisite integration also carries existing owner parser/runtime dependency declarations and corrected hash locks; no workflow permission or credential expansion is authorized.

User experience:

The consumer's protected review remains fail closed. A valid active configuration on a later page can no longer disappear from the evidence used for base/head pairing. This does not yet establish that the consumer PRs have satisfied their review and merge prerequisites.

Grounding

GitHub's official REST pagination and code-scanning analyses contracts are cited in docs/doctoring/codeql-ghas-analyses-pagination.md. This API-contract repair introduces no scientific/model feature requiring a paper PDF.

Summary by CodeRabbit

  • 버그 수정
    • GitHub API 분석 내역을 여러 페이지에 걸쳐 가져오며, 잘못되거나 불완전한 응답은 결과 일부를 반환하지 않고 처리합니다.
    • HTTP 오류 응답을 처리한 뒤에도 응답이 정상적으로 닫히도록 개선했습니다.
    • 신뢰된 아카이브 다운로드는 일시적인 서버 오류가 발생하면 제한된 횟수만큼 재시도합니다.
    • Maturin 도구 자산 다운로드의 허용 파일과 리디렉션을 제한하고 응답 크기를 확인합니다.
  • 보안 및 의존성
    • 문서 리더, Strix 및 CI 환경의 의존성을 보안 수정 버전으로 갱신했습니다.
  • 품질 및 문서
    • CI 검증 범위를 보완하고, 제품·기술 기준 문서와 변경 기록을 갱신했습니다.

Retire the reverse-PR restack path and preserve the six dependency-review owner paths while recording protected main as the second parent.
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
📝 Walkthrough

Walkthrough

Changes

CI 제어 영역 범위와 기록

Layer / File(s) Summary
저장소 범위 및 변경 기록
.Jules/palette.md, CHANGELOG.md, CHANGELOG.d/*, docs/doctoring/*, docs/product-technical-gap-baseline.md
저장소의 소유 범위를 GitHub Actions 제어 영역으로 갱신했습니다. HTTP 응답 종료, 보안 잠금, 아카이브 재시도, GHAS 페이지네이션과 검증 결과를 기록했습니다.

워크플로 및 의존성 검증

Layer / File(s) Summary
워크플로 경로 선택과 재시도
.github/workflows/*, tests/test_agent_review_runtime_quality_consolidation.py, tests/test_trusted_archive_retry_behavior.py
변경 경로에 따라 GHAS, Noema, OpenCode 및 리뷰 수리 스위트를 선택합니다. 신뢰 아카이브 다운로드는 제한된 재시도를 사용합니다.
의존성 잠금과 입력 보호
requirements-*.txt, scripts/ci/noema-document-reader/package.json, tests/test_*dependency*
파서와 보안 의존성의 고정 버전을 갱신했습니다. Noema의 fast-uri와 ip-address, Strix의 LiteLLM·PyJWT·urllib3 잠금을 검증합니다.

런타임 및 릴리스 검증

Layer / File(s) Summary
HTTP 응답 수명과 Maturin 다운로드
scripts/ci/*, tests/test_github_api_url_boundary.py, tests/test_verify_release_maturin_tool_assets.py, tests/test_sandboxed_web_e2e.py
여러 HTTP 오류 경계에서 응답을 닫습니다. Maturin 자산 다운로드는 허용된 파일명과 단일 HTTPS CDN 리디렉션만 허용하고 응답 크기를 제한합니다.
GHAS 분석 수집
scripts/ci/codeql_ghas_configuration_identity.py, tests/test_codeql_ghas_analyses_pagination.py
CodeQL 분석 목록이 검증된 Link의 다음 페이지를 순차적으로 수집합니다. 잘못된 링크, 순환, 불완전한 응답, 1,000페이지 초과를 거부합니다.
릴리스 게이트와 테스트 범위
scripts/ci/release_dependency_gate.py, scripts/ci/prescreen_release_runtime_archives.py, tests/test_release_dependency_gate*.py, tests/test_verify_release_scope_evidence_set.py
TOML parser fallback, Cargo workspace·lock 검증, 라이선스 선택, fanout 제한, runtime receipt 및 아키텍처 검증의 테스트 범위를 확장했습니다.

Priority: ⬆️ High

Estimated code review effort: 5 (Critical) | ~120 minutes

Change: Bug fix · Severity of issue fixed: High


Merge Risk: 🟡 Moderate · up to 1cda4

Python 3.10 test collection can fail. Remove the test module’s direct tomllib import before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1cda4

The inspected changes strengthen retrieval and validation controls without demonstrating a newly introduced security weakness. Some release hardening is not yet consumed by the pinned release workflow, and incomplete coverage prevents a minimal-risk assessment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected exposure is concentrated in repository security-gate decisions and trusted helper execution on CI runners. Pagination stays within the selected GitHub repository; archive retrieval obtains executable policy source for existing credentialed review workflows.

Security Findings and Attack Paths

  • inferred — The examined response-controlled pagination and release-asset paths did not establish an introduced credential escape or false-verification path. Authority and repository checks constrain authenticated pagination; filename, digest, and native-link checks constrain asset verification. This conclusion is limited to the inspected paths.

Trust Boundaries and Controls

  • observed — The head Maturin downloader admits five literal filenames, disables proxies, permits one credential-free HTTPS redirect to the release CDN, bounds downloaded bytes, and closes responses. The CDN path is not independently bound to the asset, but digest verification checks substituted content against trusted adjacent evidence.
  • observed — The evidence JSON is configuration co-located with the trusted helper, and the production caller verifies its pinned checkout identity. No independent producer or signing authority was established; this remains provenance uncertainty rather than a demonstrated verification bypass.

Resilience and Maintainability Implications

  • observed — Archive retries repeat a GET for the same immutable SHA with three configured retries. Error exit stops extraction, preserving the existing success-before-consumption ordering. The regression covers one 502 followed by success; interrupted-file cleanup, concurrent runner-path ownership, and a total transfer deadline are not established by that test.
  • observed — GHAS HTTP errors now use a 400-byte diagnostic read and finally-based closure. Review preflight also closes HTTP errors even when optional evidence recording raises, and rejects oversized Retry-After values before integer conversion.

Hardening Proposals

  • proposed — If release consumption of the new downloader is intended, advance the trusted helper pin through a separately verified rollout that preserves checkout identity and evidence integrity. The current pin does not consume this implementation.
  • proposed — For the pre-existing license-selection publication contract, consider explicit capture-directory ownership, atomic publication, and final byte-identity verification before downstream consumption. These would address interruption and concurrent-writer uncertainties without treating them as PR-introduced findings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 84.17% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 278 functions across 50 files. (43 skipped:…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 CodeQL 분석 기록을 전체 페이지에서 수집하고 실패 시 안전하게 중단하는 핵심 변경을 정확하고 간결하게 설명합니다.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head hosted RCA and executable owner replay

Consumer: central pagination PR #2555, head 331512f44460e98852c4028980af0f73d8c046b0.
Canonical security owner: existing PR #2531, inspected head 7900ba4c4d68c378023592252f2579646fad9aaa. #2530 also contains that baseline; no duplicate dependency PR was created.

The current hosted failures are distinct:

  • Trivy job 110594998201: PyO3 0.22.6 HIGH GHSA-36hh-v3qg-5jq4 in tests/fixtures/coverage-cargo/Cargo.lock; document-reader fast-uri/ip-address MEDIUM advisories also reported. This is a real inherited finding, not a pagination source defect or flake.
  • pip-audit job 110595040389: known-vulnerable urllib3 in requirements-strix-ci-hashes.txt; remediation in the actual log is 2.8.0 or later. The current lock has 2.7.0.
  • CodeQL jobs 110595026846 / 110595026956: DISPATCH_OUTCOME=success, VERDICT_STATE=pending. This is fail-closed dispatch handoff, not a completed scan or a terminal source finding.

Actual local replay:

  • Detached disposable worktree from exact fix(codeql): collect complete fail-closed GHAS analysis history #2555 head, with existing owner regression tests: 6 RED / 4 PASS.
  • Eight dependency files replayed byte-for-byte from exact owner fix(security): refresh shared PyJWT and PyO3 locks #2531: 10 GREEN.
  • All 234 pagination/identity/URL/credential/workflow-contract tests remain GREEN after this replay (scratch Bash 5 runner; not hosted acceptance).
  • Fresh Trivy database downloaded. Current baseline Cargo fixture: HIGH finding, exit 1. Owner-replayed Cargo and npm fixture scans: HIGH/CRITICAL findings 0, exit 0.
  • The eight owner blob SHA256 values and replay commands/results are retained in security-owner-replay/verification.json and logs. The scans are limited to the named fixtures and severity scope; they do not establish the full central or Python dependency gate passed.

No replay was committed, pushed, adopted into the published branch or represented as protected-main shipment. Current #2555 remains unchanged. Owner #2531 must satisfy its own exact-head gates and independent approval before protected merge; accepted protected main can then be ordinary-merged into #2555. No open-branch immutable pin, fixture exclusion, CVE suppression, gate weakening, lifecycle retirement or credential change was used.

@seonghobae
seonghobae marked this pull request as draft October 1, 2026 22:09
@seonghobae seonghobae added status: draft Draft pull request type: bug Defect or incorrect behavior priority: high High-priority or P1 work labels Oct 1, 2026 — with ChatGPT Codex Connector
Merge reviewed main 7554587 into the existing self-hosted transition branch. Keep trusted metadata and publishers separate from PR coverage execution; preserve literal auto review routing. Exact tree e0aba57 passed whole source review by the existing reviewer and full quality (5567 passed, 4 skipped, 40 subtests; coverage 100%).
- require k-csap Noema App tokens instead of legacy fallbacks
- bind PR author and reviewer App actor before and after inference
- cover unknown author, self-review, actor drift, and token selection

Local verification:
- pytest tests -q -rs -p no:cacheprovider --cov=scripts/ci --cov-branch: 5583 passed, 4 skipped, 85 subtests, 100% line/branch coverage
- focused Noema/personal route suite: 327 passed, 45 subtests
- independent review: APPROVE_SCOPE (personal-final-review.json sha256 d43d7659d2f9fefa35085eb8ca06567841223ed40c9b27b948e8391d8d4f5f5e)
Merge #2622 into the #2565 feature-stack base after locked local acceptance and independent source review. This is not protected main deployment or App approval.
Preserve reviewed self-hosted isolation and current-head receipt semantics.
Include verified shell-literal, Noema evidence and Maturin contract corrections.
Source review complete; CI, native inference and formal approval remain separate.
Reviewed-Tree: fb8b066
Source-Verdict-SHA256: 3a362294f13eb20f894d6a3ea1ccd65e41ce2da91a88e1232fa97e06ce6d6491
Publication-Adoption: msg_e3fa604a6fcd

Source publication only; runtime, formal approval and Merge remain open.
Preserve shared base bytes exactly. Reviewed-Tree: 0e4f548
Local full gate: 5993 passed, 4 skipped, 86 subtests; Python coverage/docstrings 100%.
Source publication only; runner, inference, formal App approval and protected merge remain open.
Reconcile the reviewed law lane with current base 362fed6 and keep the base-owned Noema/App producer delta. Quote the PostgreSQL server socket option and libpq host value for RUNNER_TEMP paths containing spaces. Source publication only; runtime, App approval and protected merge remain separate.
…ted-20261003

ci: stage all workflows on isolated self-hosted runners
…ral-ci

ci: centralize law-agent quality on dedicated self-hosted runners
@seonghobae
seonghobae marked this pull request as ready for review October 13, 2026 02:59
@seonghobae
seonghobae merged commit 6511762 into main Oct 13, 2026
22 of 25 checks passed
@seonghobae
seonghobae deleted the fix/sdp-ghas-analyses-pagination-20261002 branch October 13, 2026 02:59
seonghobae added a commit that referenced this pull request Oct 13, 2026
Resolve the 9 conflicts against main while preserving both contracts:
- CHANGELOG.md, docs/product-technical-gap-baseline.md: keep both entries.
- strix_report_scope.py: identical logic on both sides; take main's text.
- codeql_ghas_configuration_identity.py: keep main's Link-header pagination
  (stricter: grammar/endpoint/filter binding, page cap); PR's per_page
  validation is retained once (main already carried the same check).
  The PR's pagination regressions are adapted to emit Link headers.
- opencode-review.yml: keep main's verdict ordering and exit-0 inner
  contract (outer step still fails closed); keep the PR's file-based review
  transfer so paginated reviews never travel through argv.
- fixture_github_reads.py, verdict/rerun tests: take main's superset.
- test_codeql_ghas_configuration_identity.py: keep the PR's size-aware
  HTTPError.read stub (main's slice breaks on read(-1)).

Verification: 21-module set + analyses pagination with -W error, bash 5.3:
587 passed; 22 additional dependent modules: 1004 passed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci-cd CI, GitHub Actions, checks, release, or supply chain area: security Security boundary, hardening, or vulnerability prevention priority: high High-priority or P1 work status: draft Draft pull request type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant