fix(content-lane): first-hop source URL validation failure is a hard failure, not passed - #9892
Conversation
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-29 19:24:12 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…failure, not "passed" (JSONbored#9669) checkOneSourceUrl's first-hop path mapped a `source_host_not_checked` validation failure (plain http:// or a private/loopback host that fails isSafeHttpUrl) to status "passed", while the redirect-loop path treats the identical outcome as a hard failure. So an unfetchable canonical source silently reported the whole item OK, and a report with only such a source came back "passed". Remove the `invalidProtocol` ternary so BOTH validator failure outcomes hard-fail on the first hop, identical to the redirect path. outcome/error still carry the validator's own values. downgradeInconclusiveSourceWarnings remains the only mechanism that makes a distribution-role source_host_not_checked non-blocking, so a distribution http:// source alongside a verifiable canonical is still relaxed -- via the correct mechanism, not by mislabelling it "passed". Tests: first-hop http:// and https-loopback are hard_failure with outcome source_host_not_checked; a canonical http:// with no verifiable canonical fails the report; a distribution http:// alongside a reachable primary canonical is downgraded to blocking:false. Updated the two prior tests that pinned the old "passed" behaviour. Closes JSONbored#9669 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9892 +/- ##
==========================================
+ Coverage 79.28% 79.35% +0.07%
==========================================
Files 281 282 +1
Lines 58516 58716 +200
Branches 6777 6850 +73
==========================================
+ Hits 46393 46593 +200
Misses 11840 11840
Partials 283 283
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Closes #9669
What
checkOneSourceUrl's first-hop path (src/review/content-lane/source-evidence.ts) mapped asource_host_not_checkedvalidation failure — a plainhttp://URL, or one whose host fails the SSRF guard (isSafeHttpUrl: private/loopback/link-local) — tostatus: "passed", via aninvalidProtocol ? "hard_failure" : "passed"ternary. The redirect-loop path treats the identical outcome as ahard_failure. So an unfetchable canonical source was silently reported OK, and a report whose only source was such a URL came back"passed".The
invalidProtocolternary is removed: both validator failure outcomes now hard-fail on the first hop, identical to the redirect path.outcome/errorstill carry the validator's own values, andvalidateFetchableSourceUrl/isSafeHttpUrl/the warnings filter are untouched.downgradeInconclusiveSourceWarningsremains the only mechanism that makes a distribution-rolesource_host_not_checkednon-blocking, so a distributionhttp://source alongside a verifiable canonical is still relaxed — through the correct mechanism, not by mislabelling it "passed".Tests
http://example.com/xandhttps://127.0.0.1/x→hard_failurewith outcomesource_host_not_checked(deliverables 1 & 2).http://source with no verifiable canonical → reportstatus: "failed"(was"passed") (deliverable 4).http://source alongside a reachable primary canonical → downgraded toblocking: falseviadowngradeInconclusiveSourceWarnings(deliverable 3 — proves the intended relaxation still applies through the correct mechanism)."passed"behaviour.Verified: 87/87 in
content-lane-source-evidence.test.ts,tsc --noEmitclean,git diff --checkclean.