fix(pilot): reject placeholder or ambiguous readiness evidence - #493
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough파일럿 준비성 평가기가 중복 필드, 로컬 또는 예시 URL, 지원 채널, 증빙 참조를 검증하도록 강화되었습니다. 관련 실패 사유와 검증 테스트가 추가되었습니다. Changes파일럿 준비성 검증
Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches📝 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 |
seonghobae
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const failures = []; | ||
| for (const [label] of duplicateAuthorities) failures.push(`${label} must appear exactly once`); |
There was a problem hiding this comment.
🟡 Behavior change without a changelog entry
These new fail-closed readiness rejection rules change behavior, but no ## Unreleased bullet was added to CHANGELOG.md. The contributor rule requires a changelog entry for every behavior change, and the existing top pilot bullet covers only date/time authority, not placeholder, local, duplicate, or URL evidence.
Prompt for agents
CONTRIBUTING.md and CLAUDE.md require that CHANGELOG.md's `## Unreleased` section be updated with every behavior change. This PR adds new pilot-readiness validation behavior in scripts/lib/pilot-readiness.mjs (rejecting placeholder/example/localhost/.local evidence references, loopback / unspecified-listener / .localhost production URLs, broadened .local support-channel rejection, and duplicate authoritative fields) but adds no corresponding CHANGELOG entry. Add a Korean `## Unreleased` bullet describing this fail-closed hardening, consistent with the surrounding entries.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function fieldValue(entry, label) { | ||
| const escaped = label.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); | ||
| const match = entry.match(new RegExp(`^-\\s*${escaped}:\\s*(.+)\\s*$`, "m")); | ||
| return match ? match[1].trim() : ""; | ||
| } | ||
|
|
||
| function fieldCount(entry, label) { | ||
| const escaped = label.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); | ||
| return [...entry.matchAll(new RegExp(`^-\\s*${escaped}\\s*:`, "gm"))].length; | ||
| } |
There was a problem hiding this comment.
📝 Info: fieldValue and fieldCount treat pre-colon whitespace differently
fieldValue requires the colon to immediately follow the label, while fieldCount (pilot-readiness.mjs) allows whitespace before it. A line like - 고객명 : Acme is counted but read as empty. This only fails closed and cannot forge a duplicate, so it is not a bug, but the two regexes drifting apart invites future confusion.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function isUsableSupportChannel(value) { | ||
| const normalized = value.toLowerCase(); | ||
| return normalized.length > 0 | ||
| && !normalized.includes("@noema.local") | ||
| && !normalized.includes(".local") | ||
| && !normalized.includes("example") | ||
| && !normalized.includes("localhost"); | ||
| } | ||
|
|
||
| function isUsableEvidenceReference(value) { | ||
| const normalized = value.toLowerCase(); | ||
| return normalized.length > 0 | ||
| && !normalized.includes("example") | ||
| && !normalized.includes("localhost") | ||
| && !normalized.includes(".local"); | ||
| } |
There was a problem hiding this comment.
📝 Info: Coarse substring matching on sample markers
isUsableEvidenceReference and isUsableSupportChannel reject any value containing .local, example, or localhost as a substring. Real values like a path with .locale or a host like exampletech.com are rejected. This fail-closed broadening matches the documented buyer boundary, but the substring test is coarse and can reject genuine evidence in rare cases.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function isLocalOnlyHostname(host) { | ||
| const normalized = host.startsWith("[") && host.endsWith("]") | ||
| ? host.slice(1, -1) | ||
| : host; | ||
| if (normalized === "::" || normalized === "::1" || normalized === "0.0.0.0") return true; | ||
| if (/^::ffff:7f[0-9a-f]{2}:[0-9a-f]{1,4}$/i.test(normalized)) return true; | ||
| return /^127(?:\.\d{1,3}){3}$/.test(normalized); | ||
| } |
There was a problem hiding this comment.
📝 Info: IPv4-mapped loopback match depends on URL hex serialization
The IPv4-mapped loopback regex matches the bracket-stripped host, relying on the URL parser serializing [::ffff:127.0.0.2] to hex ::ffff:7f00:2 rather than dotted-quad. WHATWG URL does emit hex pieces, so the tested cases pass and the regex covers all of 127.0.0.0/8. Correct, but the coverage rests on serializer behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Fail closed when otherwise-complete pilot-readiness records use documented sample/local markers, local-only or non-routable listener identities, or contradictory duplicate authoritative fields to manufacture commercial completion evidence.
Repair lineage
Earlier test-first repairs on this canonical branch reject sample contract/KPI/trace references, conflicting or malformed duplicate scalar/metric authority, 127/8 and IPv6 loopback, IPv4-mapped loopback, absolute local DNS names, and
.localsupport channels.This run added a further validated production-identity boundary:
61a5263c697b10ad20657b7dc80142d43e92d0aaaddstenant.localhost,0.0.0.0, and[::]as otherwise-valid production URLs. Its CI/reviewer/Security runs were cancelled when the candidate immediately superseded that head, so cancelled runs are not claimed as terminal RED evidence.dae9b00df73d69918c104308844d09a96f24fdf0rejects.localhostdescendants plus IPv4/IPv6 unspecified listener identities while preserving private enterprise production networks.cec399a594d92f35b94f1ac07d0323f8d1011a8drebased no history: it preserves the exact fix(pilot): reject placeholder or ambiguous readiness evidence #493 source/test blobs and incorporates protectedmain8ce7c7536888ec0b596e18368eedd9379deec64fas an ancestor.Buyer boundary
docs/pilot-readiness-log.mdclassifiesexample,localhost, and.localvalues as samples that cannot count toward a completed paid pilot. Loopback, unspecified-listener, and localhost-subdomain endpoints likewise are not externally operable production identities.evaluatePilotReadinessTextfeedssaleable-readiness-audit, so these identities and duplicate evidence authorities must fail closed rather than manufacture commercial readiness.Current exact identity and evidence
Exact head:
cec399a594d92f35b94f1ac07d0323f8d1011a8d.Live protected base:
8ce7c7536888ec0b596e18368eedd9379deec64f, exact merge base, behind 0. The live diff remains exactly two files:scripts/lib/pilot-readiness.mjsand its realistic regression test.Current exact-head evidence:
32624571418: success32624571444: success32624571424: successpatch-validator-image32624571471: in progressMerge boundary
Keep Draft until every workflow actually triggered for unchanged exact head
cec399a...is terminal-success and live base/review/scanner authority are freshly unchanged. Do not reuse predecessor evidence or treat the in-progress dedicated image run as passing. If that run succeeds, re-read exact head/base/threads/central scanner authority, mark Ready, refetch again after the state change, and merge only if no new non-passing evidence appears.Summary by CodeRabbit
개선 사항
버그 수정