Skip to content

fix(review): block the RFC 6598 CGNAT range in both safe-url twin guards - #7265

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/safe-url-cgnat-both-twins-7253
Jul 19, 2026
Merged

fix(review): block the RFC 6598 CGNAT range in both safe-url twin guards#7265
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/safe-url-cgnat-both-twins-7253

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What

ipv4IsPrivateOrLocal — the SSRF-safe URL guard's IPv4 check — rejected five private/reserved ranges but not 100.64.0.0/10 (RFC 6598 "Shared Address Space" / carrier-grade NAT), which cloud/hosting providers assign to internal service endpoints. A URL resolving to a 100.64.x.x100.127.x.x host passed the check as if it were public.

How

Add one additive check — if (a === 100 && b >= 64 && b <= 127) return true; — following the identical inline style as the five existing checks, to both copies of the guard:

  • packages/loopover-engine/src/review/safe-url.ts (engine copy)
  • src/review/content-lane/safe-url.ts (the live host twin — imported by queue/processors.ts, orb/relay.ts, orb/federated-collector.ts, and the visual-capture path)

These two files are a NAMED_TWIN_PAIR enforced by scripts/check-engine-parity.ts, so patching only one would both leave a live SSRF gap and fail the parity drift-check. engine-parity:drift-check passes with both patched identically.

Validation

Boundary regression tests added to both test suites (safe-url-engine.test.ts and content-lane-safe-url.test.ts): 100.64.0.0/100.127.255.255 (inclusive bounds — blocked), 100.100.50.1 (mid-range — blocked), and 100.63.255.255/100.128.0.0 (just outside — public/safe). Confirmed both suites fail against the unfixed guards and pass with the fix; engine-parity:drift-check and typecheck pass.

Closes #7253

ipv4IsPrivateOrLocal rejected five private/reserved IPv4 ranges but not
100.64.0.0/10 (RFC 6598 shared address space / carrier-grade NAT) -- a
non-publicly-routable range cloud providers assign to internal service
endpoints, exactly what this SSRF guard exists to block. A URL resolving to
a 100.64.x.x-100.127.x.x host passed the IPv4 check as if it were public.

Add the range check to BOTH copies of the guard -- the engine copy
(packages/loopover-engine/src/review/safe-url.ts) and its live host twin
(src/review/content-lane/safe-url.ts), a NAMED_TWIN_PAIR kept in lock-step
by check-engine-parity.ts -- so the SSRF protection stays consistent across
every call site. Boundary-tested in both files' suites.

Closes JSONbored#7253
@shin-core
shin-core requested a review from JSONbored as a code owner July 19, 2026 11:17
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 19, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.81%. Comparing base (4d80214) to head (3630700).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7265   +/-   ##
=======================================
  Coverage   93.81%   93.81%           
=======================================
  Files         704      704           
  Lines       69448    69452    +4     
  Branches    18897    18899    +2     
=======================================
+ Hits        65152    65156    +4     
  Misses       3302     3302           
  Partials      994      994           
Flag Coverage Δ
shard-1 43.61% <50.00%> (+<0.01%) ⬆️
shard-2 37.32% <0.00%> (+0.10%) ⬆️
shard-3 33.19% <0.00%> (+0.07%) ⬆️
shard-4 33.74% <0.00%> (-0.60%) ⬇️
shard-5 32.89% <0.00%> (+0.71%) ⬆️
shard-6 45.49% <50.00%> (-0.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-engine/src/review/safe-url.ts 100.00% <100.00%> (ø)
src/review/content-lane/safe-url.ts 100.00% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 19, 2026
@loopover-orb

loopover-orb Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-19 11:28:12 UTC

4 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This is a correct, minimal SSRF fix: it adds the missing RFC 6598 CGNAT range (100.64.0.0/10) check to both twin copies of ipv4IsPrivateOrLocal, following the exact inline style of the five existing checks, and closes #7253. The boundary math is right (a===100 && b in [64,127] correctly spans 100.64.0.0–100.127.255.255), and both test suites add matching boundary-inclusive tests (100.64.0.0, 100.100.50.1, 100.127.255.255 blocked; 100.63.255.255, 100.128.0.0 allowed) mirrored across the engine and content-lane copies, which is exactly what the NAMED_TWIN_PAIR parity check requires.

Nits — 3 non-blocking
  • The magic numbers 64/127 aren't named as constants, but this matches the existing inline style of the other four checks in the same function, so it's consistent rather than a real issue.
  • The `b >= 64 && b <= 127` bound could alternatively be written as `(b & 0xc0) === 0x40` for bitmask consistency with the >>> 24/16 style above it, but the current form is more readable — not worth changing.
  • Consider a short comment block above ipv4IsPrivateOrLocal listing all six ranges with their RFC numbers in one place for future maintainers, rather than one inline comment per line (optional, matches existing style).
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7253
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 70 registered-repo PR(s), 45 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 70 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, Solidity, Dart, Python, CSS, PHP, Rust
  • Official Gittensor activity: 70 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@JSONbored
JSONbored merged commit 3c72fce into JSONbored:main Jul 19, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

safe-url's SSRF IPv4 guard is missing the RFC 6598 CGNAT range (100.64.0.0/10)

2 participants