Skip to content

fix: validate Strix PR metadata before scanning - #203

Merged
seonghobae merged 5 commits into
masterfrom
fix/strix-parser-location-mapping
May 27, 2026
Merged

fix: validate Strix PR metadata before scanning#203
seonghobae merged 5 commits into
masterfrom
fix/strix-parser-location-mapping

Conversation

@seonghobae

@seonghobae seonghobae commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Rebase the Strix XML location mapping branch onto current master.
  • Keep the branch's remaining unique hardening: validate PR base/head commit SHA metadata before using it in trusted Strix PR-scope operations.
  • Preserve the existing XML <file> mapping behavior from master instead of reintroducing stale parser code.

Verification

  • git diff --check
  • timeout 600 bash scripts/ci/test_strix_quick_gate.sh

Gate note

Current Strix runs may still fail before findings because the repository Strix OpenAI credential is returning quota errors. That is tracked separately on current PR comments and is not a code finding from this branch.

Summary by CodeRabbit

  • Tests

    • Added CI tests for XML-based vulnerability reports (including filenames with spaces) and for gate behavior when PR SHA metadata is invalid, ensuring failures occur as expected and no malformed SHA data is leaked.
  • Chores

    • Broadened vulnerability-report format recognition to handle additional XML-wrapped file path patterns.
  • Bug Fixes

    • Hardened pull-request commit-SHA validation and related gating checks so invalid or unreadable SHAs cause safe, fail-closed behavior with clearer diagnostics.

Review Change Stack

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 925f0e26d1107c3dcc0931de35725facf216959d:

  • 3 required check(s) are not successful on 925f0e2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 925f0e2.\n

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 55b148b0-fbb0-476d-8a39-2205f9781ce6

📥 Commits

Reviewing files that changed from the base of the PR and between a2b3e40 and 3971ef2.

📒 Files selected for processing (2)
  • scripts/ci/strix_quick_gate.sh
  • scripts/ci/test_strix_quick_gate.sh
💤 Files with no reviewable changes (2)
  • scripts/ci/test_strix_quick_gate.sh
  • scripts/ci/strix_quick_gate.sh

📝 Walkthrough

Walkthrough

Adds SHA-format validation and safer git rev-parse checks for PR base/head handling and PR-head blob copying; extends Strix report parsing to extract file paths inside XML <file>...</file> tags and adds tests for malformed-SHA rejection and XML finding mapping.

Changes

PR SHA validation & PR-head gating

Layer / File(s) Summary
SHA validators and pr_head_regular_file_mode
scripts/ci/strix_quick_gate.sh
Adds is_valid_git_commit_sha() and invalid_pull_request_sha() and updates pr_head_regular_file_mode() to validate PR_HEAD_SHA before using git rev-parse --verify --quiet.
load_pull_request_changed_files & diff terminator
scripts/ci/strix_quick_gate.sh
Validates PR_BASE_SHA/PR_HEAD_SHA, switches to git rev-parse --verify --quiet for fail-closed commit checks, and adds an explicit -- terminator to git diff --name-only.
PR-head blob copy gating for changed and trusted files
scripts/ci/strix_quick_gate.sh
Updates PR-head blob copy conditions to require valid SHA format plus git rev-parse --verify --quiet instead of git cat-file -e for both changed files and trusted-context files.
Tests: reject malformed base/head SHA cases
scripts/ci/test_strix_quick_gate.sh
Adds run_pull_request_target_rejects_invalid_sha_case, injecting invalid base/head SHAs and asserting the gate exits closed (code 2), prints “commit SHA is invalid; failing closed”, does not echo attacker marker, and does not invoke Strix; includes two invocations for base/head.

Strix XML file-path extraction

Layer / File(s) Summary
File-path extraction regex for <file> tags
scripts/ci/strix_quick_gate.sh
Adds a regex pattern to match file paths wrapped in <file> ... </file> tags and includes them as candidate vulnerability locations alongside existing extraction patterns.
Test scenario and case for <file>-tag format
scripts/ci/test_strix_quick_gate.sh
Adds pr-critical-changed-xml-file-location fake Strix scenario emitting an XML code_locations parameter targeting SysPositionController.java, plus a run_gate_case asserting the gate maps the XML finding to the PR-changed Java file and fails (exit code 1) with configured severity.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Seongho-Bae/naruon#205: Related hardening of pull_request_target gating and handling of unresolved/invalid head SHAs and self-test adjustments.
  • Seongho-Bae/naruon#201: Overlaps in PR-head/trusted-context blob handling and selection logic changes.
  • Seongho-Bae/naruon#208: Touches trusted-context bundle and PR-head copy decisions, overlapping at trusted-context blob construction.

Poem

🐰 I sniffed the SHAs, both base and head,
I hopped through tags where XML led.
I kept the markers out of sight,
I tightened checks to guard the night.
A rabbit cheers: the gate holds tight.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: validate Strix PR metadata before scanning' accurately reflects the primary change, which is strengthening PR SHA validation in the Strix quick-gate script. The title is concise, clear, and specific to the main improvement.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/strix-parser-location-mapping

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

@seonghobae

Copy link
Copy Markdown
Contributor Author

검증 증거:

  • RED: bash scripts/ci/test_strix_quick_gate.shpr-critical-changed-xml-file-location에서 Strix finding intersects files changed in this pull request. 누락으로 실패하는 것을 확인했습니다.
  • GREEN: bash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh && git diff --check 통과.
  • GREEN: bash scripts/ci/test_strix_quick_gate.shtest_strix_quick_gate: PASS.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 925f0e26d1107c3dcc0931de35725facf216959d:

  • 2 required check(s) are not successful on 925f0e2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 925f0e2.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 925f0e26d1107c3dcc0931de35725facf216959d:

  • 2 required check(s) are not successful on 925f0e2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 925f0e2.\n

@greptile-apps

greptile-apps Bot commented May 19, 2026

Copy link
Copy Markdown

Greptile Summary

PR #203은 Strix quick-gate의 두 가지 독립적인 보완 작업을 묶은 변경입니다: XML <file> 태그 기반 취약점 위치 파싱 지원과 PR SHA 검증 강화입니다.

  • XML 경로 파싱 추가 (strix_quick_gate.sh line 1326): <file>path</file> 형식을 인식하는 정규식 패턴을 extract_candidate_source_paths_from_report 파이썬 인라인 스크립트에 추가하고, 대응하는 pr-critical-changed-xml-file-location 게이트 테스트 케이스를 구성했습니다.
  • SHA 주입 방어 (strix_quick_gate.sh lines 210-219, 656-668): is_valid_git_commit_sha로 SHA를 git 명령 실행 전에 ^[0-9a-fA-F]{40,64}$ 패턴으로 선검증하고, git cat-file -egit rev-parse --verify --quiet로 교체했습니다. pull_request_target 이벤트에서 형식이 잘못된 SHA가 주입될 경우 Strix를 호출하지 않고 코드 2로 조기 종료하며, 이를 검증하는 run_pull_request_target_rejects_invalid_sha_case 테스트도 추가되었습니다.

Confidence Score: 5/5

CI 판정 로직과 테스트만 수정하며 런타임 앱·DB·API에 영향 없음 — 병합 안전

SHA 검증은 ^[0-9a-fA-F]{40,64}$ 앵커 정규식으로 완결되어 주입 벡터를 닫고, XML 경로 정규식은 [^\s<`│]+ 제외 클래스로 태그 경계를 안전하게 처리합니다. pull_request_target 이벤트에서 악의적인 SHA가 들어올 경우 Strix 호출 없이 종료 코드 2로 실패 폐쇄됨을 신규 테스트가 직접 검증합니다. 변경 범위가 CI 스크립트 두 파일에 한정되어 있으며 기존 동작을 훼손하는 부분이 없습니다.

특별히 주의가 필요한 파일 없음

Important Files Changed

Filename Overview
scripts/ci/strix_quick_gate.sh SHA 형식 선검증 함수 추가, git cat-file → rev-parse --verify 교체, XML file 태그 정규식 추가 — 로직·정규식 모두 올바름
scripts/ci/test_strix_quick_gate.sh XML 경로 파싱 게이트 케이스 및 malicious SHA 주입 거부 테스트 추가 — 픽스처·어서션 모두 올바름

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[load_changed_files_from_pull_request] --> B{base_sha / head_sha 비어있음?}
    B -- "예 + pull_request_target" --> ERR1[ERROR: metadata unavailable\nreturn 2]
    B -- "예 + 일반 PR" --> R1[return 1]
    B -- 아니오 --> C{is_valid_git_commit_sha base_sha?}
    C -- "실패 + pull_request_target" --> ERR2[invalid_pull_request_sha base\nreturn 2]
    C -- "실패 + 일반 PR" --> R2[return 1]
    C -- 성공 --> D{is_valid_git_commit_sha head_sha?}
    D -- "실패 + pull_request_target" --> ERR3[invalid_pull_request_sha head\nreturn 2]
    D -- "실패 + 일반 PR" --> R3[return 1]
    D -- 성공 --> E{git rev-parse base^commit}
    E -- 실패 --> ERR4[ERROR: base commit not found\nreturn 2 / return 1]
    E -- 성공 --> F{git rev-parse head^commit}
    F -- 실패 --> ERR5[ERROR: head commit not found\nreturn 2 / return 1]
    F -- 성공 --> G[git diff --name-only base...head --]
    G --> H[CHANGED_FILES 구축 완료\nreturn 0]
Loading

Reviews (3): Last reviewed commit: "test: clarify Strix XML fixture" | Re-trigger Greptile

Comment thread scripts/ci/test_strix_quick_gate.sh
Comment thread scripts/ci/test_strix_quick_gate.sh
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 925f0e26d1107c3dcc0931de35725facf216959d:

  • 2 unresolved current review thread(s) remain.\n- 1 required check(s) are not successful on 925f0e2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 925f0e2.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for fa66176de7198c5cd8d2da5b1c52902303451380:

  • 2 unresolved current review thread(s) remain.\n- 3 required check(s) are not successful on fa66176.\n- Missing current-head CodeRabbit/coderabbitai evidence for fa66176.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

추가 Strix 후속 조치:

  • Strix가 PR_BASE_SHA/PR_HEAD_SHA 메타데이터의 strict validation 부재를 지적해, 40/64자 hex commit SHA만 git revision 인자로 사용하도록 보강했습니다.
  • invalid base/head SHA 회귀 케이스를 추가해 fail-closed, payload 미출력, Strix 미실행을 검증했습니다.
  • 검증: bash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh && git diff --check 통과.
  • 검증: bash scripts/ci/test_strix_quick_gate.shtest_strix_quick_gate: PASS.
  • 보안 subagent review: PASS.

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for fa66176de7198c5cd8d2da5b1c52902303451380:

  • 2 unresolved current review thread(s) remain.\n- 2 required check(s) are not successful on fa66176.\n- Missing current-head CodeRabbit/coderabbitai evidence for fa66176.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for fa66176de7198c5cd8d2da5b1c52902303451380:

  • 2 unresolved current review thread(s) remain.\n- 2 required check(s) are not successful on fa66176.\n- Missing current-head CodeRabbit/coderabbitai evidence for fa66176.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for fa66176de7198c5cd8d2da5b1c52902303451380:

  • 2 unresolved current review thread(s) remain.\n- Missing current-head CodeRabbit/coderabbitai evidence for fa66176.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a2b3e401cca4f77f8bc437f73d7b8ea316ae9f11:

  • 2 unresolved current review thread(s) remain.\n- 3 required check(s) are not successful on a2b3e40.\n- Missing current-head CodeRabbit/coderabbitai evidence for a2b3e40.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a2b3e401cca4f77f8bc437f73d7b8ea316ae9f11:

  • 2 required check(s) are not successful on a2b3e40.\n- Missing current-head CodeRabbit/coderabbitai evidence for a2b3e40.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a2b3e401cca4f77f8bc437f73d7b8ea316ae9f11:

  • 2 required check(s) are not successful on a2b3e40.\n- Missing current-head CodeRabbit/coderabbitai evidence for a2b3e40.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for a2b3e401cca4f77f8bc437f73d7b8ea316ae9f11:

  • Missing current-head CodeRabbit/coderabbitai evidence for a2b3e40.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

Stale: Strix fixes already landed on master via #206-#208. This PR has conflicts.

@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 2b4b043d418e36d1a24486c6149aeed2c7a7d21e:

  • Merge state is UNKNOWN; resolve conflicts or refresh mergeability.

@seonghobae seonghobae changed the title fix: Strix XML 파일 위치 매핑 fix: validate Strix PR metadata before scanning May 27, 2026
@seonghobae
seonghobae enabled auto-merge May 27, 2026 03:41
@seonghobae

seonghobae commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Current-head merge gate evidence for 3971ef21423a677336c6f6ebe2675d37c09d7b29:

  • CodeRabbit: pass on current head.
  • security: pass on current head.
  • strix: required check fails in latest rerun job https://github.com/Seongho-Bae/naruon/actions/runs/26489334343/job/78014874149 before producing structured findings.
  • The trusted workspace, PR head fetch, and Strix gate self-test completed before the LLM call.
  • The Strix log reports litellm.RateLimitError / OpenAI quota exceeded on all 3 retry attempts, then No fallback models configured.
  • Strix is configured for openai/gpt-5.4; the direct STRIX_OPENAI_API_KEY secret is present but quota-exhausted.
  • GitHub Models catalog was checked and currently exposes openai/gpt-5 family models but no OpenAI GPT-5.4+ model, so switching to GitHub Models would not satisfy the GPT-5.4+ requirement today.

Next action: provide a quota-enabled GPT-5.4+ OpenAI credential or make an OpenAI GPT-5.4+ model available through GitHub Models, then rerun strix. Auto-merge is already enabled.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Temporary merge-gate adjustment requested by repository owner on 2026-05-27 UTC:

  • Current head: 3971ef21423a677336c6f6ebe2675d37c09d7b29.
  • security: pass on current head.
  • CodeRabbit: pass on current head.
  • strix: latest required run failed before findings due OpenAI quota exhaustion, not due a structured Strix finding.
  • Action: remove only the required strix context from master branch protection, leave security and CodeRabbit required, allow auto-merge, then restore strix immediately after the queued merges finish.

This is a temporary rollback-bound gate adjustment, not a scanner suppression in workflow code.

@seonghobae
seonghobae merged commit c3c94a9 into master May 27, 2026
14 of 15 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