fix(content-lane,federated,apr): close the seven pre-flag defects — scrub bypass, probe OOM, self-corroboration, identity farming, peer takeover, tenant binding (#9490) - #9535
Conversation
…c-safe scrub bypass, probe OOM/fan-out, self-corroborating grounding, identity farming, peer takeover, and APR tenant binding (#9490)
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-28 07:04:33 UTC
Review summary Nits — 5 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. |
Logic backtestReplayed 0 historical case(s) for Backtest comparison:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9535 +/- ##
==========================================
- Coverage 89.56% 88.67% -0.90%
==========================================
Files 843 844 +1
Lines 110213 110261 +48
Branches 26233 26250 +17
==========================================
- Hits 98717 97775 -942
- Misses 10233 11514 +1281
+ Partials 1263 972 -291
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Summary
Seven defects across the content-lane surface-verification feature, the federated peer-intelligence import, and the APR transfer path. Every one is latent behind a flag that currently ships disabled — which is exactly why they are fixed now, before a flag flip converts them into incidents.
Closes #9490
Content lane (
LOOPOVER_REVIEW_SURFACE_VERIFICATION, ships"false")1. Remote-server text reached the public comment pre-marked as safe.
probeFunctionalSurfaceinterpolated the rawcontent-typeheader — from the submitter's own server — into a detail string that flows into a finding the wire marksalreadyPublicSafe: true, the flag that skipssanitizeForCheckRunand the forbidden-terms scrub. The wire's invariant ("summary comes solely from a fixed assessment vocabulary") was simply false after #9255. Only a strictly mime-shaped token (type/subtype, ≤64 chars/side) is now ever echoed; anything else renders asunrecognized. Allowlist-from-the-start, not a scrub — nothing hostile survives beyond word chars, dots, pluses, dashes.2.
MAX_PROBE_BODY_CHARSdid not bound memory.(await response.text()).slice(0, 64000)buffered the full hostile body before slicing — a streamed multi-hundred-MB response OOMs the isolate before the fail-closed path can run. Now a streaming bounded reader (grounding-wire's house pattern) that cancels at the cap; the regression test serves an infinite stream and pins both the cancel and the chunk count.3. Unbounded probe fan-out. Each verification fetches up to 2 URLs × 5 hops × 10s, and metagraphed's spec sets
maxAppendedEntries: Infinity— a 500-entry PR was a request-amplification primitive and a subrequest-exhaustion path converting into bulkprobe_fetch_failedholds for everyone in the isolate.MAX_VERIFIED_ENTRIES_PER_RUN = 10(matching source-evidence's own cap); entries past it hold for a human, never merge unprobed — the cap bounds spend, it must not become a way to sneak entry #11 through. Counted synchronously before any await, so the concurrent map cannot race the budget; statically-closed entries don't consume it, so padding with invalid entries can't starve real ones.4. Netuid grounding was self-corroborating. With
MIN_STRONG = 1, a submitter's own page asserting "netuid 64" passed grounding — owner and host require an independent source, netuid didn't. It now holds to the strictest standard of the three: the mention must come from the independent source's text. It is the one claim whose self-corroboration was worth farming — the netuid is the subnet identity. Two pre-existing tests pinned the old semantics ("netuid path is source-independent", "grounds from the TARGET page too"); both are rewritten as regressions of the exploit shape, with invariants that source-corroborated netuids still pass and the wss-entry flow is unchanged.5. Key-order-sensitive entry identity. Both structural diffs keyed entries on raw
JSON.stringify, so reordering an existing entry's keys read as a fresh append while removing its prior self from the duplicate check's scope — a farmable path to content-free auto-merged registry PRs. Identity is now canonical (recursive key sort; arrays keep order — order is meaning there, same rule ascanonicalJson).Federated peer intelligence
6.
instanceIdtakeover across allowlisted keys. In-batch dedup and the watermark entry'skeyFingerprintboth resolved last-wins byinstanceIdregardless of signing key — so a hostile-but-allowlisted peer B could claim honest peer A's id, ratchet its watermark so A's genuine bundles reject asreplayed_or_rollbackforever (targeted suppression + stat replacement), and bypass B's own Sybil cap, since only new ids count against it. Squarely inside #9148's declared threat model. An id is now bound to the first key that verified it; a bundle for that id under any other key rejects with a newinstance_key_conflictreason. The decisive invariant test: after a takeover attempt, the honest peer's next genuine bundle still lands — neither its watermark nor its binding moved.7. Unbounded
instanceIdlength silently killed watermark persistence. Nothing bounded the id, and ~3–4 bundles of ~600 KB ids push the singlesystem_flagspeer-state blob past D1's ~2 MB value limit — after whichwriteFederatedPeerStateswallowed the failure forever, silently regressing every #9148 protection for every peer. Ids are now capped at 128 chars (empty rejected too) in the shape check, and the write failure logs at error level (structured, forwarder-visible) while still failing open — the sync tick is never failed by it.APR transfer
Tenant binding, installed now while the route is inert.
installationId,repoFullNameandnewOwnerare all caller-supplied, and the only gate wasideaComplete— hard-false today, but the module instructs implementers to "replace the body (keep the signature)". The moment a completion record lands, any authorized caller could transfer any completed APR repo — including another customer's — to themselves.requestAprRepoTransfernow requires a server-sideAprRepoBinding(own module,apr-repo-binding.ts, mirroringapr-idea-completion.tsdown to the mockable import seam and the fail-closed replace-the-body contract): no binding ⇒ reject;newOwner≠ bound customer ⇒ reject; installation mismatch ⇒ reject — all before GitHub is ever contacted.The 404 probe no longer fabricates success.
probeAprRepoTransfertreated a bare 404 (repo deleted, App uninstalled) asaccepted_departed— a terminal success in the transfer ledger. Departure is now declared only once the repo demonstrably resolves under the target owner; otherwise the transfer stays pending and the existing expiry clock gives the bounded, truthful answer.Validation
npx tsc --noEmit,db:migrations:check,selfhost:env-reference:check,git diff --check— all clean; no schema, env, or OpenAPI changesVerified solid and deliberately untouched (from the issue's own audit): redirect-hop SSRF re-checks, the three-state fail-closed verdict ordering,
safe-urlIP-literal coverage, allowlist fail-closed, timing-safe key matching, the collector's bounded reader,MAX_PULLED_BUNDLES, and median-not-mean.