test(supply-chain): reproduce Maven wrapper integrity gap on current develop - #311
test(supply-chain): reproduce Maven wrapper integrity gap on current develop#311seonghobae wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 81 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMaven Wrapper 속성에 SHA-256 체크섬을 추가했습니다. 검증 유틸리티가 필수 속성의 중복과 기대값을 확인합니다. CI 및 SBOM 워크플로는 Maven 실행 전에 검증을 수행합니다. 관련 JUnit 테스트도 추가했습니다. ChangesMaven Wrapper 무결성
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🟡 Moderate · up to The PR adds Maven-wrapper checksum and preflight enforcement, but its Windows invocation check currently fails to recognize the command form used by the workflows. That can leave a required Windows integrity check unenforced, so the PR is not merge-ready until the matcher is corrected and the relevant checks pass. Sequence Diagram(s)sequenceDiagram
participant GitHubWorkflow
participant VerifyMavenWrapperIntegrity
participant maven-wrapper.properties
participant MavenWrapper
GitHubWorkflow->>VerifyMavenWrapperIntegrity: 운영체제별 사전 검증 실행
VerifyMavenWrapperIntegrity->>maven-wrapper.properties: 속성 파일 읽기
VerifyMavenWrapperIntegrity-->>GitHubWorkflow: 검증 성공 또는 예외 반환
GitHubWorkflow->>MavenWrapper: 테스트 또는 SBOM 작업 실행
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/scripts/VerifyMavenWrapperIntegrity.java (1)
61-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value키 매칭을
=이외의 구분자까지 허용하세요.
requireUniqueProperty는key + "="접두사만 셉니다.java.util.Properties형식은key = value와key:value도 허용합니다. 누군가 공백을 넣어distributionUrl = ...로 저장하면matches가 0이 되어 "found 0" 메시지로 실패합니다. 값 자체는 정상인데 실패 원인이 오해를 유발합니다.동작은 fail-closed로 유지되므로 보안 결함은 아닙니다. 진단 정확도만 개선하는 변경입니다.
♻️ 제안 수정
private static void requireUniqueProperty(List<String> sourceLines, String key) { + java.util.regex.Pattern keyPattern = + java.util.regex.Pattern.compile("^" + java.util.regex.Pattern.quote(key) + "\\s*[=:]"); long matches = sourceLines.stream() .map(String::trim) - .filter(line -> line.startsWith(key + "=")) + .filter(line -> keyPattern.matcher(line).find()) .count();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/VerifyMavenWrapperIntegrity.java around lines 61 - 71, Update requireUniqueProperty to recognize java.util.Properties key separators: optional whitespace followed by either '=' or ':' after the key, while preserving the existing exactly-one match validation and error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@etl-service/src/test/java/com/xtrmetl/etl/documentation/MavenWrapperIntegrityTest.java`:
- Around line 138-140: Update containsWrapperInvocation to match Windows wrapper
commands using the mvnw.cmd pattern so workflow text is detected correctly,
while preserving Unix detection. Revise the related invocation-count validation
to require at least one Unix and one Windows invocation rather than assuming
wrapperInvocations == 2.
---
Nitpick comments:
In @.github/scripts/VerifyMavenWrapperIntegrity.java:
- Around line 61-71: Update requireUniqueProperty to recognize
java.util.Properties key separators: optional whitespace followed by either '='
or ':' after the key, while preserving the existing exactly-one match validation
and error behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d3bba6d-64a8-4a90-8d50-1c002db12a74
📒 Files selected for processing (5)
.github/scripts/VerifyMavenWrapperIntegrity.java.github/workflows/ci.yml.github/workflows/sbom.yml.mvn/wrapper/maven-wrapper.propertiesetl-service/src/test/java/com/xtrmetl/etl/documentation/MavenWrapperIntegrityTest.java
| private static boolean containsWrapperInvocation(String line) { | ||
| return line.contains("./mvnw ") || line.contains(".\\mvnw.cmd "); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- 워크플로 실제 텍스트 ---"
rg -n 'mvnw' .github/workflows/ci.yml .github/workflows/sbom.yml
echo "--- 단일 백슬래시 패턴 존재 여부 (0건이면 테스트가 Windows 스텝을 못 잡음) ---"
rg -nF '.\mvnw.cmd ' .github/workflows/ci.yml .github/workflows/sbom.yml || echo "matches=0"
echo "--- 이중 백슬래시 패턴 ---"
rg -nF '.\\mvnw.cmd ' .github/workflows/ci.yml .github/workflows/sbom.yml || trueRepository: ContextualWisdomLab/mightyETL
Length of output: 1324
Windows 래퍼 호출 매칭 조건을 수정하세요.
.github/workflows/ci.yml과 .github/workflows/sbom.yml의 Windows 명령은 텍스트 .\\mvnw.cmd 를 사용합니다. 현재 containsWrapperInvocation의 Java 리터럴 ".\\mvnw.cmd "는 실행 시 .\mvnw.cmd 가 되므로 Windows 호출을 매칭하지 못합니다.
Windows 호출을 mvnw.cmd 기준으로 매칭하도록 수정하세요. 워크플로별 호출 수가 다르므로 wrapperInvocations == 2로 고정하지 말고, Unix 및 Windows 호출이 각각 존재하는지 검증하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@etl-service/src/test/java/com/xtrmetl/etl/documentation/MavenWrapperIntegrityTest.java`
around lines 138 - 140, Update containsWrapperInvocation to match Windows
wrapper commands using the mvnw.cmd pattern so workflow text is detected
correctly, while preserving Unix detection. Revise the related invocation-count
validation to require at least one Unix and one Windows invocation rather than
assuming wrapperInvocations == 2.
Sources: Coding guidelines, Learnings
1 participant
Rebuilds issue #265 test-first from exact protected
develop@4408b6086fd725ff1629d8f048cf28f3a2b40b1bbecause PR #281 is two protected commits behind and no longer descends from the live tip.This branch intentionally contains only
MavenWrapperIntegrityTest. The test requires the fixed Maven 3.9.11 distribution URL to remain bound to its reviewed SHA-256 and requires an integrity preflight immediately before every CI/SBOM Maven Wrapper invocation. Protected source at this base does not yet satisfy either requirement, so exact-head CI must produce causal RED before production/workflow changes are copied forward.The two protected commits since #281's merge base touch only CDC source/tests and do not overlap this supply-chain lane. No old-head checks, reviews, or model evidence transfer. After current-head RED, apply the smallest fail-closed GREEN and rerun all exact-head security/SBOM/CI gates.
Summary by CodeRabbit