fix(orb): harden the federated peer-intelligence trust boundary (#9147, #9148, #9149, #9150, #9166)#9243
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | c64100c | Commit Preview URL Branch Preview URL |
Jul 27 2026, 09:25 AM |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
JSONbored
force-pushed
the
fix/9147-9148-9149-9150-9166-federated-trust
branch
2 times, most recently
from
July 27, 2026 09:15
c20e249 to
adebfff
Compare
…non secret (#9147) Enrolling a federated peer previously handed them this instance's dedicated anonymization secret (the SAME key that HMACs repo/pr identifiers on the always-on #1255 orb export), since buildFederatedBundle signed with getOrCreateAnonSecret. A peer holding that key could dictionary-invert any repo_hash it observed and forge this instance's x-orb-signature. - Add getOrCreateFederatedSigningSecret: a separate, dedicated system_flags secret used only to sign federated bundles. - Decouple instanceId from the anon secret via getOrCreateInstanceIdentitySecret, a third independent secret seeded from the current anon secret on first use (so an upgrading instance keeps its existing instanceId) but stored in its own row from then on, so rotating the anon secret no longer resets the export watermark or the federated peer-median's per-instance accounting. - Correct anonymize.ts's key-separation docstring, which was false for any federated-enabled instance.
…gest comments (#9166) The documented per-collector rate limit (6/min) was inert at every real call site: rateLimitAllows treated a caller-omitted bucket as "unlimited" instead of falling back to a real bucket, and the only production caller (buildFederatedBenchmark) never supplied one. evaluateLocalRateLimit is also pure (read-only), so nothing was ever advancing a bucket's count either. - Add module-level default push/pull buckets in federated-collector.ts; rateLimitAllows now always consults (and advances) a real bucket, whether caller-supplied or the module default. - Bound pullPeerBundles' response read with the same reader /v1/orb/ingest uses (generalized readOrbIngestBody to accept any body-bearing source, so it works for both a Request and a collector Response), and cap the parsed array before any per-element shape check. - Remove two stale "FAIL-OPEN by default" comment blocks in routes.ts left over from #9046, which made isAuthorizedIngest's ingest gate (now fail-closed) read backwards from what the code actually does. pushFederatedBundle wiring into a real tick lands with the #9148 commit, which introduces the background refresh job this naturally slots into.
orb_relay_pending was scoped only by installation_id, so any enrollment secret valid for an installation could drain and destructively-ack another live enrollment's queued webhooks -- a stale re-enrolled container silently stealing a fresh one's events (blue/green swap, secret rotation, or a rebuilt container issued a fresh secret while the old one is still inside its drain timer). - Add orb_relay_pending.enroll_id, tagged at enqueue time with the SAME winning enrollment forwardOrbEvent already elects for the push path (#1783's deterministic ORDER BY). NULL for config_push rows (installation- wide, not consumer-specific) and for any pre-#9150 row, so an untagged row is never orphaned -- pullRelayPending's WHERE matches enroll_id = ? OR enroll_id IS NULL. - forwardOrbEvent now fetches every live enrollment row for an installation (not just the elected winner) so a second live enrollment is observable rather than silent, and increments a new counter, loopover_orb_relay_multiple_live_enrollments_total, whenever more than one exists. - pullRelayPending/enqueueRelayPending both accept an optional enrollId to scope the SELECT and the ack-DELETE together, closing the hole where one container could ack another's rows.
One allowlisted key could mint unlimited Sybil peers and own the median outright: verifyFederatedBundle only checked whether SOME allowlisted key signed a bundle, never bound a key to an instanceId, and importPeerBundles had no per-instance dedup, no range checks, no freshness/replay protection, and pulled peer bundles synchronously on every maintainer dashboard load. - Range-validate every numeric field (rates in [0,1], non-negative counts, cycleP50Ms <= cycleP95Ms) as its own "out_of_range" rejection, distinct from a wrong-typed "malformed" field. - Reject a windowDays that doesn't match the local instance's own resolved window, and a generatedAt outside a bounded freshness window (7 days) or more than 5 minutes in the future. - Enforce decided >= MIN_DECIDED receiver-side rather than trusting the sender's own eligibility claim. - Dedup a batch to the LAST bundle per instanceId (importPeerBundles), then add a persisted, cross-tick per-instance replay/rollback watermark and a per-key Sybil cap (MAX_INSTANCES_PER_KEY) via the new applyFederatedPeerWatermarks, backed by a system_flags JSON blob rather than a new table -- a self-hosted federation's peer count is expected to stay small enough that an in-memory scan/cap-check is trivially cheap. - Move the peer pull off the dashboard's request path entirely: a new "federated-peer-sync" queue job (10-minute cadence, gated on the loopover self-repo's federatedIntelligence.enabled) now runs the pull + trust-gate + persist pipeline AND pushes this instance's own bundle (wiring pushFederatedBundle into a real tick, closing the other half of #9166). buildFederatedBenchmark still computes the local half live (a fast local DB query) but reads the peer half from a cache the tick refreshes. - peerCount now naturally counts distinct contributing INSTANCES (a side effect of the per-instance dedup), matching what its own doc comment always claimed but the old code never enforced.
…r broker enrollments (#9149) issueOrbEnrollment was a bare INSERT: it never revoked, updated, or even counted sibling rows for the same installation_id, so re-running the install flow after a secret leak minted a SECOND simultaneously-valid secret and left the leaked one working forever. The only revoke path sat behind INTERNAL_JOB_TOKEN, unreachable by a self-hosting maintainer. - issueOrbEnrollment takes an optional { rotate: true }: when set, every prior live enrollment for the installation is revoked before the new one is minted. Defaults to false (append) so a blue/green container swap can still rely on two live enrollments briefly overlapping (#9150's sibling fix is what makes that overlap safe, not this one). - The OAuth landing page's callback now recognizes state=<installationId>: rotate / :revoke (installation_id has no channel back through a bare GitHub login/oauth/authorize bounce other than state) -- re-proving admin-of-installation via the SAME verifyInstallationAdmin gate the enrollment flow already uses, so a maintainer can rotate or revoke without operator involvement. Revoke deliberately skips the suspended/self-enrollment-disabled checks: locking down a leaked secret should never be blocked by unrelated administrative state. - The secret page now surfaces how many enrollments are live for the installation and links to the rotate/revoke actions; its response sets Cache-Control: no-store (it was previously a cacheable GET response rendering a plaintext secret). - The internal installations list and enrollment-issue routes gained the same live-enrollment-count visibility and an optional rotate flag, for operator-issued parity with the OAuth self-service path.
JSONbored
force-pushed
the
fix/9147-9148-9149-9150-9166-federated-trust
branch
from
July 27, 2026 09:22
adebfff to
c64100c
Compare
❌ 16 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 5 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
4 tasks
philluiz2323
pushed a commit
to philluiz2323/gittensory
that referenced
this pull request
Jul 27, 2026
Follows JSONbored#9276, which fixed 8 files / 12 cases and left these 4 files / 5 cases still red on main (and therefore red on every open PR). Each diagnosed to its introducing commit; two are real source gaps, two are stale sibling tests a deliberate behavior change never updated. SOURCE fixes: - selfhost/metrics.ts: register two counters emitted since JSONbored#9243 and JSONbored#9245 but never added to DEFAULT_METRIC_META, so renderMetrics() emitted them as bare undocumented samples with no HELP/TYPE — exactly what that drift guard exists to catch. Both commits even name the counter in their own body text; a pure registration miss, not a behavior decision. TEST fixes (source verified correct in both cases): - selfhost-pg-retention: the fake Postgres still matched the pre-JSONbored#9083 ctid semi-join, so every DELETE fell through to rowCount 0. JSONbored#9083 deliberately moved retention to an index-backed PK range delete with ORDER BY (the fix for prune-retention blowing its timeout and stalling permanently); it updated the SQLite twin but not the Postgres one. Regex now captures whichever key column is in play, keeping BOTH the mapped-PK and ctid-fallback paths exercised. - salvageability: JSONbored#9085 made an absent blocker confidence degrade to CONFIDENCE_WHEN_UNSTATED (0.5) instead of 1.0, so it is sub-floor against the 0.93 default and the low-confidence hold owns the case. It renamed both sibling assertions in rules.test.ts but missed this third consumption site. Both calls kept — they are the only coverage of the default-floor arm and the nullish-confidence arm respectively. - worker-entry-boundary: false positive, not a real dependency leak. The check grepped whole-file text, so JSONbored#9230's user-facing string "crossed the visual-diff threshold" failed a green tree over a sentence, in a file worker-reachable since JSONbored#4120 that imports none of these deps. Narrowed to scan module specifiers (reusing the file's own parseImportSpecifiers) — the only way a Node-only dep can actually reach the bundle. Added a discriminating test so it cannot pass vacuously; verified by injecting a real `import sharp` and confirming it fails, naming the file and specifier.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five related trust-boundary fixes in the ORB federated peer-intelligence subsystem, bundled together because they touch the same call paths and were tracked as a single audit pass.
orb:federated_signing_secret, and decouplesinstanceIdfrom the anon secret via a third independent secret, so rotating either no longer breaks the other.MIN_DECIDEDenforcement, per-instance dedup (last bundle wins), a persisted replay/rollback watermark, and a per-key Sybil cap. Moves the peer pull off the dashboard's request path onto a new 10-minutefederated-peer-syncbackground queue job, which also finally wirespushFederatedBundleinto a real tick (closing the other half of orb(federated): the documented peer rate limit is inert at every real call site, push mode is a no-op configuration, and the ingest comments still claim a fail-open that #9046 removed #9166).issueOrbEnrollmentwas a bare INSERT: re-running the install flow after a secret leak minted a second simultaneously-valid secret and revoked nothing; the only revoke path requiredINTERNAL_JOB_TOKEN, unreachable by a self-hosting maintainer. Adds an opt-inrotate(revoke-then-reissue) and a maintainer-reachable revoke path through the OAuth callback (state=<installationId>:rotate/:revoke), gated by the sameverifyInstallationAdmincheck the enrollment flow already uses. Surfaces the live-enrollment count on both the secret page and the internal installations list, and setsCache-Control: no-storeon the secret page.installation_idonly, so any valid enrollment secret for an installation could drain and destructively-ack another live enrollment's queued webhooks (blue/green swap, secret rotation, or a rebuilt container). Addsorb_relay_pending.enroll_id, tagged at enqueue time with the same winning-enrollment election the push path already uses ([Bug]: Orb relay forward picks an arbitrary enrollment when multiple enrolled rows exist #1783), and scopes both the SELECT and the ack-DELETE to it. Also makes a second live enrollment for an installation observable via a new counter instead of silent.Judgment calls
system_flags(orb:federated_peer_state), not a new table — a self-hosted federation's peer count is expected to stay small enough (tens, not thousands) that an in-memory scan/cap-check per tick is trivially cheap, and this avoids a migration for what's fundamentally bookkeeping state rather than a queryable relation.MAX_INSTANCES_PER_KEYis a fixed constant (10), not operator-configurable yet.issueOrbEnrollmentcall — orb(relay): pull-mode drain is scoped by installation, not enrollment — a stale container steals and destructively acks the live one's webhooks, silently #9150 explicitly relies on two live enrollments briefly overlapping during a blue/green swap, so auto-revoking on reissue would have broken that. The OAuth callback'sstatefield is the only channel available to carry a rotate/revoke intent + installation id through GitHub'slogin/oauth/authorizebounce back to our fixed callback URL (GitHub only echoescode+state, not arbitrary query params).Validation
git diff --checknpm run typecheck(full repo, clean)npm run db:migrations:check/npm run db:schema-drift:check(both clean; migration0191_orb_relay_pending_enroll_id.sqlis the next contiguous number)npm run ui:openapi:check/npm run selfhost:env-reference:check(no drift)npm run test:coverage(full, unsharded) — not run in this pass per the task's explicit instruction to keep to directly-relevant test files rather than the full local gate; a maintainer follow-up should run the full gate before mergenpm run test:workers/npm run build:mcp/npm run ui:build— not run, same reason as aboveSafety
Notes
Two pre-existing, unrelated test failures were investigated and confirmed NOT caused by this branch: both were latent on the branch's original fork point and are already fixed on
origin/mainby #9225 (rate-limit DB-error handling) and an unrelated drizzle-related fix; rebasing ontoorigin/mainbefore pushing resolved both, and the full targeted suite is green.Closes #9147
Closes #9148
Closes #9149
Closes #9150
Closes #9166